From dcc3a1772b9ad8f7dd3077f6c1d408f834215814 Mon Sep 17 00:00:00 2001 From: Alessandro Pasotti Date: Tue, 2 Jan 2024 17:53:17 +0100 Subject: [PATCH 1/4] GeoJSON fast extent --- autotest/ogr/ogr_geojson.py | 214 +++++++++++++++++- ogr/ogrsf_frmts/geojson/ogr_geojson.h | 5 + ogr/ogrsf_frmts/geojson/ogrgeojsonlayer.cpp | 49 +++++ ogr/ogrsf_frmts/geojson/ogrgeojsonreader.cpp | 215 ++++++++++++++++++- ogr/ogrsf_frmts/geojson/ogrgeojsonreader.h | 15 +- 5 files changed, 486 insertions(+), 12 deletions(-) diff --git a/autotest/ogr/ogr_geojson.py b/autotest/ogr/ogr_geojson.py index 1fe232c98fa5..2b20ee2a5bf9 100755 --- a/autotest/ogr/ogr_geojson.py +++ b/autotest/ogr/ogr_geojson.py @@ -4734,8 +4734,6 @@ def test_ogr_geojson_foreign_members_feature(tmp_vsimem): ############################################################################### - - def test_ogr_json_getextent3d(tmp_vsimem): jdata = r"""{ @@ -4743,7 +4741,7 @@ def test_ogr_json_getextent3d(tmp_vsimem): "features": [ { "type": "Feature", - "properties": {}, + "properties": {"foo": "bar"}, "geometry": { "type": "%s", "coordinates": %s @@ -4751,7 +4749,7 @@ def test_ogr_json_getextent3d(tmp_vsimem): }, { "type": "Feature", - "properties": {}, + "properties": {"foo": "baz"}, "geometry": { "type": "%s", "coordinates": %s @@ -4792,7 +4790,8 @@ def test_ogr_json_getextent3d(tmp_vsimem): assert gdal.GetLastErrorMsg() == "" lyr = ds.GetLayer(0) - assert not lyr.TestCapability(ogr.OLCFastGetExtent3D) + assert lyr.TestCapability(ogr.OLCFastGetExtent3D) + assert lyr.TestCapability(ogr.OLCFastGetExtent) dfn = lyr.GetLayerDefn() assert dfn.GetGeomFieldCount() == 1 ext2d = lyr.GetExtent() @@ -4800,6 +4799,32 @@ def test_ogr_json_getextent3d(tmp_vsimem): ext3d = lyr.GetExtent3D() assert ext3d == (1.0, 2.0, 1.0, 2.0, float("inf"), float("-inf")) + # Test capabilities and extent with filters and round trip + lyr.SetAttributeFilter("foo = 'baz'") + assert not lyr.TestCapability(ogr.OLCFastGetExtent3D) + assert not lyr.TestCapability(ogr.OLCFastGetExtent) + ext2d = lyr.GetExtent() + assert ext2d == (2.0, 2.0, 2.0, 2.0) + ext3d = lyr.GetExtent3D() + assert ext3d == (2.0, 2.0, 2.0, 2.0, float("inf"), float("-inf")) + + lyr.SetAttributeFilter(None) + assert lyr.TestCapability(ogr.OLCFastGetExtent3D) + assert lyr.TestCapability(ogr.OLCFastGetExtent) + ext2d = lyr.GetExtent() + assert ext2d == (1.0, 2.0, 1.0, 2.0) + ext3d = lyr.GetExtent3D() + assert ext3d == (1.0, 2.0, 1.0, 2.0, float("inf"), float("-inf")) + + # Test capability with geometry filter + lyr.SetSpatialFilterRect(1.5, 1.5, 2.5, 2.5) + assert not lyr.TestCapability(ogr.OLCFastGetExtent3D) + assert not lyr.TestCapability(ogr.OLCFastGetExtent) + ext2d = lyr.GetExtent() + assert ext2d == (2.0, 2.0, 2.0, 2.0) + ext3d = lyr.GetExtent3D() + assert ext3d == (2.0, 2.0, 2.0, 2.0, float("inf"), float("-inf")) + # Test mixed 2D gdal.FileFromMemBuffer( tmp_vsimem / "test.json", @@ -4819,3 +4844,182 @@ def test_ogr_json_getextent3d(tmp_vsimem): assert ext2d == (1.0, 2.0, 1.0, 2.0) ext3d = lyr.GetExtent3D() assert ext3d == (1.0, 2.0, 1.0, 2.0, 1.0, 1.0) + + # Text mixed geometry types + gdal.FileFromMemBuffer( + tmp_vsimem / "test.json", + jdata % ("Point", "[1, 1, 1]", "LineString", "[[2, 2, 2], [3, 3, 3]]"), + ) + + ds = gdal.OpenEx(tmp_vsimem / "test.json", gdal.OF_VECTOR) + + assert gdal.GetLastErrorMsg() == "" + + lyr = ds.GetLayer(0) + dfn = lyr.GetLayerDefn() + assert dfn.GetGeomFieldCount() == 1 + # Check geometry type is unknown + assert dfn.GetGeomFieldDefn(0).GetType() == ogr.wkbUnknown + + # Test a polygon + gdal.FileFromMemBuffer( + tmp_vsimem / "test.json", + """{ + "type": "FeatureCollection", + "features": [ + { + "type": "Feature", + "properties": {}, + "geometry": { + "type": "Polygon", + "coordinates": [[[1, 1], [2, 1], [2, 2], [1, 2], [1, 1]]] + } + } + ] + }""", + ) + + ds = gdal.OpenEx(tmp_vsimem / "test.json", gdal.OF_VECTOR) + + assert gdal.GetLastErrorMsg() == "" + + lyr = ds.GetLayer(0) + dfn = lyr.GetLayerDefn() + assert dfn.GetGeomFieldCount() == 1 + ext2d = lyr.GetExtent() + assert ext2d == (1.0, 2.0, 1.0, 2.0) + ext3d = lyr.GetExtent3D() + assert ext3d == (1.0, 2.0, 1.0, 2.0, float("inf"), float("-inf")) + + # Test a polygon with a hole + gdal.FileFromMemBuffer( + tmp_vsimem / "test.json", + """{ + "type": "FeatureCollection", + "features": [ + { + "type": "Feature", + "properties": {}, + "geometry": { + "type": "Polygon", + "coordinates": [[[1, 1], [2, 1], [2, 2], [1, 2], [1, 1]], [[1.5, 1.5], [1.5, 1.6], [1.6, 1.6], [1.6, 1.5], [1.5, 1.5]]] + } + } + ] + }""", + ) + + ds = gdal.OpenEx(tmp_vsimem / "test.json", gdal.OF_VECTOR) + + assert gdal.GetLastErrorMsg() == "" + + lyr = ds.GetLayer(0) + dfn = lyr.GetLayerDefn() + assert dfn.GetGeomFieldCount() == 1 + ext2d = lyr.GetExtent() + assert ext2d == (1.0, 2.0, 1.0, 2.0) + ext3d = lyr.GetExtent3D() + assert ext3d == (1.0, 2.0, 1.0, 2.0, float("inf"), float("-inf")) + + # Test a series of different 2D geometries including polygons with holes + gdal.FileFromMemBuffer( + tmp_vsimem / "test.json", + """{ + "type": "FeatureCollection", + "features": [ + { + "type": "Feature", + "properties": {}, + "geometry": { + "type": "Point", + "coordinates": [1, 1] + } + }, + { + "type": "Feature", + "properties": {}, + "geometry": { + "type": "LineString", + "coordinates": [[2, 2], [3, 3]] + } + }, + { + "type": "Feature", + "properties": {}, + "geometry": { + "type": "Polygon", + "coordinates": [[[4, 4], [5, 4], [5, 5], [4, 5], [4, 4]]] + } + }, + { + "type": "Feature", + "properties": {}, + "geometry": { + "type": "Polygon", + "coordinates": [[[6, 6], [7, 6], [7, 7], [6, 7], [6, 6]], [[6.5, 6.5], [6.5, 6.6], [6.6, 6.6], [6.6, 6.5], [6.5, 6.5]]] + } + } + ] + }""", + ) + + ds = gdal.OpenEx(tmp_vsimem / "test.json", gdal.OF_VECTOR) + + assert gdal.GetLastErrorMsg() == "" + + lyr = ds.GetLayer(0) + + assert lyr.GetExtent() == (1.0, 7.0, 1.0, 7.0) + assert lyr.GetExtent3D() == (1.0, 7.0, 1.0, 7.0, float("inf"), float("-inf")) + + # Test a series of different 3D geometries including polygons with holes + + gdal.FileFromMemBuffer( + tmp_vsimem / "test.json", + """{ + "type": "FeatureCollection", + "features": [ + { + "type": "Feature", + "properties": {}, + "geometry": { + "type": "Point", + "coordinates": [1, 1, 1] + } + }, + { + "type": "Feature", + "properties": {}, + "geometry": { + "type": "LineString", + "coordinates": [[2, 2, 2], [3, 3, 3]] + } + }, + { + "type": "Feature", + "properties": {}, + "geometry": { + "type": "Polygon", + "coordinates": [[[4, 4, 4], [5, 4, 4], [5, 5, 5], [4, 5, 5], [4, 4, 4]]] + } + }, + { + "type": "Feature", + "properties": {}, + "geometry": { + "type": "Polygon", + "coordinates": [[[6, 6, 6], [7, 6, 6], [7, 7, 7], [6, 7, 7], [6, 6, 6]], [[6.5, 6.5, 6.5], [6.5, 6.6, 6.5], [6.6, 6.6, 6.5], [6.6, 6.5, 6.5], [6.5, 6.5, 6.5]]] + } + } + ] + }""", + ) + + ds = gdal.OpenEx(tmp_vsimem / "test.json", gdal.OF_VECTOR) + + assert gdal.GetLastErrorMsg() == "" + + lyr = ds.GetLayer(0) + + assert lyr.GetExtent() == (1.0, 7.0, 1.0, 7.0) + assert lyr.GetExtent3D() == (1.0, 7.0, 1.0, 7.0, 1.0, 7.0) diff --git a/ogr/ogrsf_frmts/geojson/ogr_geojson.h b/ogr/ogrsf_frmts/geojson/ogr_geojson.h index 38399e473789..910f1e500abc 100644 --- a/ogr/ogrsf_frmts/geojson/ogr_geojson.h +++ b/ogr/ogrsf_frmts/geojson/ogr_geojson.h @@ -91,6 +91,11 @@ class OGRGeoJSONLayer final : public OGRMemLayer int nFlags) override; virtual OGRErr CreateGeomField(const OGRGeomFieldDefn *poGeomField, int bApproxOK = TRUE) override; + virtual OGRErr GetExtent(OGREnvelope *psExtent, int bForce = TRUE) override; + virtual OGRErr GetExtent(int iGeomField, OGREnvelope *psExtent, + int bForce = TRUE) override; + virtual OGRErr GetExtent3D(int iGeomField, OGREnvelope3D *psExtent3D, + int bForce = TRUE) override; // // OGRGeoJSONLayer Interface diff --git a/ogr/ogrsf_frmts/geojson/ogrgeojsonlayer.cpp b/ogr/ogrsf_frmts/geojson/ogrgeojsonlayer.cpp index fb27a926607b..7b0dd84767c5 100644 --- a/ogr/ogrsf_frmts/geojson/ogrgeojsonlayer.cpp +++ b/ogr/ogrsf_frmts/geojson/ogrgeojsonlayer.cpp @@ -438,6 +438,52 @@ OGRErr OGRGeoJSONLayer::CreateGeomField(const OGRGeomFieldDefn *poGeomField, return OGRMemLayer::CreateGeomField(poGeomField, bApproxOK); } +OGRErr OGRGeoJSONLayer::GetExtent(OGREnvelope *psExtent, int bForce) +{ + return GetExtent(0, psExtent, bForce); +} + +OGRErr OGRGeoJSONLayer::GetExtent(int iGeomField, OGREnvelope *psExtent, + int bForce) +{ + if (iGeomField != 0) + { + return OGRERR_FAILURE; + } + + if (poReader_ && poReader_->ExtentRead() && + TestCapability(OLCFastGetExtent)) + { + *psExtent = poReader_->GetExtent(); + return OGRERR_NONE; + } + else + { + return OGRMemLayer::GetExtentInternal(iGeomField, psExtent, bForce); + } +} + +OGRErr OGRGeoJSONLayer::GetExtent3D(int iGeomField, OGREnvelope3D *psExtent3D, + int bForce) +{ + + if (iGeomField != 0) + { + return OGRERR_FAILURE; + } + + if (poReader_ && poReader_->ExtentRead() && + TestCapability(OLCFastGetExtent3D)) + { + *psExtent3D = poReader_->GetExtent3D(); + return OGRERR_NONE; + } + else + { + return OGRMemLayer::GetExtent3D(iGeomField, psExtent3D, bForce); + } +} + /************************************************************************/ /* TestCapability() */ /************************************************************************/ @@ -451,6 +497,9 @@ int OGRGeoJSONLayer::TestCapability(const char *pszCap) return TRUE; else if (EQUAL(pszCap, OLCStringsAsUTF8)) return TRUE; + else if (EQUAL(pszCap, OLCFastGetExtent) || + EQUAL(pszCap, OLCFastGetExtent3D)) + return m_poFilterGeom == nullptr && m_poAttrQuery == nullptr; return OGRMemLayer::TestCapability(pszCap); } diff --git a/ogr/ogrsf_frmts/geojson/ogrgeojsonreader.cpp b/ogr/ogrsf_frmts/geojson/ogrgeojsonreader.cpp index 2aa5cbaf6888..60921322fb40 100644 --- a/ogr/ogrsf_frmts/geojson/ogrgeojsonreader.cpp +++ b/ogr/ogrsf_frmts/geojson/ogrgeojsonreader.cpp @@ -1722,14 +1722,27 @@ bool OGRGeoJSONBaseReader::GenerateFeatureDefn( poObj, poObjProps, nPrevFieldIdx, oMapFieldNameToIdx, apoFieldDefn, dag, bFeatureLevelIdAsFID_, bFeatureLevelIdAsAttribute_, m_bNeedFID64); - if (m_bDetectLayerGeomType) + json_object *poGeomObj = CPL_json_object_object_get(poObj, "geometry"); + if (poGeomObj && json_object_get_type(poGeomObj) == json_type_object) { - json_object *poGeomObj = CPL_json_object_object_get(poObj, "geometry"); - if (poGeomObj && json_object_get_type(poGeomObj) == json_type_object) + const auto eType = OGRGeoJSONGetOGRGeometryType(poGeomObj); + + OGRGeoJSONUpdateLayerGeomType(m_bFirstGeometry, eType, + m_eLayerGeomType); + + if (eType != wkbNone && eType != wkbUnknown) { - const auto eType = OGRGeoJSONGetOGRGeometryType(poGeomObj); - m_bDetectLayerGeomType = OGRGeoJSONUpdateLayerGeomType( - m_bFirstGeometry, eType, m_eLayerGeomType); + // This is maybe too optimistic: it assumes that the geometry + // coordinates array is in the correct format + if (wkbHasZ(eType)) + { + m_bExtentRead |= + OGRGeoJSONGetExtent3D(poGeomObj, &m_oEnvelope3D); + } + else + { + m_bExtentRead |= OGRGeoJSONGetExtent(poGeomObj, &m_oEnvelope3D); + } } } @@ -2318,6 +2331,25 @@ OGRFeature *OGRGeoJSONBaseReader::ReadFeature(OGRLayer *poLayer, return poFeature; } +/************************************************************************/ +/* Extent getters */ +/************************************************************************/ + +bool OGRGeoJSONBaseReader::ExtentRead() const +{ + return m_bExtentRead; +} + +OGREnvelope OGRGeoJSONBaseReader::GetExtent() const +{ + return m_oEnvelope3D; +} + +OGREnvelope3D OGRGeoJSONBaseReader::GetExtent3D() const +{ + return m_oEnvelope3D; +} + /************************************************************************/ /* ReadFeatureCollection() */ /************************************************************************/ @@ -3157,3 +3189,174 @@ json_object *CPL_json_object_object_get(struct json_object *obj, json_object_object_get_ex(obj, key, &poRet); return poRet; } + +bool OGRGeoJSONGetExtent(json_object *poObj, OGREnvelope *poEnvelope) +{ + if (!poEnvelope || !poObj) + { + return false; + } + + // Get the "coordinates" array from the JSON object + json_object *poObjCoords = OGRGeoJSONFindMemberByName(poObj, "coordinates"); + + // Return if not found or not an array + if (!poObjCoords || json_object_get_type(poObjCoords) != json_type_array) + { + return false; + } + + // poObjCoords can be an array of arrays, this lambda function will + // recursively parse the array + std::function fParseCoords; + fParseCoords = [&fParseCoords](json_object *poObjCoordsIn, + OGREnvelope *poEnvelopeIn) -> bool + { + if (json_type_array == json_object_get_type(poObjCoordsIn)) + { + const auto nItems = json_object_array_length(poObjCoordsIn); + + double dXVal = std::numeric_limits::quiet_NaN(); + + for (auto i = decltype(nItems){0}; i < nItems; ++i) + { + + // Get the i element + json_object *poObjCoordsElement = + json_object_array_get_idx(poObjCoordsIn, i); + + const json_type eType{json_object_get_type(poObjCoordsElement)}; + + // if it is an array, recurse + if (json_type_array == eType) + { + if (!fParseCoords(poObjCoordsElement, poEnvelopeIn)) + { + return false; + } + } + else if (json_type_double == eType || json_type_int == eType) + { + switch (i) + { + case 0: + { + dXVal = json_object_get_double(poObjCoordsElement); + break; + } + case 1: + { + const double dYVal = + json_object_get_double(poObjCoordsElement); + poEnvelopeIn->Merge(dXVal, dYVal); + dXVal = std::numeric_limits::quiet_NaN(); + break; + } + default: + return false; + } + } + else + { + return false; + } + } + return true; + } + else + { + return false; + } + }; + + return fParseCoords(poObjCoords, poEnvelope); +} + +bool OGRGeoJSONGetExtent3D(json_object *poObj, OGREnvelope3D *poEnvelope) +{ + if (!poEnvelope || !poObj) + { + return false; + } + + // Get the "coordinates" array from the JSON object + json_object *poObjCoords = OGRGeoJSONFindMemberByName(poObj, "coordinates"); + + // Return if not found or not an array + if (!poObjCoords || json_object_get_type(poObjCoords) != json_type_array) + { + return false; + } + + // poObjCoords can be an array of arrays, this lambda function will + // recursively parse the array + std::function fParseCoords; + fParseCoords = [&fParseCoords](json_object *poObjCoordsIn, + OGREnvelope3D *poEnvelopeIn) -> bool + { + if (json_type_array == json_object_get_type(poObjCoordsIn)) + { + const auto nItems = json_object_array_length(poObjCoordsIn); + + double dXVal = std::numeric_limits::quiet_NaN(); + double dYVal = std::numeric_limits::quiet_NaN(); + + for (auto i = decltype(nItems){0}; i < nItems; ++i) + { + + // Get the i element + json_object *poObjCoordsElement = + json_object_array_get_idx(poObjCoordsIn, i); + + const json_type eType{json_object_get_type(poObjCoordsElement)}; + + // if it is an array, recurse + if (json_type_array == eType) + { + if (!fParseCoords(poObjCoordsElement, poEnvelopeIn)) + { + return false; + } + } + else if (json_type_double == eType || json_type_int == eType) + { + switch (i) + { + case 0: + { + dXVal = json_object_get_double(poObjCoordsElement); + break; + } + case 1: + { + dYVal = json_object_get_double(poObjCoordsElement); + break; + } + case 2: + { + const double dZVal = + json_object_get_double(poObjCoordsElement); + poEnvelopeIn->Merge(dXVal, dYVal, dZVal); + dXVal = std::numeric_limits::quiet_NaN(); + dYVal = std::numeric_limits::quiet_NaN(); + break; + } + default: + return false; + } + } + else + { + return false; + } + } + return true; + } + else + { + return false; + } + }; + + return fParseCoords(poObjCoords, poEnvelope); +} diff --git a/ogr/ogrsf_frmts/geojson/ogrgeojsonreader.h b/ogr/ogrsf_frmts/geojson/ogrgeojsonreader.h index dc9bc4e6e730..9e2a27d0f31a 100644 --- a/ogr/ogrsf_frmts/geojson/ogrgeojsonreader.h +++ b/ogr/ogrsf_frmts/geojson/ogrgeojsonreader.h @@ -116,6 +116,11 @@ class OGRGeoJSONBaseReader OGRFeature *ReadFeature(OGRLayer *poLayer, json_object *poObj, const char *pszSerializedObj); + bool ExtentRead() const; + + OGREnvelope GetExtent() const; + OGREnvelope3D GetExtent3D() const; + protected: bool bGeometryPreserve_ = true; bool bAttributesSkip_ = false; @@ -139,8 +144,10 @@ class OGRGeoJSONBaseReader bool bFeatureLevelIdAsFID_ = false; bool m_bNeedFID64 = false; - bool m_bDetectLayerGeomType = true; bool m_bFirstGeometry = true; + OGREnvelope3D m_oEnvelope3D; + // Becomes true when extent has been read from data + bool m_bExtentRead = false; OGRwkbGeometryType m_eLayerGeomType = wkbUnknown; CPL_DISALLOW_COPY_ASSIGN(OGRGeoJSONBaseReader) @@ -269,6 +276,12 @@ bool OGRGeoJSONUpdateLayerGeomType(bool &bFirstGeom, OGRwkbGeometryType eGeomType, OGRwkbGeometryType &eLayerGeomType); +// Get the extent from the geometry coordinates of a feature +bool OGRGeoJSONGetExtent(json_object *poObj, OGREnvelope *poEnvelope); + +// Get the 3D extent from the geometry coordinates of a feature +bool OGRGeoJSONGetExtent3D(json_object *poObj, OGREnvelope3D *poEnvelope); + /************************************************************************/ /* GeoJSON Geometry Translators */ /************************************************************************/ From e877fcc48116d886f86ed5e8bc5b298d964e8bb9 Mon Sep 17 00:00:00 2001 From: Alessandro Pasotti Date: Wed, 3 Jan 2024 09:03:50 +0100 Subject: [PATCH 2/4] Simplify code --- ogr/ogrsf_frmts/geojson/ogrgeojsonreader.cpp | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/ogr/ogrsf_frmts/geojson/ogrgeojsonreader.cpp b/ogr/ogrsf_frmts/geojson/ogrgeojsonreader.cpp index 60921322fb40..4f55d7a215a2 100644 --- a/ogr/ogrsf_frmts/geojson/ogrgeojsonreader.cpp +++ b/ogr/ogrsf_frmts/geojson/ogrgeojsonreader.cpp @@ -3216,7 +3216,7 @@ bool OGRGeoJSONGetExtent(json_object *poObj, OGREnvelope *poEnvelope) { const auto nItems = json_object_array_length(poObjCoordsIn); - double dXVal = std::numeric_limits::quiet_NaN(); + double dXVal; for (auto i = decltype(nItems){0}; i < nItems; ++i) { @@ -3249,7 +3249,6 @@ bool OGRGeoJSONGetExtent(json_object *poObj, OGREnvelope *poEnvelope) const double dYVal = json_object_get_double(poObjCoordsElement); poEnvelopeIn->Merge(dXVal, dYVal); - dXVal = std::numeric_limits::quiet_NaN(); break; } default: @@ -3298,8 +3297,8 @@ bool OGRGeoJSONGetExtent3D(json_object *poObj, OGREnvelope3D *poEnvelope) { const auto nItems = json_object_array_length(poObjCoordsIn); - double dXVal = std::numeric_limits::quiet_NaN(); - double dYVal = std::numeric_limits::quiet_NaN(); + double dXVal; + double dYVal; for (auto i = decltype(nItems){0}; i < nItems; ++i) { @@ -3337,8 +3336,6 @@ bool OGRGeoJSONGetExtent3D(json_object *poObj, OGREnvelope3D *poEnvelope) const double dZVal = json_object_get_double(poObjCoordsElement); poEnvelopeIn->Merge(dXVal, dYVal, dZVal); - dXVal = std::numeric_limits::quiet_NaN(); - dYVal = std::numeric_limits::quiet_NaN(); break; } default: From 30880f375d5fde7b95b2f5233004fe5e36e3f3fe Mon Sep 17 00:00:00 2001 From: Alessandro Pasotti Date: Wed, 3 Jan 2024 17:25:23 +0100 Subject: [PATCH 3/4] Handle geometry collections and simplify. --- autotest/ogr/ogr_geojson.py | 33 ++++ ogr/ogrsf_frmts/geojson/ogrgeojsonlayer.cpp | 2 +- ogr/ogrsf_frmts/geojson/ogrgeojsonreader.cpp | 166 +++++++------------ ogr/ogrsf_frmts/geojson/ogrgeojsonreader.h | 4 - 4 files changed, 98 insertions(+), 107 deletions(-) diff --git a/autotest/ogr/ogr_geojson.py b/autotest/ogr/ogr_geojson.py index 2b20ee2a5bf9..8fad3db57da7 100755 --- a/autotest/ogr/ogr_geojson.py +++ b/autotest/ogr/ogr_geojson.py @@ -5023,3 +5023,36 @@ def test_ogr_json_getextent3d(tmp_vsimem): assert lyr.GetExtent() == (1.0, 7.0, 1.0, 7.0) assert lyr.GetExtent3D() == (1.0, 7.0, 1.0, 7.0, 1.0, 7.0) + + # Test geometrycollection + gdal.FileFromMemBuffer( + tmp_vsimem / "test.json", + r""" + { + "type": "FeatureCollection", + "features": [{ + "type": "Feature", + "properties": {}, + "geometry": { + "type": "GeometryCollection", + "geometries": [{ + "type": "Point", + "coordinates": [6, 7] + }, { + "type": "Polygon", + "coordinates": [[[3, 4, 2], [5, 4, 4], [5, 5, 5], [4, 5, 5], [3, 4, 2]]] + }] + } + }] + } + """, + ) + + ds = gdal.OpenEx(tmp_vsimem / "test.json", gdal.OF_VECTOR) + + assert gdal.GetLastErrorMsg() == "" + + lyr = ds.GetLayer(0) + + assert lyr.GetExtent() == (3.0, 6.0, 4.0, 7.0) + assert lyr.GetExtent3D() == (3.0, 6.0, 4.0, 7.0, 2.0, 5.0) diff --git a/ogr/ogrsf_frmts/geojson/ogrgeojsonlayer.cpp b/ogr/ogrsf_frmts/geojson/ogrgeojsonlayer.cpp index 7b0dd84767c5..d16f1d3795c6 100644 --- a/ogr/ogrsf_frmts/geojson/ogrgeojsonlayer.cpp +++ b/ogr/ogrsf_frmts/geojson/ogrgeojsonlayer.cpp @@ -454,7 +454,7 @@ OGRErr OGRGeoJSONLayer::GetExtent(int iGeomField, OGREnvelope *psExtent, if (poReader_ && poReader_->ExtentRead() && TestCapability(OLCFastGetExtent)) { - *psExtent = poReader_->GetExtent(); + *psExtent = poReader_->GetExtent3D(); return OGRERR_NONE; } else diff --git a/ogr/ogrsf_frmts/geojson/ogrgeojsonreader.cpp b/ogr/ogrsf_frmts/geojson/ogrgeojsonreader.cpp index 4f55d7a215a2..e668c2b83298 100644 --- a/ogr/ogrsf_frmts/geojson/ogrgeojsonreader.cpp +++ b/ogr/ogrsf_frmts/geojson/ogrgeojsonreader.cpp @@ -35,6 +35,7 @@ #include #include +#include /************************************************************************/ /* OGRGeoJSONReaderStreamingParser */ @@ -1734,15 +1735,7 @@ bool OGRGeoJSONBaseReader::GenerateFeatureDefn( { // This is maybe too optimistic: it assumes that the geometry // coordinates array is in the correct format - if (wkbHasZ(eType)) - { - m_bExtentRead |= - OGRGeoJSONGetExtent3D(poGeomObj, &m_oEnvelope3D); - } - else - { - m_bExtentRead |= OGRGeoJSONGetExtent(poGeomObj, &m_oEnvelope3D); - } + m_bExtentRead |= OGRGeoJSONGetExtent3D(poGeomObj, &m_oEnvelope3D); } } @@ -2340,11 +2333,6 @@ bool OGRGeoJSONBaseReader::ExtentRead() const return m_bExtentRead; } -OGREnvelope OGRGeoJSONBaseReader::GetExtent() const -{ - return m_oEnvelope3D; -} - OGREnvelope3D OGRGeoJSONBaseReader::GetExtent3D() const { return m_oEnvelope3D; @@ -3190,33 +3178,26 @@ json_object *CPL_json_object_object_get(struct json_object *obj, return poRet; } -bool OGRGeoJSONGetExtent(json_object *poObj, OGREnvelope *poEnvelope) +bool OGRGeoJSONGetExtent3D(json_object *poObj, OGREnvelope3D *poEnvelope) { if (!poEnvelope || !poObj) { return false; } - // Get the "coordinates" array from the JSON object - json_object *poObjCoords = OGRGeoJSONFindMemberByName(poObj, "coordinates"); - - // Return if not found or not an array - if (!poObjCoords || json_object_get_type(poObjCoords) != json_type_array) - { - return false; - } - // poObjCoords can be an array of arrays, this lambda function will // recursively parse the array - std::function fParseCoords; + std::function fParseCoords; fParseCoords = [&fParseCoords](json_object *poObjCoordsIn, - OGREnvelope *poEnvelopeIn) -> bool + OGREnvelope3D *poEnvelopeIn) -> bool { if (json_type_array == json_object_get_type(poObjCoordsIn)) { const auto nItems = json_object_array_length(poObjCoordsIn); - double dXVal; + double dXVal = std::numeric_limits::quiet_NaN(); + double dYVal = std::numeric_limits::quiet_NaN(); + double dZVal = std::numeric_limits::quiet_NaN(); for (auto i = decltype(nItems){0}; i < nItems; ++i) { @@ -3246,9 +3227,12 @@ bool OGRGeoJSONGetExtent(json_object *poObj, OGREnvelope *poEnvelope) } case 1: { - const double dYVal = - json_object_get_double(poObjCoordsElement); - poEnvelopeIn->Merge(dXVal, dYVal); + dYVal = json_object_get_double(poObjCoordsElement); + break; + } + case 2: + { + dZVal = json_object_get_double(poObjCoordsElement); break; } default: @@ -3260,6 +3244,20 @@ bool OGRGeoJSONGetExtent(json_object *poObj, OGREnvelope *poEnvelope) return false; } } + + if (!std::isnan(dXVal) && !std::isnan(dYVal)) + { + if (std::isnan(dZVal)) + { + static_cast(poEnvelopeIn) + ->Merge(dXVal, dYVal); + } + else + { + poEnvelopeIn->Merge(dXVal, dYVal, dZVal); + } + } + return true; } else @@ -3268,92 +3266,56 @@ bool OGRGeoJSONGetExtent(json_object *poObj, OGREnvelope *poEnvelope) } }; - return fParseCoords(poObjCoords, poEnvelope); -} - -bool OGRGeoJSONGetExtent3D(json_object *poObj, OGREnvelope3D *poEnvelope) -{ - if (!poEnvelope || !poObj) - { - return false; - } - - // Get the "coordinates" array from the JSON object - json_object *poObjCoords = OGRGeoJSONFindMemberByName(poObj, "coordinates"); - - // Return if not found or not an array - if (!poObjCoords || json_object_get_type(poObjCoords) != json_type_array) + // This function looks for "coordinates" and for "geometries" to handle + // geometry collections. It will recurse on itself to handle nested geometry. + std::function fParseGeometry; + fParseGeometry = [&fParseGeometry, &fParseCoords]( + json_object *poObj, OGREnvelope3D *poEnvelope) -> bool { - return false; - } + // Get the "coordinates" array from the JSON object + json_object *poObjCoords = + OGRGeoJSONFindMemberByName(poObj, "coordinates"); - // poObjCoords can be an array of arrays, this lambda function will - // recursively parse the array - std::function fParseCoords; - fParseCoords = [&fParseCoords](json_object *poObjCoordsIn, - OGREnvelope3D *poEnvelopeIn) -> bool - { - if (json_type_array == json_object_get_type(poObjCoordsIn)) + // Return if found and not an array + if (poObjCoords && json_object_get_type(poObjCoords) != json_type_array) { - const auto nItems = json_object_array_length(poObjCoordsIn); + return false; + } + else if (poObjCoords) + { + return fParseCoords(poObjCoords, poEnvelope); + } - double dXVal; - double dYVal; + // Try "geometries" + if (!poObjCoords) + { + poObjCoords = OGRGeoJSONFindMemberByName(poObj, "geometries"); + } + // Return if not found or not an array + if (!poObjCoords || + json_object_get_type(poObjCoords) != json_type_array) + { + return false; + } + else + { + // Loop thgrough the geometries + const auto nItems = json_object_array_length(poObjCoords); for (auto i = decltype(nItems){0}; i < nItems; ++i) { + json_object *poObjGeometry = + json_object_array_get_idx(poObjCoords, i); - // Get the i element - json_object *poObjCoordsElement = - json_object_array_get_idx(poObjCoordsIn, i); - - const json_type eType{json_object_get_type(poObjCoordsElement)}; - - // if it is an array, recurse - if (json_type_array == eType) - { - if (!fParseCoords(poObjCoordsElement, poEnvelopeIn)) - { - return false; - } - } - else if (json_type_double == eType || json_type_int == eType) - { - switch (i) - { - case 0: - { - dXVal = json_object_get_double(poObjCoordsElement); - break; - } - case 1: - { - dYVal = json_object_get_double(poObjCoordsElement); - break; - } - case 2: - { - const double dZVal = - json_object_get_double(poObjCoordsElement); - poEnvelopeIn->Merge(dXVal, dYVal, dZVal); - break; - } - default: - return false; - } - } - else + // Recurse + if (!fParseGeometry(poObjGeometry, poEnvelope)) { return false; } } return true; } - else - { - return false; - } }; - return fParseCoords(poObjCoords, poEnvelope); + return fParseGeometry(poObj, poEnvelope); } diff --git a/ogr/ogrsf_frmts/geojson/ogrgeojsonreader.h b/ogr/ogrsf_frmts/geojson/ogrgeojsonreader.h index 9e2a27d0f31a..9c592c783695 100644 --- a/ogr/ogrsf_frmts/geojson/ogrgeojsonreader.h +++ b/ogr/ogrsf_frmts/geojson/ogrgeojsonreader.h @@ -118,7 +118,6 @@ class OGRGeoJSONBaseReader bool ExtentRead() const; - OGREnvelope GetExtent() const; OGREnvelope3D GetExtent3D() const; protected: @@ -276,9 +275,6 @@ bool OGRGeoJSONUpdateLayerGeomType(bool &bFirstGeom, OGRwkbGeometryType eGeomType, OGRwkbGeometryType &eLayerGeomType); -// Get the extent from the geometry coordinates of a feature -bool OGRGeoJSONGetExtent(json_object *poObj, OGREnvelope *poEnvelope); - // Get the 3D extent from the geometry coordinates of a feature bool OGRGeoJSONGetExtent3D(json_object *poObj, OGREnvelope3D *poEnvelope); From b490976948eb5e128d76756c2a761c740d3e2e3c Mon Sep 17 00:00:00 2001 From: Alessandro Pasotti Date: Thu, 4 Jan 2024 09:34:00 +0100 Subject: [PATCH 4/4] Fix shadow error --- ogr/ogrsf_frmts/geojson/ogrgeojsonreader.cpp | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/ogr/ogrsf_frmts/geojson/ogrgeojsonreader.cpp b/ogr/ogrsf_frmts/geojson/ogrgeojsonreader.cpp index e668c2b83298..fdf58480c32d 100644 --- a/ogr/ogrsf_frmts/geojson/ogrgeojsonreader.cpp +++ b/ogr/ogrsf_frmts/geojson/ogrgeojsonreader.cpp @@ -3269,12 +3269,13 @@ bool OGRGeoJSONGetExtent3D(json_object *poObj, OGREnvelope3D *poEnvelope) // This function looks for "coordinates" and for "geometries" to handle // geometry collections. It will recurse on itself to handle nested geometry. std::function fParseGeometry; - fParseGeometry = [&fParseGeometry, &fParseCoords]( - json_object *poObj, OGREnvelope3D *poEnvelope) -> bool + fParseGeometry = [&fParseGeometry, + &fParseCoords](json_object *poObjIn, + OGREnvelope3D *poEnvelopeIn) -> bool { // Get the "coordinates" array from the JSON object json_object *poObjCoords = - OGRGeoJSONFindMemberByName(poObj, "coordinates"); + OGRGeoJSONFindMemberByName(poObjIn, "coordinates"); // Return if found and not an array if (poObjCoords && json_object_get_type(poObjCoords) != json_type_array) @@ -3283,13 +3284,13 @@ bool OGRGeoJSONGetExtent3D(json_object *poObj, OGREnvelope3D *poEnvelope) } else if (poObjCoords) { - return fParseCoords(poObjCoords, poEnvelope); + return fParseCoords(poObjCoords, poEnvelopeIn); } // Try "geometries" if (!poObjCoords) { - poObjCoords = OGRGeoJSONFindMemberByName(poObj, "geometries"); + poObjCoords = OGRGeoJSONFindMemberByName(poObjIn, "geometries"); } // Return if not found or not an array @@ -3308,7 +3309,7 @@ bool OGRGeoJSONGetExtent3D(json_object *poObj, OGREnvelope3D *poEnvelope) json_object_array_get_idx(poObjCoords, i); // Recurse - if (!fParseGeometry(poObjGeometry, poEnvelope)) + if (!fParseGeometry(poObjGeometry, poEnvelopeIn)) { return false; }