[asimage] Fix filled shapes drawn with a semi-transparent colour - #23023
[asimage] Fix filled shapes drawn with a semi-transparent colour#23023tekinertekin wants to merge 2 commits into
Conversation
|
Question - is this code generate with AI-assist? But if you find a solution - it is really grate! |
468737b to
90c3b2c
Compare
Thank you very much for your interest @linev. Yes, AI was used as a helper during both the fix and the test. I fully understand the code and accept responsibility for it. I have added an Assisted-by: line to both commit messages and force-pushed. I traced the root cause. I compiled and measured Again thanks a lot for checking and helping. |
90c3b2c to
d7e86df
Compare
libAfterImage builds a filled shape by drawing its outline into a scratch canvas, flood-filling the interior, and only then merging the scratch into the image. That merge already applies the brush alpha, because it hands the scratch value to alpha_blend_point_argb32() as the blend ratio -- but the colored tool and fill functions were scaling what they wrote by the same alpha. It was therefore applied twice, and the value the flood fill writes came to depend on it while the threshold that fill compares against did not. Two failures followed whenever TASImage::DrawCircle() and friends were given a colour that is not fully opaque. At an alpha of 0x8C or below the value written never left the range the fill accepts, so every filled pixel was rediscovered and ctx_flood_fill() never returned. Between 0x8D and 0xFE it did return, but the anti-aliased outline no longer reached the threshold either, so the fill leaked past it and covered the whole image instead of the shape. Writing the coverage unscaled fixes both, because the threshold comparison stops depending on the brush. Output for a fully opaque brush is unchanged: 255 * ratio / 255 == ratio, and a circle, a manual filled path and an unfilled circle all render bit-identically before and after. Refs root-project#23014 Assisted-by: Claude (Anthropic); the change and the test were AI-assisted, then reviewed, measured and verified by the author.
Covers both ways DrawCircle() failed before the previous commit: at an alpha of 0x7F it hung, and at 0xC0 it filled the whole image rather than the circle. The opaque case guards the common path against a regression. Corner pixels are compared against their own values from before the draw, so the test does not depend on how a fresh TASImage is initialised. Refs root-project#23014 Assisted-by: Claude (Anthropic); the change and the test were AI-assisted, then reviewed, measured and verified by the author.
d7e86df to
c3b5906
Compare
Test Results 22 files 22 suites 3d 11h 31m 38s ⏱️ For more details on these failures, see this check. Results for commit c3b5906. |
This Pull request:
Filled shapes that are drawn with a colour that's not fully opaque are broken.
The
TASImage::DrawCircle(x, y, r, colour, -1)function either never returns or it paints the whole image instead of just the circle.Changes or fixes:
Cause. The libAfterImage library creates a filled shape in three steps: it draws the outline into a canvas, fills the inside, and then combines the temporary canvas with the image. When it combines the canvas with the image it already applies the brush transparency. However the functions
apply_tool_point_colored()andfill_hline_notile_colored()were also applying this alpha to what they drew. This means the alpha was applied twice. As a result the value that the fill function writes depends on the transparency. The threshold it compares against does not.This one mistake causes two problems.
When the alpha is 0x8C or lower the value that is written never leaves the range that the fill function accepts. This means that every filled pixel is rediscovered and the
ctx_flood_fill()function never finishes.When the alpha is between 0x8D and 0xFE the
ctx_flood_fill()function does finish. The outline that is smoothed to reduce jagged edges no longer reaches the threshold. This causes the fill to leak past the outline and cover the image.This is why nobody noticed this problem: the default colors are completely opaque. With these colors the alpha has no effect because
255 * ratio / 255is equal toratio.Fix. We need to write the coverage without scaling it in both functions. The threshold comparison stops depending on the brush.
Verification. I tested this by compiling
builtins/libAfterImage/draw.con its own because asimage is not enabled in my local ROOT build. Here are the results:When I use an opaque brush the output is exactly the same before and after. I compared the raw ARGB buffers using
cmpfor a circle, a filled path, an unfilled circle andellips2: all 4 tests had identical results.I also checked for hanging cases: there were 8 before. None after.
For a circle with alpha 0xC0 on a 400x400 image the number of filled pixels changed from 160000/160000 to 32173/160000 which is 20.1% and matches the area of the circle. The corner pixels changed from 4/4 painted to 0/4.
I tested the boundary on a 64x64 image with r=16: 0xFF and 0xFE worked fine but 0xF8 down to 0x8D filled the whole image and 0x8C and below caused it to hang.
Test. I added a test in
graf2d/asimage/test/following the same pattern asgraf2d/gpad/test. I used three colours:#FFFF0000guards the opaque path andworks on master too,
#C0FF0000fails on master, and#7FFF0000hangs on masterand is the colour from the report. The test compares the corner pixels to their
values before the draw so it does not depend on how a fresh
TASImageis set up.Not addressed here. The
asim_ellips2function still fills the canvas at every alpha, including 0xFF. This is a problem that was already there and this patch does not fix it or make it worse and its checksum is still the same.Checklist:
This PR fixes #23014