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 764552 - Painting using LCH blend modes produces solid squares
Painting using LCH blend modes produces solid squares
Status: RESOLVED FIXED
Product: GIMP
Classification: Other
Component: Tools
git master
Other All
: Normal normal
: 2.10
Assigned To: GIMP Bugs
GIMP Bugs
Depends on:
Blocks:
 
 
Reported: 2016-04-03 16:49 UTC by Elle Stone
Modified: 2017-02-14 16:18 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Solid squares when using an LCH blend mode (209.80 KB, image/jpeg)
2016-04-03 16:49 UTC, Elle Stone
  Details
quick hack (3.74 KB, patch)
2016-04-04 07:06 UTC, Massimo
none Details | Review
proposed patch (7.36 KB, patch)
2016-04-04 13:45 UTC, Massimo
reviewed Details | Review

Description Elle Stone 2016-04-03 16:49:08 UTC
Created attachment 325270 [details]
Solid squares when using an LCH blend mode

Painting using LCH blend modes sometimes produces solid squares instead of whatever pattern the brush tip actually has. This might be a recent issue as I'm sure I've painted using LCH blend modes in the past. GIMP commit c3c2e29 shows this problem. 

I'm not sure what exact combination of layers/etc triggers the problem because it doesn't happen immediately. But once it gets triggered, it persists across closing and opening the image, closing and opening GIMP, and across prefixes (that is, closing the image in GIMP installed in one prefix and opening the image in GIMP installed in another prefix).
Comment 1 Elle Stone 2016-04-03 16:55:02 UTC
Removing the alpha channel seems to allow to paint without producing just squares of color. Adding the alpha channel back in restarts the painting of squares.
Comment 2 Elle Stone 2016-04-03 17:27:46 UTC
I was planning to file a separate bug report on the smudge tool, as smudging across out-of-gamut channel values results in the immediate clipping instead of smooth blending of the out-of-gamut channel values. Plus smudging over out-of-gamut channel values creates square edges at the start of the smudge-brushed area.

But it turns out that removing the layer alpha channel allows to smudge across out-of-gamut channel values without any clipping or square-edged "smudging". 

So there seems to be an unwanted interaction between painting and alpha channels.
Comment 3 Massimo 2016-04-04 07:06:54 UTC
Created attachment 325304 [details] [review]
quick hack

It is an invalid assumption I made. The paint engine is passing
the same buffer for input and output and so it is wrong to store
there intermediate values because the input is still used later.

I will check later whether the same problem affects other
LCH based layer modes
Comment 4 Massimo 2016-04-04 13:45:49 UTC
Created attachment 325342 [details] [review]
proposed patch

The problem affects painting on layers with alpha
channel of images whose precision is floating point
for all lch layer modes as it was written in the 
original report.
Comment 5 Elle Stone 2016-04-04 14:45:57 UTC
I applied the patch and so far I haven't been able to trigger the painting of squares for the LCH blend modes even when the layer has an alpha channel. Many thanks!

Should I file a separate bug report for the Smudge tool?
Comment 6 Massimo 2016-04-04 17:29:14 UTC
(In reply to Elle Stone from comment #5)
> I applied the patch and so far I haven't been able to trigger the painting
> of squares for the LCH blend modes even when the layer has an alpha channel.
> Many thanks!
> 
> Should I file a separate bug report for the Smudge tool?

Well, I'd say it is unrelated and so yes.

I've rapidly looked for the first time to smudge tool
code and (it's code I don't know) I didn't find anything
evidently wrong.
Comment 7 Elle Stone 2016-04-04 18:04:04 UTC
(In reply to Massimo from comment #6)
> (In reply to Elle Stone from comment #5)
> > I applied the patch and so far I haven't been able to trigger the painting
> > of squares for the LCH blend modes even when the layer has an alpha channel.
> > Many thanks!
> > 
> > Should I file a separate bug report for the Smudge tool?
> 
> Well, I'd say it is unrelated and so yes.
> 
> I've rapidly looked for the first time to smudge tool
> code and (it's code I don't know) I didn't find anything
> evidently wrong.

Here's the bug report: https://bugzilla.gnome.org/show_bug.cgi?id=764608

I also looked through the code. But I was looking for clamping code because I hadn't realized that the issue disappears if the alpha channel is removed.
Comment 8 Michael Natterer 2016-04-05 13:46:13 UTC
Review of attachment 325342 [details] [review]:

Do we really want to allocate that temp buffer instead of just
allocating it on the stack (g_alloca)?
Comment 9 Massimo 2016-04-05 15:43:15 UTC
(In reply to Michael Natterer from comment #8)
> Review of attachment 325342 [details] [review] [review]:
> 
> Do we really want to allocate that temp buffer instead of just
> allocating it on the stack (g_alloca)?

I did not want to, in fact I made sure the op classes are

->want_in_place = FALSE

but the paint engine is not using that flag.

I did not opt for g_alloca because the _pre_process
functions are already using a variable length array
and the allocation happens only when painting with
layer formats "RGBA float" or "R'G'B'A float". The 
projection does not pay that cost.

Anyway feel free to clean it as you like
Comment 10 Michael Schumacher 2017-02-14 13:19:11 UTC
Comment on attachment 325342 [details] [review]
proposed patch

Per comments.
Comment 11 Michael Natterer 2017-02-14 15:03:31 UTC
Everything here has changed, does this still happen at all?
Comment 12 Elle Stone 2017-02-14 15:52:17 UTC
I did some testing using GIMP as of this AM, trying to replicate the squares in attachment 325270 [details]. 

I wasn't able to trigger the "squares" while painting with any of the LCH blend modes. So maybe this doesn't happen any more.
Comment 13 Michael Natterer 2017-02-14 16:18:38 UTC
Thanks, I guess this got fixed by the complete rewrite all of that
code has seen.