[postgis-tickets] [SCM] PostGIS branch master updated. 3.2.0-626-gc273f3943
git at osgeo.org
git at osgeo.org
Fri Mar 4 03:12:48 PST 2022
This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "PostGIS".
The branch, master has been updated
via c273f3943df45ef06907be12282c20caea6784c2 (commit)
from 2fe49a4b5c7441bee2c2e987e93d0521de51957e (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -----------------------------------------------------------------
commit c273f3943df45ef06907be12282c20caea6784c2
Author: Sandro Santilli <strk at kbt.io>
Date: Thu Mar 3 20:38:06 2022 +0100
Speed up face MBR computation during topology editing
Closes #5111
Update test for #4706 as we won't throw an exception now,
from ST_ChangeEdgeGeom, on invalid topology
diff --git a/liblwgeom/liblwgeom_topo.h b/liblwgeom/liblwgeom_topo.h
index c49ba0bda..276d9d7cf 100644
--- a/liblwgeom/liblwgeom_topo.h
+++ b/liblwgeom/liblwgeom_topo.h
@@ -853,6 +853,17 @@ typedef struct LWT_BE_CALLBACKS_T {
uint64_t *numelems,
int fields);
+ /**
+ * Compute minimum bounding rectangle of a face
+ *
+ * @param topo the topology to act upon
+ * @param face the face identifier
+ *
+ * @return a GBOX describing the minimum bounding rectangle of the face.
+ *
+ */
+ GBOX *(*computeFaceMBR)(const LWT_BE_TOPOLOGY *topo, LWT_ELEMID face);
+
} LWT_BE_CALLBACKS;
diff --git a/liblwgeom/lwgeom_topo.c b/liblwgeom/lwgeom_topo.c
index 97e0515eb..479a65386 100644
--- a/liblwgeom/lwgeom_topo.c
+++ b/liblwgeom/lwgeom_topo.c
@@ -412,6 +412,12 @@ lwt_be_getClosestEdge(const LWT_TOPOLOGY *topo, const LWPOINT *pt, uint64_t *num
CBT3(topo, getClosestEdge, pt, numelems, fields);
}
+static GBOX *
+lwt_be_computeFaceMBR(const LWT_TOPOLOGY *topo, LWT_ELEMID face)
+{
+ CBT1(topo, computeFaceMBR, face);
+}
+
/************************************************************************
*
* Utility functions
@@ -2043,7 +2049,7 @@ _lwt_AddFaceSplit( LWT_TOPOLOGY* topo,
lwerror("Backend error: %s", lwt_be_lastErrorMessage(topo->be_iface));
return -2;
}
- LWDEBUGF(1, "lwt_be_getEdgeByFace returned %d edges", numfaceedges);
+ LWDEBUGF(1, "_lwt_AddFaceSplit: lwt_be_getEdgeByFace returned %d edges", numfaceedges);
if ( numfaceedges )
{
@@ -2831,6 +2837,7 @@ lwt_GetFaceGeometry(LWT_TOPOLOGY* topo, LWT_ELEMID faceid)
lwerror("Backend error: %s", lwt_be_lastErrorMessage(topo->be_iface));
return NULL;
}
+ LWDEBUGF(1, "lwt_GetFaceGeometry: lwt_be_getEdgeByFace returned %d edges", numfaceedges);
if ( numfaceedges == 0 )
{
@@ -3078,6 +3085,7 @@ lwt_GetFaceEdges(LWT_TOPOLOGY* topo, LWT_ELEMID face_id, LWT_ELEMID **out )
return -1;
}
if ( ! numfaceedges ) return 0; /* no edges in output */
+ LWDEBUGF(1, "lwt_GetFaceEdges: lwt_be_getEdgeByFace returned %d edges", numfaceedges);
/* order edges by occurrence in face */
@@ -3480,89 +3488,53 @@ lwt_ChangeEdgeGeom(LWT_TOPOLOGY* topo, LWT_ELEMID edge_id, LWLINE *geom)
const GBOX* newbox = lwgeom_get_bbox(lwline_as_lwgeom(geom));
if ( ! gbox_same(oldbox, newbox) )
{
+ GBOX* updatedBox;
uint64_t facestoupdate = 0;
LWT_ISO_FACE faces[2];
- LWGEOM *nface1 = NULL;
- LWGEOM *nface2 = NULL;
if ( oldedge->face_left > 0 )
{
- nface1 = lwt_GetFaceGeometry(topo, oldedge->face_left);
- if ( ! nface1 )
- {
- lwerror("lwt_ChangeEdgeGeom could not construct face %"
- LWTFMT_ELEMID ", on the left of edge %" LWTFMT_ELEMID,
- oldedge->face_left, edge_id);
- return -1;
- }
- #if 0
- {
- size_t sz;
- char *wkt = lwgeom_to_wkt(nface1, WKT_EXTENDED, 2, &sz);
- LWDEBUGF(1, "new geometry of face left (%d): %s", (int)oldedge->face_left, wkt);
- lwfree(wkt);
- }
- #endif
- lwgeom_add_bbox(nface1);
- if ( ! nface1->bbox )
+ updatedBox = lwt_be_computeFaceMBR(topo, oldedge->face_left);
+ if ( ! updatedBox )
{
lwerror("Corrupted topology: face %d, left of edge %d, has no bbox",
oldedge->face_left, edge_id);
return -1;
}
faces[facestoupdate].face_id = oldedge->face_left;
- /* ownership left to nface */
- faces[facestoupdate++].mbr = nface1->bbox;
+ /* ownership transferred to faces[] */
+ faces[facestoupdate++].mbr = updatedBox;
}
if ( oldedge->face_right > 0
/* no need to update twice the same face.. */
&& oldedge->face_right != oldedge->face_left )
{
- nface2 = lwt_GetFaceGeometry(topo, oldedge->face_right);
- if ( ! nface2 )
- {
- lwerror("lwt_ChangeEdgeGeom could not construct face %"
- LWTFMT_ELEMID ", on the right of edge %" LWTFMT_ELEMID,
- oldedge->face_right, edge_id);
- return -1;
- }
- #if 0
- {
- size_t sz;
- char *wkt = lwgeom_to_wkt(nface2, WKT_EXTENDED, 2, &sz);
- LWDEBUGF(1, "new geometry of face right (%d): %s", (int)oldedge->face_right, wkt);
- lwfree(wkt);
- }
- #endif
- lwgeom_add_bbox(nface2);
- if ( ! nface2->bbox )
+ updatedBox = lwt_be_computeFaceMBR(topo, oldedge->face_right);
+ if ( ! updatedBox )
{
lwerror("Corrupted topology: face %d, right of edge %d, has no bbox",
oldedge->face_right, edge_id);
return -1;
}
faces[facestoupdate].face_id = oldedge->face_right;
- faces[facestoupdate++].mbr = nface2->bbox; /* ownership left to nface */
+ /* ownership transferred to faces[] */
+ faces[facestoupdate++].mbr = updatedBox;
}
LWDEBUGF(1, "%d faces to update", facestoupdate);
if ( facestoupdate )
{
- uint64_t updatedFaces = lwt_be_updateFacesById(topo, &(faces[0]), facestoupdate);
+ uint64_t updatedFaces = lwt_be_updateFacesById(topo, &(faces[0]), facestoupdate);
if (updatedFaces != facestoupdate)
{
- if (nface1)
- lwgeom_free(nface1);
- if (nface2)
- lwgeom_free(nface2);
+ while ( facestoupdate-- ) lwfree(faces[facestoupdate].mbr);
_lwt_release_edges(oldedge, 1);
if (updatedFaces == UINT64_MAX)
lwerror("Backend error: %s", lwt_be_lastErrorMessage(topo->be_iface));
else
- lwerror("Unexpected error: %d faces found when expecting 1", i);
+ lwerror("Unexpected error: %d faces updated when expecting 1", updatedFaces);
return -1;
}
}
- if ( nface1 ) lwgeom_free(nface1);
- if ( nface2 ) lwgeom_free(nface2);
+ while ( facestoupdate-- ) lwfree(faces[facestoupdate].mbr);
}
else
{
diff --git a/topology/postgis_topology.c b/topology/postgis_topology.c
index dd799e56a..f67af1be7 100644
--- a/topology/postgis_topology.c
+++ b/topology/postgis_topology.c
@@ -3046,6 +3046,100 @@ cb_getClosestEdge( const LWT_BE_TOPOLOGY* topo, const LWPOINT* pt, uint64_t *num
return edges;
}
+static GBOX *
+cb_computeFaceMBR(const LWT_BE_TOPOLOGY *topo, LWT_ELEMID face)
+{
+ //LWT_ELEMID *edges;
+ int spi_result;
+ HeapTuple row;
+ //TupleDesc rowdesc;
+ bool isnull;
+ Datum dat;
+ GSERIALIZED *geom;
+ LWGEOM *lwg;
+ StringInfoData sqldata;
+ StringInfo sql = &sqldata;
+ const GBOX *box;
+ MemoryContext oldcontext = CurrentMemoryContext;
+
+ initStringInfo(sql);
+ appendStringInfo(
+ sql,
+ "SELECT ST_BoundingDiagonal(ST_Collect("
+ "ST_BoundingDiagonal(geom, true)"
+ "), true) FROM \"%s\".edge "
+ "WHERE left_face != right_face AND "
+ "( left_face = %" LWTFMT_ELEMID
+ " OR right_face = %" LWTFMT_ELEMID
+ ")",
+ topo->name,
+ face,
+ face
+ );
+
+ POSTGIS_DEBUGF(1, "cb_computeFaceMBR query: %s", sql->data);
+ spi_result = SPI_execute(sql->data, !topo->be_data->data_changed, 0);
+ MemoryContextSwitchTo( oldcontext ); /* switch back */
+ if ( spi_result != SPI_OK_SELECT )
+ {
+ cberror(topo->be_data, "unexpected return (%d) from query execution: %s", spi_result, sql->data);
+ pfree(sqldata.data);
+ return NULL;
+ }
+ pfree(sqldata.data);
+
+ if ( ! SPI_processed )
+ {
+ cberror(
+ topo->be_data,
+ "Face with id %" LWTFMT_ELEMID" in topology \"%s\" has no edges",
+ face,
+ topo->name
+ );
+ return NULL;
+ }
+
+ row = SPI_tuptable->vals[0];
+ dat = SPI_getbinval(row, SPI_tuptable->tupdesc, 1, &isnull);
+
+ if ( isnull )
+ {
+ cberror(
+ topo->be_data,
+ "Face with id %" LWTFMT_ELEMID" in topology \"%s\" has null edges ?",
+ face,
+ topo->name
+ );
+ return NULL;
+ }
+
+ geom = (GSERIALIZED *)PG_DETOAST_DATUM(dat);
+ lwg = lwgeom_from_gserialized(geom);
+
+ lwgeom_refresh_bbox(lwg); /* Ensure we use a fit mbr, see #4149 */
+ box = lwgeom_get_bbox(lwg);
+ if ( box )
+ {
+ POSTGIS_DEBUGF(1, "Face %" LWTFMT_ELEMID " bbox xmin is %.15g", face, box->xmin);
+ }
+ else
+ {
+ cberror(
+ topo->be_data,
+ "Face with id %" LWTFMT_ELEMID" in topology \"%s\" has empty MBR ?",
+ face,
+ topo->name
+ );
+ return NULL;
+ }
+ lwgeom_free(lwg);
+ if ( DatumGetPointer(dat) != (Pointer)geom ) pfree(geom);
+
+ SPI_freetuptable(SPI_tuptable);
+
+ return gbox_clone(box);
+}
+
static int
cb_deleteFacesById(const LWT_BE_TOPOLOGY *topo, const LWT_ELEMID *ids, uint64_t numelems)
{
@@ -3423,7 +3517,8 @@ static LWT_BE_CALLBACKS be_callbacks =
cb_getFaceWithinBox2D,
cb_checkTopoGeomRemIsoNode,
cb_checkTopoGeomRemIsoEdge,
- cb_getClosestEdge
+ cb_getClosestEdge,
+ cb_computeFaceMBR
};
static void
diff --git a/topology/test/regress/st_changeedgegeom.sql b/topology/test/regress/st_changeedgegeom.sql
index 9cedd7677..7822035e0 100644
--- a/topology/test/regress/st_changeedgegeom.sql
+++ b/topology/test/regress/st_changeedgegeom.sql
@@ -136,20 +136,13 @@ SELECT 'T13.3', TopoGeo_AddPoint('city_data', 'POINT(-1.1697 47.7825)'::geometry
SELECT 'T13.4', ST_ChangeEdgeGeom('city_data', 29, '01020000001D000000E42CEC69873FF2BF9E98F56228E347400EDB16653648F2BF4985B18520E34740E92B4833164DF2BF3A1E335019E34740A94D9CDCEF50F2BF33F9669B1BE347407DAEB6627F59F2BF2CF180B229E34740758E01D9EB5DF2BFD0D556EC2FE34740533F6F2A5261F2BFD717096D39E34740F4893C49BA66F2BFC8073D9B55E34740B8239C16BC68F2BF33A7CB6262E34740AA2B9FE57970F2BF4165FCFB8CE347406DC5FEB27B72F2BFBA4E232D95E34740978BF84ECC7AF2BF24EEB1F4A1E34740E527D53E1D8FF2BF8F8D40BCAEE3474036CD3B4ED191F2BF649291B3B0E34740841266DAFE95F2BF1DE6CB0BB0E34740E3361AC05BA0F2BFB2632310AFE347405C5A0D897BACF2BF72F90FE9B7E3474031D3F6AFACB4F2BF4F232D95B7E347402B137EA99FB7F2BFD656EC2FBBE347402D431CEBE2B6F2BF551344DD07E4474011E4A08499B6F2BF15E3FC4D28E447406519E25817B7F2BF63EE5A423EE447409DD7D825AAB7F2BFE3FC4D2844E447405969520ABABDF2BF2384471B47E44740A31EA2D11DC4F2BFB1F9B83654E447400473F4F8BDCDF2BFEA5BE67459E447405070B1A206D3F2BFF19D98F562E4474062670A9DD7D8F2BF0E4FAF9465E447407FF6234564D8F2BFF1BA7EC16EE4474
0' );
-- See https://trac.osgeo.org/postgis/ticket/4706
-DO $$
-BEGIN
- -- 1. corrupt topology
- UPDATE city_data.edge_data SET right_face = 3 WHERE edge_id = 19;
- -- 2. Try to change the edge of a face with no bbox
- BEGIN
- PERFORM ST_ChangeEdgeGeom('city_data', 7, 'LINESTRING(21 22,28 20,35 22)' );
- EXCEPTION
- WHEN OTHERS THEN
- -- Strip details, we only want the first part
- RAISE EXCEPTION '%', regexp_replace(SQLERRM, '([^:]): .*', '\1 (see #4706)');
- END;
-END;
-$$ LANGUAGE 'plpgsql';
+BEGIN;
+-- 1. Corrupt topology to make face geometry unbuildable
+UPDATE city_data.edge_data SET right_face = 3 WHERE edge_id = 19;
+-- 2. Try to change an edge of the corrupted face
+-- (we're only interested in this operation not crashing the backend)
+SELECT NULL FROM ST_ChangeEdgeGeom('city_data', 7, 'LINESTRING(21 22,28 20,35 22)' );
+ROLLBACK;
--
---- TODO: test changing some clockwise closed edges..
diff --git a/topology/test/regress/st_changeedgegeom_expected b/topology/test/regress/st_changeedgegeom_expected
index 64acf3f58..9e29700c7 100644
--- a/topology/test/regress/st_changeedgegeom_expected
+++ b/topology/test/regress/st_changeedgegeom_expected
@@ -35,5 +35,4 @@ T13.1|29
T13.2|Edge 29 changed
T13.3|26
ERROR: Edge motion collision at POINT(-1.1697 47.7825)
-ERROR: Corrupted topology (see #4706)
Topology 'city_data' dropped
-----------------------------------------------------------------------
Summary of changes:
liblwgeom/liblwgeom_topo.h | 11 +++
liblwgeom/lwgeom_topo.c | 72 ++++++------------
topology/postgis_topology.c | 97 +++++++++++++++++++++++-
topology/test/regress/st_changeedgegeom.sql | 21 ++---
topology/test/regress/st_changeedgegeom_expected | 1 -
5 files changed, 136 insertions(+), 66 deletions(-)
hooks/post-receive
--
PostGIS
More information about the postgis-tickets
mailing list