ST_AsMVTGeom: Do resolution check before deserializing

Closes #4294
Closes https://github.com/postgis/postgis/pull/358

git-svn-id: http://svn.osgeo.org/postgis/trunk@17148 b70326c6-7e19-0410-871a-916f4a2858ee

diff --git a/NEWS b/NEWS
index 575f2165c9f3ef3e9164f0b5f132c0be29d44861..48bf79521b7055dffb1d3c55ba6cd322817d2316 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -35,7 +35,8 @@ PostGIS 3.0.0
            Praliaskouski).
   - #4162, ST_DWithin documentation examples for storing geometry and
            radius in table (Darafei Praliaskouski, github user Boscop).
-  - #4161, MVT: Drop geometries smaller than the resolution (Raúl Marín)
+  - #4161 and #4294, ST_AsMVTGeom: Shortcut geometries smaller than the
+           resolution (Raúl Marín)
   - #4176, ST_Intersects supports GEOMETRYCOLLECTION (Darafei Praliaskouski)
   - #4181, ST_AsMVTGeom: Avoid type changes due to validation (Raúl Marín)
   - #4183, ST_AsMVTGeom: Drop invalid geometries after simplification (Raúl Marín)

diff --git a/liblwgeom/g_serialized.c b/liblwgeom/g_serialized.c
index c671ccc4a73d1c0b3dcaa0538f778c6b749033a7..6b9877c11d259775f9b7e99d90f58d4533d2899f 100644 (file)
--- a/liblwgeom/g_serialized.c
+++ b/liblwgeom/g_serialized.c
@@ -457,7 +457,8 @@ int gserialized_read_gbox_p(const GSERIALIZED *g, GBOX *gbox)
 * Populate a bounding box *without* allocating an LWGEOM. Useful
 * for some performance purposes.
 */
-static int gserialized_peek_gbox_p(const GSERIALIZED *g, GBOX *gbox)
+int
+gserialized_peek_gbox_p(const GSERIALIZED *g, GBOX *gbox)
 {
        uint32_t type = gserialized_get_type(g);
 

diff --git a/liblwgeom/liblwgeom_internal.h b/liblwgeom/liblwgeom_internal.h
index 64eee3ef237a6732cadf1c306b586a0855359c71..57e8ca73d8a8f0e0f5cadb21aec6b1da0d96e3cb 100644 (file)
--- a/liblwgeom/liblwgeom_internal.h
+++ b/liblwgeom/liblwgeom_internal.h
@@ -261,6 +261,12 @@ double lwtriangle_area(const LWTRIANGLE *triangle);
 */
 int gserialized_read_gbox_p(const GSERIALIZED *g, GBOX *gbox);
 
+/*
+ * Populate a bounding box *without* allocating an LWGEOM. Useful for some performance
+ * purposes. Use only if gserialized_read_gbox_p failed
+ */
+int gserialized_peek_gbox_p(const GSERIALIZED *g, GBOX *gbox);
+
 /*
 * Length calculations
 */

diff --git a/postgis/lwgeom_out_mvt.c b/postgis/lwgeom_out_mvt.c
index e2e05298eee8fbe70b5c9408c6cf7c859bc5a53e..61660119937fa6289bf3d0e079b92570338524f5 100644 (file)
--- a/postgis/lwgeom_out_mvt.c
+++ b/postgis/lwgeom_out_mvt.c
@@ -45,26 +45,77 @@ Datum ST_AsMVTGeom(PG_FUNCTION_ARGS)
        elog(ERROR, "Missing libprotobuf-c");
        PG_RETURN_NULL();
 #else
-       LWGEOM *lwgeom_in, *lwgeom_out;
+       GBOX *bounds = NULL;
+       int32_t extent = 0;
+       int32_t buffer = 0;
+       bool clip_geom = true;
        GSERIALIZED *geom_in, *geom_out;
-       GBOX *bounds;
-       int extent, buffer;
-       bool clip_geom;
+       LWGEOM *lwgeom_in, *lwgeom_out;
+       uint8_t type = 0;
+
        if (PG_ARGISNULL(0))
+       {
                PG_RETURN_NULL();
-       geom_in = PG_GETARG_GSERIALIZED_P_COPY(0);
-       lwgeom_in = lwgeom_from_gserialized(geom_in);
+       }
+
        if (PG_ARGISNULL(1))
-               elog(ERROR, "%s: parameter bounds cannot be null", __func__);
-       bounds = (GBOX *) PG_GETARG_POINTER(1);
+       {
+               elog(ERROR, "%s: Geometric bounds cannot be null", __func__);
+               PG_RETURN_NULL();
+       }
+       bounds = (GBOX *)PG_GETARG_POINTER(1);
+       if (bounds->xmax - bounds->xmin <= 0 || bounds->ymax - bounds->ymin <= 0)
+       {
+               elog(ERROR, "%s: Geometric bounds are too small", __func__);
+               PG_RETURN_NULL();
+       }
+
        extent = PG_ARGISNULL(2) ? 4096 : PG_GETARG_INT32(2);
+       if (extent <= 0)
+       {
+               elog(ERROR, "%s: Extent must be greater than 0", __func__);
+               PG_RETURN_NULL();
+       }
+
        buffer = PG_ARGISNULL(3) ? 256 : PG_GETARG_INT32(3);
        clip_geom = PG_ARGISNULL(4) ? true : PG_GETARG_BOOL(4);
-       // NOTE: can both return in clone and in place modification so
-       // not known if lwgeom_in can be freed
+
+       geom_in = PG_GETARG_GSERIALIZED_P_COPY(0);
+       type = gserialized_get_type(geom_in);
+
+       /* If possible, peek into the bounding box before deserializing it to discard small geometries
+        * We don't check COLLECTIONTYPE since that might be a collection of points */
+       if (type == LINETYPE || type == POLYGONTYPE || type == MULTILINETYPE || type == MULTIPOLYGONTYPE)
+       {
+               GBOX gserialized_box;
+               /* We only apply the optimization if the bounding box is available */
+               if ((gserialized_read_gbox_p(geom_in, &gserialized_box) == LW_SUCCESS) ||
+                   (gserialized_peek_gbox_p(geom_in, &gserialized_box) == LW_SUCCESS))
+               {
+                       /* Shortcut to drop geometries smaller than the resolution */
+                       double geom_width = gserialized_box.xmax - gserialized_box.xmin;
+                       double geom_height = gserialized_box.ymax - gserialized_box.ymin;
+                       double geom_area = geom_width * geom_height;
+
+                       double bounds_width = (bounds->xmax - bounds->xmin) / extent;
+                       double bounds_height = (bounds->ymax - bounds->ymin) / extent;
+
+                       /* We use 1/4th of the grid square area as the minimum resolution */
+                       double min_resolution_area = bounds_width * bounds_height / 4.0;
+
+                       if (geom_area < min_resolution_area)
+                       {
+                               PG_RETURN_NULL();
+                       }
+               }
+       }
+
+       lwgeom_in = lwgeom_from_gserialized(geom_in);
+
        lwgeom_out = mvt_geom(lwgeom_in, bounds, extent, buffer, clip_geom);
        if (lwgeom_out == NULL)
                PG_RETURN_NULL();
+
        geom_out = geometry_serialize(lwgeom_out);
        lwgeom_free(lwgeom_out);
        PG_FREE_IF_COPY(geom_in, 0);

diff --git a/postgis/mvt.c b/postgis/mvt.c
index fde37abe89764fb9d3a3255ef25edaa37b8cf38e..0b50b1f621b6df93dd042cf3c8084f67c7459e01 100644 (file)
--- a/postgis/mvt.c
+++ b/postgis/mvt.c
@@ -845,28 +845,12 @@ LWGEOM *mvt_geom(LWGEOM *lwgeom, const GBOX *gbox, uint32_t extent, uint32_t buf
        if (lwgeom_is_empty(lwgeom))
                return NULL;
 
-       if (width == 0 || height == 0)
-               elog(ERROR, "mvt_geom: bounds width or height cannot be 0");
-
-       if (extent == 0)
-               elog(ERROR, "mvt_geom: extent cannot be 0");
-
        resx = width / extent;
        resy = height / extent;
        res = (resx < resy ? resx : resy)/2;
        fx = extent / width;
        fy = -(extent / height);
 
-       if (basic_type == LINETYPE || basic_type == POLYGONTYPE)
-       {
-               // Shortcut to drop geometries smaller than the resolution
-               const GBOX *lwgeom_gbox = lwgeom_get_bbox(lwgeom);
-               double bbox_width = lwgeom_gbox->xmax - lwgeom_gbox->xmin;
-               double bbox_height = lwgeom_gbox->ymax - lwgeom_gbox->ymin;
-               if (bbox_height * bbox_height + bbox_width * bbox_width < res * res)
-                       return NULL;
-       }
-
        /* Remove all non-essential points (under the output resolution) */
        lwgeom_remove_repeated_points_in_place(lwgeom, res);
        lwgeom_simplify_in_place(lwgeom, res, preserve_collapsed);

diff --git a/regress/core/mvt.sql b/regress/core/mvt.sql
index 2c1218bfcd67f5f618ccd6b08730d9dad7705acb..7d0bb6e6730a081f31c67e5203c4bb54862f6b3e 100644 (file)
--- a/regress/core/mvt.sql
+++ b/regress/core/mvt.sql
@@ -1,3 +1,10 @@
+-- Input validation
+select 'I1', ST_AsMVTGeom(NULL, ST_MakeEnvelope(10, 10, 20, 20), 4096);
+select 'I2', ST_AsMVTGeom(ST_Point(1, 2), NULL, 4096);
+select 'I3', ST_AsMVTGeom(ST_Point(1, 2), ST_MakeBox2D(ST_Point(0, 0), ST_Point(0, 0)));
+select 'I4', ST_AsMVTGeom(ST_Point(1, 2), ST_MakeEnvelope(10, 10, 20, 20), -10);
+select 'I5', ST_AsMVTGeom(ST_Point(1, 2), ST_MakeEnvelope(10, 10, 20, 20), 0);
+
 -- geometry preprocessing tests
 select 'PG1', ST_AsText(ST_AsMVTGeom(
        ST_Point(1, 2),
@@ -281,6 +288,12 @@ SELECT 'PG46', St_AsEWKT(ST_AsMVTGeom(
        16,
        true));
 
+-- Geometry fastpath
+SELECT 'PG47', ST_AsText(ST_AsMVTGeom(
+       ST_GeomFromText('LINESTRING(0 0, 0 4, 4 4, 4 0, 0 0)'),
+       ST_MakeBox2D(ST_Point(0, 0), ST_Point(1000, 1000)),
+       100, 0, false));
+
 -- geometry encoding tests
 SELECT 'TG1', encode(ST_AsMVT(q, 'test', 4096, 'geom'), 'base64') FROM (SELECT 1 AS c1,
        ST_AsMVTGeom(ST_GeomFromText('POINT(25 17)'),

diff --git a/regress/core/mvt_expected b/regress/core/mvt_expected
index 52f9d972b3d8634a20d30926a839ab9756ce5463..5cab796abe53f1b41726e8fd014b6c9655f90d79 100644 (file)
--- a/regress/core/mvt_expected
+++ b/regress/core/mvt_expected
@@ -1,3 +1,8 @@
+I1|
+ERROR:  ST_AsMVTGeom: Geometric bounds cannot be null
+ERROR:  ST_AsMVTGeom: Geometric bounds are too small
+ERROR:  ST_AsMVTGeom: Extent must be greater than 0
+ERROR:  ST_AsMVTGeom: Extent must be greater than 0
 PG1|POINT(1 4094)
 PG2|POINT(0 4095)
 PG3|POINT(2 4092)
@@ -49,6 +54,7 @@ PG43 - OFF|MULTIPOLYGON(((5 5,-1 -1,11 -1,5 5)),((5 5,11 11,-1 11,5 5)))
 PG44|
 PG45|
 PG46|SRID=3857;MULTIPOLYGON(((3245 2224,3262 2030,3253 2158,3245 2224)))
+PG47|
 TG1|GiEKBHRlc3QSDBICAAAYASIECTLePxoCYzEiAigBKIAgeAI=
 TG2|GiMKBHRlc3QSDhICAAAYASIGETLePwIBGgJjMSICKAEogCB4Ag==
 TG3|GiYKBHRlc3QSERICAAAYAiIJCQCAQArQD88PGgJjMSICKAEogCB4Ag==