Skip to content

Commit 918e8c4

Browse files
iveraseiverase
authored
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.
1 parent f7ecddb commit 918e8c4

File tree

3 files changed

+29
-28
lines changed

3 files changed

+29
-28
lines changed

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

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,6 @@
2121
import java.io.IOException;
2222
import java.util.Locale;
2323

24-
import static org.elasticsearch.common.geo.GeoUtils.normalizeLat;
25-
import static org.elasticsearch.common.geo.GeoUtils.normalizeLon;
2624
import static org.elasticsearch.common.geo.GeoUtils.quantizeLat;
2725

2826
/**
@@ -113,15 +111,13 @@ public static int checkPrecisionRange(int precision) {
113111
* Calculates the x-coordinate in the tile grid for the specified longitude given
114112
* the number of tile columns for a pre-determined zoom-level.
115113
*
116-
* @param longitude the longitude to use when determining the tile x-coordinate
114+
* @param longitude the longitude to use when determining the tile x-coordinate. Longitude is in degrees
115+
* and must be between -180 and 180 degrees.
117116
* @param tiles the number of tiles per row for a pre-determined zoom-level
118117
*/
119118
public static int getXTile(double longitude, int tiles) {
120-
// normalizeLon treats this as 180, which is not friendly for tile mapping
121-
if (longitude == -180) {
122-
return 0;
123-
}
124-
final double xTile = (normalizeLon(longitude) + 180.0) / 360.0 * tiles;
119+
assert longitude >= -180 && longitude <= 180 : "Longitude must be between -180 and 180 degrees";
120+
final double xTile = (longitude + 180.0) / 360.0 * tiles;
125121
// Edge values may generate invalid values, and need to be clipped.
126122
return Math.max(0, Math.min(tiles - 1, (int) Math.floor(xTile)));
127123
}
@@ -130,11 +126,13 @@ public static int getXTile(double longitude, int tiles) {
130126
* Calculates the y-coordinate in the tile grid for the specified longitude given
131127
* the number of tile rows for pre-determined zoom-level.
132128
*
133-
* @param latitude the latitude to use when determining the tile y-coordinate
129+
* @param latitude the latitude to use when determining the tile y-coordinate. Latitude is in degrees
130+
* and must be between -90 and 90 degrees.
134131
* @param tiles the number of tiles per column for a pre-determined zoom-level
135132
*/
136133
public static int getYTile(double latitude, int tiles) {
137-
final double latSin = SloppyMath.cos(PI_DIV_2 - Math.toRadians(normalizeLat(latitude)));
134+
assert latitude >= -90 && latitude <= 90 : "Latitude must be between -90 and 90 degrees";
135+
final double latSin = SloppyMath.cos(PI_DIV_2 - Math.toRadians(latitude));
138136
final double yTile = (0.5 - (ESSloppyMath.log((1.0 + latSin) / (1.0 - latSin)) / PI_TIMES_4)) * tiles;
139137
// Edge values may generate invalid values, and need to be clipped.
140138
// For example, polar regions (above/below lat 85.05112878) get normalized.

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
import org.apache.lucene.document.InetAddressPoint;
1313
import org.apache.lucene.util.BytesRef;
14+
import org.elasticsearch.common.geo.GeoUtils;
1415
import org.elasticsearch.common.io.stream.BytesStreamOutput;
1516
import org.elasticsearch.common.io.stream.NamedWriteableAwareStreamInput;
1617
import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
@@ -173,8 +174,8 @@ public void testGeoTileFormat() {
173174
assertEquals("29/536869420/0", DocValueFormat.GEOTILE.format(longEncode(179.999, 89.999, 29)));
174175
assertEquals("29/1491/536870911", DocValueFormat.GEOTILE.format(longEncode(-179.999, -89.999, 29)));
175176
assertEquals("2/2/1", DocValueFormat.GEOTILE.format(longEncode(1, 1, 2)));
176-
assertEquals("1/1/0", DocValueFormat.GEOTILE.format(longEncode(13, 95, 1)));
177-
assertEquals("1/1/1", DocValueFormat.GEOTILE.format(longEncode(13, -95, 1)));
177+
assertEquals("1/1/0", DocValueFormat.GEOTILE.format(longEncode(13, GeoUtils.normalizeLat(95), 1)));
178+
assertEquals("1/1/1", DocValueFormat.GEOTILE.format(longEncode(13, GeoUtils.normalizeLat(-95), 1)));
178179
}
179180

180181
public void testRawParse() {

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

Lines changed: 18 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616
import org.elasticsearch.test.ESTestCase;
1717
import org.hamcrest.Matchers;
1818

19+
import static org.elasticsearch.common.geo.GeoUtils.normalizeLat;
20+
import static org.elasticsearch.common.geo.GeoUtils.normalizeLon;
1921
import static org.elasticsearch.search.aggregations.bucket.geogrid.GeoTileUtils.MAX_ZOOM;
2022
import static org.elasticsearch.search.aggregations.bucket.geogrid.GeoTileUtils.checkPrecisionRange;
2123
import static org.elasticsearch.search.aggregations.bucket.geogrid.GeoTileUtils.hashToGeoPoint;
@@ -53,20 +55,20 @@ public void testLongEncode() {
5355
assertEquals(0x77FFFF4580000000L, longEncode(179.999, 89.999, 29));
5456
assertEquals(0x740000BA7FFFFFFFL, longEncode(-179.999, -89.999, 29));
5557
assertEquals(0x0800000040000001L, longEncode(1, 1, 2));
56-
assertEquals(0x0C00000060000000L, longEncode(-20, 100, 3));
58+
assertEquals(0x0C00000060000000L, longEncode(-20, normalizeLat(100), 3));
5759
assertEquals(0x71127D27C8ACA67AL, longEncode(13, -15, 28));
5860
assertEquals(0x4C0077776003A9ACL, longEncode(-12, 15, 19));
59-
assertEquals(0x140000024000000EL, longEncode(-328.231870, 16.064082, 5));
60-
assertEquals(0x6436F96B60000000L, longEncode(-590.769588, 89.549167, 25));
61-
assertEquals(0x6411BD6BA0A98359L, longEncode(999.787079, 51.830093, 25));
62-
assertEquals(0x751BD6BBCA983596L, longEncode(999.787079, 51.830093, 29));
63-
assertEquals(0x77CF880A20000000L, longEncode(-557.039740, -632.103969, 29));
61+
assertEquals(0x140000024000000EL, longEncode(normalizeLon(-328.231870), 16.064082, 5));
62+
assertEquals(0x6436F96B60000000L, longEncode(normalizeLon(-590.769588), 89.549167, 25));
63+
assertEquals(0x6411BD6BA0A98359L, longEncode(normalizeLon(999.787079), 51.830093, 25));
64+
assertEquals(0x751BD6BBCA983596L, longEncode(normalizeLon(999.787079), 51.830093, 29));
65+
assertEquals(0x77CF880A20000000L, longEncode(normalizeLon(-557.039740), normalizeLat(-632.103969), 29));
6466
assertEquals(0x7624FA4FA0000000L, longEncode(13, 88, 29));
6567
assertEquals(0x7624FA4FBFFFFFFFL, longEncode(13, -88, 29));
6668
assertEquals(0x0400000020000000L, longEncode(13, 89, 1));
6769
assertEquals(0x0400000020000001L, longEncode(13, -89, 1));
68-
assertEquals(0x0400000020000000L, longEncode(13, 95, 1));
69-
assertEquals(0x0400000020000001L, longEncode(13, -95, 1));
70+
assertEquals(0x0400000020000000L, longEncode(13, normalizeLat(95), 1));
71+
assertEquals(0x0400000020000001L, longEncode(13, normalizeLat(-95), 1));
7072

7173
expectThrows(IllegalArgumentException.class, () -> longEncode(0, 0, -1));
7274
expectThrows(IllegalArgumentException.class, () -> longEncode(-1, 0, MAX_ZOOM + 1));
@@ -78,20 +80,20 @@ public void testLongEncodeFromString() {
7880
assertEquals(0x77FFFF4580000000L, longEncode(stringEncode(longEncode(179.999, 89.999, 29))));
7981
assertEquals(0x740000BA7FFFFFFFL, longEncode(stringEncode(longEncode(-179.999, -89.999, 29))));
8082
assertEquals(0x0800000040000001L, longEncode(stringEncode(longEncode(1, 1, 2))));
81-
assertEquals(0x0C00000060000000L, longEncode(stringEncode(longEncode(-20, 100, 3))));
83+
assertEquals(0x0C00000060000000L, longEncode(stringEncode(longEncode(-20, normalizeLat(100), 3))));
8284
assertEquals(0x71127D27C8ACA67AL, longEncode(stringEncode(longEncode(13, -15, 28))));
8385
assertEquals(0x4C0077776003A9ACL, longEncode(stringEncode(longEncode(-12, 15, 19))));
84-
assertEquals(0x140000024000000EL, longEncode(stringEncode(longEncode(-328.231870, 16.064082, 5))));
85-
assertEquals(0x6436F96B60000000L, longEncode(stringEncode(longEncode(-590.769588, 89.549167, 25))));
86-
assertEquals(0x6411BD6BA0A98359L, longEncode(stringEncode(longEncode(999.787079, 51.830093, 25))));
87-
assertEquals(0x751BD6BBCA983596L, longEncode(stringEncode(longEncode(999.787079, 51.830093, 29))));
88-
assertEquals(0x77CF880A20000000L, longEncode(stringEncode(longEncode(-557.039740, -632.103969, 29))));
86+
assertEquals(0x140000024000000EL, longEncode(stringEncode(longEncode(normalizeLon(-328.231870), 16.064082, 5))));
87+
assertEquals(0x6436F96B60000000L, longEncode(stringEncode(longEncode(normalizeLon(-590.769588), 89.549167, 25))));
88+
assertEquals(0x6411BD6BA0A98359L, longEncode(stringEncode(longEncode(normalizeLon(999.787079), 51.830093, 25))));
89+
assertEquals(0x751BD6BBCA983596L, longEncode(stringEncode(longEncode(normalizeLon(999.787079), 51.830093, 29))));
90+
assertEquals(0x77CF880A20000000L, longEncode(stringEncode(longEncode(normalizeLon(-557.039740), normalizeLat(-632.103969), 29))));
8991
assertEquals(0x7624FA4FA0000000L, longEncode(stringEncode(longEncode(13, 88, 29))));
9092
assertEquals(0x7624FA4FBFFFFFFFL, longEncode(stringEncode(longEncode(13, -88, 29))));
9193
assertEquals(0x0400000020000000L, longEncode(stringEncode(longEncode(13, 89, 1))));
9294
assertEquals(0x0400000020000001L, longEncode(stringEncode(longEncode(13, -89, 1))));
93-
assertEquals(0x0400000020000000L, longEncode(stringEncode(longEncode(13, 95, 1))));
94-
assertEquals(0x0400000020000001L, longEncode(stringEncode(longEncode(13, -95, 1))));
95+
assertEquals(0x0400000020000000L, longEncode(stringEncode(longEncode(13, normalizeLat(95), 1))));
96+
assertEquals(0x0400000020000001L, longEncode(stringEncode(longEncode(13, normalizeLat(-95), 1))));
9597

9698
expectThrows(IllegalArgumentException.class, () -> longEncode("12/asdf/1"));
9799
expectThrows(IllegalArgumentException.class, () -> longEncode("foo"));

0 commit comments

Comments
 (0)