After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 355178 - transformation tools with Lanczos interpolation brighten the result
transformation tools with Lanczos interpolation brighten the result
Status: RESOLVED FIXED
Product: GIMP
Classification: Other
Component: General
git master
Other All
: Normal minor
: 2.4
Assigned To: GIMP Bugs
GIMP Bugs
Depends on:
Blocks:
 
 
Reported: 2006-09-09 19:28 UTC by che
Modified: 2006-09-24 22:27 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Sample PNG with alpha channel (2.73 KB, image/png)
2006-09-12 05:34 UTC, Mike
  Details
test image very prone to this defect (45.44 KB, image/png)
2006-09-20 09:30 UTC, gg
  Details
patch to correct this bug (13.89 KB, patch)
2006-09-20 09:39 UTC, gg
none Details | Review
comparison of bicubic and corrected lanczos (57.64 KB, image/png)
2006-09-20 09:44 UTC, gg
  Details
filter comparison on clearly pixelated line (91.81 KB, image/png)
2006-09-20 15:36 UTC, gg
  Details
patch to correct this bug , update to #9 (19.10 KB, patch)
2006-09-20 19:02 UTC, gg
committed Details | Review
corrects #15 slip and reverts to lanczos3 (1.11 KB, patch)
2006-09-21 03:26 UTC, gg
none Details | Review
Patch to fix lanczos interpollation in gimpdrawable-transform (12.19 KB, patch)
2006-09-22 18:17 UTC, geert jordaens
none Details | Review
Fixes lanczos in gimpdrawable-transform.c (12.19 KB, patch)
2006-09-22 19:04 UTC, geert jordaens
committed Details | Review

Description che 2006-09-09 19:28:12 UTC
Please describe the problem:
Layer tranformation tools have an unwanted effect on image colors when Lanczos interpolation is chosen.

Steps to reproduce:
1. Choose a layer.
2. Apply rotate, scale, shear or perspective tool on it with Lanczos interpolation.


Actual results:
Layer color are broken on transformed layer. The exception is the "perspective" tool, which divides the layer in two parts, and color are broken only in one of them.

Expected results:
Layer transformation w/o any side effect.

Does this happen every time?
yes

Other information:
Comment 1 Sven Neumann 2006-09-11 10:21:34 UTC
Isn't this already handled in bug #167956 ?
Comment 2 Sven Neumann 2006-09-11 10:30:36 UTC
Can you describe what you mean when you say "the colors are broken?".
Comment 3 che 2006-09-11 16:36:42 UTC
I don't think this was handled in bug #167956, although it might be a side effect, as I started to notice the issue after the following commit:

2006-09-06  Bill Skaggs  <weskaggs@primate.ucdavis.edu>

	* app/paint-funcs/scale-funcs.[ch]: apply patch from Geert
	Jordaens to improve Lanczos scaling, with coding style
	cleanups; partly fixes bug #167956.

However, I don't remember if I used the feature just before that, so I'm not sure.


By "broken colors" I mean something lokking quite like contrast enhancement. See examples:
before: http://pokazde.jinak.cz/misc/before.jpg
after Lanczos-interpolated rotation: http://pokazde.jinak.cz/misc/lanczos.jpg
both layers, together with example of perspective toll "splitting effect" are in http://pokazde.jinak.cz/misc/interpolation.xcf.bz2 (800KB)

These files were produced with Saturday's CVS HEAD.
Comment 4 Mike 2006-09-12 05:34:03 UTC
Created attachment 72586 [details]
Sample PNG with alpha channel
Comment 5 Mike 2006-09-12 05:36:45 UTC
I've attached sample 32 bit RGBA PNG image.
Downscaling this image to 31x31 with Lanczos is OK,
but upscaling to 33x33 make colors brighter ("mess up").
PS. GIMP 2.3.11 on Windows (SourceForge build)
Comment 6 weskaggs 2006-09-12 15:55:23 UTC
It is very important to distinguish between scaling using the menu commands ("Scale Image" and "Scale Layer") and scaling using the Scale Tool.  The two use different code.  Only the code for the menu commands was affected by the commit discussed in comment #3 -- the behavior of the tool should not have changed.
Comment 7 geert jordaens 2006-09-13 16:59:12 UTC
The behavior of the tool should not be changed, I agree and not the patch changed the header file scale-funcs.h which is also used in gimp-drawable-transform.
I completely forgot it was shared, i'll put some more time in it.
Comment 8 gg 2006-09-20 09:30:32 UTC
Created attachment 73076 [details]
test image very prone to this defect

This image shows extreme brightening with current 2.3.11 and CVS HEAD at this time. Filter is not usable in this state.

test condition, scale to width 800 with lanczos
Comment 9 gg 2006-09-20 09:39:08 UTC
Created attachment 73077 [details] [review]
patch to correct this bug

this patch corrects the normalisation of the transformation kernels that was causing this problem.

Also some renaming of variables to make the code more easy to understand and maintain.

The function for creation of lanczos lookup table is added *.h as a first step to consolidation  of code duplication in tools and filters.

corrects some nomenclature, eg.: lanczos_kernel() creates the lookup table not the kernel. Renamed.
Comment 10 gg 2006-09-20 09:44:37 UTC
Created attachment 73078 [details]
comparison of bicubic and corrected lanczos

magnified screenshot of resize of the test image using both bicubic and the corrected lanczos.

the latter is slightly softer but seems to interpolate better. The two are very close.
Comment 11 gg 2006-09-20 15:36:43 UTC
Created attachment 73092 [details]
filter comparison on clearly pixelated line 

the right of this image shows a lanczos enlarged image of a badly pixelated line drawn in gimp.

despite the ringing , a known defect of sinc techniques , it is clear that the single pixel quantisation has been completely eliminated. This is outstanding, even compared to bicubic on the left which merely smoothes it a bit. 

The remaining larger order jaggedness of the line is an artifact of the gimp line tool and is therefore to be regareded at data. This was correctly preserved.

There seem to be no other unexpected artifacts. 

It seems that the filter is now working correctly on expand. (I have not tested other uses of the filter in gimp).

This underlines that this filter works best on raw data as pixelisation remover.
Comment 12 weskaggs 2006-09-20 16:11:56 UTC
Geert, could you take a look at the patch in comment #9 and give your opinion?

Gg, from a quick scan of the patch, the one thing I don't like is the variable name adsrc_yance_dest, which is completely mystifying.  (Was this a "search and replace" blunder?)  Can you come up with something more meaningful?  And congratulation on following GIMP coding style so well -- hardly anybody ever gets it right on the first try.
Comment 13 gg 2006-09-20 17:34:01 UTC
oops, indeed. c/v/src_y/g  seems to have got me. 

c/adsrc_yance_dest/advance_dest/g should get you back, not much chance of a false match there.

I'll clean that up in a subsequent patch.

That's one of the reasons I wanted to remove variable names like v and u : you get so many false hits on a search.

Since it compiled I did not even spot it , appologies.


I'll do a bit more work and some tidying now I know that sort of change is welcome. (Many flame wars have started over code style I was not sure how it would be recd. )

I'm glad GIMP follows good coding practice ;) I just did what I needed to make it more readable. Glad to see we're on the same wavelength.

Comment 14 geert jordaens 2006-09-20 17:41:30 UTC
I already scrolled through the patch #9 , it seems prommessing (not yet tested).
One thing I'm already happy with is the change of lanczos kernel to lookup.
The kernel name was badly choosen.

I think this might be the help I needed. As mentioned earlier, i have limited time to spend on it. I already started on the gimp-drawable-transform code.
This already went in this direction only the result was not yet up to par.
Comment 15 gg 2006-09-20 19:02:01 UTC
Created attachment 73103 [details] [review]
patch to correct this bug , update to  #9

small changes and edit correction.
Comment 16 weskaggs 2006-09-20 20:33:43 UTC
Hmm.  I tested this by applying the patch from comment #15, building, starting gimp, then running "Image->Scale Image" on a 300x290 RGB image, scaling to make width=400 using Lanczos.  Result: it took about 10 sec for the command to finish -- much longer than before -- and the result looked reasonable except that there were evenly-spaced one-pixel-wide vertical black stripes up and down it.  So I think some work is still needed before this can be applied.
 
Comment 17 gg 2006-09-20 20:56:22 UTC
I also see some lines now I will look into this.

pls try #9, that did not show the prob, unless affected by some cvs changes today. (the odd var names is irrel to function) 

Thanks for testing.
Comment 18 geert jordaens 2006-09-20 21:19:07 UTC
hmm #15

#define LANCZOS_MIN      (1/LANCZOS_SPP)

result = 1 instead of 0.0001 dividing integer by integer not?
Comment 19 gg 2006-09-20 22:46:03 UTC
oops, cheers Geert.

Bill , slow in relation to which "before" . Note that current CVS is using 
#define LANCZOS_WIDTH    (4)

That still seems to be mobile and needs to be looked at in another bug , just make sure you're not comparing this with an earlier build done on 3.

Some optimisation can surely be done once the code actually gives the desired results.

See the attachment I sent earlier the results of this filter are very good now.
Comment 20 gg 2006-09-21 03:26:48 UTC
Created attachment 73122 [details] [review]
corrects #15 slip and reverts to lanczos3

this patch applies after #15 to correct the slip on LANCZOS_MIN and reverts the filter to Lanczos3. This is indeed the cause of the slowdown noted by Bill.

Once the code has stabalised I suggest a discussion on ML about the merits. That will be long and technical so no need to clutter bugzilla with it. Suffice to say that Lanczos3 is almost univerally std and makes a good default for testing.
Comment 21 gg 2006-09-21 03:45:54 UTC
damn, it's getting too late , I need sleep. No point in messing with patches for two trivial edits.

forget the patch, just apply #15 and correct these lines scale-funcs.h  

#define LANCZOS_MIN      (1.0 / LANCZOS_SPP)
#define LANCZOS_WIDTH    (3)

good night.
Comment 22 weskaggs 2006-09-21 15:15:38 UTC
gg,

  Looks like it works now, as far as I can tell.  Since it's okay
with Geert too, I'll commit your changes.  If you would like a credit,
please tell me what name to use -- otherwise the credit will go to
"gg".
Comment 23 gg 2006-09-21 18:37:40 UTC
 
"gg" is fine.
 
I'll give it a while to see if anymore issues comes up and in the meantime I'll see if I can optimise a bit.
 
Expand/resample with Lanczos should now be good.
 
I am well on the way to expanding the convolution matrix to take 7x7 what will allow it do lanczos3 as well.
 
Since applying Lanczos to an already expanded image with pixelisation does nothing in Gimp at the moment, it may be useful to be able to define a scaling factor to remove pixelisation.
 
This is what lanczos is really aimed at doing.
 
Expand/resample with Lanczos should now be good.
 
I would like to add it as a plugin , like blur et al. , where the user can choose the range (frequency) of the pixelisation.
 
I have a partially working prototype for that now. I'll open another bug for that feature.
 
cheers, gg
Comment 24 weskaggs 2006-09-21 19:51:20 UTC
Okay, I have committed the patch from comment #15 with the correction from comment #21.

2006-09-21  Bill Skaggs  <weskaggs@primate.ucdavis.edu>

	* app/paint-funcs/scale-funcs.[ch]:  commit patch from "gg"
	to fix Lanczos problems, addresses bug #167956 and 
	bug #355178.
Comment 25 gg 2006-09-22 00:32:08 UTC
Thanks Bill,

just in case anyone is unsure how much use this filter is try this:
http://tinyurl.com/kg4no

this was an image taken from a digital camera with no processing before the new gimp Lanczos3.

I did an 800% resize using L3 then zoomed the original to the same size on the screen.

Although the quality of the image itself is not brilliant at this magnification, I am astounded by the complete absence any trace of pixelisation on the filtered image. The smooth contours of the original have been cleanly restored.

This just underlines the importance of correct use of this filter. 

Bicubic is as good as an all round smoother but I defy anyone to match that for depixelisation.

Last week I decided I wanted to write a sync filter to see how good it would be for depixelisation. As luck would have it one was in the pipeline. Many thanks to Geert for his excellent work.

I never expected the results to be this good. 

gg
Comment 26 geert jordaens 2006-09-22 18:17:06 UTC
Created attachment 73232 [details] [review]
Patch to fix lanczos interpollation in gimpdrawable-transform

This patch gave good results for me (rotating), I'll do some more tests
for the other transformations
Comment 27 geert jordaens 2006-09-22 19:04:14 UTC
Created attachment 73236 [details] [review]
Fixes lanczos in gimpdrawable-transform.c


To solve the warning
changed :       
read_pixel_data_1 (orig_tiles, pu, pv, &lwin[pos]);  
to :
read_pixel_data_1 (orig_tiles, pu, pv, &lwin[pos][0]);
Comment 28 weskaggs 2006-09-24 16:47:09 UTC
Looks good to me -- I went ahead and committed the patch from comment #27.

2006-09-24  Bill Skaggs  <weskaggs@primate.ucdavis.edu>

	* app/core/gimpdrawable-transform.c: apply patch from
	Geert Jordaens to improve Lanczos performance;
	probably fixes bug #355178.

That probably fixes this bug report, but I'll leave it open a little while for testing.
Comment 29 gg 2006-09-24 18:24:13 UTC
getting some odd results on rotate I just posted on the offset bug since it does not really fit here on the brightening issue. The brightening does seem to have been correctly fixed by my earlier patch.

I think this one can be closed and further issues with offset followed up in the appropriate bug:


http://bugzilla.gnome.org/show_bug.cgi?id=167956
Comment 30 che 2006-09-24 18:39:52 UTC
Current CVS fixes the brightening for me, thanks!
Comment 31 weskaggs 2006-09-24 22:27:08 UTC
Resolving as FIXED on the basis of comments.  Further work on Lanczos should go to bug #167956, or a new bug report if there is a new problem.