This problem does not seem to be reproducible with current SVN. Please give a more detailed description with exact pixel dimensions.

For example:
                 width x height
Original image:    800 x 800
Destination   :    801 x 799

or
                 width x height
Original image:    512 x 512
Destination   :    513 x 511

This bug was maybe fixed in svn.
I'll check svn.

It is still reproducible with these values.

Created attachment 89337 [details] [review]
proposed fix

This change fixes the crash. I am however not sure if it introduces other problems.

+    case GIMP_INTERPOLATION_LANCZOS:
+      scale_region_lanczos (srcPR, destPR, progress_callback, progress_data);
+      break;

This "break" is "return", isn't it?

yes that would make more sense or else the same code will still get called with GIMP_INTERPOLATION_LANCZOS.

IIRC this test was put in by Geert since it was thought that lanczos was not a good choice for image reduction. I think the idea was that it falls back to cubic.

ie.


  if (interpolation == GIMP_INTERPOLATION_LANCZOS && orig_height <= height)
    {
      scale_region_lanczos (srcPR, destPR, progress_callback, progress_data);
      return;
    }
  else 
    {
     interpolation=GIMP_INTERPOLATION_CUBIC;
    }

I'm not sure if sneakily changing this behind the users back is the best choice but it's at least correct code.

I seem to remember suggesting about 9 mth back when this came up that a better way would be to modify the drop down list of interp types if there is a difference in the way the code handles this.

Unless that idea gets more support than it did last time the short term fix may be what Sven intended: to reinstate lanczos on reduction and let the decide if it's what he wants.

It will be different from cubic so it's better than a secret spec violation via an unnotified change of interp.

I'll try to run some tests on lanczos reducution later.

The problem here is that you can't decide whether you are doing reduction or expansion because the user might expand the size in one direction and reduce it in the other. That's exactly the situation that causes the test to fail and GIMP to crash. It would be better if the Lanczos scaling code handled reduction.

Created attachment 89424 [details] [review]
proposed fix, revised

well it can "handle" it but the results are different.

It has some of the blockiness of NONE but with less distortion than nearest neighbours rather crude interp.

Both linear and cubic give a smoother result but lanczos seems to be a lot closer to original colour contrast and brightness. Lin and cubic seem rather soft and less contrasty by comparison.

[The contrast issue should probably be looked at elsewhere since there seems to be a net difference between lin/cubic scaled 25% and original viewed at 25%, photo image.]


While lanczos produces very good results as a pixelisation filter, I'm still not convinced of validity of using an imperfect bandpass filter when scaling down an image since there should be no higher frequencies to filter out and all that is left is the ringing and the imperfections of the filter.

I dont see this so much as lanczos not handling this situation so much as it being an inappropriate filter to use on reduction.

Good point about possible opposed scaling factors. I'd overlooked that but it is conceivable that someone could want to make an image wider and shorter, esp. with factors close to 1:1

The effect of attempting this with lanczos should be ... interesting.

/gg

The proposed patch is probably the best thing to do then. The code then does what the user asks it to do. Since we aren't proposing Lanczos as the default interpolation type, this should be fine. The user will get what they asked for and the crash should be fixed.

I have committed this change then, with some additional code cleanups.

2007-06-06  Sven Neumann  <sven@gimp.org>

	* app/paint-funcs/scale-funcs.c (scale_region): always use
	scale_region_lanczos() for LANCZOS interpolation. Fixes bug #443640.