Control: tags -1 + patch pending On Sat, 24 Aug 2024 at 09:24:02 -0400, Jeremy Bícha wrote: > A new test added in gtk4 4.15.* is failing on armel but not on any > other release architecture. ... > not ok 1 /<<PKGBUILDDIR>>/testsuite/css/parser/math2.css
The bug is that GTK implements CSS expressions like `round(up, 18px, 5px)` by using fesetround() to select the desired rounding mode, and nearbyint() to do the rounding. On all release architectures except armel, this works as intended: floating point arithmetic is done in hardware, and the FPU supports all four of the rounding modes required by GTK's CSS implementation (FE_TONEAREST, FE_UPWARD, FE_DOWNWARD, FE_TOWARDZERO). On armel, the software implementation of floating-point only implements the default FE_TONEAREST[1], and the other modes are not available. This causes the test to fail: `round(to-zero, 18px, 5px)` comes out as 20px instead of the intended 15px, and similar for `round(down, 18px, 5px)`. [1] see also https://lists.debian.org/debian-arm/2008/03/msg00001.html I was able to get this test passing with GTK 4.15.6 by applying the attached patch, also available as part of the update to 4.15.6 in <https://salsa.debian.org/gnome-team/gtk4/-/merge_requests/26> (I have not tried to backport it to 4.15.5 but I suspect it would apply cleanly there too). I have intentionally not tried to upstream this patch, because I can already predict what will happen: upstream will ask why I'm trying to run GTK on an old embedded device with no FPU and no useful GPU, and I will not have a good answer, because I, personally, have never wanted to do that. I find being upstream's punching bag for all of Debian's architecture support decisions to be really demotivating, and even if I had unlimited motivation, I think upstreaming this myself would be tactically a bad idea: every time I try to make upstream spend their limited time on an architecture that they don't have any interest in supporting, I feel as though I'm coming one step closer to the point where they start intentionally ignoring me, which would significantly harm my ability to do other work that the project expects from me. So, I'm invoking Debian Constitution §2.1.1 and declining to attempt to fix this upstream myself. If someone from the ARM porting team feels strongly that armel should continue to be a first-class-citizen architecture for GTK for whatever reason, please take over here. Thanks, smcv
From: Simon McVittie <s...@debian.org> Date: Sat, 31 Aug 2024 10:13:48 +0100 Subject: gtkcssnumbervalue: Don't use fesetround() on ARM softfloat Older ARM CPUs have to emulate floating-point in software, and do not implement rounding modes other than the default, FE_TONEAREST. Implement nearbyint() the hard way when targeting an affected platform. Bug-Debian: https://bugs.debian.org/1079545 Signed-off-by: Simon McVittie <s...@debian.org> --- gtk/gtkcssnumbervalue.c | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/gtk/gtkcssnumbervalue.c b/gtk/gtkcssnumbervalue.c index 1181c1a..d9cdf5f 100644 --- a/gtk/gtkcssnumbervalue.c +++ b/gtk/gtkcssnumbervalue.c @@ -1846,11 +1846,21 @@ gtk_css_dimension_value_is_zero (const GtkCssValue *value) return value->dimension.value == 0; } +#if defined(__arm__) && !defined(__ARM_PCS_VFP) +/* Floating-point emulated in software. Setting the rounding mode to + * anything other than FE_TONEAREST doesn't work */ +#undef HAVE_WORKING_FESETROUND +#else +#define HAVE_WORKING_FESETROUND +#endif + static double _round (guint mode, double a, double b) { +#ifdef HAVE_WORKING_FESETROUND int old_mode; int modes[] = { FE_TONEAREST, FE_UPWARD, FE_DOWNWARD, FE_TOWARDZERO }; +#endif double result; if (b == 0) @@ -1880,12 +1890,36 @@ _round (guint mode, double a, double b) } } +#ifdef HAVE_WORKING_FESETROUND old_mode = fegetround (); fesetround (modes[mode]); result = nearbyint (a/b) * b; fesetround (old_mode); +#else + result = a / b; + + switch (mode) + { + case ROUND_NEAREST: + result = round (result); + break; + case ROUND_TO_ZERO: + result = (result >= 0 ? floor (result) : ceil (result)); + break; + case ROUND_UP: + result = ceil (result); + break; + case ROUND_DOWN: + result = floor (result); + break; + default: + g_assert_not_reached (); + } + + result *= b; +#endif return result; }
