Draft: Fix artifacts on transform mask (free and maybe perspective)
Bug detected on this thread in KA https://krita-artists.org/t/krita-transform-tool-produces-aliased-edges/87883
Bug report: BUG:484677
It seems that when using free transform and maybe perspective transform in the transform mask, if the source device position is <= 0 in x or y then there are some aliasing artifacts in the fast path of the accurate preview on the top/left edges (to recreate the situation, add an empty layer, fill it completely with a color different from the background one, add a transform mask, and free transform by scaling down and rotating). This is even more noticeable when the change to the high quality transformed image happens, since that is not aliased (maybe due to being computed by the KisTransformWorker
instead of the KisPerspectiveTransformWorker
).
First I thought it was just the bilinear filtering on KisRandomSubAccessor
, since the aliasing happens only on the left/top edges. Maybe there was some issue with negative coordinates. Well, there was an issue, but not specific to negative coordinates. For some fractional coordinate, say (4.25, 5.63)
, the bilinear filtering there floors the coordinate and then always uses the pixel at that location plus the right/bottom/bottom-right pixels. So, using the example coordinate, the pixels involved would be: [current] (4, 5)
, [right] (5, 5)
, [bottom] (4, 6)
and [bottom-right] (5, 6)
. This causes some miss-alignment/offset with respect to the true transform. If a coordinate's fractional part is less than 0.5 then the left/top pixels should be used. So for the example point the pixels involved on the bilinear filtering should be: [left] (3, 5)
, [current] (4, 5)
, [bottom-left] (3, 6)
and [bottom] (4, 6)
. Example image (left column, previous; right column, fix. The red dot indicates the fractional coordinate):
This get rid of the aliasing and shifting issue, but not totally, there was still some offset. So I looked into how the coordinates are mapped from the transformed space to the original space. I found that when transforming the points in KisPerspectiveTransformWorker::runPartialDst
from dst points to src points, the integer coordinated are used, so this can also introduce some shifting. What I did was changing it so that what are transformed are the dst pixel centers. This aligned the result completely in my opinion. Now the "anti-aliased" edges are totally aligned and symmetric with respect to the decoration box lines. Using a flat rectangle that fills all the parent layer, when transformed by the mask, the edges are mixed with transparent default pixels, and the value of the mixed transparency seems to correspond with the coverage of that pixel (intersected with the transform decoration cage).
But there was another issue. The coordinate mapping may map some points outside of the src rect. That should be allowed because even if a point is mapped to (1.8, 2.3)
and the rect is (x=2, y=2, w=4, h=4)
the dst pixel should be colored because a pixel inside the rect contributed to it (pixel (2, 2)
) in the bilinear filtering. So I added some extra pading to the src rect in runPartialDst
. This seemed to completely fix the issue:
I also added the fix to KisPerspectiveTransformWorker::runImpl
and m_srcRect
but I'm not sure how well that would work. In perspective it is harder to evaluate the result by eye due to naturally being more aliasing when squashing the image. It seems to work, but it also seem still to be some discrepancy between runPartialDst
and runImpl
(some shifting).
The same type of discrepancy seems to happen with affine transforms between runPartialDst
and KisTransformWorker
. This was present before, but due to the artifacts it was harder to analyze, although it was visible on the right/bottom edges. Now, there is still a small shift between the 2 (this is mostly noticeable using bilinear filtering, since it is what is used by runPartialDst
, but if one look carefully, it can be seen that it happens with the other filters as well, having the cage decoration as reference). I think that KisTransformWorker
maps the coordinates a bit off, maybe similar to how runPartialDst
did it. And given that when executing KisPerspectiveTransformWorker::runImpl
it seems to also happen, maybe the issue is before those are called, computing some rect:
Formalities Checklist
-
I confirmed this builds. -
I confirmed Krita ran and the relevant functions work. -
I tested the relevant unit tests and can confirm they are not broken. (If not possible, don't hesitate to ask for help!) -
I made sure my commits build individually and have good descriptions as per KDE guidelines. -
I made sure my code conforms to the standards set in the HACKING file. -
I can confirm the code is licensed and attributed appropriately, and that unattributed code is mine, as per KDE Licensing Policy.
Reminder: the reviewer is responsible for merging the patch, this is to ensure at the least two people can build the patch. In case a patch breaks the build, both the author and the reviewer should be contacted to fix the build. If this is not possible, the commits shall be reverted, and a notification with the reasoning and any relevant logs shall be sent to the mailing list, kimageshop@kde.org.