PintaProject / Pinta

Simple GTK# Paint Program
http://www.pinta-project.com/
MIT License
1.74k stars 267 forks source link

When pixels are moved, they get the layer opacity remultiplied into them, and their opacity is used to blend them into the target location in the same layer #819

Open logiclrd opened 1 month ago

logiclrd commented 1 month ago

Description If you select a set of pixels in a layer, the Move Selected Pixels tool can be used to change their position within the layer. When the move operation completes and the pixels are deselected, they are recomposited into the layer. But, if the layer's opacity is not 100%, the pixels which were moved end up having the layer opacity remultiplied against them as part of the commit operation.

Similarly, if the pixels themselves are not 100% opaque, that opacity is used to blend them back into the layer they're being moved within. Committing a move should simply overwrite the pixels -- including the opacity, so that they bring the opacity they had where they were moved from but don't get blended back into their layer.

To Reproduce

  1. Create an image.

image

  1. Paste another image in as a new layer.

image

  1. Set the layer opacity to 50%.

image

  1. Select a region of pixels in the layer.

image

  1. Using the Move Selected Pixels tool, move the pixels to another location.

image

The problem is already apparent here, because the moved region is transparent and the part of the layer behind where the pixels have been moved to can be seen through the selection.

  1. Deselect the pixels to commit the move operation.

image

The pixels have been cut from the layer and then pasted into the new location, but the layer's opacity was included in the cut and applied in the paste. The destination for the paste, though, is the layer which has that opacity as a persistent property as well, so now the opacity is applied to those pixels twice. In addition, the newly-pasted pixels show through what was beneath them in the layer. They should have simply replaced the pixels in the layer. With respect to the paste operation, they should be 100% opaque.

Version Operating system: Ubuntu 24.04 Pinta version: 2.2 built from 60cabb813f8397fc9a2b4ab946ce373259d550db

logiclrd commented 1 month ago

This issue happens with alpha in the actual pixels as well, not just layer alpha. I set up an image with this configuration:

image

The layer has a soft circle erased out of the middle, so that the pixel alpha drops to 0.0 in the very centre, but is still 1.0 at the edges. When a selection is made and pixels are moved, this happens:

https://youtu.be/FMG2FIWJRxE

This is what should happen:

https://youtu.be/b3I25k-Q5Os

The pixels should take their alpha with them, but should not be blended when the move operation is completed.

(The "alpha good" demonstration was faked by simply merging the layer down into a solid white layer, so that the pixels don't actually have alpha any more, but it should look that way with the layer configured as in the image at the top of this comment too.)

cameronwhite commented 1 month ago

I think you might want to use the Cairo.Operator.Source blend mode when flattening the selection back on to the layer? That should just replace the pixels rather than blending them with what's underneath

(https://www.cairographics.org/operators/)

logiclrd commented 1 month ago

That seems reasonable, though fixing this is going to be more than a one-line fix I suspect. During the drag operation, if I'm understanding correctly, the selection layer is modelled as just another layer, composited on top of the layers that are part of the image. It simply can't work that way with respect to alpha. You would need to have some sort of hierarchical layer system.

Let's say you have two layers, layer 1 background and layer 2 with some shape with partial translucency on it. If you select some of the pixels in layer 2 and start moving them, you now need to have a separate stack of layers within layer 2 that represent the selection overlaid onto the remainder of layer 2's pixels. This operation preserves the full RGBA for every pixel but doesn't process alpha at all, simply replaces the alpha in pixels. Then, the result of that compositing is the pixels used for layer 2 in the main stack of layers, and there it does get composited with full alpha processing. With this scheme, layer 1 shows through where layer 2 is translucent, but within layer 2's processing, there is no alpha interaction at all with the selection that's being moved -- but that selection still keeps its alpha channel data. Does that make sense?

cameronwhite commented 1 month ago

Yeah I think that makes sense to me. Probably a good first step would be to fix the final update of the layer so at least the final result is as expected, even if the preview is a bit off

logiclrd commented 1 month ago

Could you point me at where that takes place? :-)

cameronwhite commented 1 month ago

Could you point me at where that takes place? :-)

I think the final write back will happen in Document.FinishSelection ()

logiclrd commented 1 month ago

Thanks, I'll take a look.

cameronwhite commented 2 weeks ago

This got auto-closed by the PR, but I think we still want to keep this open for getting the correct blending before the selection is committed

logiclrd commented 2 weeks ago

Makes sense :-)