GNOME Bugzilla – Bug 128118
Apply layer mask by comparing instead of multiplying
Last modified: 2018-05-24 10:58:04 UTC
First, some introduction: When editing an image with transparent areas, there are several indirect ways to modify the alpha channel: using the eraser, adding a mask and drawing in it, creating a selection (maybe using the quick mask) and then clearing its contents, etc. Using the mask as a way to edit the opacity of the image is useful because many operations can be applied to that mask, including plug-ins, and it is easy to toggle between viewing the image and viewing the mask. Viewing the mask can be very useful because it shows precisely what areas are made transparent by the mask, without being disturbed by the features present in the image. However, this does not work as well after the mask has been applied: even if it is possible to re-create the mask by copying it from the alpha channel, applying it to the image later will not produce the expected result because the mask and the alpha channel will be multiplied. As a result, the areas that were partially transparent before will be made more transparent even if they were not edited. This multiplicative effect is useful in some cases, but usually not when one creates the layer mask from the alpha channel. Current (but bad) solutions: A known but tedious workaround to this problem is to use the eraser in "anti-erase" mode and "un-erase" the partially transparent areas of the picture before applying the mask, so that the multiplicative effect is avoided. This is not an elegant solution, because it relies on a hack in the eraser. Besides, "un-erasing" large areas of the picture takes time and is error-prone. Another solution has been proposed in bug #127930 by Pedro Gimeno: add a new option for creating the layer mask. Allow the alpha channel to be "transferred" to the layer mask. So besides creating a mask from the alpha channel, it also resets the alpha channel to full opacity, so that the problems with the multiplication will be avoided when the mask is applied later. However, this presents some problems, because the user could also decide to discard the layer mask, or to make some fully transparent areas opaque. What happens then is that some internal data (the "color" of fully transparent pixels) is revealed to the user. The image would then contain arbitrary data, depending on how it was created or from which file format it was loaded. This is bad because it reveals some random internal data, but also because some users could start relying on this and could complain if we ever try to optimize the handling of transparent pixels in a future version of the GIMP (maybe via GEGL). A better solution (IMNSHO): Instead of having a special case for creating the mask, let's change the way the mask is applied to the image: use a comparison instead of a multiplication. In pseudo-code: for each pixel: { if opacity(mask) < opacity(A) then opacity(A) = opacity(mask) } So any part of the mask that has been edited to reduce its opacity will be transfered to the alpha channel. Any part that has not been edited will be left unchanged. This avoids the drawbacks of the default way to apply the layer mask (which performs the unconditional operation: opacity(A) = opacity(A) * opacity(mask)). In fact, this comparison is similar to combining grayscale layers with the "Darken Only" mode. Advantages: - allows easy editing of the alpha channel via the layer mask - does not require any hacks relying on "un-erase" - no problems if the user discards the layer mask The code: Well, I created a patch two days ago, but I think that I was a bit too ambitious because I tried to have several ways to apply the mask, like the different modes for combining layers. But after getting lost for a while trying to propagate the right modes to the paint core, I decided that it was the wrong way to go and I started on a simpler track involving only two ways to apply the layer mask: "Apply (multiply)" and "Apply (compare)". This also made the menus simpler and easier to understand. I worked on this yesterday, but I still have to finish the patch and test it. I will attach it to this bug report as soon as possible (hopefully this evening, maybe tomorrow) but I am opening this bug report already now, in order to have a good reason to ask for the patch in bug #127930 to be reverted (even if the patch is nice, I think that it solves the wrong problem and introduces several new ones).
That is IMO the wrong approach. Instead there should be a toggle button that allows me to replace the alpha channel with the mask when it is applied. That would be a lot easier to code, easier to understand and it wouldn't change existing behaviour.
I don't think so. If you simply replace the layer mask, then you will have the same problems with fully transparent areas that may be made opaque by accident, revealing unwanted internal data. Also, it will have the same problem as I just described in bug #127930: if you start painting the mask in black to make some areas transparent and then want to correct this by painting over it in white, then you will have to be very careful when painting close to the previous edge of the transparent area. It will be almost impossible to restore any smooth edges that were there previously. You do not have this problem with the solution proposed above, or if you start a new mask from scratch. In fact, the problem is that some people get used to one way of working with the masks: paint freely in black or white, and correct mistakes with the opposite color. This is easy and this works very well when starting with a new mask. But this does not work with the solution proposed in bug #127930 or with any solution that just replaces the alpha channel without taking into account what it contained previously.
We can not possibly do this change because it changes existing behaviour in a completely unforseeable way and I expect that quite some scripts will break. I strongly suggest to close as WONTFIX.
Please read my description carefully: I am proposing to add a new way to apply the mask. As I wrote above, there would be: - Apply (multiply) - Apply (combine) This will not break any scripts and this would let the user select what works best for them.
Another note: depending on how you see it, the way I propose to apply the mask (comparison instead of multiplication) could be seen as the "replace" mode that you are proposing, except that it does not create pixels where there was nothing before.
I agree with Sven. This seems to change behaviour in a way which is quite unintuitive, and not at all like the current behaviour. If I understand correctly, what is projected in the image window before applying the layer mask will be different than after applying the layer mask. Since applying a layer mask is (and should be) a simple merging of a mask into the alpha channel, this seems wrong to me. Thanks for the proposition (and the work) Raphael, but I feel that we should perhaps be spending as much time as possible bug-fixing now, and not make any drastic interface changes. Cheers, Dave.
It does not change behaviour, it adds a new option, as I just mentioned in my comment to Sven. And if I would agree with your last sentence, I would say that the patch that was so quickly applied in bug #127930 has more reasons to be rejected: it does change the user interface by adding a new option in the mask creation menu (and it has side-effects, contrary to the other options in the same menu) and it only takes two simple operations to see that it introduces new problems: "transfer" the layer mask, then discard it. Although I think that it would be a pity, I wouldn't mind if the discussion about *both* new features (bug #127930 and this one) was postponed until 2.2, but accepting one and not the other does not really make sense from my point of view.
This feature has been discussed in #gimp for a couple of months. It came up several times and everyone agreed that it is a good thing to add this additional action. It is additional, it doesn't change anything. Adding a new mode to apply the mask could make sense and it should be considered, but I don't see why the change in bug #127930 should be reverted.
Bolsh is absolutely correct here. Applying a layer mask must not change the projected image. That's a principle that we stick to for too long to change it. Especially not a few days before a major prerelease. We could leave this report open for discussion but considering this change for 2.0 is not an option.
Regarding the discussion in #gimp, I was unfortunately not part of it. The only thing that I saw was on the developer's mailing list, and no consensus was reached there. The reason for reverting the patch in #127930 is that it introduces new problems that could certainly be filed as bugs. It is really worth trying to experiment with different files that you load and then discard the alpha channel (transfer to mask, then discard mask) or try to paint in the mask alternatively with black and white and see how difficult it is to correct any mistakes (if it is too late for undo). Really, try it honestly: you will probably see that it is harder than you think to work correctly with this. That's why I said above that if you think that we should discuss this further, then it would be better to revert the previous patch before the problems propagate further. Another, more general, problem is that revealing the internal data is wrong. The "anti-erase" mode of the eraser does that already, and it is useful sometimes to "un-erase" some parts of the image that you have erased by accident but that you do not want to undo as a whole. This is useful, but this is the wrong solution to the problem: other programs have invented the "undo brush" for this. It allows some small parts to be corrected without having to undo everything. Implementing the undo brush would reduce the importance of the hack offered by the anti-erase mode. As there is some hope to have a better solution to that problem without side-effects, I think that it would be the wrong time to introduce another wrong solution to a similar problem with layer masks. Hmmm... Let's try to calm down this discussion a bit... Before implementing the patch described above, I thought about another solution that would avoid the problems associated with bug #127930. I rejected it in favor of the solution described above, but maybe you would like it. Here is how it goes: use the same principle as in Pedro's patch (transfer alpha channel to layer mask), but do not discard the previous alpha channel. Instead, keep a temporary copy in memory and use that as a constraint for the layer mask. There would be two ways to apply this constraint: either the painting operations would be prevented from increasing the opacity beyond what was in the old alpha channel, or all painting operations would be allowed but the mask would be automatically compared with the previous alpha channel when applying it. I didn't like these solutions because the first one is too complex and would require some modifications comparable to what happens when a selection is active, but only in one direction. The second one is less complex but still involves some "hidden" data (the old layer mask) and this does not feel right. So I thought that the solution presented above was the best compromise because it was simpler. But the other options can also be a basis for discussion, if you think that they would be better.
Raphael, could you try to write less? It is almost impossible to read the amount of text you are writing. I am certainly not willing to spend that much time on this and I know that others think the same and will not take part in this discussion simply because of the shear amount of text you create. Discarding the layer mask is a destructive operation and has always been. Thus, I cannot follow your argumention. I also don't follow you on the track that there's internal data that shouldn't be unveiled.
oh, well, I know exactly what he is taking about as far as internal data goes. The "internal" data is the color associated with totally transparent pixels, which, in many cases, is uninitialized, and so contains garbage. I do remember the previous flame war about this topic, the behaviour of alpha and the desire to have an unerase brush. I totally agree with Raphael here. The unerase should go away and be replaced with an undo brush that does more or less the same thing. A pixel with 0 alpha has undefined color (in the same way that value=0 leads to an undefined hue and saturation) and should probably be filled with the background color, or black, if necessary. An undo brush would fill transparent pixels with whatever defined color was there before, and totally transparent pixels should be filled with a background color only. Or, we could have a shape channel and a transparency channel, and insist that everywhere the shape is non-zero, the color must be defined. This is really the best solution, IMO, and is what I was getting at during that flame war. This is totally irrelevent for the current discussion.
The problem is quite simple. My memory is now jogged as well. There are two ways to consider transparent pixels - either when someone makes a pixel transparent, they are deleting it, or they are hiding it. If they are deleting it, it is invalid, and should not be restored. If they are hiding it, then an alpha channel and a layer mask are conceptually identical. The problem is that if we assume the former situation in any operation, then we break things for people who expect the second operation (hiding). Whereas exposing random data that's not supposed to be exposed doesn't break anything. I favour the alpha channel as a hiding mechanism. I think that treating pixels with alpha 0 as undefined in the RGB channels is wrong. Dave.
The alpha channel should not be considered as a hiding mechanism, or at least not always. There is a significant difference between the pixels that were made transparent during the current editing session and those that were loaded from disk. For the former, one could see the alpha channel as a hiding mechanism (although an undo brush would be a better way to un-erase these pixels). But for the latter, the pixels were never there, so the concept of "hiding" does not apply. This is what prompted me to ask for the new feature in bug #127930 to be removed, because it can expose RGB values of pixels that should not exist. I will post a slighly better explanation of the differences and their consequences (depending on file formats, etc.) to the gimp-developer mailing list in a few hours. I might also attach the patch here if I have enough time to test it thoroughly. I did a cvs update and I am waiting for the stuff to compile. Anyway, I suggest that we exchange a few messages on the list and see if there is any consensus about how alpha should be handled before adding more comments to this bug report. (And I am sorry for not replying earlier. Work and real life got in the way: I had to travel abroad and my trip was shifted earlier than expected, on short notice.)
Created attachment 22466 [details] Operate directly on alpha?
Noone worked on this for a while so it's probably not going to be 2.2 feature. Bumping off the 2.2 milestone.
-- GitLab Migration Automatic Message -- This bug has been migrated to GNOME's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/gimp/issues/58.