From 29eba9301d0a8b5a80f71b8502767abac1459192 Mon Sep 17 00:00:00 2001 From: Tekin Ertekin Date: Wed, 5 Aug 2026 17:02:39 +0300 Subject: [PATCH 1/2] [asimage] Do not fold the brush alpha into the scratch coverage 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 #23014 Assisted-by: Claude (Anthropic); the change and the test were AI-assisted, then reviewed, measured and verified by the author. --- builtins/libAfterImage/draw.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/builtins/libAfterImage/draw.c b/builtins/libAfterImage/draw.c index b1606bd1c3fbe..e4d10bb6fa2b8 100644 --- a/builtins/libAfterImage/draw.c +++ b/builtins/libAfterImage/draw.c @@ -340,7 +340,7 @@ apply_tool_point_colored(ASDrawContext *ctx, int curr_x, int curr_y, CARD32 rati dst += curr_y * cw + curr_x; if (get_flags(ctx->flags, ASDrawCTX_UsingScratch)) { - CARD32 value = (ARGB32_ALPHA8(ctx->tool->matrix[0])*ratio)/255 ; + CARD32 value = ratio ; /* coverage only; alpha applied on merge */ if( *dst < value ) *dst = value ; } @@ -391,7 +391,7 @@ fill_hline_notile_colored(ASDrawContext *ctx, int x_from, int y, int x_to, CARD3 { while( x1 <= x2 ) { - CARD32 value = (ARGB32_ALPHA8(ctx->tool->matrix[0])*ratio)/255 ; + CARD32 value = ratio ; /* coverage only; alpha applied on merge */ if( dst[x1] < value ) dst[x1] = value ; ++x1 ; From c3b5906456d3637d8a5f5f20f72d60b6bdfba795 Mon Sep 17 00:00:00 2001 From: Tekin Ertekin Date: Wed, 5 Aug 2026 17:02:39 +0300 Subject: [PATCH 2/2] [asimage] Add a regression test for filled shapes drawn with alpha 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 #23014 Assisted-by: Claude (Anthropic); the change and the test were AI-assisted, then reviewed, measured and verified by the author. --- graf2d/asimage/CMakeLists.txt | 2 + graf2d/asimage/test/CMakeLists.txt | 7 +++ graf2d/asimage/test/tasimage_draw.cxx | 68 +++++++++++++++++++++++++++ 3 files changed, 77 insertions(+) create mode 100644 graf2d/asimage/test/CMakeLists.txt create mode 100644 graf2d/asimage/test/tasimage_draw.cxx diff --git a/graf2d/asimage/CMakeLists.txt b/graf2d/asimage/CMakeLists.txt index 8f835cc7e16ad..8a0a67e7012ab 100644 --- a/graf2d/asimage/CMakeLists.txt +++ b/graf2d/asimage/CMakeLists.txt @@ -53,3 +53,5 @@ ROOT_STANDARD_LIBRARY_PACKAGE(ASImageGui ) ROOT_INSTALL_HEADERS() + +ROOT_ADD_TEST_SUBDIRECTORY(test) diff --git a/graf2d/asimage/test/CMakeLists.txt b/graf2d/asimage/test/CMakeLists.txt new file mode 100644 index 0000000000000..c08327d3e21db --- /dev/null +++ b/graf2d/asimage/test/CMakeLists.txt @@ -0,0 +1,7 @@ +# Copyright (C) 1995-2026, Rene Brun and Fons Rademakers. +# All rights reserved. +# +# For the licensing terms see $ROOTSYS/LICENSE. +# For the list of contributors see $ROOTSYS/README/CREDITS. + +ROOT_ADD_GTEST(TASImageDraw tasimage_draw.cxx LIBRARIES ASImage) diff --git a/graf2d/asimage/test/tasimage_draw.cxx b/graf2d/asimage/test/tasimage_draw.cxx new file mode 100644 index 0000000000000..bc8b942e2fb5c --- /dev/null +++ b/graf2d/asimage/test/tasimage_draw.cxx @@ -0,0 +1,68 @@ +#include "gtest/gtest.h" + +#include "TASImage.h" + +namespace { + +constexpr UInt_t kSize = 64; + +constexpr UInt_t kPixels = kSize * kSize; + +// Index of the four canvas corners. +constexpr UInt_t kCorners[4] = {0, kSize - 1, (kSize - 1) * kSize, kPixels - 1}; + +// Draw a filled circle of `colour` well inside a kSize x kSize image and check +// that the fill stayed inside it. The corner values are compared against what +// they were before drawing rather than against a constant, so the test does not +// depend on how a fresh TASImage is initialised. +void CheckFilledCircleStaysInside(const char *colour) +{ + TASImage img(kSize, kSize); + + UInt_t *argb = img.GetArgbArray(); + ASSERT_NE(argb, nullptr); + + UInt_t before[4]; + for (int i = 0; i < 4; ++i) + before[i] = argb[kCorners[i]]; + const UInt_t centre = (kSize / 2) * kSize + kSize / 2; + const UInt_t centreBefore = argb[centre]; + + img.DrawCircle(kSize / 2, kSize / 2, kSize / 4, colour, -1); + + argb = img.GetArgbArray(); + ASSERT_NE(argb, nullptr); + + for (int i = 0; i < 4; ++i) + EXPECT_EQ(argb[kCorners[i]], before[i]) << "the fill escaped the circle and reached corner " << i; + + EXPECT_NE(argb[centre], centreBefore) << "the circle was not filled at all"; +} + +} // namespace + +// https://github.com/root-project/root/issues/23014 +// +// libAfterImage scaled the coverage it wrote into the scratch canvas by the +// brush alpha, which made the flood fill that closes a filled shape depend on +// that alpha. Two symptoms followed, and this geometry shows both: at an alpha +// of 0x8C or below the fill never terminated, and between 0x8D and 0xFE it +// returned but leaked through the anti-aliased outline and covered the whole +// image. Only a fully opaque brush behaved correctly. + +TEST(TASImage, FilledCircleOpaque) +{ + CheckFilledCircleStaysInside("#FFFF0000"); +} + +// Used to leak out of the circle and fill the whole image. +TEST(TASImage, FilledCircleHighAlpha) +{ + CheckFilledCircleStaysInside("#C0FF0000"); +} + +// Used to hang: the colour from the issue report. +TEST(TASImage, FilledCircleSemiTransparent) +{ + CheckFilledCircleStaysInside("#7FFF0000"); +}