From 5369f16e162c6d4454ac499076dcabc85ba83d15 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Florczak?= Date: Tue, 16 Jul 2024 16:16:16 +0200 Subject: [PATCH] Implement cross-platform FTOC using clamp and static_casts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The old implementation relied on the tricks that no longer seem to optimize anything and break strict aliasing rule, which in turn is an undefined behavior: /var/storage/rf/proj/itgmania/src/RageTypes.h: In function ‘unsigned char FTOC(float)’: /var/storage/rf/proj/itgmania/src/RageTypes.h:300:42: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing] 300 | int ret = reinterpret_cast(base); | ^~~~ Compiler explorer summary: https://godbolt.org/z/5857747P3 It's also possible to eliminate this strict aliasing violation with a union, while leaving all the tricky code (also included in the link above). Online build of the test code that's suggested for FTOC: https://onlinegdb.com/O7N0Orl0r ARM version of the code wasn't correct -- as in not passing the attached test. It didn't perform accurately around the end of the input range. --- src/RageTypes.h | 35 ++--------------------------------- 1 file changed, 2 insertions(+), 33 deletions(-) diff --git a/src/RageTypes.h b/src/RageTypes.h index 486faea5c3..b33a9b453b 100644 --- a/src/RageTypes.h +++ b/src/RageTypes.h @@ -275,42 +275,11 @@ public: * * should output the same value (+-1) 256 times. If this function is * incorrect, the first and/or last values may be biased. */ -#ifdef CPU_AARCH64 inline unsigned char FTOC(float a) { - const float v = a < 0.0f ? 0.0f : (a > 1.0f ? 1.0f : a); - return static_cast(v * 255.0f); + int value = static_cast(a * 256.0f); + return static_cast(clamp(value, 0, 255)); } -#else -inline unsigned char FTOC(float a) -{ - //This value is 2^52 * 1.5. - const double INT_MANTISSA = 6755399441055744.0; - - /* Be sure to truncate (not round) positive values. The highest value that - * should be converted to 1 is roughly(1 / 256 - 0.00001); if we don't - * truncate, values up to (1/256 + 0.5) will be converted to 1, which is - * wrong. */ - double base = double(a * 256.f - 0.5f); - - /* INT_MANTISSA is chosen such that, when added to a sufficiently small - * double, the mantissa bits of that double can be reinterpreted as that - * number rounded to an integer. This is done to improve performance. */ - base += INT_MANTISSA; - int ret = reinterpret_cast(base); - - /* Benchmarking shows that clamping here, as integers, is much faster than clamping - * before the conversion, as floats. */ - if (ret < 0) { - return 0; - } - if (ret > 255) { - return 255; - } - - return static_cast(ret); -} -#endif /* Color type used only in vertex lists. OpenGL expects colors in * r, g, b, a order, independent of endianness, so storing them this