> [EMAIL PROTECTED] (2004-01-17 at 1724.31 +0100):
> > a) It has a table of presets that is constructed in an ad hoc manner
> > without any explantation what the advantages to the user are.
> Did your copy of the first mail I posted lacked:
> "Comments about the presets also welcomed, I just made a list of the
> ones that seemed interesting while working always around some given
> factor."

Well, I should have sent my first two mails in a different order then
perhaps. The second mail does exactly this: It discusses the presets.
So I guess thats not the reason for you to be so offended.

The first mail I sent discusses the way this IMHO should be implemented.
In fact I did implement it in my local tree here yesterday and it turns
out that it is a pretty big change, since I noted a lot of brokeness
in the gimpdisplayshell API (the ratio 16:1 is encoded as both 1601
(decimal) and 0x1601 (hexadecimal) in different places of the code).
Because of the size of my patch it might be too late to put this
into 2.0. I think it is the right thing to do, but it might have to
wait (the patch is attached to bug 131563).

So in the retrospective the only reproach I have to make to myself is
the choice of the word "evil" in IRC. I'd like to apologize for that - 
rest assured that I would not have used that word in a more formal
medium like mailing lists (Ok, I used it after you brought it up).
Please accept my apologies and I hope we can focus on a more technical

[technical discussion  :)]

I think I already explained why I prefer the set of ratios based on
the idea of "homogenous zooming". So the rest of this Mail focuses 
on the technical issues of your patch.

I am not an expert on optimization levels, Assembler or whatever.
The only thing I know is, that it is hard to compare two floats
for equality. Part of the (huge) patch I mentioned above
is a function, that calculates the next zoom step (float) based on a
given (float) zoom step. It looks like this:

gimp_display_shell_scale_zoom_step (GimpZoomType zoom_type,
                                    gdouble      scale)
  gint    i, n_presets;
  gdouble new_scale;

  /* This table is constructed to have fractions, that approximate
   * sqrt(2)^k. This gives a smooth feeling regardless of the starting
   * zoom level.

  gdouble presets[] = {
    1.0 / 256, 1.0 / 180, 1.0 / 128, 1.0 / 90,
    1.0 / 64,  1.0 / 45,  1.0 / 32,  1.0 / 23,
    1.0 / 16,  1.0 / 11,  1.0 / 8,   2.0 / 11,
    1.0 / 4,   1.0 / 3,   1.0 / 2,   2.0 / 3,
               3.0 / 2,      2.0,      3.0,
      4.0,    11.0 / 2,      8.0,     11.0,
      16.0,     23.0,       32.0,     45.0,
      64.0,     90.0,      128.0,    180.0,

  n_presets = G_N_ELEMENTS (presets);
  /* Zooming in/out always jumps to a zoom step from the list above.
   * However, we try to guarantee a certain size of the step, to
   * avoid silly jumps from 101% to 100%.

  switch (zoom_type)
    case GIMP_ZOOM_IN:
      scale *= 1.1;

      new_scale = presets[n_presets-1];

      for (i = n_presets - 1; i >= 0 && presets[i] > scale; i--)
        new_scale = presets[i];

      scale = new_scale;

    case GIMP_ZOOM_OUT:
      scale /= 1.1;

      new_scale = presets[0];

      for (i = 0; i < n_presets && presets[i] < scale; i++)
        new_scale = presets[i];

      scale = new_scale;

    case GIMP_ZOOM_TO:

  return CLAMP (scale, 1.0/256.0, 256.0);

As you can see it compares floats with <= and >= and so avoids
tests for real equalness. The little "jump" done by multiplying by
1.1 (which is a bit arbitrary chosen, but should be smaller than the
factors between the presets) makes it even more robust IMHO.

Something similiar could be implemented directly in
gimp_display_shell_scale (). The only trick is, to get the numerator and
denominator for the fraction in the gimpdisplayshell.

In current CVS there is the function
gimp_display_shell_scale_calc_fraction (), that calculates a fraction
for a given float. It uses continued fractions, which kind of ensures,
that things like 2.0 / 3.0 really result in 2/3. You could use this
to determine the fraction for the gimpdisplayshell (similiar as to how
it is used now).

I hope you can accept this as a technical criticism of your patch, it
might solve your floating point problems with a different approach.
It also should work with a different set of presets.

Gimp-developer mailing list

Reply via email to