This is an automated email from the git hooks/post-receive script.
git pushed a commit to branch overline-textgrid
in repository efl.
View the commit online.
commit 1211f6a5e474854c7bd7e17f367737640a81612f
Author: Cedric BAIL <[email protected]>
AuthorDate: Sun Aug 2 22:07:46 2026 -0600
ector: fix NEON premultiply rounding one step too high
The NEON path in efl_draw_argb_premul() applied the +1 twice:
lrgba.val[3] = vaddl_u8(rgba.val[3], mask_0x01); /* alpha + 1 */
rgba.val[0] = vshrn_n_u16(vmlaq_u16(lrgba.val[0],
lrgba.val[0],
lrgba.val[3]), 8);
vmlaq is multiply *accumulate*, so that computes c + c * (alpha + 1),
which is c * (alpha + 2). The scalar loop a few lines below computes
c * (alpha + 1) >> 8, and it is the one that is right.
The difference is a single step, but it lands on the wrong side of the
invariant the whole software compositor depends on. For c = 255,
alpha = 3 the scalar path yields 3 and the vector path yields 4 - a
colour channel larger than its own alpha, which premultiplied ARGB may
never contain. The blend kernels then do
s + MUL_256(256 - a, d) = 4 + ((255 * 253) >> 8) = 4 + 252 = 256
overflowing the 8 bit channel to zero and carrying into the next one. A
pixel that is one percent opaque, and should leave the destination almost
untouched, comes out as opaque black with a stray colour cast from the
carry.
Anything with a soft edge over a light background shows it. expedite's
bulb.png icon has 343 such pixels out of 1024 and renders with a black
halo and blue speckles around it; the same image composited by hand from
its own decoded data is clean.
This is longstanding - it reproduces on master - and it is invisible to
EVAS_CPU_NO_NEON, because this code tests eina_cpu_features_get()
directly rather than going through evas' feature mask, and so cannot be
switched off from there.
Let vmlaq supply the +1 and pass it the alpha itself. Verified against
the decoded image data: 343 pixels violating the invariant before, none
after.
The accompanying test checks the invariant itself rather than any one
code path - no channel above its alpha, for every alpha against the
colour values most likely to round the wrong way. It also compares the
two paths against each other directly: the vector loop only engages for
runs of eight or more, so premultiplying the same pixels one at a time
exercises the scalar fallback, and the two must agree exactly. Both fail
without the fix.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
---
src/static_libs/draw/draw_convert.c | 8 ++-
src/tests/evas/evas_suite.c | 1 +
src/tests/evas/evas_suite.h | 1 +
src/tests/evas/evas_test_premul.c | 118 ++++++++++++++++++++++++++++++++++++
src/tests/evas/meson.build | 1 +
5 files changed, 128 insertions(+), 1 deletion(-)
diff --git a/src/static_libs/draw/draw_convert.c b/src/static_libs/draw/draw_convert.c
index 81c727fd7d..3674649d32 100644
--- a/src/static_libs/draw/draw_convert.c
+++ b/src/static_libs/draw/draw_convert.c
@@ -560,7 +560,13 @@ efl_draw_argb_premul(uint32_t *data, unsigned int len)
lrgba.val[0] = vmovl_u8(rgba.val[0]);
lrgba.val[1] = vmovl_u8(rgba.val[1]);
lrgba.val[2] = vmovl_u8(rgba.val[2]);
- lrgba.val[3] = vaddl_u8(rgba.val[3], mask_0x01);
+ /* vmlaq below is c + c * val[3], so val[3] must be the alpha
+ * itself to yield c * (alpha + 1) - the same thing the scalar
+ * loop computes. Adding one here applied the +1 twice and
+ * produced c * (alpha + 2), which rounds faint pixels up past
+ * their own alpha and breaks the premultiplied invariant the
+ * blend kernels rely on. */
+ lrgba.val[3] = vmovl_u8(rgba.val[3]);
rgba.val[0] = vshrn_n_u16(vmlaq_u16(lrgba.val[0], lrgba.val[0],
lrgba.val[3]), 8);
diff --git a/src/tests/evas/evas_suite.c b/src/tests/evas/evas_suite.c
index dedf6841db..05faf08ff8 100644
--- a/src/tests/evas/evas_suite.c
+++ b/src/tests/evas/evas_suite.c
@@ -26,6 +26,7 @@ static const Efl_Test_Case etc[] = {
{ "Events", evas_test_events },
{ "Efl Canvas Animation", efl_test_canvas_animation },
{ "Map", evas_test_map },
+ { "Premul", evas_test_premul },
{ NULL, NULL }
};
diff --git a/src/tests/evas/evas_suite.h b/src/tests/evas/evas_suite.h
index af5decaf51..76a10a19b5 100644
--- a/src/tests/evas/evas_suite.h
+++ b/src/tests/evas/evas_suite.h
@@ -20,5 +20,6 @@ void evas_test_object_smart(TCase *tc);
void evas_test_events(TCase *tc);
void efl_test_canvas_animation(TCase *tc);
void evas_test_map(TCase *tc);
+void evas_test_premul(TCase *tc);
#endif /* _EVAS_SUITE_H */
diff --git a/src/tests/evas/evas_test_premul.c b/src/tests/evas/evas_test_premul.c
new file mode 100644
index 0000000000..ee95e6dd69
--- /dev/null
+++ b/src/tests/evas/evas_test_premul.c
@@ -0,0 +1,118 @@
+/* Premultiplication is the invariant the whole software compositor rests on:
+ * every blend kernel assumes no colour channel exceeds its own alpha, and
+ * s + MUL_256(256 - a, d) overflows its 8 bit channel the moment that stops
+ * being true - carrying into the neighbouring channel and turning a barely
+ * visible pixel into an opaque one of some unrelated colour.
+ *
+ * A NEON path that rounded one step too high was enough to do exactly that,
+ * so these tests check the invariant itself rather than any one code path.
+ */
+
+#ifdef HAVE_CONFIG_H
+# include "config.h"
+#endif
+
+#include <Evas.h>
+
+#include "evas_suite.h"
+
+/* Declared in evas_common_private.h, which pulls in far more than a test
+ * needs; it is exported from libevas. */
+EVAS_API unsigned int evas_common_convert_argb_premul(unsigned int *data,
+ unsigned int len);
+
+#define ARGB(a, r, g, b) \
+ (((unsigned int)(a) << 24) | ((unsigned int)(r) << 16) | \
+ ((unsigned int)(g) << 8) | (unsigned int)(b))
+
+/* Premultiplying must never leave a channel above the alpha it was scaled by.
+ * Sweeping every alpha against the channel values most likely to round the
+ * wrong way covers the whole space that matters. */
+EFL_START_TEST(evas_premul_invariant)
+{
+ static const unsigned int vals[] =
+ { 0, 1, 2, 3, 4, 127, 128, 129, 200, 250, 253, 254, 255 };
+ const unsigned int nvals = sizeof(vals) / sizeof(vals[0]);
+ unsigned int a, i;
+
+ for (a = 0; a <= 255; a++)
+ for (i = 0; i < nvals; i++)
+ {
+ /* eight identical pixels, so the vector path is used where one exists */
+ unsigned int px[8];
+ unsigned int k;
+
+ for (k = 0; k < 8; k++) px[k] = ARGB(a, vals[i], vals[i], vals[i]);
+ evas_common_convert_argb_premul(px, 8);
+
+ for (k = 0; k < 8; k++)
+ {
+ unsigned int ga = (px[k] >> 24) & 0xff;
+ unsigned int gr = (px[k] >> 16) & 0xff;
+ unsigned int gg = (px[k] >> 8) & 0xff;
+ unsigned int gb = px[k] & 0xff;
+
+ ck_assert_int_eq(ga, a);
+ ck_assert_msg((gr <= ga) && (gg <= ga) && (gb <= ga),
+ "alpha %u colour %u premultiplied to %08x: a "
+ "channel exceeds its alpha, which overflows the "
+ "blend and corrupts the pixel", a, vals[i], px[k]);
+ }
+ }
+}
+EFL_END_TEST
+
+/* The vector path only engages for runs of eight or more, so premultiplying
+ * the same pixels one at a time exercises the scalar fallback instead. The two
+ * must agree exactly - they are the same formula. */
+EFL_START_TEST(evas_premul_vector_matches_scalar)
+{
+ unsigned int a, c;
+
+ for (a = 0; a <= 255; a++)
+ for (c = 0; c <= 255; c++)
+ {
+ unsigned int bulk[8], one[8];
+ unsigned int k;
+
+ for (k = 0; k < 8; k++)
+ bulk[k] = one[k] = ARGB(a, c, (c + 85) & 0xff, (c + 170) & 0xff);
+
+ evas_common_convert_argb_premul(bulk, 8); /* vectorised */
+ for (k = 0; k < 8; k++)
+ evas_common_convert_argb_premul(&one[k], 1); /* scalar */
+
+ for (k = 0; k < 8; k++)
+ ck_assert_msg(bulk[k] == one[k],
+ "alpha %u colour %u: bulk premultiply gave %08x but "
+ "one at a time gave %08x", a, c, bulk[k], one[k]);
+ }
+}
+EFL_END_TEST
+
+/* The count of trivially opaque or transparent pixels feeds the sparse alpha
+ * flag, so it has to be right whichever path produced it. */
+EFL_START_TEST(evas_premul_trivial_count)
+{
+ unsigned int px[8], i, n;
+
+ for (i = 0; i < 8; i++) px[i] = ARGB(255, 10, 20, 30);
+ n = evas_common_convert_argb_premul(px, 8);
+ ck_assert_int_eq(n, 8);
+
+ for (i = 0; i < 8; i++) px[i] = ARGB(0, 10, 20, 30);
+ n = evas_common_convert_argb_premul(px, 8);
+ ck_assert_int_eq(n, 8);
+
+ for (i = 0; i < 8; i++) px[i] = ARGB(128, 10, 20, 30);
+ n = evas_common_convert_argb_premul(px, 8);
+ ck_assert_int_eq(n, 0);
+}
+EFL_END_TEST
+
+void evas_test_premul(TCase *tc)
+{
+ tcase_add_test(tc, evas_premul_invariant);
+ tcase_add_test(tc, evas_premul_vector_matches_scalar);
+ tcase_add_test(tc, evas_premul_trivial_count);
+}
diff --git a/src/tests/evas/meson.build b/src/tests/evas/meson.build
index e5758c2bb7..112b291145 100644
--- a/src/tests/evas/meson.build
+++ b/src/tests/evas/meson.build
@@ -22,6 +22,7 @@ evas_suite_src = [
'efl_test_canvas3.c',
'efl_canvas_animation.c',
'evas_test_map.c',
+ 'evas_test_premul.c',
]
evas_suite = executable('evas_suite',
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
