[postgis-tickets] r17148 - ST_AsMVTGeom: Do resolution check before deserializing
Raul
raul at rmr.ninja
Mon Jan 14 10:02:29 PST 2019
Author: algunenano
Date: 2019-01-14 10:02:29 -0800 (Mon, 14 Jan 2019)
New Revision: 17148
Modified:
trunk/NEWS
trunk/liblwgeom/g_serialized.c
trunk/liblwgeom/liblwgeom_internal.h
trunk/postgis/lwgeom_out_mvt.c
trunk/postgis/mvt.c
trunk/regress/core/mvt.sql
trunk/regress/core/mvt_expected
Log:
ST_AsMVTGeom: Do resolution check before deserializing
Closes #4294
Closes https://github.com/postgis/postgis/pull/358
Modified: trunk/NEWS
===================================================================
--- trunk/NEWS 2019-01-14 16:53:22 UTC (rev 17147)
+++ trunk/NEWS 2019-01-14 18:02:29 UTC (rev 17148)
@@ -35,7 +35,8 @@
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)
Modified: trunk/liblwgeom/g_serialized.c
===================================================================
--- trunk/liblwgeom/g_serialized.c 2019-01-14 16:53:22 UTC (rev 17147)
+++ trunk/liblwgeom/g_serialized.c 2019-01-14 18:02:29 UTC (rev 17148)
@@ -457,7 +457,8 @@
* 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);
Modified: trunk/liblwgeom/liblwgeom_internal.h
===================================================================
--- trunk/liblwgeom/liblwgeom_internal.h 2019-01-14 16:53:22 UTC (rev 17147)
+++ trunk/liblwgeom/liblwgeom_internal.h 2019-01-14 18:02:29 UTC (rev 17148)
@@ -262,6 +262,12 @@
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
*/
double lwcompound_length(const LWCOMPOUND *comp);
Modified: trunk/postgis/lwgeom_out_mvt.c
===================================================================
--- trunk/postgis/lwgeom_out_mvt.c 2019-01-14 16:53:22 UTC (rev 17147)
+++ trunk/postgis/lwgeom_out_mvt.c 2019-01-14 18:02:29 UTC (rev 17148)
@@ -45,26 +45,77 @@
elog(ERROR, "Missing libprotobuf-c");
PG_RETURN_NULL();
#else
+ GBOX *bounds = NULL;
+ int32_t extent = 0;
+ int32_t buffer = 0;
+ bool clip_geom = true;
+ GSERIALIZED *geom_in, *geom_out;
LWGEOM *lwgeom_in, *lwgeom_out;
- GSERIALIZED *geom_in, *geom_out;
- GBOX *bounds;
- int extent, buffer;
- bool clip_geom;
+ 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);
Modified: trunk/postgis/mvt.c
===================================================================
--- trunk/postgis/mvt.c 2019-01-14 16:53:22 UTC (rev 17147)
+++ trunk/postgis/mvt.c 2019-01-14 18:02:29 UTC (rev 17148)
@@ -845,12 +845,6 @@
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;
@@ -857,16 +851,6 @@
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);
Modified: trunk/regress/core/mvt.sql
===================================================================
--- trunk/regress/core/mvt.sql 2019-01-14 16:53:22 UTC (rev 17147)
+++ trunk/regress/core/mvt.sql 2019-01-14 18:02:29 UTC (rev 17148)
@@ -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 @@
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)'),
Modified: trunk/regress/core/mvt_expected
===================================================================
--- trunk/regress/core/mvt_expected 2019-01-14 16:53:22 UTC (rev 17147)
+++ trunk/regress/core/mvt_expected 2019-01-14 18:02:29 UTC (rev 17148)
@@ -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 @@
PG44|
PG45|
PG46|SRID=3857;MULTIPOLYGON(((3245 2224,3262 2030,3253 2158,3245 2224)))
+PG47|
TG1|GiEKBHRlc3QSDBICAAAYASIECTLePxoCYzEiAigBKIAgeAI=
TG2|GiMKBHRlc3QSDhICAAAYASIGETLePwIBGgJjMSICKAEogCB4Ag==
TG3|GiYKBHRlc3QSERICAAAYAiIJCQCAQArQD88PGgJjMSICKAEogCB4Ag==
More information about the postgis-tickets
mailing list