From c0a99576b1a9bc30dad17caebea1aaf72b33a3c3 Mon Sep 17 00:00:00 2001 From: Tekin Ertekin Date: Wed, 5 Aug 2026 17:02:39 +0300 Subject: [PATCH 1/3] [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. (cherry picked from commit 2eb51d4ac93d56491ee712b4daa8f9ec46ceec60) --- graf2d/asimage/src/libAfterImage/draw.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/graf2d/asimage/src/libAfterImage/draw.c b/graf2d/asimage/src/libAfterImage/draw.c index b1606bd1c3fbe..e4d10bb6fa2b8 100644 --- a/graf2d/asimage/src/libAfterImage/draw.c +++ b/graf2d/asimage/src/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 649e61278fa86babce69f0b4eae3049722ffdd8e Mon Sep 17 00:00:00 2001 From: Tekin Ertekin Date: Wed, 5 Aug 2026 17:02:39 +0300 Subject: [PATCH 2/3] [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. (cherry picked from commit 1d28aaa85c68eb72b73bf3c293dd49e4c89b387e) --- 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 3b5188c126edb..870cceea03d37 100644 --- a/graf2d/asimage/CMakeLists.txt +++ b/graf2d/asimage/CMakeLists.txt @@ -68,3 +68,5 @@ if (x11) endif() 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"); +} From aee219844b8a8e2b01122a7dfd7f5f3e06471d82 Mon Sep 17 00:00:00 2001 From: Sergey Linev Date: Thu, 20 Aug 2026 16:36:54 +0200 Subject: [PATCH 3/3] [stressgraphics] adjust ref file after fill circle fix --- test/stressGraphics.ref | 2 +- test/stressGraphics_zlibng.ref | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/stressGraphics.ref b/test/stressGraphics.ref index 38df8ed5a50e5..1416b0ac19ec0 100644 --- a/test/stressGraphics.ref +++ b/test/stressGraphics.ref @@ -49,7 +49,7 @@ waves 3303861 100 712368 3000 62688 19000 123056 7000 3396789 1000 tf12 3954 100 14714 300 17378 7800 9629 2000 3794 200 tspline 9073 100 22210 300 27691 7800 20712 2000 9157 200 - tscatter 15595 200 22620 400 28599 7800 42618 2000 15460 200 + tscatter 15595 200 22620 400 28599 7800 44900 2000 15460 200 tefficiency 12926 200 24364 300 24021 6800 12637 2000 12584 200 profile_2d 29664 200 19414 300 23024 6500 12821 2000 31317 200 profile_2dpoly 26463 400 23542 300 38792 8500 30458 4000 26759 400 diff --git a/test/stressGraphics_zlibng.ref b/test/stressGraphics_zlibng.ref index b7ca1568c66ee..f8d586158d9ff 100644 --- a/test/stressGraphics_zlibng.ref +++ b/test/stressGraphics_zlibng.ref @@ -49,7 +49,7 @@ waves 3303861 100 709707 3000 62761 19000 122867 7000 3396789 1000 tf12 3954 100 14714 300 17378 7800 9629 2000 3794 200 tspline 9073 100 22210 300 27691 7800 20712 2000 9157 200 - tscatter 15595 200 22420 400 28599 7800 42618 2000 15460 200 + tscatter 15595 200 22420 400 28599 7800 44760 2000 15460 200 tefficiency 12926 200 24364 300 24021 6800 12637 2000 12584 200 profile_2d 29664 200 19414 300 23024 6500 12821 2000 31317 200 profile_2dpoly 26463 400 23542 300 38792 8500 30458 4000 26759 400