From cc5c478d1d847f0c8d49e65e0a2b2d67f13fe6a5 Mon Sep 17 00:00:00 2001 From: TD-er Date: Mon, 17 Jul 2023 22:02:42 +0200 Subject: [PATCH 1/3] Fix Map function - IntegerDivideByZero #8938 Fixes: #8938 Also increase accuracy of mapping and fixing map issues with large values. Still only using integer operations. Test function to see absolute error compared to floating point map function using `double` precision floating point values. ```c++ # include # include long test_map(long x, long in_min, long in_max, long out_min, long out_max) { const double out_length = out_max - out_min; const double in_length = in_max - in_min; if (in_length == 0) { return in_min; } if (out_length == 0) { return out_min; } const double delta = x - in_min; const double value = (delta * out_length + (in_length / 2)) / in_length + out_min; // return std::round(value); return value; } long check(long x, long in_min, long in_max, long out_min, long out_max) { const long floatvalue = test_map(x, in_min, in_max, out_min, out_max); const long map_value = map(x, in_min, in_max, out_min, out_max); return floatvalue - map_value; } int main() { long input = 0; for (size_t i = 0; i < 15; ++i) { input = (i / 3) * 2500 - 1 + i % 3; const long values[] = { check(input, 0, 1000, 255, 0), check(input, 0, 1000, 0, 255), check(input, 10000, 0, 100, 10100), check(input, 10000, 0, 10100, 100), check(input, 10000, 0, 0, 1024), check(input, 10000, 0, 1024, 0), check(input, 1000, 0, 0, 10240), check(input, 1000, 0, 10240, 0), check(input, 0, 1000, 0, 10240), check(input, 0, 1000, 10240, 0), check(input, 0, 10000, 10240, 0), check(input, 10234567, -12345, 10234567, 100), check(input, 10000, 0, 10234567, 0), check(input, 50, 1, 10234567, 0) }; std::cout << input << ":"; constexpr size_t nrvalues = sizeof(values) / sizeof(values[0]); for (size_t i = 0; i < nrvalues; ++i) { std::cout << "\t" << values[i]; } std::cout << "\n"; } } ``` Output: ``` -1: 0 -1 0 0 0 -1 0 2 2 0 0 -1 2 2 0: 0 0 0 0 0 0 0 0 0 0 0 -1 0 1 1: -1 0 0 0 -1 0 1 0 0 1 1 -1 0 0 2499: 1 -1 0 0 0 0 1 1 1 1 0 0 0 0 2500: 1 1 0 0 0 0 1 0 0 1 0 0 1 0 2501: 0 0 0 0 0 0 2 0 0 2 1 0 0 1 4999: 1 -1 0 0 0 0 1 1 1 1 0 0 0 0 5000: 1 0 0 0 0 0 1 0 0 1 0 0 1 1 5001: 0 0 0 0 0 0 2 0 0 2 1 0 1 0 7499: 1 -1 0 0 0 0 1 1 1 1 0 0 1 1 7500: 1 1 0 0 0 0 1 0 0 1 0 0 0 0 7501: 0 0 0 0 0 0 2 0 0 2 1 0 1 0 9999: 1 -1 0 0 0 -1 1 1 1 1 0 0 1 0 10000: 1 0 0 0 0 0 1 0 0 1 0 0 0 0 10001: 0 0 0 0 -1 0 2 0 0 2 2 0 0 1 ``` --- cores/esp8266/WMath.cpp | 64 ++++++++++++++++++++++++++++++++++++++--- 1 file changed, 60 insertions(+), 4 deletions(-) diff --git a/cores/esp8266/WMath.cpp b/cores/esp8266/WMath.cpp index d8dc1f82f2..c822346abd 100644 --- a/cores/esp8266/WMath.cpp +++ b/cores/esp8266/WMath.cpp @@ -72,11 +72,67 @@ long secureRandom(long howsmall, long howbig) { } long map(long x, long in_min, long in_max, long out_min, long out_max) { - const long dividend = out_max - out_min; - const long divisor = in_max - in_min; - const long delta = x - in_min; + const long out_length = out_max - out_min; + const long in_length = in_max - in_min; - return (delta * dividend + (divisor / 2)) / divisor + out_min; + if (in_length == 0) { return in_min; } + + if (out_length == 0) { return out_min; } + + long delta = x - in_min; + + if (out_length == in_length) { + return out_min + delta; + } + + if ((out_length < 0) && (in_length < 0)) { + return map(x, in_max, in_min, out_max, out_min); + } else if (out_length < 0) { + return map(in_max - delta, in_min, in_max, out_max, out_min); + } else if (in_length < 0) { + return map(in_max - delta, in_max, in_min, out_min, out_max); + } + + // We now know in_min < in_max and out_min < out_max + // Make sure x is within range of in_min ... in_max + if ((x < in_min) || (x > in_max)) { + long shift_factor = 0; + + if (x < in_min) { + const long before_min = in_min - x; + shift_factor = -1 - (before_min / in_length); + } else if (x > in_max) { + const long passed_max = x - in_max; + shift_factor = 1 + (passed_max / in_length); + } + + if (shift_factor != 0) { + const long in_shift = shift_factor * in_length; + const long out_shift = shift_factor * out_length; + in_min += in_shift; + in_max += in_shift; + out_min += out_shift; + out_max += out_shift; + delta = x - in_min; + } + } + + if (out_length > in_length) { + // Map to larger range + const long factor = out_length / in_length; + const long error_mod = out_length % in_length; + const long error = (delta * error_mod) / in_length; + return (delta * factor) + out_min + error; + } + + // abs(out_length) < abs(in_length) + // Map to smaller range + const long factor = (in_length / out_length); + + const long estimate_full = in_length / factor + out_min; + const long error = (delta * (out_max - estimate_full)) / in_length; + + return delta / factor + out_min + error; } uint16_t makeWord(uint16_t w) { From 7b50d84c71caf9939c3a8ef3941666e6d749cbd6 Mon Sep 17 00:00:00 2001 From: TD-er Date: Tue, 18 Jul 2023 10:28:50 +0200 Subject: [PATCH 2/3] Simplify code & remove recursive call Not really a need to make a recursive call for simply swapping values. --- cores/esp8266/WMath.cpp | 59 ++++++++++++++++++++++++----------------- 1 file changed, 35 insertions(+), 24 deletions(-) diff --git a/cores/esp8266/WMath.cpp b/cores/esp8266/WMath.cpp index c822346abd..736cdf75d1 100644 --- a/cores/esp8266/WMath.cpp +++ b/cores/esp8266/WMath.cpp @@ -72,12 +72,12 @@ long secureRandom(long howsmall, long howbig) { } long map(long x, long in_min, long in_max, long out_min, long out_max) { - const long out_length = out_max - out_min; - const long in_length = in_max - in_min; + long in_length = in_max - in_min; + long out_length = out_max - out_min; - if (in_length == 0) { return in_min; } - - if (out_length == 0) { return out_min; } + if (in_length == 0 || out_length == 0) { + return out_min; + } long delta = x - in_min; @@ -86,51 +86,62 @@ long map(long x, long in_min, long in_max, long out_min, long out_max) { } if ((out_length < 0) && (in_length < 0)) { - return map(x, in_max, in_min, out_max, out_min); + std::swap(in_min, in_max); + std::swap(out_min, out_max); } else if (out_length < 0) { - return map(in_max - delta, in_min, in_max, out_max, out_min); + x = in_max - delta; + std::swap(out_min, out_max); } else if (in_length < 0) { - return map(in_max - delta, in_max, in_min, out_min, out_max); + x = in_max - delta; + std::swap(in_min, in_max); } + // Update length and delta as in/out values may have changed. + delta = x - in_min; + in_length = in_max - in_min; + out_length = out_max - out_min; + // We now know in_min < in_max and out_min < out_max // Make sure x is within range of in_min ... in_max + // Shift the in/out range to contain 'x' if ((x < in_min) || (x > in_max)) { long shift_factor = 0; if (x < in_min) { const long before_min = in_min - x; shift_factor = -1 - (before_min / in_length); - } else if (x > in_max) { + } else { + // x > in_max const long passed_max = x - in_max; shift_factor = 1 + (passed_max / in_length); } - if (shift_factor != 0) { - const long in_shift = shift_factor * in_length; - const long out_shift = shift_factor * out_length; - in_min += in_shift; - in_max += in_shift; - out_min += out_shift; - out_max += out_shift; - delta = x - in_min; - } + const long in_shift = shift_factor * in_length; + const long out_shift = shift_factor * out_length; + in_min += in_shift; + in_max += in_shift; + out_min += out_shift; + out_max += out_shift; + delta = x - in_min; } if (out_length > in_length) { // Map to larger range - const long factor = out_length / in_length; - const long error_mod = out_length % in_length; - const long error = (delta * error_mod) / in_length; + // Do not 'simplify' this calculation + // as this will result in integer overflow errors + const long factor = out_length / in_length; + const long error_mod = out_length % in_length; + const long error = (delta * error_mod) / in_length; return (delta * factor) + out_min + error; } // abs(out_length) < abs(in_length) // Map to smaller range - const long factor = (in_length / out_length); - + // Do not 'simplify' this calculation + // as this will result in integer overflow errors + const long factor = (in_length / out_length); const long estimate_full = in_length / factor + out_min; - const long error = (delta * (out_max - estimate_full)) / in_length; + const long error = (delta * (out_max - estimate_full)) / in_length; return delta / factor + out_min + error; } From 0962edc5453884e901b46dc1df788f977677c024 Mon Sep 17 00:00:00 2001 From: TD-er Date: Tue, 18 Jul 2023 10:46:02 +0200 Subject: [PATCH 3/3] Simplify map to inverse range of same length --- cores/esp8266/WMath.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/cores/esp8266/WMath.cpp b/cores/esp8266/WMath.cpp index 736cdf75d1..e5bccf0782 100644 --- a/cores/esp8266/WMath.cpp +++ b/cores/esp8266/WMath.cpp @@ -81,10 +81,6 @@ long map(long x, long in_min, long in_max, long out_min, long out_max) { long delta = x - in_min; - if (out_length == in_length) { - return out_min + delta; - } - if ((out_length < 0) && (in_length < 0)) { std::swap(in_min, in_max); std::swap(out_min, out_max); @@ -101,6 +97,10 @@ long map(long x, long in_min, long in_max, long out_min, long out_max) { in_length = in_max - in_min; out_length = out_max - out_min; + if (out_length == in_length) { + return out_min + delta; + } + // We now know in_min < in_max and out_min < out_max // Make sure x is within range of in_min ... in_max // Shift the in/out range to contain 'x'