GNOME Bugzilla – Bug 136702
Rotations with interpolation introduce blurring
Last modified: 2004-03-13 17:47:10 UTC
When rotating an image with interpolation active, some blurring is introduced. See http://perso.wanadoo.es/p.g.f/temp/hair.png (original), http://perso.wanadoo.es/p.g.f/temp/hair_withSS.png (which is what is happening now) and http://perso.wanadoo.es/p.g.f/temp/hair_noSS.png (was created with a modified version of GIMP that disables the patch applied in bug #109817). Just in case the images above are unavailable, I'm attaching a test image which, when rotated by 0.01 degrees, becomes blurred and gets a half-transparent border (enough for test purposes).
Created attachment 25422 [details] Test case for this bug (png image)
Created attachment 25423 [details] Result of rotating the above by 0.01 degrees. Note the half-transparent borders.
N.B. The testcase image needs zooming in to properly see the result.
I don't think that the patch for bug #109817 should be unconditionally reverted. Some of its effects such as the introduction of the half- transparent border are useful, for example when you are trying to make a composition from several objects (in different layers) and you want to have smooth edges. But I agree that the lack of sharpness can be a serious drawback in some cases. What about making the supersampling optional? By the way, I once saw the full version of the "hair" picture. The comment attached to it was "I don't know how it works, but if you concentrate for a few minutes on this image, you will see a waterfall in the back." Of course, you have to see the full version to understand why. ;-)
I'm not saying that it should be just reverted; indeed I'm all for including the option. I'd like to see it implemented just as in the blend tool, with an adjustment for level and another one for threshold (assuming the supersampling is adaptive). However it's a bad timing for that. I suggest to revert the patch to bug #109817 for 2.0 and add it again with a supersampling option to the tool options for 2.2. I think that the patch also needs to be revised, since the algorithm is implemented in 16.16 fixed point using ints and it overflows easily (see bug #128594). A similar effect can be obtained by offsetting the layer by 0.5 pixels (currently doable with the Displace filter using a white image as displacement source). Note that repeated application of a 0.5 offset degrades (blurs) the image at each step. With respect to the image, it's been just cropped to center the attention in the problematic part but yes, that's the one :)
You are right: adding a new option is not appropriate for 2.0. This should only be done in 2.0.1 or 2.2. So we basically have to decide if we keep the current RECURSION_LEVEL value of if we set it to 0 for version 2.0 (this is probably better than reverting the whole patch). There are pros and cons in both cases. We should collect some opinions about this. I suggest that those who are interested in this perform a few tests by rotating and (down)scaling some images, and compare the results with and without the supersampling (using two different versions of the GIMP). Applying more than one transformation on an image is probably a good test.
Currently there's a huge change in behaviour between scaling slightly up (which works as always) or slightly down (which introduces the blurring reporting here); thus I think that RECURSION_LEVEL should be set to zero and bug #109817 be reopened. Making the supersampling optional would be the proper fix for 109817.
Created attachment 25583 [details] Summary of several tests with GIMP 1.2 and 1.3/2.0. Composite PNG image, 600x1000, 815 KB.
After running several tests, the results are quite clear: for most cases, the supersampling introduces undesirable blurring. The (large) image attached above shows the results of several rotation tests. The test image is rotated by +15 degrees, then rotated back by -15 degrees. The results in the left column were obtained with GIMP 1.2 (or with GIMP 2.0 if the recursion level is set to 0). The results in the right column were obtained with GIMP 1.3/2.0 (current CVS). The differences are obvious, so the best solution for 2.0 is to set the recursion level to 0. 2004-03-12 Raphaël Quinet <quinet@gamers.org> * app/core/gimpdrawable-transform.c (RECURSION_LEVEL): Set to 0 in order to avoid nasty blurring effects. Fixes bug #136702, but re-opens bug #109817. This is the best compromise for 2.0. Later, we will have to make the supersampling optional as suggested in the original patch to bug #109817.
Hmmm... I should have posted a link to that large image instead of attaching it directly to this bug report and wasting some of Bugzilla's precious resources. I will keep that in mind for next time...
Rotating back and forth is not really a common use case. I am not convinced that it is a good idea to set the recursion level to 0. I think for the common case it should have been left at 3. Also it would be a trivial task to add a user interface for this setting. IMO the change should be reverted and instead a toggle or a scale should be introduced that allows to control this setting.
I rotated back and forth in order to have an easy comparison with the original image, but you can already see the results with a single rotation, as Pedro has shown in his first comment. In fact, I haven't found a single example of a rotated image that looked better with the recursion level set to 3 than with no recursion (I tried various images, photographic and cartoonish). The problem is that the supersampling is applied to all affine transforms. For almost all rotations and scaling with a ratio >= 1, the recursion level 3 produces a blurred result that is visibly inferior to the results produced by GIMP 1.2. If you use the scaling or perspective tool for shrinking an image, the supersampling may produce better results (but not always - this seems to depend on the scaling factor and type of image). But these cases are relatively rare compared to the larger number of cases in which the result appears blurred. So I think that the best solution for the moment is to disable this feature until we can find a better solution. In a future version, we could try to make the supersampling optional or configurable by the user. It may also be possible to find some heuristics that set the best values depending on the scaling factor, dimensions of the image, contrast, etc. But this requires some experimentation, so I don't think that we can provide a good solution right now.
Couldn't we just add a "Supersample" toggle to the transform tools? A scale to control the level of supersampling might be overkill but we could easily add a toggle.
Well, yes we could add that option (off by default). But documenting this option will not be easy. Also, we are supposed to be in a kind of feature freeze so I am a bit reluctant to add this just before a major release. But you are the judge in this case, so if you think that an option would be better and would be needed in 2.0, then I could try to work on it tomorrow.
It would mean adding a property to GimpTransformOptions and connecting a property widget to it. That can hardly break anything and I think it would be useful. After all there was a reason to introduce supersampling to the transform tools.
OK. I added the new property "supersample" and told the tools to use it. It defaults to FALSE for all tools, but the user can easily toggle the "Supersample" check box in the tool options for any transform tool. I did not want to change the PDB API (it is already missing the interpolation_type and clip_result parameters), so I decided to set "supersample" to FALSE for rotate and shear and to set to TRUE for perspective, scale and transform_2d. This seems to give the best results even if supersampling may not be desirable when scaling up. But we would need to change the PDB API for that. 2004-03-13 Raphaël Quinet <quinet@gamers.org> * app/tools/gimptransformoptions.[ch]: added new "supersample" property to GimpTransformOptions and added corresponding check button in the option dialog for the transform tools. * app/core/gimpdrawable-transform.[ch], * app/core/gimpdrawable.c, * app/tools/gimptransformtool.c: new "gboolean supersample" parameter added to gimp_drawable_transform_tiles_affine() and gimp_drawable_transform_affine(). * tools/pdbgen/pdb/transform_tools.pdb: ditto. For the PDB calls, the supersample parameter is set to FALSE for "rotate" and "shear" and set to TRUE for "perspective", "scale" and "transform_2d". * app/pdb/transform_tools_cmds.c: regenerated. The new "supersample" option lets the user decide if the transformations should use supersampling (RECURSION_LEVEL 3) or not. This fixes both bug #136702 and bug #109817. Hopefully for good, this time.
Unfortunately this fix is incomplete. It changes the function signature of gimp_drawable_transform() which is an implementation of GimpItem::transform. There's a compiler warning that could have told you about this problem. As a result, transformation of linked items lead to a crash. Mitch is looking into this and I think we will have this fixed soon. Please withstand from reverting the change or touching the code w/o coordinating with Mitch. I am reopening this report, setting severity to blocker so that it appears on the list of bugs to close before 2.0.
There qre too many warnings printed during the compilation of other functions, so I missed this one. Anyway, I think that I fixed this problem in my local tree. I added the new parameter "supersample" to gimp_item_transform() and updated the related files (gimplayer.c, gimpitem-linked.c, gimptransformtool.c). Can I commit or should I wait for a signal from Mitch?
There are many warnings? What warnings? I can count the warnings from a GIMP compilation on one hand. There shouldn't be any but we added some warnings to remind us of open issues... No, please don't commit. Mitch has fixed this more thoroughly already and I expect him to commit his changes soon.
Sorry, too late ;) Fixed it and half-way addressed the configureable-recursion-level issue. We can easily decide at any time now to have this option in the GUI without changing too much code: 2004-03-13 Michael Natterer <mitch@gimp.org> Completed the fix for bug #136702: * app/core/gimpitem.[ch]: added "gboolean supersample" and "gint recursion_level" to GimpItem::transform(). * app/core/gimpitem-linked.[ch] (gimp_item_linked_transform): ditto. * app/core/gimpdrawable-transform.[ch]: added "recursion_level" parameters and removed the RECURSION_LEVEL #define. * app/core/gimpchannel.c * app/core/gimpdrawable.c * app/core/gimplayer.c * app/vectors/gimpvectors.c: changed accordingly. * app/tools/gimptransformoptions.[ch]: added new property "recursion_level" which is not serializable and has no GUI. Pretty useless, but it's IMHO better to hardcode the default value here than in gimpdrawable-transform.c * app/tools/gimptransformtool.c: changed accordingly. * tools/pdbgen/pdb/transform_tools.pdb: hardcode "recursion_level" to 3. * app/pdb/transform_tools_cmds.c: regenerated.