dojox.image.lightbox window resize erroneous algorithm

[Eric Pasquier]

The algoritm used in _scaleToFit is completly erroneous to resize an image to a viewport. Please find below a correction :

_scaleToFit: function(/* Object */size){
  // summary: resize an image to fit within the bounds of the viewport
  // size: Object
  //		The 'size' object passed around for this image
  var ns = {};   // New size
  var nvp = {};  // New viewport
		
  // Take borders/title into account 
  nvp.w = this._vp.w - 80;
  nvp.h = this._vp.h - 60 - this._lastTitleSize.h;
		
  // Calculate aspect ratio
  var viewportAspect = nvp.w / nvp.h;
  var imageAspect    = size.w / size.h;
      
  // Calculate new image size
  if (imageAspect >= viewportAspect) {
    ns.h = nvp.w / imageAspect;
    ns.w = nvp.w;
  }  
  else {
    ns.w = imageAspect * nvp.h;
    ns.h = nvp.h;
  }

  // we actually have to style this image, it's too big
  this._wasStyled = true;
  this._setImageSize(ns);

  ns.duration = size.duration;
  return ns; // Object
}

Eric.

[Eric Pasquier]

I'd like to add that this defect is also present on 1.5.0b3: large images are not correctly resized with the actual function and can be larger than the screen, with no possibility to close the popup (button not visible).

[dante]

sorry -- my comment was lost somehow:

@Eric - do you have a CLA on file?

Also, not a super large deal, but the preferred way for patch submission is via unified diff against trunk attached with a .diff or .patch extension for easy review.

Do any of the tests show this resizing error? I have the one large 1:1 aspect ratio test that seems fine, but I presume this comes from images with odd/excessive aspects?

[Eric Pasquier]

Hi, Dante, The first test I made was to display a 2590x1920 image (taken with my 10-years old camera): the image displayed is bigger than the screen. Then, I built 2590*1000 and 1000x1920 images that confirm the problem.

The algorithm implemented surprised me, and contrast a lot with the work done for the entire control: testing the "biggest edge of the viewport" to resize an image is really something innovative.

Anyway, feel free to use my suggestion. I have been using this algorithm for years in a personal VB application developped by myself (a control to display & print images !).

I don't know anything about CLA, unified diff and so on. If you are the developper of the control, it should be easy for you to correct it. Otherwise, tell me how can I help. I can supply the complete new js file.

Eric.

[Eric Pasquier]

Hi, Dante. I just sent the signed CLA by Fax.

[dante]

Awesome! I've created tall and wide test images, and am only seeing the error in really wide images. Will apply your patch and see if that resolves it.

[dante]

a .patch version of a [slightly adjusted for style] the above code against trunk

[dante]

@Eric - everything looks great. the above attachment is a diff from svn diff after applying [and slightly modifying] your patch. Once I get the confirmation of the CLA I'll go ahead and commit the fix. Thanks for fixing my sloppy algo! I see know how very 'creative' my solution was. I may have been drunk. :)

[Eric Pasquier]

Yes, now your control is very smooth to use, even if you resize the browser during display. I didn't do it, you can add a test when the browser's window is very small: the nvp values become negatives. In this case, define a fixed size (just to be perfect !)

[dante]

[22507]