LUCENE-7312: fix geo3d's encoding to always round down

lucene/CHANGES.txt

@@ -137,6 +137,9 @@ Bug Fixes
   one update batch could be applied in the wrong order resulting in
   the wrong updated value (Ishan Chattopadhyaya, hossman, Mike McCandless)
 
+* LUCENE-7312: Fix geo3d's x/y/z double to int encoding to ensure it always
+  rounds down (Karl Wright, Mike McCandless)
+
 Documentation
 
 * LUCENE-7223: Improve XXXPoint javadocs to make it clear that you

lucene/spatial3d/src/java/org/apache/lucene/spatial3d/Geo3DUtil.java

@@ -44,8 +44,9 @@ class Geo3DUtil {
   private static final double MAX_VALUE = PlanetModel.WGS84.getMaximumMagnitude();
   private static final int BITS = 32;
   private static final double MUL = (0x1L<<BITS)/(2*MAX_VALUE);
-  static final double DECODE = 1/MUL;
+  static final double DECODE = getNextSafeDouble(1/MUL);
   private static final int MIN_ENCODED_VALUE = encodeValue(-MAX_VALUE);
+  private static final int MAX_ENCODED_VALUE = encodeValue(MAX_VALUE);
 
   public static int encodeValue(double x) {
     if (x > MAX_VALUE) {
@@ -54,20 +55,26 @@ class Geo3DUtil {
     if (x < -MAX_VALUE) {
       throw new IllegalArgumentException("value=" + x + " is out-of-bounds (less than than WGS84's -planetMax=" + -MAX_VALUE + ")");
     }
-    // the maximum possible value cannot be encoded without overflow
-    if (x == MAX_VALUE) {
-      x = Math.nextDown(x);
-    }
     long result = (long) Math.floor(x / DECODE);
-    //System.out.println("    enc: " + x + " -> " + result);
     assert result >= Integer.MIN_VALUE;
     assert result <= Integer.MAX_VALUE;
     return (int) result;
   }
 
   public static double decodeValue(int x) {
-    // We decode to the center value; this keeps the encoding stable
+    double result;
-    return (x+0.5) * DECODE;
+    if (x == MIN_ENCODED_VALUE) {
+      // We must special case this, because -MAX_VALUE is not guaranteed to land precisely at a floor value, and we don't ever want to
+      // return a value outside of the planet's range (I think?).  The max value is "safe" because we floor during encode:
+      result = -MAX_VALUE;
+    } else if (x == MAX_ENCODED_VALUE) {
+      result = MAX_VALUE;
+    } else {
+      // We decode to the center value; this keeps the encoding stable
+      result = (x+0.5) * DECODE;
+    }
+    assert result >= -MAX_VALUE && result <= MAX_VALUE;
+    return result;
   }
 
   /** Returns smallest double that would encode to int x. */
@@ -76,14 +83,30 @@ class Geo3DUtil {
     return x * DECODE;
   }
   
+  /** Returns a double value >= x such that if you multiply that value by an int, and then
+   *  divide it by that int again, you get precisely the same value back */
+  private static double getNextSafeDouble(double x) {
+
+    // Move to double space:
+    long bits = Double.doubleToLongBits(x);
+
+    // Make sure we are beyond the actual maximum value:
+    bits += Integer.MAX_VALUE;
+
+    // Clear the bottom 32 bits:
+    bits &= ~((long) Integer.MAX_VALUE);
+
+    // Convert back to double:
+    double result = Double.longBitsToDouble(bits);
+    assert result > x;
+    return result;
+  }
+
   /** Returns largest double that would encode to int x. */
   // NOTE: keep this package private!!
   static double decodeValueCeil(int x) {
-    if (x == Integer.MAX_VALUE) {
+    assert x < Integer.MAX_VALUE;
-      return MAX_VALUE;
+    return Math.nextDown((x+1) * DECODE);
-    } else {
-      return Math.nextDown((x+1) * DECODE);
-    }
   }
   
   /** Converts degress to radians */

lucene/spatial3d/src/test/org/apache/lucene/spatial3d/TestGeo3DPoint.java

@@ -23,6 +23,7 @@ import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.HashSet;
 import java.util.List;
+import java.util.Random;
 import java.util.Set;
 
 import org.apache.lucene.codecs.Codec;
@@ -55,8 +56,6 @@ import org.apache.lucene.index.Term;
 import org.apache.lucene.search.IndexSearcher;
 import org.apache.lucene.search.Query;
 import org.apache.lucene.search.SimpleCollector;
-import org.apache.lucene.spatial3d.geom.XYZSolid;
-import org.apache.lucene.spatial3d.geom.XYZSolidFactory;
 import org.apache.lucene.spatial3d.geom.GeoArea;
 import org.apache.lucene.spatial3d.geom.GeoAreaFactory;
 import org.apache.lucene.spatial3d.geom.GeoBBox;
@@ -71,6 +70,8 @@ import org.apache.lucene.spatial3d.geom.Plane;
 import org.apache.lucene.spatial3d.geom.PlanetModel;
 import org.apache.lucene.spatial3d.geom.SidedPlane;
 import org.apache.lucene.spatial3d.geom.XYZBounds;
+import org.apache.lucene.spatial3d.geom.XYZSolid;
+import org.apache.lucene.spatial3d.geom.XYZSolidFactory;
 import org.apache.lucene.store.Directory;
 import org.apache.lucene.util.DocIdSetBuilder;
 import org.apache.lucene.util.FixedBitSet;
@@ -1179,6 +1180,50 @@ public class TestGeo3DPoint extends LuceneTestCase {
     }
   }
 
+  // poached from TestGeoEncodingUtils.testLatitudeQuantization:
+
+  /**
+   * step through some integers, ensuring they decode to their expected double values.
+   * double values start at -90 and increase by LATITUDE_DECODE for each integer.
+   * check edge cases within the double range and random doubles within the range too.
+   */
+  public void testQuantization() throws Exception {
+    Random random = random();
+    for (int i = 0; i < 10000; i++) {
+      int encoded = random.nextInt();
+      double min = encoded * Geo3DUtil.DECODE;
+      double decoded = Geo3DUtil.decodeValueFloor(encoded);
+      // should exactly equal expected value
+      assertEquals(min, decoded, 0.0D);
+      // should round-trip
+      assertEquals(encoded, Geo3DUtil.encodeValue(decoded));
+      // test within the range
+      if (encoded != Integer.MAX_VALUE) {
+        // this is the next representable value
+        // all double values between [min .. max) should encode to the current integer
+        // all double values between (min .. max] should encodeCeil to the next integer.
+        double max = min + Geo3DUtil.DECODE;
+        assertEquals(max, Geo3DUtil.decodeValueFloor(encoded+1), 0.0D);
+        assertEquals(encoded+1, Geo3DUtil.encodeValue(max));
+
+        // first and last doubles in range that will be quantized
+        double minEdge = Math.nextUp(min);
+        double maxEdge = Math.nextDown(max);
+        assertEquals(encoded, Geo3DUtil.encodeValue(minEdge));
+        assertEquals(encoded, Geo3DUtil.encodeValue(maxEdge));
+
+        // check random values within the double range
+        long minBits = NumericUtils.doubleToSortableLong(minEdge);
+        long maxBits = NumericUtils.doubleToSortableLong(maxEdge);
+        for (int j = 0; j < 100; j++) {
+          double value = NumericUtils.sortableLongToDouble(TestUtil.nextLong(random, minBits, maxBits));
+          // round down
+          assertEquals(encoded,   Geo3DUtil.encodeValue(value));
+        }
+      }
+    }
+  }
+
   public void testEncodeDecodeIsStable() throws Exception {
 
     int iters = atLeast(1000);