From d9cf0c8deb5c7c1e1223bd9bab2d9a8d61ed7de0 Mon Sep 17 00:00:00 2001 From: Even Rouault Date: Sat, 9 Dec 2023 16:49:28 +0100 Subject: [PATCH] Geopackage: tune GetExtent3D() --- autotest/ogr/data/gpkg/no_envelope.gpkg | Bin 98304 -> 98304 bytes autotest/ogr/ogr_gpkg.py | 12 +-- .../gpkg/ogrgeopackagedatasource.cpp | 18 +++++ .../gpkg/ogrgeopackagetablelayer.cpp | 42 +++++------ ogr/ogrsf_frmts/gpkg/ogrgeopackageutility.cpp | 69 +++++++++--------- 5 files changed, 78 insertions(+), 63 deletions(-) diff --git a/autotest/ogr/data/gpkg/no_envelope.gpkg b/autotest/ogr/data/gpkg/no_envelope.gpkg index 7fecb0b949de39d10ddb55e76aacc1a83d55a29d..d64ca9bb9d8409b4864177bc9e6aa8652ef08e84 100644 GIT binary patch delta 158 zcmZo@U~6b#n;^}|JyFJ)m777&{^rJ%Zhbz>5JOWdV?!$gV?6@{%gulF>+Ul$POsr- zbmL}Z00PEW%nS@n6B`w`zvE{VGhkuj|HiPfpp$>%gj6nOCT0dsW=0^fEr8LSTb~J} k0SLev?Hj;s1{e+GF*yJU2Ot5VH#W{w?3a~h=HY*si~ELfu5y-`R2d+b@v$sr`PZ^ wx^W9K00HAGW(EePiH(Ze-|;hw87yYmAh4Y!fYF$HW6MdFMF#|y6)*|_0Dh(!wEzGB diff --git a/autotest/ogr/ogr_gpkg.py b/autotest/ogr/ogr_gpkg.py index 6a2996ff31a3..8ffe8718e124 100755 --- a/autotest/ogr/ogr_gpkg.py +++ b/autotest/ogr/ogr_gpkg.py @@ -6622,9 +6622,9 @@ def test_ogr_gpkg_spatial_view_computed_geom_column(tmp_vsimem, tmp_path): ds = None pytest.skip("spatialite missing") - lyr = ds.CreateLayer("foo", geom_type=ogr.wkbPoint) + lyr = ds.CreateLayer("foo", geom_type=ogr.wkbPoint25D) f = ogr.Feature(lyr.GetLayerDefn()) - f.SetGeometry(ogr.CreateGeometryFromWkt("POINT(1 2)")) + f.SetGeometry(ogr.CreateGeometryFromWkt("POINT Z (1 2 3)")) lyr.CreateFeature(f) ds.ExecuteSQL( @@ -6634,7 +6634,7 @@ def test_ogr_gpkg_spatial_view_computed_geom_column(tmp_vsimem, tmp_path): "INSERT INTO gpkg_contents (table_name, identifier, data_type, srs_id) VALUES ( 'geom_view', 'geom_view', 'features', 4326 )" ) ds.ExecuteSQL( - "INSERT INTO gpkg_geometry_columns (table_name, column_name, geometry_type_name, srs_id, z, m) values ('geom_view', 'my_geom', 'MULTIPOINT', 4326, 0, 0)" + "INSERT INTO gpkg_geometry_columns (table_name, column_name, geometry_type_name, srs_id, z, m) values ('geom_view', 'my_geom', 'MULTIPOINT', 4326, 1, 0)" ) ds.ExecuteSQL( "INSERT INTO gpkg_extensions VALUES('geom_view', 'my_geom', 'gdal_spatialite_computed_geom_column', 'https://gdal.org/drivers/vector/gpkg_spatialite_computed_column.html', 'read-write')" @@ -6661,10 +6661,12 @@ def test_ogr_gpkg_spatial_view_computed_geom_column(tmp_vsimem, tmp_path): ds = ogr.Open(filename) lyr = ds.GetLayerByName("geom_view") - assert lyr.GetGeomType() == ogr.wkbMultiPoint + assert lyr.GetGeomType() == ogr.wkbMultiPoint25D assert lyr.GetSpatialRef().GetAuthorityCode(None) == "4326" f = lyr.GetNextFeature() - assert f.GetGeometryRef().ExportToWkt() == "MULTIPOINT (1 2)" + assert f.GetGeometryRef().ExportToIsoWkt() == "MULTIPOINT Z ((1 2 3))" + assert lyr.GetExtent() == (1, 1, 2, 2) + assert lyr.GetExtent3D() == (1, 1, 2, 2, 3, 3) ds = None diff --git a/ogr/ogrsf_frmts/gpkg/ogrgeopackagedatasource.cpp b/ogr/ogrsf_frmts/gpkg/ogrgeopackagedatasource.cpp index 9b6944f37bd1..6d9f7f901999 100644 --- a/ogr/ogrsf_frmts/gpkg/ogrgeopackagedatasource.cpp +++ b/ogr/ogrsf_frmts/gpkg/ogrgeopackagedatasource.cpp @@ -7724,7 +7724,10 @@ static void OGRGeoPackageSTMinX(sqlite3_context *pContext, int argc, { GPkgHeader sHeader; if (!OGRGeoPackageGetHeader(pContext, argc, argv, &sHeader, true, false)) + { + sqlite3_result_null(pContext); return; + } sqlite3_result_double(pContext, sHeader.MinX); } @@ -7737,7 +7740,10 @@ static void OGRGeoPackageSTMinY(sqlite3_context *pContext, int argc, { GPkgHeader sHeader; if (!OGRGeoPackageGetHeader(pContext, argc, argv, &sHeader, true, false)) + { + sqlite3_result_null(pContext); return; + } sqlite3_result_double(pContext, sHeader.MinY); } @@ -7750,7 +7756,10 @@ static void OGRGeoPackageSTMaxX(sqlite3_context *pContext, int argc, { GPkgHeader sHeader; if (!OGRGeoPackageGetHeader(pContext, argc, argv, &sHeader, true, false)) + { + sqlite3_result_null(pContext); return; + } sqlite3_result_double(pContext, sHeader.MaxX); } @@ -7763,7 +7772,10 @@ static void OGRGeoPackageSTMaxY(sqlite3_context *pContext, int argc, { GPkgHeader sHeader; if (!OGRGeoPackageGetHeader(pContext, argc, argv, &sHeader, true, false)) + { + sqlite3_result_null(pContext); return; + } sqlite3_result_double(pContext, sHeader.MaxY); } @@ -7776,7 +7788,10 @@ static void OGRGeoPackageSTIsEmpty(sqlite3_context *pContext, int argc, { GPkgHeader sHeader; if (!OGRGeoPackageGetHeader(pContext, argc, argv, &sHeader, false, false)) + { + sqlite3_result_null(pContext); return; + } sqlite3_result_int(pContext, sHeader.bEmpty); } @@ -7933,7 +7948,10 @@ static void OGRGeoPackageSTSRID(sqlite3_context *pContext, int argc, { GPkgHeader sHeader; if (!OGRGeoPackageGetHeader(pContext, argc, argv, &sHeader, false, false)) + { + sqlite3_result_null(pContext); return; + } sqlite3_result_int(pContext, sHeader.iSrsId); } diff --git a/ogr/ogrsf_frmts/gpkg/ogrgeopackagetablelayer.cpp b/ogr/ogrsf_frmts/gpkg/ogrgeopackagetablelayer.cpp index 586f5f8c51fc..23baca823714 100644 --- a/ogr/ogrsf_frmts/gpkg/ogrgeopackagetablelayer.cpp +++ b/ogr/ogrsf_frmts/gpkg/ogrgeopackagetablelayer.cpp @@ -8704,34 +8704,32 @@ static void OGR_GPKG_GeometryExtent3DAggregate_Step(sqlite3_context *pContext, if (pabyBLOB != nullptr) { GPkgHeader sHeader; - const int nBLOBLen = sqlite3_value_bytes(argv[0]); - if (GPkgHeaderFromWKB(pabyBLOB, nBLOBLen, &sHeader) == OGRERR_NONE && - static_cast(nBLOBLen) >= sHeader.nHeaderLen + 5) - { - if (!OGRGeoPackageGetHeader(pContext, 0, argv, &sHeader, true, - true)) + if (OGRGeoPackageGetHeader(pContext, 0, argv, &sHeader, true, true)) + { + OGREnvelope3D extent3D; + extent3D.MinX = sHeader.MinX; + extent3D.MaxX = sHeader.MaxX; + extent3D.MinY = sHeader.MinY; + extent3D.MaxY = sHeader.MaxY; + extent3D.MinZ = sHeader.MinZ; + extent3D.MaxZ = sHeader.MaxZ; + poContext->m_oExtent3D.Merge(extent3D); + } + else if (!sHeader.bEmpty) + { + // Try also spatialite geometry blobs + const int nBLOBLen = sqlite3_value_bytes(argv[0]); + OGRGeometry *poGeom = nullptr; + if (OGRSQLiteImportSpatiaLiteGeometry(pabyBLOB, nBLOBLen, + &poGeom) == OGRERR_NONE && + poGeom && !poGeom->IsEmpty()) { - OGRGeometry *poGeom = - GPkgGeometryToOGR(pabyBLOB, nBLOBLen, nullptr); - if (poGeom == nullptr || poGeom->IsEmpty()) - { - delete poGeom; - return; - } OGREnvelope3D extent3D; poGeom->getEnvelope(&extent3D); poContext->m_oExtent3D.Merge(extent3D); - return; } + delete poGeom; } - OGREnvelope3D extent3D; - extent3D.MinX = sHeader.MinX; - extent3D.MaxX = sHeader.MaxX; - extent3D.MinY = sHeader.MinY; - extent3D.MaxY = sHeader.MaxY; - extent3D.MinZ = sHeader.MinZ; - extent3D.MaxZ = sHeader.MaxZ; - poContext->m_oExtent3D.Merge(extent3D); } } diff --git a/ogr/ogrsf_frmts/gpkg/ogrgeopackageutility.cpp b/ogr/ogrsf_frmts/gpkg/ogrgeopackageutility.cpp index a125570ec5b6..3163987e34af 100644 --- a/ogr/ogrsf_frmts/gpkg/ogrgeopackageutility.cpp +++ b/ogr/ogrsf_frmts/gpkg/ogrgeopackageutility.cpp @@ -370,6 +370,7 @@ OGRErr GPkgHeaderFromWKB(const GByte *pabyGpkg, size_t nGpkgLen, if (nGpkgLen < 8 || pabyGpkg[0] != 0x47 || pabyGpkg[1] != 0x50 || pabyGpkg[2] != 0) /* Version (only 0 supported at this time)*/ { + memset(poHeader, 0, sizeof(*poHeader)); return OGRERR_FAILURE; } @@ -511,7 +512,7 @@ OGRGeometry *GPkgGeometryToOGR(const GByte *pabyGpkg, size_t nGpkgLen, /* OGRGeoPackageGetHeader() */ /************************************************************************/ -bool OGRGeoPackageGetHeader(sqlite3_context *pContext, int /*argc*/, +bool OGRGeoPackageGetHeader(sqlite3_context * /*pContext*/, int /*argc*/, sqlite3_value **argv, GPkgHeader *psHeader, bool bNeedExtent, bool bNeedExtent3D, int iGeomIdx) { @@ -521,15 +522,19 @@ bool OGRGeoPackageGetHeader(sqlite3_context *pContext, int /*argc*/, if (sqlite3_value_type(argv[iGeomIdx]) != SQLITE_BLOB) { - sqlite3_result_null(pContext); + memset(psHeader, 0, sizeof(*psHeader)); return false; } - int nBLOBLen = sqlite3_value_bytes(argv[iGeomIdx]); + const int nBLOBLen = sqlite3_value_bytes(argv[iGeomIdx]); const GByte *pabyBLOB = reinterpret_cast(sqlite3_value_blob(argv[iGeomIdx])); - if (nBLOBLen < 8 || - GPkgHeaderFromWKB(pabyBLOB, nBLOBLen, psHeader) != OGRERR_NONE) + if (nBLOBLen < 8) + { + memset(psHeader, 0, sizeof(*psHeader)); + return false; + } + else if (GPkgHeaderFromWKB(pabyBLOB, nBLOBLen, psHeader) != OGRERR_NONE) { bool bEmpty = false; memset(psHeader, 0, sizeof(*psHeader)); @@ -540,56 +545,48 @@ bool OGRGeoPackageGetHeader(sqlite3_context *pContext, int /*argc*/, { psHeader->bEmpty = bEmpty; psHeader->bExtentHasXY = !bEmpty; - if (!(bEmpty && bNeedAnyExtent)) + if (!bNeedExtent3D && !(bEmpty && bNeedAnyExtent)) return true; } - sqlite3_result_null(pContext); return false; } if (psHeader->bEmpty && bNeedAnyExtent) { - sqlite3_result_null(pContext); return false; } else if (!psHeader->bExtentHasXY && bNeedExtent && !bNeedExtent3D) { - if (static_cast(nBLOBLen) >= psHeader->nHeaderLen + 5) + OGREnvelope sEnvelope; + if (OGRWKBGetBoundingBox(pabyBLOB + psHeader->nHeaderLen, + static_cast(nBLOBLen) - + psHeader->nHeaderLen, + sEnvelope)) { - OGREnvelope sEnvelope; - if (OGRWKBGetBoundingBox(pabyBLOB + psHeader->nHeaderLen, - static_cast(nBLOBLen) - - psHeader->nHeaderLen, - sEnvelope)) - { - psHeader->MinX = sEnvelope.MinX; - psHeader->MaxX = sEnvelope.MaxX; - psHeader->MinY = sEnvelope.MinY; - psHeader->MaxY = sEnvelope.MaxY; - return true; - } + psHeader->MinX = sEnvelope.MinX; + psHeader->MaxX = sEnvelope.MaxX; + psHeader->MinY = sEnvelope.MinY; + psHeader->MaxY = sEnvelope.MaxY; + return true; } return false; } else if (!psHeader->bExtentHasZ && bNeedExtent3D) { - if (static_cast(nBLOBLen) >= psHeader->nHeaderLen + 5) + OGREnvelope3D sEnvelope3D; + if (OGRWKBGetBoundingBox(pabyBLOB + psHeader->nHeaderLen, + static_cast(nBLOBLen) - + psHeader->nHeaderLen, + sEnvelope3D)) { - OGREnvelope3D sEnvelope3D; - if (OGRWKBGetBoundingBox(pabyBLOB + psHeader->nHeaderLen, - static_cast(nBLOBLen) - - psHeader->nHeaderLen, - sEnvelope3D)) - { - psHeader->MinX = sEnvelope3D.MinX; - psHeader->MaxX = sEnvelope3D.MaxX; - psHeader->MinY = sEnvelope3D.MinY; - psHeader->MaxY = sEnvelope3D.MaxY; - psHeader->MinZ = sEnvelope3D.MinZ; - psHeader->MaxZ = sEnvelope3D.MaxZ; - return true; - } + psHeader->MinX = sEnvelope3D.MinX; + psHeader->MaxX = sEnvelope3D.MaxX; + psHeader->MinY = sEnvelope3D.MinY; + psHeader->MaxY = sEnvelope3D.MaxY; + psHeader->MinZ = sEnvelope3D.MinZ; + psHeader->MaxZ = sEnvelope3D.MaxZ; + return true; } return false; }