Added buffer pixels to vector tile spec parsing (#84710)

* Added buffer pixels to vector tile spec parsing

Previously this was hard-coded to 5, but now is configurable using the
format z/x/y@extent:buffer, where both extent and buffer are optional
and default to 4096 and 5 pixels respectively.

Co-authored-by: James Rodewig <james.rodewig@elastic.co>

docs/changelog/84710.yaml

@@ -0,0 +1,6 @@
+pr: 84710
+summary: Added buffer pixels to vector tile spec parsing
+area: Geo
+type: enhancement
+issues:
+ - 84492

docs/reference/search/search.asciidoc

@@ -408,10 +408,11 @@ http://www.geojson.org[GeoJSON]
 `wkt`:::
 {wikipedia}/Well-known_text_representation_of_geometry[Well Known Text]
 
-`mvt(<zoom>/<x>/<y>@<extent>)` or `mvt(<zoom>/<x>/<y>)`:::
+`mvt(<spec>)`:::
 Binary
 https://docs.mapbox.com/vector-tiles/specification[Mapbox vector tile]. The API
 returns the tile as a base64-encoded string.
+The `<spec>` has the format `<zoom>/<x>/<y>` with two optional suffixes: `@<extent>` and/or `:<buffer>`. For example, `2/0/1` or `2/0/1@4096:5`.
 +
 .`mvt` parameters
 [%collapsible%open]
@@ -428,6 +429,10 @@ returns the tile as a base64-encoded string.
 `<extent>`::
 (Optional, integer) Size, in pixels, of a side of the tile. Vector tiles are
 square with equal sides. Defaults to `4096`.
+
+`<buffer>`::
+(Optional, integer) Size, in pixels, of a clipping buffer outside the tile.
+This allows renderers to avoid outline artifacts from geometries that extend past the extent of the tile. Defaults to `5`.
 ========
 --
 // end::fields-param-props[]

server/src/main/java/org/elasticsearch/common/geo/SimpleVectorTileFormatter.java

@@ -13,13 +13,22 @@
 import java.util.List;
 import java.util.Locale;
 import java.util.function.Function;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
 
 /**
  * A facade for SimpleFeatureFactory that converts it into FormatterFactory for use in GeoPointFieldMapper
  */
 public class SimpleVectorTileFormatter implements GeoFormatterFactory.FormatterFactory<GeoPoint> {
 
     public static final String MVT = "mvt";
+    public static final int DEFAULT_EXTENT = 4096;
+    public static final int DEFAULT_BUFFER_PIXELS = 5;
+    public static final String EXTENT_PREFIX = "@";
+    public static final String BUFFER_PREFIX = ":";
+
+    // we expect either z/x/y or z/x/y@extent or z/x/y@extent:buffer or z/x/y:buffer
+    private static final Pattern pattern = Pattern.compile("(\\d+)/(\\d+)/(\\d+)(" + EXTENT_PREFIX + "\\d+)?(" + BUFFER_PREFIX + "\\d+)?");
 
     @Override
     public String getName() {
@@ -36,28 +45,42 @@ public Function<String, Function<List<GeoPoint>, List<Object>>> getFormatterBuil
     }
 
     /**
-     * Parses string in the format we expect either z/x/y or z/x/y@extent to an array of integer parameters
+     * Parses string in the format we expect, primarily z/x/y, to an array of integer parameters.
+     * There are also two optional additions to the format:
+     * <ul>
+     *     <li><code>@extent</code> - number of pixels across</li>
+     *     <li><code>:buffer</code> - number of pixels by which to widen each tile</li>
+     * </ul>
+     * The resulting array will always have five elements, with the extent and buffer set to the default
+     * values if not provided.
+     * Examples:
+     * <ul>
+     *     <li><code>3/2/2</code> produces <code>[3,2,2,4096,5]</code></li>
+     *     <li><code>2/1/1@5000</code> produces <code>[2,1,1,5000,5]</code></li>
+     *     <li><code>2/1/1@5000:10</code> produces <code>[2,1,1,5000,10]</code></li>
+     * </ul>
      */
     public static int[] parse(String param) {
-        // we expect either z/x/y or z/x/y@extent
-        final String[] parts = param.split("@", 3);
-        if (parts.length > 2) {
+        Matcher matcher = pattern.matcher(param);
+        if (matcher.matches() == false) {
             throw new IllegalArgumentException(
-                "Invalid mvt formatter parameter [" + param + "]. Must have the form \"zoom/x/y\" or \"zoom/x/y@extent\"."
+                "Invalid mvt formatter parameter ["
+                    + param
+                    + "]. Must have the form \"zoom/x/y\" or \"zoom/x/y@extent\" or \"zoom/x/y@extent:buffer\" or \"zoom/x/y:buffer\"."
             );
         }
-        final int extent = parts.length == 2 ? Integer.parseInt(parts[1]) : 4096;
-        final String[] tileBits = parts[0].split("/", 4);
-        if (tileBits.length != 3) {
-            throw new IllegalArgumentException("Invalid tile string [" + parts[0] + "]. Must be three integers in a form \"zoom/x/y\".");
-        }
-        final int z = GeoTileUtils.checkPrecisionRange(Integer.parseInt(tileBits[0]));
+        final int z = GeoTileUtils.checkPrecisionRange(Integer.parseInt(matcher.group(1)));
         final int tiles = 1 << z;
-        final int x = Integer.parseInt(tileBits[1]);
-        final int y = Integer.parseInt(tileBits[2]);
+        final int x = Integer.parseInt(matcher.group(2));
+        final int y = Integer.parseInt(matcher.group(3));
         if (x < 0 || y < 0 || x >= tiles || y >= tiles) {
             throw new IllegalArgumentException(String.format(Locale.ROOT, "Zoom/X/Y combination is not valid: %d/%d/%d", z, x, y));
         }
-        return new int[] { z, x, y, extent };
+        final int extent = matcher.group(4) == null ? DEFAULT_EXTENT : Integer.parseInt(matcher.group(4).substring(1));
+        if (extent <= 0) {
+            throw new IllegalArgumentException(String.format(Locale.ROOT, "Extent is not valid: %d is not > 0", extent));
+        }
+        final int bufferPixels = matcher.group(5) == null ? DEFAULT_BUFFER_PIXELS : Integer.parseInt(matcher.group(5).substring(1));
+        return new int[] { z, x, y, extent, bufferPixels };
     }
 }

server/src/test/java/org/elasticsearch/common/geo/SimpleVectorTileFormatterTests.java

@@ -0,0 +1,102 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0 and the Server Side Public License, v 1; you may not use this file except
+ * in compliance with, at your election, the Elastic License 2.0 or the Server
+ * Side Public License, v 1.
+ */
+
+package org.elasticsearch.common.geo;
+
+import org.elasticsearch.test.ESTestCase;
+
+import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.equalTo;
+
+public class SimpleVectorTileFormatterTests extends ESTestCase {
+
+    public void testEmptyString() {
+        var exception = expectThrows(IllegalArgumentException.class, () -> SimpleVectorTileFormatter.parse(""));
+        assertThat(exception.getMessage(), containsString("Invalid mvt formatter parameter"));
+    }
+
+    public void testInvalid() {
+        var exception = expectThrows(IllegalArgumentException.class, () -> SimpleVectorTileFormatter.parse("invalid"));
+        assertThat(exception.getMessage(), containsString("Invalid mvt formatter parameter"));
+    }
+
+    public void testInvalidCombination() {
+        var exception = expectThrows(IllegalArgumentException.class, () -> SimpleVectorTileFormatter.parse("1/2/3"));
+        assertThat(exception.getMessage(), containsString("Zoom/X/Y combination is not valid"));
+    }
+
+    public void testLeadingWhitespace() {
+        var exception = expectThrows(IllegalArgumentException.class, () -> SimpleVectorTileFormatter.parse(" 2/1/0"));
+        assertThat(exception.getMessage(), containsString("Invalid mvt formatter parameter"));
+    }
+
+    public void testTrailingWhitespace() {
+        var exception = expectThrows(IllegalArgumentException.class, () -> SimpleVectorTileFormatter.parse("2/1/0 "));
+        assertThat(exception.getMessage(), containsString("Invalid mvt formatter parameter"));
+    }
+
+    public void testValidSimple() {
+        int[] fields = SimpleVectorTileFormatter.parse("2/1/0");
+        assertThat(fields.length, equalTo(5));
+        assertThat(fields[0], equalTo(2));
+        assertThat(fields[1], equalTo(1));
+        assertThat(fields[2], equalTo(0));
+        assertThat(fields[3], equalTo(SimpleVectorTileFormatter.DEFAULT_EXTENT));
+        assertThat(fields[4], equalTo(SimpleVectorTileFormatter.DEFAULT_BUFFER_PIXELS));
+    }
+
+    public void testValidWithExtent() {
+        int[] fields = SimpleVectorTileFormatter.parse("2/1/0@5000");
+        assertThat(fields.length, equalTo(5));
+        assertThat(fields[0], equalTo(2));
+        assertThat(fields[1], equalTo(1));
+        assertThat(fields[2], equalTo(0));
+        assertThat(fields[3], equalTo(5000));
+        assertThat(fields[4], equalTo(SimpleVectorTileFormatter.DEFAULT_BUFFER_PIXELS));
+    }
+
+    public void testInvalidExtent() {
+        var exception = expectThrows(IllegalArgumentException.class, () -> SimpleVectorTileFormatter.parse("2/1/0@invalid"));
+        assertThat(exception.getMessage(), containsString("Invalid mvt formatter parameter"));
+    }
+
+    public void testInvalidExtentValue() {
+        var exception = expectThrows(IllegalArgumentException.class, () -> SimpleVectorTileFormatter.parse("2/1/0@0"));
+        assertThat(exception.getMessage(), containsString("Extent is not valid"));
+    }
+
+    public void testValidWithExtentAndBuffer() {
+        int[] fields = SimpleVectorTileFormatter.parse("2/1/0@5000:55");
+        assertThat(fields.length, equalTo(5));
+        assertThat(fields[0], equalTo(2));
+        assertThat(fields[1], equalTo(1));
+        assertThat(fields[2], equalTo(0));
+        assertThat(fields[3], equalTo(5000));
+        assertThat(fields[4], equalTo(55));
+    }
+
+    public void testValidWithBuffer() {
+        int[] fields = SimpleVectorTileFormatter.parse("2/1/0:55");
+        assertThat(fields.length, equalTo(5));
+        assertThat(fields[0], equalTo(2));
+        assertThat(fields[1], equalTo(1));
+        assertThat(fields[2], equalTo(0));
+        assertThat(fields[3], equalTo(SimpleVectorTileFormatter.DEFAULT_EXTENT));
+        assertThat(fields[4], equalTo(55));
+    }
+
+    public void testInvalidBuffer() {
+        var exception = expectThrows(IllegalArgumentException.class, () -> SimpleVectorTileFormatter.parse("2/1/0:invalid"));
+        assertThat(exception.getMessage(), containsString("Invalid mvt formatter parameter"));
+    }
+
+    public void testInvalidBufferValue() {
+        var exception = expectThrows(IllegalArgumentException.class, () -> SimpleVectorTileFormatter.parse("2/1/0:-1"));
+        assertThat(exception.getMessage(), containsString("Invalid mvt formatter parameter"));
+    }
+}

x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/index/mapper/GeoShapeWithDocValuesFieldTypeTests.java

@@ -11,6 +11,7 @@
 import org.elasticsearch.common.geo.GeoFormatterFactory;
 import org.elasticsearch.common.geo.GeometryNormalizer;
 import org.elasticsearch.common.geo.Orientation;
+import org.elasticsearch.common.geo.SimpleVectorTileFormatter;
 import org.elasticsearch.geo.GeometryTestUtils;
 import org.elasticsearch.geometry.Geometry;
 import org.elasticsearch.geometry.utils.WellKnownText;
@@ -111,18 +112,22 @@ private void fetchVectorTile(Geometry geometry) throws IOException {
         final int z = randomIntBetween(1, 10);
         int x = randomIntBetween(0, (1 << z) - 1);
         int y = randomIntBetween(0, (1 << z) - 1);
-        final FeatureFactory featureFactory;
-        final String mvtString;
+        final StringBuilder mvtString = new StringBuilder("mvt(");
+        mvtString.append(z).append("/").append(x).append("/").append(y);
+        int extent = SimpleVectorTileFormatter.DEFAULT_EXTENT;
+        int padPixels = SimpleVectorTileFormatter.DEFAULT_BUFFER_PIXELS;
         if (randomBoolean()) {
-            int extent = randomIntBetween(1 << 8, 1 << 14);
-            mvtString = "mvt(" + z + "/" + x + "/" + y + "@" + extent + ")";
-            featureFactory = new FeatureFactory(z, x, y, extent);
-        } else {
-            mvtString = "mvt(" + z + "/" + x + "/" + y + ")";
-            featureFactory = new FeatureFactory(z, x, y, 4096);
+            extent = randomIntBetween(1 << 8, 1 << 14);
+            mvtString.append(SimpleVectorTileFormatter.EXTENT_PREFIX).append(extent);
+        }
+        if (randomBoolean()) {
+            padPixels = randomIntBetween(0, extent);
+            mvtString.append(SimpleVectorTileFormatter.BUFFER_PREFIX).append(padPixels);
         }
+        mvtString.append(")");
+        final FeatureFactory featureFactory = new FeatureFactory(z, x, y, extent, padPixels);
 
-        final List<?> sourceValue = fetchSourceValue(mapper, WellKnownText.toWKT(geometry), mvtString);
+        final List<?> sourceValue = fetchSourceValue(mapper, WellKnownText.toWKT(geometry), mvtString.toString());
         List<byte[]> features;
         try {
             features = featureFactory.getFeatures(normalize(geometry));

x-pack/plugin/vector-tile/src/main/java/org/elasticsearch/xpack/vectortile/SpatialGeometryFormatterExtension.java

@@ -37,7 +37,7 @@ public Function<String, Function<List<Geometry>, List<Object>>> getFormatterBuil
 
                 return (params) -> {
                     int[] parsed = SimpleVectorTileFormatter.parse(params);
-                    final FeatureFactory featureFactory = new FeatureFactory(parsed[0], parsed[1], parsed[2], parsed[3]);
+                    final FeatureFactory featureFactory = new FeatureFactory(parsed[0], parsed[1], parsed[2], parsed[3], parsed[4]);
                     return geometries -> {
                         final Geometry geometry = (geometries.size() == 1) ? geometries.get(0) : new GeometryCollection<>(geometries);
                         return new ArrayList<>(featureFactory.getFeatures(geometry));

x-pack/plugin/vector-tile/src/main/java/org/elasticsearch/xpack/vectortile/feature/FeatureFactory.java

@@ -56,19 +56,26 @@ public class FeatureFactory {
     private final CoordinateSequenceFilter sequenceFilter;
     // pixel precision of the tile in the mercator projection.
     private final double pixelPrecision;
-    // size of the buffer in pixels for the clip envelope. we choose a values that makes sure
-    // we have values outside the tile for polygon crossing the tile so the outline of the
-    // tile is not part of the final result.
-    // TODO: consider exposing this parameter so users have control of the buffer's size.
-    private static final int BUFFER_SIZE_PIXELS = 5;
 
-    public FeatureFactory(int z, int x, int y, int extent) {
+    /**
+     * The vector-tile feature factory will produce tiles as features based on the tile specification.
+     * @param z - zoom level
+     * @param x - x position of the tile at that zoom
+     * @param y - y position of the tile at that zoom
+     * @param extent - the full extent of entire area in pixels
+     * @param padPixels - a buffer around each tile in pixels
+     * The parameter padPixels is the size of the buffer in pixels for the clip envelope.
+     * We choose a value that ensures we have values outside the tile for polygons crossing
+     * the tile so the outline of the tile is not part of the final result. The default
+     * value is set in SimpleVectorTileFormatter.DEFAULT_BUFFER_PIXELS (currently 5 pixels).
+     */
+    public FeatureFactory(int z, int x, int y, int extent, int padPixels) {
         this.pixelPrecision = 2 * SphericalMercatorUtils.MERCATOR_BOUNDS / ((1L << z) * extent);
         final Rectangle r = SphericalMercatorUtils.recToSphericalMercator(GeoTileUtils.toBoundingBox(x, y, z));
         final Envelope tileEnvelope = new Envelope(r.getMinX(), r.getMaxX(), r.getMinY(), r.getMaxY());
         final Envelope clipEnvelope = new Envelope(tileEnvelope);
         // expand enough the clip envelope to prevent visual artefacts
-        clipEnvelope.expandBy(BUFFER_SIZE_PIXELS * this.pixelPrecision, BUFFER_SIZE_PIXELS * this.pixelPrecision);
+        clipEnvelope.expandBy(padPixels * this.pixelPrecision, padPixels * this.pixelPrecision);
         final GeometryFactory geomFactory = new GeometryFactory();
         this.builder = new JTSGeometryBuilder(geomFactory);
         this.clipTile = geomFactory.toGeometry(clipEnvelope);

x-pack/plugin/vector-tile/src/test/java/org/elasticsearch/xpack/vectortile/feature/FeatureFactoriesConsistencyTests.java

@@ -27,9 +27,10 @@ public void testPoint() throws IOException {
         int x = randomIntBetween(0, (1 << z) - 1);
         int y = randomIntBetween(0, (1 << z) - 1);
         int extent = randomIntBetween(1 << 8, 1 << 14);
+        int padPixels = randomIntBetween(0, extent);
         Rectangle rectangle = GeoTileUtils.toBoundingBox(x, y, z);
         SimpleFeatureFactory builder = new SimpleFeatureFactory(z, x, y, extent);
-        FeatureFactory factory = new FeatureFactory(z, x, y, extent);
+        FeatureFactory factory = new FeatureFactory(z, x, y, extent, padPixels);
         List<Point> points = new ArrayList<>();
         List<GeoPoint> geoPoints = new ArrayList<>();
         for (int i = 0; i < 10; i++) {
@@ -52,8 +53,9 @@ public void testRectangle() throws IOException {
         int x = randomIntBetween(0, (1 << z) - 1);
         int y = randomIntBetween(0, (1 << z) - 1);
         int extent = randomIntBetween(1 << 8, 1 << 14);
+        int padPixels = randomIntBetween(0, extent);
         SimpleFeatureFactory builder = new SimpleFeatureFactory(z, x, y, extent);
-        FeatureFactory factory = new FeatureFactory(z, x, y, extent);
+        FeatureFactory factory = new FeatureFactory(z, x, y, extent, padPixels);
         Rectangle r = GeoTileUtils.toBoundingBox(x, y, z);
         for (int i = 0; i < extent; i++) {
             byte[] b1 = builder.box(r.getMinLon(), r.getMaxLon(), r.getMinLat(), r.getMaxLat());
@@ -67,8 +69,9 @@ public void testDegeneratedRectangle() throws IOException {
         int x = randomIntBetween(1, (1 << z) - 1);
         int y = randomIntBetween(1, (1 << z) - 1);
         int extent = randomIntBetween(1 << 8, 1 << 14);
+        int padPixels = randomIntBetween(0, extent);
         SimpleFeatureFactory builder = new SimpleFeatureFactory(z, x, y, extent);
-        FeatureFactory factory = new FeatureFactory(z, x, y, extent);
+        FeatureFactory factory = new FeatureFactory(z, x, y, extent, padPixels);
         {
             Rectangle r = GeoTileUtils.toBoundingBox(x, y, z);
             // box is a point