Don't normalize coordinates in GeoTileUtils (#114929) (#115030)

The main users of this class use as input latitudes and longitudes read from doc values. These coordinates are always 
on bounds so there is no point to try to normalise them, more over when this piece of code is in the hot path for 
aggregations.

server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoTileUtils.java

@@ -21,8 +21,6 @@ import org.elasticsearch.xcontent.XContentParser;
 import java.io.IOException;
 import java.util.Locale;
 
-import static org.elasticsearch.common.geo.GeoUtils.normalizeLat;
-import static org.elasticsearch.common.geo.GeoUtils.normalizeLon;
 import static org.elasticsearch.common.geo.GeoUtils.quantizeLat;
 
 /**
@@ -113,15 +111,13 @@ public final class GeoTileUtils {
      * Calculates the x-coordinate in the tile grid for the specified longitude given
      * the number of tile columns for a pre-determined zoom-level.
      *
-     * @param longitude the longitude to use when determining the tile x-coordinate
+     * @param longitude the longitude to use when determining the tile x-coordinate. Longitude is in degrees
+     *                  and must be between -180 and 180 degrees.
      * @param tiles     the number of tiles per row for a pre-determined zoom-level
      */
     public static int getXTile(double longitude, int tiles) {
-        // normalizeLon treats this as 180, which is not friendly for tile mapping
-        if (longitude == -180) {
-            return 0;
-        }
-        final double xTile = (normalizeLon(longitude) + 180.0) / 360.0 * tiles;
+        assert longitude >= -180 && longitude <= 180 : "Longitude must be between -180 and 180 degrees";
+        final double xTile = (longitude + 180.0) / 360.0 * tiles;
         // Edge values may generate invalid values, and need to be clipped.
         return Math.max(0, Math.min(tiles - 1, (int) Math.floor(xTile)));
     }
@@ -130,11 +126,13 @@ public final class GeoTileUtils {
      * Calculates the y-coordinate in the tile grid for the specified longitude given
      * the number of tile rows for pre-determined zoom-level.
      *
-     * @param latitude  the latitude to use when determining the tile y-coordinate
+     * @param latitude  the latitude to use when determining the tile y-coordinate. Latitude is in degrees
+     *                  and must be between -90 and 90 degrees.
      * @param tiles     the number of tiles per column for a pre-determined zoom-level
      */
     public static int getYTile(double latitude, int tiles) {
-        final double latSin = SloppyMath.cos(PI_DIV_2 - Math.toRadians(normalizeLat(latitude)));
+        assert latitude >= -90 && latitude <= 90 : "Latitude must be between -90 and 90 degrees";
+        final double latSin = SloppyMath.cos(PI_DIV_2 - Math.toRadians(latitude));
         final double yTile = (0.5 - (ESSloppyMath.log((1.0 + latSin) / (1.0 - latSin)) / PI_TIMES_4)) * tiles;
         // Edge values may generate invalid values, and need to be clipped.
         // For example, polar regions (above/below lat 85.05112878) get normalized.

server/src/test/java/org/elasticsearch/search/DocValueFormatTests.java

@@ -11,6 +11,7 @@ package org.elasticsearch.search;
 
 import org.apache.lucene.document.InetAddressPoint;
 import org.apache.lucene.util.BytesRef;
+import org.elasticsearch.common.geo.GeoUtils;
 import org.elasticsearch.common.io.stream.BytesStreamOutput;
 import org.elasticsearch.common.io.stream.NamedWriteableAwareStreamInput;
 import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
@@ -173,8 +174,8 @@ public class DocValueFormatTests extends ESTestCase {
         assertEquals("29/536869420/0", DocValueFormat.GEOTILE.format(longEncode(179.999, 89.999, 29)));
         assertEquals("29/1491/536870911", DocValueFormat.GEOTILE.format(longEncode(-179.999, -89.999, 29)));
         assertEquals("2/2/1", DocValueFormat.GEOTILE.format(longEncode(1, 1, 2)));
-        assertEquals("1/1/0", DocValueFormat.GEOTILE.format(longEncode(13, 95, 1)));
-        assertEquals("1/1/1", DocValueFormat.GEOTILE.format(longEncode(13, -95, 1)));
+        assertEquals("1/1/0", DocValueFormat.GEOTILE.format(longEncode(13, GeoUtils.normalizeLat(95), 1)));
+        assertEquals("1/1/1", DocValueFormat.GEOTILE.format(longEncode(13, GeoUtils.normalizeLat(-95), 1)));
     }
 
     public void testRawParse() {

server/src/test/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoTileUtilsTests.java

@@ -16,6 +16,8 @@ import org.elasticsearch.geometry.Rectangle;
 import org.elasticsearch.test.ESTestCase;
 import org.hamcrest.Matchers;
 
+import static org.elasticsearch.common.geo.GeoUtils.normalizeLat;
+import static org.elasticsearch.common.geo.GeoUtils.normalizeLon;
 import static org.elasticsearch.search.aggregations.bucket.geogrid.GeoTileUtils.MAX_ZOOM;
 import static org.elasticsearch.search.aggregations.bucket.geogrid.GeoTileUtils.checkPrecisionRange;
 import static org.elasticsearch.search.aggregations.bucket.geogrid.GeoTileUtils.hashToGeoPoint;
@@ -53,20 +55,20 @@ public class GeoTileUtilsTests extends ESTestCase {
         assertEquals(0x77FFFF4580000000L, longEncode(179.999, 89.999, 29));
         assertEquals(0x740000BA7FFFFFFFL, longEncode(-179.999, -89.999, 29));
         assertEquals(0x0800000040000001L, longEncode(1, 1, 2));
-        assertEquals(0x0C00000060000000L, longEncode(-20, 100, 3));
+        assertEquals(0x0C00000060000000L, longEncode(-20, normalizeLat(100), 3));
         assertEquals(0x71127D27C8ACA67AL, longEncode(13, -15, 28));
         assertEquals(0x4C0077776003A9ACL, longEncode(-12, 15, 19));
-        assertEquals(0x140000024000000EL, longEncode(-328.231870, 16.064082, 5));
-        assertEquals(0x6436F96B60000000L, longEncode(-590.769588, 89.549167, 25));
-        assertEquals(0x6411BD6BA0A98359L, longEncode(999.787079, 51.830093, 25));
-        assertEquals(0x751BD6BBCA983596L, longEncode(999.787079, 51.830093, 29));
-        assertEquals(0x77CF880A20000000L, longEncode(-557.039740, -632.103969, 29));
+        assertEquals(0x140000024000000EL, longEncode(normalizeLon(-328.231870), 16.064082, 5));
+        assertEquals(0x6436F96B60000000L, longEncode(normalizeLon(-590.769588), 89.549167, 25));
+        assertEquals(0x6411BD6BA0A98359L, longEncode(normalizeLon(999.787079), 51.830093, 25));
+        assertEquals(0x751BD6BBCA983596L, longEncode(normalizeLon(999.787079), 51.830093, 29));
+        assertEquals(0x77CF880A20000000L, longEncode(normalizeLon(-557.039740), normalizeLat(-632.103969), 29));
         assertEquals(0x7624FA4FA0000000L, longEncode(13, 88, 29));
         assertEquals(0x7624FA4FBFFFFFFFL, longEncode(13, -88, 29));
         assertEquals(0x0400000020000000L, longEncode(13, 89, 1));
         assertEquals(0x0400000020000001L, longEncode(13, -89, 1));
-        assertEquals(0x0400000020000000L, longEncode(13, 95, 1));
-        assertEquals(0x0400000020000001L, longEncode(13, -95, 1));
+        assertEquals(0x0400000020000000L, longEncode(13, normalizeLat(95), 1));
+        assertEquals(0x0400000020000001L, longEncode(13, normalizeLat(-95), 1));
 
         expectThrows(IllegalArgumentException.class, () -> longEncode(0, 0, -1));
         expectThrows(IllegalArgumentException.class, () -> longEncode(-1, 0, MAX_ZOOM + 1));
@@ -78,20 +80,20 @@ public class GeoTileUtilsTests extends ESTestCase {
         assertEquals(0x77FFFF4580000000L, longEncode(stringEncode(longEncode(179.999, 89.999, 29))));
         assertEquals(0x740000BA7FFFFFFFL, longEncode(stringEncode(longEncode(-179.999, -89.999, 29))));
         assertEquals(0x0800000040000001L, longEncode(stringEncode(longEncode(1, 1, 2))));
-        assertEquals(0x0C00000060000000L, longEncode(stringEncode(longEncode(-20, 100, 3))));
+        assertEquals(0x0C00000060000000L, longEncode(stringEncode(longEncode(-20, normalizeLat(100), 3))));
         assertEquals(0x71127D27C8ACA67AL, longEncode(stringEncode(longEncode(13, -15, 28))));
         assertEquals(0x4C0077776003A9ACL, longEncode(stringEncode(longEncode(-12, 15, 19))));
-        assertEquals(0x140000024000000EL, longEncode(stringEncode(longEncode(-328.231870, 16.064082, 5))));
-        assertEquals(0x6436F96B60000000L, longEncode(stringEncode(longEncode(-590.769588, 89.549167, 25))));
-        assertEquals(0x6411BD6BA0A98359L, longEncode(stringEncode(longEncode(999.787079, 51.830093, 25))));
-        assertEquals(0x751BD6BBCA983596L, longEncode(stringEncode(longEncode(999.787079, 51.830093, 29))));
-        assertEquals(0x77CF880A20000000L, longEncode(stringEncode(longEncode(-557.039740, -632.103969, 29))));
+        assertEquals(0x140000024000000EL, longEncode(stringEncode(longEncode(normalizeLon(-328.231870), 16.064082, 5))));
+        assertEquals(0x6436F96B60000000L, longEncode(stringEncode(longEncode(normalizeLon(-590.769588), 89.549167, 25))));
+        assertEquals(0x6411BD6BA0A98359L, longEncode(stringEncode(longEncode(normalizeLon(999.787079), 51.830093, 25))));
+        assertEquals(0x751BD6BBCA983596L, longEncode(stringEncode(longEncode(normalizeLon(999.787079), 51.830093, 29))));
+        assertEquals(0x77CF880A20000000L, longEncode(stringEncode(longEncode(normalizeLon(-557.039740), normalizeLat(-632.103969), 29))));
         assertEquals(0x7624FA4FA0000000L, longEncode(stringEncode(longEncode(13, 88, 29))));
         assertEquals(0x7624FA4FBFFFFFFFL, longEncode(stringEncode(longEncode(13, -88, 29))));
         assertEquals(0x0400000020000000L, longEncode(stringEncode(longEncode(13, 89, 1))));
         assertEquals(0x0400000020000001L, longEncode(stringEncode(longEncode(13, -89, 1))));
-        assertEquals(0x0400000020000000L, longEncode(stringEncode(longEncode(13, 95, 1))));
-        assertEquals(0x0400000020000001L, longEncode(stringEncode(longEncode(13, -95, 1))));
+        assertEquals(0x0400000020000000L, longEncode(stringEncode(longEncode(13, normalizeLat(95), 1))));
+        assertEquals(0x0400000020000001L, longEncode(stringEncode(longEncode(13, normalizeLat(-95), 1))));
 
         expectThrows(IllegalArgumentException.class, () -> longEncode("12/asdf/1"));
         expectThrows(IllegalArgumentException.class, () -> longEncode("foo"));