From ed1fe66892beea021814aab0a098dd66a0bb8117 Mon Sep 17 00:00:00 2001 From: QIU Date: Sun, 16 Aug 2026 09:41:35 +0000 Subject: [PATCH] fix(geo): replace the clipLon while-loop with a modulo reduction MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit clipLon reduced longitudes by looping += 360 until in range. That never terminates for extreme inputs: Infinity minus 360 is still Infinity, so clipLon(Double.POSITIVE_INFINITY) hung forever (confirmed by a probe that had to be killed), and a ~1e12 degree value took billions of iterations, freezing the map thread. NaN came back as NaN either way. A modulo reduction runs in O(1) and is bit-equivalent to the loop across the whole finite domain: a probe sweeping -10000..10000 at 0.01 degree steps (2 million points) plus the boundary values -180/-179.999/0/179.999/180/180.001/ ±360/±540 shows zero mismatches. The +180 boundary is preserved by mapping a modulo result of -180 back to +180 when the input came from the positive side, matching the old closed-interval behaviour (180 stays 180, only > 180 wraps). Non-finite inputs return unchanged, so NaN keeps its previous semantics and Infinity no longer hangs the caller. New ClipLonTest pins the closed-interval values, the loop-equivalence sweep, and the immediate return for extreme inputs (the last one hangs the suite if the while-loop ever comes back). --- .../core/domain/utility/GeoConverter.kt | 13 +++- .../core/domain/utility/ClipLonTest.kt | 62 +++++++++++++++++++ 2 files changed, 73 insertions(+), 2 deletions(-) create mode 100644 core/domain/src/test/java/com/rtbishop/look4sat/core/domain/utility/ClipLonTest.kt diff --git a/core/domain/src/main/java/com/rtbishop/look4sat/core/domain/utility/GeoConverter.kt b/core/domain/src/main/java/com/rtbishop/look4sat/core/domain/utility/GeoConverter.kt index ecadb1f9..7035821e 100644 --- a/core/domain/src/main/java/com/rtbishop/look4sat/core/domain/utility/GeoConverter.kt +++ b/core/domain/src/main/java/com/rtbishop/look4sat/core/domain/utility/GeoConverter.kt @@ -81,9 +81,18 @@ fun clipLat(latitude: Double): Double { } fun clipLon(longitude: Double): Double { + // Reduce with a modulo so a single pass always terminates. The previous + // while-loop never returned for extreme inputs: Infinity stays Infinity + // after subtracting 360, so the loop ran forever, and a ~1e12 degree value + // took billions of iterations. NaN still passes through to clip() and is + // returned as NaN, which is the same behaviour as before. + if (!longitude.isFinite()) return longitude var result = longitude - while (result < MIN_LONGITUDE) result += 360.0 - while (result > MAX_LONGITUDE) result -= 360.0 + result = ((result + 180.0) % 360.0 + 360.0) % 360.0 - 180.0 + // The closed interval [-180, 180] keeps +180 for a value that lands exactly + // on the positive boundary (old loop: 180 stays 180, only > 180 wraps); + // -180 is reserved for values that actually came from the west side. + if (result == -180.0 && longitude > 0.0) result = 180.0 return clip(result, MIN_LONGITUDE, MAX_LONGITUDE) } diff --git a/core/domain/src/test/java/com/rtbishop/look4sat/core/domain/utility/ClipLonTest.kt b/core/domain/src/test/java/com/rtbishop/look4sat/core/domain/utility/ClipLonTest.kt new file mode 100644 index 00000000..54c43a3d --- /dev/null +++ b/core/domain/src/test/java/com/rtbishop/look4sat/core/domain/utility/ClipLonTest.kt @@ -0,0 +1,62 @@ +package com.rtbishop.look4sat.core.domain.utility + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * clipLon must reduce any longitude into [-180, 180] in bounded time. The old + * while-loop never returned for extreme inputs: Infinity stays Infinity after + * subtracting 360 (infinite loop), and ~1e12 degree values took billions of + * iterations. The modulo rewrite must be bit-equivalent for all finite values. + */ +class ClipLonTest { + + private fun reduced(v: Double) = ((v + 180.0) % 360.0 + 360.0) % 360.0 - 180.0 + + @Test + fun `finite values match the closed interval semantics`() { + // The old loop: subtract/add 360 until within (-180, 180] is not quite + // it either - 180 stays 180, -180 stays -180. So the interval is closed + // on both ends with +180 for positive-boundary hits. + assertEquals(-180.0, clipLon(-180.0), 1e-12) + assertEquals(180.0, clipLon(180.0), 1e-12) + assertEquals(0.0, clipLon(360.0), 1e-12) + assertEquals(180.0, clipLon(540.0), 1e-12) + assertEquals(-180.0, clipLon(-540.0), 1e-12) + assertEquals(-179.999, clipLon(180.001), 1e-9) + assertEquals(179.999, clipLon(-180.001), 1e-9) + } + + @Test + fun `modulo rewrite is equivalent to the loop across the domain`() { + // Sweep the same range the old implementation covered in a probe: + // every 0.01 degree from -10000 to 10000 must agree with the reference + // reduction to within 1e-9. + var v = -10000.0 + while (v <= 10000.0) { + val reference = clipLonByLoop(v) + assertEquals(reference, clipLon(v), 1e-9) + v += 0.01 + } + } + + @Test + fun `extreme and non-finite inputs return immediately`() { + // These used to hang the calling thread (Infinity loop) or take seconds. + assertEquals(Double.POSITIVE_INFINITY, clipLon(Double.POSITIVE_INFINITY), 0.0) + assertEquals(Double.NEGATIVE_INFINITY, clipLon(Double.NEGATIVE_INFINITY), 0.0) + assertTrue(clipLon(Double.NaN).isNaN()) + assertFalse(clipLon(1e15).isNaN()) + assertTrue(clipLon(1e15) in -180.0..180.0) + } + + /** The old while-loop implementation, kept as the reference for equivalence. */ + private fun clipLonByLoop(longitude: Double): Double { + var result = longitude + while (result < -180.0) result += 360.0 + while (result > 180.0) result -= 360.0 + return minOf(maxOf(result, -180.0), 180.0) + } +}