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 783755 - Smudge should blend the smudged colors using linear RGB
Smudge should blend the smudged colors using linear RGB
Status: RESOLVED FIXED
Product: GIMP
Classification: Other
Component: Tools
git master
Other All
: Normal blocker
: 2.10
Assigned To: GIMP Bugs
GIMP Bugs
Depends on:
Blocks:
 
 
Reported: 2017-06-13 14:47 UTC by Elle Stone
Modified: 2018-02-12 18:45 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Smudge using perceptual encoding produces gamma artifacts (70.54 KB, image/jpeg)
2017-06-13 14:47 UTC, Elle Stone
Details
heal gamma artifacts (106.40 KB, image/png)
2017-08-12 23:31 UTC, Elle Stone
Details

Description Elle Stone 2017-06-13 14:47:57 UTC
Created attachment 353697 [details]
Smudge using perceptual encoding produces gamma artifacts

Currently the smudge tool is hard-coded to only allow blending of the smudged colors using Normal (Legacy) blend mode. This means that the smudging is done on RGB encoded using the sRGB TRC instead of on linear RGB.

Smudge painting using Normal (Legacy) blend mode produces gamma artifacts, as illustrated by the left side of the attached jpeg. 

The right side of the attached jpeg was produced in a linear gamma version of the sRGB color space (using my patched version of GIMP) and consequently doesn't show any gamma artifacts.

If the user isn't given any choice in the matter, then smudging should be hard-coded to be blended using linear RGB ("Default" Normal blend).
Comment 1 Elle Stone 2017-06-13 16:46:24 UTC
Hmm, if the layer has an alpha channel, the gamma artifacts while smudging disappear. Is this intended behavior?
Comment 2 Michael Natterer 2017-06-13 17:06:35 UTC
I don't think anything is intended here :)
Comment 3 SenlinOS 2017-06-14 01:24:09 UTC
GIMP 2.9.5 new layer, layer blending mode is "Default".
(I forgot, I set up or by default.)
The "Default" layer blending mode is good.
I like using the "Default" layer blending mode.

Just try the "Legacy" mode, and the smudge tool has black edges.
(New Layer-alpha channel. If The bottom layer is white and the black edge does not show.) (But I do not use "Legacy" mode.)
Comment 4 Michael Natterer 2017-08-11 10:27:06 UTC
Ok here is what happens:

When smudging on a layer without alpha, the code falls back to another
painting function that needs a paint mode, we always used NORMAL for
that, now we 1:1 ported that to GIMP_LAYER_MODE_NORMAL_LEGACY.

We should probably use GIMP_LAYER_MODE_NORMAL there and make sure
it blends linearly, because this function is only used by:

Convolve (blur / sharpen)
Dodge / Burn
Heal
Smudge

all of which are not really "painting".

Can you check if they are all affected by the gamma artifact problem?
We can fix them all in once place.
Comment 5 Elle Stone 2017-08-11 15:28:26 UTC
Using 32f "perceptual gamma" built-in sRGB test image, a green rectangle surrounded by red:

1. Convolve blur and sharpen didn't show any difference between layers with and without an alpha channel - no gamma artifacts, but these are perfectly vertical and horizontal edges already, with no area of transition. Checking an area smudged to have soft diagonal lines, there still doesn't appear to be any gamma artifacts with our without an alpha channel.

2. Dodge/Burn shows an obvious difference depending on whether the layer has or doesn't have an alpha channel. I'm assuming "without" (much darker from Burn than "with") is showing gamma artifacts. But I don't use these tools for editing except for modifying layer masks, so I don't know what the "right" results are. Also dodging/burning pure red and green is a bit of an odd thing as each color has two channels at 0 and one channel at 1, so what's a shadow and what's a highlight?

3. Heal shows gamma artifacts with *and* without an alpha channel. 

4. Smudge shows gamma artifacts without an alpha channel, but no gamma artifacts with an alpha channel.

5. Clone shows gamma artifacts with *and* without an alpha channel. You didn't ask about Clone, so maybe clone is not relevant to this bug report, but the gamma artifacts are obvious.

I can check using linear precision, if you think it might make a difference.
Comment 6 Michael Natterer 2017-08-11 17:47:11 UTC
Yes Clone is unrelated, do you have the legacy normal mode selected?

Before we go back and forth twice, can you go to app/paint/gimppaintcore.c
line 990, replace the layer mode by GIMP_LAYER_MODE_NORMAL, and check
if it fixes all the other issues (except clone)?
Comment 7 Elle Stone 2017-08-11 19:06:10 UTC
(In reply to Michael Natterer from comment #6)

> Yes Clone is unrelated, do you have the legacy normal mode selected?
> 
For the layer itself? Or for the tool? Either way, my apologies, but I don't remember. I have a lot of trouble remembering to keep resetting stuff back to "not legacy".


> Before we go back and forth twice, can you go to app/paint/gimppaintcore.c
> line 990, replace the layer mode by GIMP_LAYER_MODE_NORMAL, and check
> if it fixes all the other issues (except clone)?

After the above code change, if there is no alpha channel, then there is the following terminal complaint while trying to use Smudge or Heal (and nothing happens, the tools don't work):

(gimp-2.9:32468): Gimp-Paint-CRITICAL **: do_layer_blend: assertion 'gimp_temp_buf_get_format (paint_buf) == iterator_format' failed

If there is an alpha channel, then Smudge works properly but Heal has gamma artifacts. I didn't check blur/sharpen or dodge/burn.
Comment 8 Michael Natterer 2017-08-11 20:24:43 UTC
Hmm well ok I was expecting more of a "all works fine now" :)

Will look into it.
Comment 9 Michael Natterer 2017-08-12 19:39:47 UTC
This should fix it, please test:

commit b926de5adad6d5b6adf4bfa2b784910aba1f2eeb
Author: Michael Natterer <mitch@gimp.org>
Date:   Sat Aug 12 21:35:47 2017 +0200

    Bug 783755 - Smudge should blend the smudged colors using linear RGB
    
    Use GIMP_LAYER_MODE_NORMAL (not NORMAL_LEGACY) when falling back from
    gimp_paint_core_replace() to gimp_paint_core_paste() for layers
    without alpha. Adapt the format of the used paint buffers accordingly.

 app/paint/gimpconvolve.c  |  2 +-
 app/paint/gimpdodgeburn.c |  2 +-
 app/paint/gimpheal.c      | 36 ++++++++++++++++++++++++++++++++++--
 app/paint/gimppaintcore.c |  2 +-
 app/paint/gimpsmudge.c    |  4 ++--
 5 files changed, 39 insertions(+), 7 deletions(-)
Comment 10 Elle Stone 2017-08-12 23:31:35 UTC
Created attachment 357500 [details]
heal gamma artifacts

I think smudge/convolve/dodge-burn are fine but I'm not 100% sure because there are so many combinations one could try.

The attached png shows gamma artifacts still affect heal.

To be honest, these days I'm having a difficult time testing or using default GIMP for anything more complicated than making screenshots, because there are simply too many "this/that" options:

* The actual operation(s) being tested or used

* Linear vs gamma precision

* The bit depth and type

* Having or not having an alpha channel

* Layer at legacy vs layer at default blend mode, though default blend mode isn't the default and legacy blend mode *is* the default

* The paint tool itself might have legacy vs default blend mode, though I think all the paint tools in this bug report are set to legacy blend mode with no user choice, even though some of them don't produce gamma artifacts any more and so clearly are operating on linearized RGB


I never have been sure what "heal" was supposed to do. But the attached png clearly shows gamma artifacts, except for the "yellow" areas pointed to by the arrows, which show what you should get when you mix green and red.

So I think, but I'm not sure, that convolve, dodge/burn, and smudge now work properly with and without a gamma channel. But heal remains problematic.

It would help a great deal if this whole "legacy" thing disappeared from the user interface, at least as the default for anything. 

Usually legacy means perceptual, but not always. Usually default means linear, but not always. So how is one to know what any given operation is actually doing? 

What's wrong with using "perceptual" for operations working on perceptually uniform RGB, and "linear" for operations working on linearized RGB?
Comment 11 Michael Natterer 2017-08-18 21:59:24 UTC
Please try this, leaving the bug open for now.

commit 94c6bb46030e544f5f9267cd08bdd15fc84fee8a
Author: Michael Natterer <mitch@gimp.org>
Date:   Fri Aug 18 23:54:26 2017 +0200

    Bug 783755 - Smudge should blend the smudged colors using linear RGB
    
    Looked a bit deeper into heal: while I didn't try to understand what
    it's actually doing, this is strange: there is a comment that says
    that healing should done in perceptual space, and the code uses
    R'G'B'A float (at least it completely did before the last commit).
    
    On the other hand, the code adds and subtracts temporary buffers,
    which screams "gamma artifacts" unless done in linear space.
    
    This commit changes everything to use linear float buffers,
    and removes the comment. It "looks" right to me now, please test.

 app/paint/gimpheal.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)
Comment 12 Elle Stone 2017-08-19 11:09:42 UTC
(In reply to Michael Natterer from comment #11)
> Please try this, leaving the bug open for now.
> 
> commit 94c6bb46030e544f5f9267cd08bdd15fc84fee8a
> Author: Michael Natterer <mitch@gimp.org>
> Date:   Fri Aug 18 23:54:26 2017 +0200
> 
>     Bug 783755 - Smudge should blend the smudged colors using linear RGB
>     
>     Looked a bit deeper into heal: while I didn't try to understand what
>     it's actually doing, this is strange: there is a comment that says
>     that healing should done in perceptual space, and the code uses
>     R'G'B'A float (at least it completely did before the last commit).
>     
>     On the other hand, the code adds and subtracts temporary buffers,
>     which screams "gamma artifacts" unless done in linear space.
>     
>     This commit changes everything to use linear float buffers,
>     and removes the comment. It "looks" right to me now, please test.
> 
>  app/paint/gimpheal.c | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)

Hmm, https://docs.gimp.org/en/gimp-tool-heal.html says the point of the heal tool is to remove scratches and blemishes, and references this article:

http://www.tgeorgiev.net/Invariant.pdf

Page 3 of the pdf has an image of some pebbles with a scratch. Using the GIMP heal tool to try to repair the scratch in my "CCE" version of GIMP (which allows to control the TRC by making ICC profile conversions), results are very bad when using linear gamma sRGB, and pretty good when using regular ("perceptual") sRGB. 

Based on the pebble example, the heal tool really should use perceptually uniform RGB. The pebble image is a low-chroma image, so I want to also check some high-chroma images. Some algorithms do produce expected/desired results when done on perceptually uniform RGB, but the red-green gamma artifacts from using "perceptual" sRGB are bothersome.
Comment 13 Elle Stone 2017-08-19 13:50:42 UTC
Looking at the code in gimpheal.c, there is this comment:

 * The method used here is similar to the lighting invariant correction
 * method but slightly different: we do not divide the RGB components,
 * but subtract them I2 = I0 - I1, where I0 is the sample image to be
 * corrected, I1 is the reference pattern. 

And the code does subtract. 

If subtract is used as a measure of similarity, then subtract should be performed on perceptually uniform RGB. This is because when an image is encoded linearly, then in the shadows "visually similar colors" requires RGB values that are much closer together than is the case with "visually similar colors" in the midtones and highlights. 

Given that the GIMP algorithm uses subtraction to calculate the distance between the source and destination colors:

* If the source and destination tonalities are similar, then it doesn't matter much whether the image is encoded as linear or as perceptually uniform RGB, results will be very similar regardless of the encoding. 

* If the source and destintation tonalities are far apart (as using a brightly lit area to heal a dark area or vice versa), then the image needs to be encoded using perceptually uniform RGB.

Trying to "heal" solid green using solid red is a wildly inappropriate way to test the heal algorithm (and as an aside also produces out of gamut colors). My apologies for not reading the documentation and checking the code before complaining about gamma artifacts! But the "heal" code does need to use perceptually uniform RGB.
Comment 14 Jehan 2017-12-04 23:12:33 UTC
Setting to blocker because I guess we want to have this right before release, especially if, as Elle says in comment 12 that the heal tool now performs badly after the changes.
Comment 15 Michael Natterer 2018-02-12 16:29:33 UTC
Back to the old way of smudging, and closing as FIXED. Please reopen
if there are still issues:

commit 6f324b867af50898bfb40abc0f4dcfd83bdc21ea
Author: Michael Natterer <mitch@gimp.org>
Date:   Mon Feb 12 17:27:05 2018 +0100

    Bug 783755 - Smudge should blend the smudged colors using linear RGB
    
    This reverts commit 94c6bb46030e544f5f9267cd08bdd15fc84fee8a, because
    unlike what the bug title suggests, Smudge apparently *should* use
    perceptual RGB, so now everything works as before.

 app/paint/gimpheal.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)
Comment 16 Elle Stone 2018-02-12 17:01:50 UTC
(In reply to Michael Natterer from comment #15)
> Back to the old way of smudging, and closing as FIXED. Please reopen
> if there are still issues:
> 
> commit 6f324b867af50898bfb40abc0f4dcfd83bdc21ea
> Author: Michael Natterer <mitch@gimp.org>
> Date:   Mon Feb 12 17:27:05 2018 +0100
> 
>     Bug 783755 - Smudge should blend the smudged colors using linear RGB
>     
>     This reverts commit 94c6bb46030e544f5f9267cd08bdd15fc84fee8a, because
>     unlike what the bug title suggests, Smudge apparently *should* use
>     perceptual RGB, so now everything works as before.
> 
>  app/paint/gimpheal.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)

This commit message is confusing because it looks like the commit changes *heal* back to using perceptual RGB as it originally did, which is necessary because "heal" uses "subtract" as a stand-in for perceptual difference.

The original bug report was about *smudge*, which to avoid gamma artifacts (and also produce nicer looking results) should work on linear RGB.
Comment 17 Michael Natterer 2018-02-12 17:08:43 UTC
Argh, yes I reverted the wrong commit :)
Comment 18 Michael Natterer 2018-02-12 17:10:21 UTC
Hmm, upon looking closer, I think the same applies to heal, doesn't
it? It's about color similarities, and those similarities are obviously (?)
perceptual...?
Comment 19 Michael Natterer 2018-02-12 17:13:27 UTC
Argh, so this is what happened: I did that revert last night, rebased
it today, and went ahead writing the commit message, writing "smudge"
because of "heal" because that's what the bug title says :)

Maybe before I do any more git mess, can you check if things in
both heal and smudge now work as they should?
Comment 20 Elle Stone 2018-02-12 18:18:13 UTC
(In reply to Michael Natterer from comment #19)
> 
> can you check if things in
> both heal and smudge now work as they should?

Yes, heal and smudge both now work as expected. Smudge should work on linear RGB, and Heal should work on perceptually uniform RGB, and checking with sample images, that seems to be exactly what's happening.

(In reply to Michael Natterer from comment #18)
> Hmm, upon looking closer, I think the same applies to heal, doesn't
> it? It's about color similarities, and those similarities are obviously (?)
> perceptual...?

Well, the thing about Heal is that it copies patterns from one place to another, using the RGB channel difference (subtracting source from destination, or maybe it's vice versa, or the absolute difference? I'm not looking at the code) as a metric for "perceptually uniform distance". Which works OK for perceptually uniform RGB. But it doesn't work for linear RGB, because for the same numeric channel differences "delta_r,delta_g,delta_b", in linear RGB this difference is *perceptually* a much greater distance in the shadows than it is in the highlights.

Smudge is different because Smudge is about blending colors. So for example smudging red and green together should produce a yellow hue that's midway in tonality between red and green. When done on perceptually uniform RGB the result is gamma artifacts, which too much darkens the blended colors.
Comment 21 Michael Natterer 2018-02-12 18:45:05 UTC
Ok thanks a lot for double checking, so it was the right revert after all,
closing as FIXED for good now :)