Created attachment 13572 [details]
patch for app/interface.c to address bug (manually created diff)

Looks reasonable and is simple enough to be applied to the 1.2 branch.

I haven't checked but I think the proposed solution has some
drawbacks. Think of an image of 2000x399 pixels. If we'd go for your
proposed change, the image would be displayed with a zoom ratio of 1:1
while earlier versions of Gimp chose 1:2 or 1:3 to be able to display
the entire image. I don't want the behaviour for such common formats
to change.

I'd like to propose two different solutions:

(1) Limit the initial zoom ratio to 1:16. This would be consequent
    since zooming in and out using the shortcuts does currently not
    work if you are outside the 16:1 .. 1:16 range.

(2) Add code that assures that the initial zoom ratio is choosen in
    a way that assures that the smaller axis is not scaled down below
    a minimal size of 1 pixel.

I'd prefer solution (1) since it would reduce the time needed to open
a large image and create the initial display. Of course it means that
very large images (larger than 16 times the screen size) are not
displayed entirely when they are opened. As outlined in bug #62313,
this is broken anyway. So perhaps we should go for (1) but remove the
limitation for the zoom in / zoom out commands as I've already done in
the 1.3 tree.

Hi Sven,

Thanks very much for your reply and consideration.

First of all, let me say that I think your suggestion of removing the 
16:1 zoom factor limit as you have done in 1.3 is probably a good 
idea.  In fact, I have already made the corresponding changes to my 
copy of gimp 1.2.4, and it seems to work well.  Now, on to your my 
proposal and your suggested alternatives...

Upon further consideration, I must agree with your concern regarding 
a case such as the 2000 x 399 example that you gave.  However, I 
don't think that either of the alternatives that you suggested would 
work well either...

I don't agree that your first suggestion (16X limit) is reasonable 
all by itself.  Consider my typical image, 2000 x 100000 pixels.  If 
we went with your first alternative, then the initial display of this 
image on my 1600 x 1200 screen would be at a 16X reduction, resulting 
in an image width of only 125 pixels, or just about 1.25 inches out 
of my available 12 inch display width.  Your second alternative would 
produce even worse results.

My delay in getting back to you on this is due to the fact that I 
have spent almost the whole day thinking about this problem and 
tinkering with various potential solutions.  I finally came up with 
something that is fairly simple and provides reasonable results on a 
very wide range of image sizes and aspect ratios.  The attached files 
provide both my algorithm (scaleImage.sas), and the results that it 
would produce (scaleImage.txt) for test cases covering a wide range 
of possibilities from 10 to 100000 pixels in 25 diffferent 
combinations on both axes.  I actually evaluated even more cases than 
that, but didn't include those in the attachment because they
merely made the results more voluminous to wade through.

I wrote the attached algorithm in SAS to simplify my code changes 
while evaluating various options and to simplify reporting the 
results.  However, this algorithm can be easily converted to C, and I 
will be happy to do so and provide a new patch if you agree that the 
results it produces are desirable.  SAS is fairly straightforward as 
far as the code for the algorithm is concerned, so hopefully you will 
be able to interpret my code without any significant difficulty.

The new algorithm provides a maximum of compatibility with prior 
behavior for typical images, while also solving the high aspect ratio 
problem that I initially reported.  It solves the problems that you 
found with my previous proposal, as well as those that I found with 
the alternatives that you suggested.  It maintains the 16 fold 
automatic scale reduction limit that was intended by prior versions 
of app/interface.c, and also addresses the 1 pixel minimum issue that 
you raised.

I'd appreciate it if you could look all of this over and let me know 
if you think it is acceptable.  If so, then I will recode in C and 
provide a new patch as soon as possible.

Thanks once again for considering my input.

Regards,
s/KAM

Created attachment 13600 [details]
revised algorithm and test case code as written in SAS

Created attachment 13601 [details]
results of test cases for revised algorithm

Well, considering your typical image, 2000 x 100000 pixels.  If we
went with my first alternative, then the initial display of this image
on my 1600 x 1200 screen would be at a 16X reduction, resulting in an
image width of 125 pixels. Wouldn't that be a great step forward?
After all your images are not very typical and the casual user expects
to the see the full image when it is opened.

I think that showing your image with a width of 125 pixels will give a
good impression of the aspect ratio of the image. Showing it at full
width may be desired for your use but I doubt that the casual user
would expect this behaviour. IMO your usage pattern is so very special
that it doesn't make much sense to try to find the optimal solution
for it. You are free to modify your copy of gimp to fit your needs but
I don't see a complex algorithm such as the one you suggested going
into the 1.2 tree. After all nothing but bug fixes should go into the
stable tree.

Unfortunately I just realized there was a minor problem with my 
previously submitted script and test cases. The problem involves my 
pfactor variable, the "previous scale factor" which is equivalent to 
the combined effect of scalesrc and scaledest prior to the scaling 
algorithm of create_display_shell in app/interface.c .  

This problem really only affects the test results; there still isn't 
anything wrong with the basic algorithm that I previously submitted.  
However, I'm going to attach both updated files anyway.  Sorry for 
any confusion.

s/KAM

Created attachment 13602 [details]
corrected algorithm as written in SAS

Created attachment 13603 [details]
corrected results from SAS test cases

Hi Sven -

Although I definitely appreciate your help, and can understand the 
need to be wary of changes to the "stable" image, I don't agree with 
your conclusion here...

My proposed algorithm is no more complex than the one that is 
currently in app/interface.c.  My algorithm consists of 11 lines of 
code involving only a few simple comparisons and some straightforward 
multiplcation and division.  By comparison, the existing algorithm 
takes only 9 lines of code, but also does some very unnecessary 
looping to accomplish the same task.

Perhaps you didn't realize that a significant amount of the SAS 
program that I provided is only overhead that is used to produce all 
of the information in the test results listing?  My core algorithm 
consists of only the following statements:

if factor * ximage > xmax or factor * yimage > ymax then do;
   xfactor = xmax / ximage;
   yfactor = ymax / yimage;
   factor = min(xfactor, yfactor);
   if factor < 1 / 16 then
      factor = min(max(xfactor, yfactor, 1 / 16), pfactor);
   factor = max(factor, 1 / ximage, 1 / yimage);
end; /* if ximage > xmax or yimage > ymax */

if factor > 1 then do; numerator = int(factor); denominator = 1; end;
else do; numerator = 1; denominator = ceil(1 / factor); end;
factor = numerator / denominator;

xscaled = int(ximage * factor);
yscaled = int(yimage * factor);


I strongly hope that you'll reconsider.  I don't want to get into 
maintaining my own "custom" version of the gimp.  The changes that I 
have suggested will only help others, or at worst have no impact on 
them whatsoever, as illustrated by the test case results that I 
provided.

Thanks again for your consideration in any case.

s/KAM

1. I just realized that mixed use of tabs and spaces did a number on 
the formatting of my previously submitted source code attachment.  
Hopefully the code snippet in my prior entry provides a better view 
of the necessary code.

2. Concerning Sven's comment regarding the casual user expecting to 
see the whole image, I must point out that the casual user will NOT 
necessarily see the whole image with Sven's proposed solution, nor 
with ANY other solution that involves a max 16X reduction cutoff.  My 
suggested processing ONLY changes how things are handled once the 16X 
reduction factor has been EXCEEDED.  So, my solution will NOT 
adversely affect the casual user in any way compared to Sven's 
proposed alternative.

Let's compare a few specific scenarios:

Screen size: 1280 x 1024, "previous" scale factor = 1:1

Incoming image size: 800 x 600
Sven's scale factor: 1:1, image size: 800 x 600
Kevin's scale factor: 1:1, image size: 800 x 600, no difference

Incoming image size: 1280 x 1024
Sven's scale factor: 2:1, image size: 640 x 512
Kevin's scale factor: 2:1, image size: 640 x 512, no difference

Incoming image size: 32000 x 600
Sven's scale factor: 16:1, image size: 2000 x 38, horizontal 
scrolling required, unusable tiny vertical scale
Kevin's scale factor: 1:1, image size: 32000 x 600, horizontal 
scrolling required, normal vertical scale
 
Incoming image size: 32000 x 1024
Sven's scale factor: 16:1, image size: 2000 x 64, horizontal 
scrolling required, unusable tiny vertical scale
Kevin's scale factor: 2:1, image size: 16000 x 256, horizontal 
scrolling required, reasonable vertical scale

Incoming image size: 32000 x 32000
Sven's scale factor: 16:1, image size: 2000 x 2000, horizontal and 
vertical scrolling required
Kevin's scale factor: 16:1, image size: 2000 x 2000, horizontal and 
vertical scrolling required, no difference

As you can see from these examples:

1. As long as the initial image can fit into 75% of the screen area 
(that part is old code) with up to a 16X reduction in scale, Sven's 
proposal and mine produce the same result.  

2. Once the 16X factor is exceeded, both our proposals also produce 
the same result when the aspect ratio is in the same ballpark as the 
screen, or when both image dimensions are vastly larger than the 
screen.

3. BUT, when a scale reduction exceeding 16X would be required to fit 
the entire image onto the screen, AND the image aspect ratio is 
significantly different from that of the screen, then my proposal 
produces an IMHO much more usable scaled image size.  The difference 
becomes more and more profound as the aspect ratio increases.  This 
difference is particularly significant for my admittedly somewhat 
unusual images, but as you can see from the examples above the 
differences can also be quite significant for images of more 
reasonable sizes and aspect ratios.

One final comment:  It is specifically because my usage of the GIMP 
is NOT casual that my concerns in this area should be given careful 
consideration and more weight than usual.  Something that a more 
casual user can easily get by with, such as simply rescaling an image 
prior to working with it, becomes much more problematic when you're 
dealing with thousands of images like I am.  And once again, there is 
nothing in my proposal as compared to Sven's that makes things worse 
in any way for the casual user.

Hope this helps clarify things.  Now I'll sit back and try to do my 
very best to stop spamming bugzilla.  Sorry for all the email!!!

s/KAM

I still believe the casual user expects to be presented with the whole
image when it is initially displayed. IMO, we shouldn't change this
behaviour. My proposal was to stop making the smaller axis very small
at some point. If you disagree that stopping at a display ratio of
1:16 is a reasonable choice, we should perhaps go for the second
proposal and set the initial display ratio so that the smaller axis is
at least one pixel wide. This would still give the user useful
information about the image.

You are looking at this from the view-point of someone who knows what
the images look like and you know you will be working by scrolling
your way thru the image. The casual GIMP user doesn't know what image
she is opening and probably wants to take a look at the image in its
entirety first. We both agree that it doesn't make sense to present
the image in a display ratio where no pixels are displayed at all but
I doubt that it makes much sense to try to fill the screen with a very
long or tall image.

When looking at your examples I fid it surprising to see that an image
of 32000 x 600 pixels is supposed to be displayed with the same screen
width as an image of 800 x 600 pixels. On first sight, I could get the
impression that both images are almost square. Let's see what happens
for the same images if we double their height. The small one, 1600 x
600 pixels gets displayed with 300 pixels screen width while the
larger one ends up using the same screen width as before. That does
sound odd to me and indicates that the algorithm you propose will
confuse people because it behaves very differently for some image sizes.

For GIMP-1.3 we could perhaps add the possibility to choose the
initial display size when opening the image or make it a preference
option (although I think we should be careful about adding more
preference options). For 1.2 we are basically looking for that
one-liner that fixes the problem this bug-report is all about but
doesn't change the behaviour for almost all other images.

Hi Sven -

Thanks for your comments.  My responses interspersed below:

> ------- Additional Comments From sven@gimp.org  2003-01-15 21:44 ---
----
> I still believe the casual user expects to be presented with the 
whole
> image when it is initially displayed. IMO, we shouldn't change this
> behaviour. My proposal was to stop making the smaller axis very 
small
> at some point. If you disagree that stopping at a display ratio of
> 1:16 is a reasonable choice, we should perhaps go for the second
> proposal and set the initial display ratio so that the smaller axis 
is
> at least one pixel wide. This would still give the user useful
> information about the image.

I agree with your proposal to stop making the image smaller at some 
point, and agree that the ratio of 16 to 1 is reasonable.  However, 
that conflicts with your statement that you want to maintain the 
present behavior of seeing the whole image when it is initially 
displayed.  Once the image exceeds the 16 to 1 limit, it cannot be 
displayed in its entireity with your approach!  I am NOT trying to 
make a case that this is a bad thing, nor that 16X is an 
inappropriate cutoff point to avoid further scale reductions.  I'm
only trying to point out that images above that cutoff will NOT be 
displayed in their entireity, no matter whether using your approach 
or mine.

>
> You are looking at this from the view-point of someone who knows 
what
> the images look like and you know you will be working by scrolling
> your way thru the image. The casual GIMP user doesn't know what 
image
> she is opening and probably wants to take a look at the image in its
> entirety first. We both agree that it doesn't make sense to present
> the image in a display ratio where no pixels are displayed at all 
but
> I doubt that it makes much sense to try to fill the screen with a 
very
> long or tall image.
>

How many users do you think there are out there who would even run 
across images of this size without having some idea of what to 
expect?  Not many!  Remember, for the specific points that we are 
discussing to even matter, one dimension of the image must exceed 16X 
the desired display area, AND the aspect ratio must also be 
significantly different from the screen aspect ratio.
The only folks out there who are going to have images like this are 
people like me who have experience in dealing with them and are NOT 
novice, unsophisticated users.

> When looking at your examples I fid it surprising to see that an 
image
> of 32000 x 600 pixels is supposed to be displayed with the same 
screen
> width as an image of 800 x 600 pixels. On first sight, I could get 
the
> impression that both images are almost square. Let's see what 
happens
> for the same images if we double their height. The small one, 1600 x
> 600 pixels gets displayed with 300 pixels screen width while the
> larger one ends up using the same screen width as before. That does
> sound odd to me and indicates that the algorithm you propose will
> confuse people because it behaves very differently for some image 
sizes.

Consider for a moment that all of my images are actually much taller
than their width, rather than the other way around (even though the 
example dimensions that I provided weren't that way).  I know that 
seems a minor distinction, but consider your impressions in that 
context rather than a horizontally wide document for a moment.  
People are entriely accustomed to the need to scroll vertically 
through a document whose width fits the screen, but whose length does 
not.  This is the most common paradigm for scrolling through long 
files and documents of any type.  Certainly not something that anyone
who has ever used a computer before would be confused by!

>
> For GIMP-1.3 we could perhaps add the possibility to choose the
> initial display size when opening the image or make it a preference
> option (although I think we should be careful about adding more
> preference options). For 1.2 we are basically looking for that
> one-liner that fixes the problem this bug-report is all about but
> doesn't change the behaviour for almost all other images.

Philosophically I am generally in agreement with you on most of those
points, even though I still believe that my algorithm is simple 
enough and low enough impact to warrant implemenation.  However, I 
also agree that even your simple 16X limit would be an improvement 
over the present situation.  I am also going take a closer look at my 
code to see if there is any way that it could be greatly simplified 
and turned into more of a one liner.  I suspect this *might* be
possible, because when I first started looking at this earlier today 
I was initially considering something more complicated, and what I 
have now may include unnecessary left over artifacts from that.  I 
haven't really spent any time yet attempting to optimize what I 
provided yet in terms of minimizing code changes.  So far, all I have 
attempted to minimize are any significant changes that would have
an actual impact on typical end users.

I'll get back to you as soon as I can.

Regards,
s/KAM

Created attachment 13611 [details]
C version of previously submitted scaling algorithm for app/interface.c

I've attached a C version of my previously submitted initial scaling 
algorithm for the relevant portion of create_display_shell in 
app/interface.c .

Regarding ongoing discussions between Sven and myself concerning the 
proper fix for this proposed bug, I think that it may be worth noting 
that my proposed fix and Sven's are equivalent to using the following 
two chunks of code to break out of the loop in the previously 
existing algorithm:

Sven's version:
 if (scalesrc >= 16) break; /* limit zoom to 16X maximum */

My version:
 if (scalesrc >= 16)
 {
  factor = max(xfactor, yfactor); /* largest that will fit */
  factor = max(factor, (gdouble) 1 / 16); /* or 16X if larger */
  factor = max(factor, pfactor); /* but don't exceed prior scale */
  if (factor > 1) {scaledest = factor; scalesrc = 1;}
  else {scaledest = 1; 
        scalesrc = (1 / factor) % 1 ? (1 / factor) + 1 : 1 / factor;}
  break;
 }
 
The key difference here is the first line within my version of the 
primary if block.  Without that line, my approach will produce 
exactly the same results as Sven's.  That line first attempts to use 
the largest scale that will fit into the desired display area for 
either axis, before finally settling on 16X max scale reduction if 
that would produce a larger image.  

Sven's version simply limits the max reduction to 16X, without 
considering that it might be possible to display the image at a 
larger, more usable size.

Neither of these solutions affect the display of images unless at 
least one axis must be reduced by 16X or more to fit the desired 
display area.  My solution produces identical results to Sven's if 
both image dimensions are more than 16X the desired display size, or 
if the aspect ratio of the image is in the same ballpark as the 
screen.  It only differs from Sven's when both the size of at least 
one axis is greater than 16X the desired display area AND the aspect 
ratio of the image is significantly different from that of the screen.

The code that I have provided in an attachment as the actual version 
of my algorithm to be used is slightly different from the above in 
order to avoid unnecessary loop overhead and compute a few 
preliminary variables that I use later.  I just provided the above to 
simplify comparison of our propsed solutions.

s/KAM

Wow!  Many comments since yesterday...  I would just like to point out
that I tend to agree with Sven: I think that some kind of consistency
is important in the user interface, and most users would expect to see
the image scaled to fit the screen (even if it is only 1 pixel wide)
because this gives them a good idea of the aspect ratio and it is also
easy to understand and predict how the GIMP will open an image.

The problem is the limitation to the 16:1 or 1:16 aspect ratio of the
stable GIMP, which prevents very thin images from being scaled to the
appropriate size for fitting entirely on the screen.

If we really want to change the stable GIMP in order to fix this bug,
then I would prefer to invest some time in removing the 16:1 and 1:16
limits than to implement a different algorithm for the initial
scaling factor.  Note that even if the image is intially scaled in
such a way that one a 1-pixel line or column is visible, it only takes
one keystroke (pressing the "1" key) to get the 1:1 aspect ratio.

Just to add that I tend to agree with KAM.

If we can fit the whole image in at regular scales, fine. If we can't,
then let's at least make what we show in the window useful - if aspect
ratios get up towards 30 or 40 to 1, we should ensure that the 1
doesn't get swamped out in attempting to accommodate the 30. If we
make incorrect assumptions about the scale the user would like, then
the zoom tools can take up the slack once we have a display open.

It seems more liely to me that someone dealing with banner-type images
wants the smaller scale to be reasonable than to be able to see the
maximum amount of the image possible.

Whether it's a change that should go into 1.2 is questionable - the
current behaviour of the zoom tool is definitely a bug, but this
doesn't just fix the problem, it adds a new mini feature too. I
personally wouldn't mind seeing it go in. But for 1.3, I would
definitely like to see something like this go in.

In any case, if there's an argument to be had on the matter, it should
probably happen on the mailing list :)

Cheers,
Dave.

I'm setting the target milestone to 1.3 and severity to enhancement
since I think the bug is fixed in the 1.2 tree and the improved
behaviour suggested here should IMHO only go into the development tree.

See also bug report 106730.

In my opinion the fix for bug 106730 fixes this issue reasonably well.
You can now choose to have all images opened with a scale of 1:1.
If you agree that this is good enough for your usage behaviour, please
close this report as FIXED.

I agree that the fix for bug 106730 seems like a reasonable way to 
address this problem.  Marking FIXED per Sven's request.

The fix for one part of this bug is included in the stable release
1.2.4.  The other part was addressed by bug #106730.  Closing this bug.