Skip to content

Commit 5d5ce4e

Browse files
committed
RFC 107: make OGRLayer::SetSpatialFilter[Rect]() non-virtual and add virtual ISetSpatialFilter()
1 parent 7fb8317 commit 5d5ce4e

File tree

89 files changed

+845
-1258
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

89 files changed

+845
-1258
lines changed

MIGRATION_GUIDE.TXT

+7
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,13 @@ MIGRATION GUIDE FROM GDAL 3.10 to GDAL 3.11
1212
``bool bForce``). Drivers must implement IGetExtent3D(int iGeomField, OGREnvelope3D*,
1313
bool bForce). The user-facing method checks that the iGeomField value is in range.
1414

15+
- The OGRLayer::SetSpatialFilter() and SetSpatialFilterRect() methods are
16+
no longer virtual methods that are implemented by drivers. They are now
17+
user facing methods (with the change that they return OGRErr instead of void,
18+
and accept a ``const OGRGeometry*``). Drivers must implement
19+
ISetSpatialFilter(int iGeomField, const OGRGeometry*). The user-facing methods
20+
check that the iGeomField value is in range.
21+
1522
- GDAL drivers may now return raster bands with the new data types
1623
GDT_Float16 or GDT_CFloat16. Code that use the GDAL API must be
1724
ready to react to the new data type, possibly by doing RasterIO()

apps/ogr2ogr_lib.cpp

+3-19
Original file line numberDiff line numberDiff line change
@@ -795,26 +795,10 @@ class OGRSplitListFieldLayer : public OGRLayer
795795
return poSrcLayer->GetStyleTable();
796796
}
797797

798-
virtual void SetSpatialFilter(OGRGeometry *poGeom) override
798+
virtual OGRErr ISetSpatialFilter(int iGeom,
799+
const OGRGeometry *poGeom) override
799800
{
800-
poSrcLayer->SetSpatialFilter(poGeom);
801-
}
802-
803-
virtual void SetSpatialFilter(int iGeom, OGRGeometry *poGeom) override
804-
{
805-
poSrcLayer->SetSpatialFilter(iGeom, poGeom);
806-
}
807-
808-
virtual void SetSpatialFilterRect(double dfMinX, double dfMinY,
809-
double dfMaxX, double dfMaxY) override
810-
{
811-
poSrcLayer->SetSpatialFilterRect(dfMinX, dfMinY, dfMaxX, dfMaxY);
812-
}
813-
814-
virtual void SetSpatialFilterRect(int iGeom, double dfMinX, double dfMinY,
815-
double dfMaxX, double dfMaxY) override
816-
{
817-
poSrcLayer->SetSpatialFilterRect(iGeom, dfMinX, dfMinY, dfMaxX, dfMaxY);
801+
return poSrcLayer->SetSpatialFilter(iGeom, poGeom);
818802
}
819803

820804
virtual OGRErr SetAttributeFilter(const char *pszFilter) override

autotest/ogr/ogr_geojson.py

+2-1
Original file line numberDiff line numberDiff line change
@@ -648,7 +648,8 @@ def test_ogr_geojson_23(tmp_vsimem):
648648
lyr.CreateFeature(feat)
649649
assert lyr.GetExtent() == (1.0, 2.0, 10.0, 20.0)
650650
assert lyr.GetExtent(geom_field=0) == (1.0, 2.0, 10.0, 20.0)
651-
assert lyr.GetExtent(geom_field=1, can_return_null=True) is None
651+
with gdaltest.disable_exceptions():
652+
assert lyr.GetExtent(geom_field=1, can_return_null=True) is None
652653
lyr = None
653654
ds = None
654655

autotest/ogr/ogr_oapif.py

+74-47
Original file line numberDiff line numberDiff line change
@@ -548,7 +548,7 @@ def NO_LONGER_USED_test_ogr_oapif_fc_links_next_headers():
548548
###############################################################################
549549

550550

551-
def test_ogr_oapif_spatial_filter():
551+
def test_ogr_oapif_spatial_filter_deprecated_api():
552552

553553
# Deprecated API
554554
handler = webserver.SequentialHandler()
@@ -568,7 +568,31 @@ def test_ogr_oapif_spatial_filter():
568568
ds = ogr.Open("OAPIF:http://localhost:%d/oapif" % gdaltest.webserver_port)
569569
lyr = ds.GetLayer(0)
570570
assert lyr.TestCapability(ogr.OLCFastGetExtent)
571-
assert lyr.GetExtent() == (-10.0, 15.0, 40.0, 50.0)
571+
572+
handler = webserver.SequentialHandler()
573+
_add_dummy_root_and_api_pages(handler)
574+
handler.add(
575+
"GET",
576+
"/oapif/collections/foo/items?limit=20",
577+
200,
578+
{"Content-Type": "application/geo+json"},
579+
"""{ "type": "FeatureCollection", "features": [
580+
{
581+
"type": "Feature",
582+
"properties": {
583+
"foo": "bar"
584+
}
585+
}
586+
] }""",
587+
)
588+
with webserver.install_http_handler(handler):
589+
assert lyr.GetExtent() == (-10.0, 15.0, 40.0, 50.0)
590+
591+
592+
###############################################################################
593+
594+
595+
def test_ogr_oapif_spatial_filter():
572596

573597
# Nominal API
574598
handler = webserver.SequentialHandler()
@@ -591,7 +615,6 @@ def test_ogr_oapif_spatial_filter():
591615
with webserver.install_http_handler(handler):
592616
ds = ogr.Open("OAPIF:http://localhost:%d/oapif" % gdaltest.webserver_port)
593617
lyr = ds.GetLayer(0)
594-
assert lyr.GetExtent() == (-10.0, 15.0, 40.0, 50.0)
595618

596619
handler = webserver.SequentialHandler()
597620
_add_dummy_root_and_api_pages(handler)
@@ -610,6 +633,7 @@ def test_ogr_oapif_spatial_filter():
610633
] }""",
611634
)
612635
with webserver.install_http_handler(handler):
636+
assert lyr.GetExtent() == (-10.0, 15.0, 40.0, 50.0)
613637
assert lyr.GetLayerDefn().GetFieldCount() == 1
614638

615639
lyr.SetSpatialFilterRect(2, 49, 3, 50)
@@ -1365,11 +1389,6 @@ def test_ogr_oapif_storage_crs_easting_northing():
13651389
with webserver.install_http_handler(handler):
13661390
ds = ogr.Open("OAPIF:http://localhost:%d/oapif" % gdaltest.webserver_port)
13671391
lyr = ds.GetLayer(0)
1368-
minx, maxx, miny, maxy = lyr.GetExtent()
1369-
assert (minx, miny, maxx, maxy) == pytest.approx(
1370-
(-611288.854779237, 4427761.561734099, 1525592.2813932528, 5620112.89047953),
1371-
abs=1e-3,
1372-
)
13731392

13741393
handler = webserver.SequentialHandler()
13751394
_add_dummy_root_and_api_pages(handler)
@@ -1389,10 +1408,16 @@ def test_ogr_oapif_storage_crs_easting_northing():
13891408
] }""",
13901409
)
13911410
with webserver.install_http_handler(handler):
1392-
srs = lyr.GetSpatialRef()
1393-
assert srs
1394-
assert srs.GetAuthorityCode(None) == "32631"
1395-
assert lyr.GetLayerDefn().GetFieldCount() == 1
1411+
minx, maxx, miny, maxy = lyr.GetExtent()
1412+
assert (minx, miny, maxx, maxy) == pytest.approx(
1413+
(-611288.854779237, 4427761.561734099, 1525592.2813932528, 5620112.89047953),
1414+
abs=1e-3,
1415+
)
1416+
1417+
srs = lyr.GetSpatialRef()
1418+
assert srs
1419+
assert srs.GetAuthorityCode(None) == "32631"
1420+
assert lyr.GetLayerDefn().GetFieldCount() == 1
13961421

13971422
handler = webserver.SequentialHandler()
13981423
handler.add(
@@ -1467,8 +1492,6 @@ def test_ogr_oapif_storage_crs_latitude_longitude():
14671492
with webserver.install_http_handler(handler):
14681493
ds = ogr.Open("OAPIF:http://localhost:%d/oapif" % gdaltest.webserver_port)
14691494
lyr = ds.GetLayer(0)
1470-
minx, maxx, miny, maxy = lyr.GetExtent()
1471-
assert (minx, miny, maxx, maxy) == pytest.approx((-10, 40, 15, 50), abs=1e-3)
14721495

14731496
handler = webserver.SequentialHandler()
14741497
_add_dummy_root_and_api_pages(handler)
@@ -1488,12 +1511,15 @@ def test_ogr_oapif_storage_crs_latitude_longitude():
14881511
] }""",
14891512
)
14901513
with webserver.install_http_handler(handler):
1491-
srs = lyr.GetSpatialRef()
1492-
assert srs
1493-
assert srs.GetAuthorityCode(None) == "4326"
1494-
assert srs.GetDataAxisToSRSAxisMapping() == [2, 1]
1495-
assert srs.GetCoordinateEpoch() == 2022.5
1496-
assert lyr.GetLayerDefn().GetFieldCount() == 1
1514+
minx, maxx, miny, maxy = lyr.GetExtent()
1515+
assert (minx, miny, maxx, maxy) == pytest.approx((-10, 40, 15, 50), abs=1e-3)
1516+
1517+
srs = lyr.GetSpatialRef()
1518+
assert srs
1519+
assert srs.GetAuthorityCode(None) == "4326"
1520+
assert srs.GetDataAxisToSRSAxisMapping() == [2, 1]
1521+
assert srs.GetCoordinateEpoch() == 2022.5
1522+
assert lyr.GetLayerDefn().GetFieldCount() == 1
14971523

14981524
handler = webserver.SequentialHandler()
14991525
# Coordinates must be in lat, lon order in the GeoJSON answer
@@ -1573,11 +1599,6 @@ def test_ogr_oapif_storage_crs_latitude_longitude_non_compliant_server():
15731599
open_options=["SERVER_FEATURE_AXIS_ORDER=GIS_FRIENDLY"],
15741600
)
15751601
lyr = ds.GetLayer(0)
1576-
minx, maxx, miny, maxy = lyr.GetExtent()
1577-
assert (minx, miny, maxx, maxy) == pytest.approx((-10, 40, 15, 50), abs=1e-3)
1578-
1579-
supported_srs_list = lyr.GetSupportedSRSList()
1580-
assert supported_srs_list is None
15811602

15821603
handler = webserver.SequentialHandler()
15831604
_add_dummy_root_and_api_pages(handler)
@@ -1597,12 +1618,18 @@ def test_ogr_oapif_storage_crs_latitude_longitude_non_compliant_server():
15971618
] }""",
15981619
)
15991620
with webserver.install_http_handler(handler):
1600-
srs = lyr.GetSpatialRef()
1601-
assert srs
1602-
assert srs.GetAuthorityCode(None) == "4326"
1603-
assert srs.GetDataAxisToSRSAxisMapping() == [2, 1]
1604-
assert srs.GetCoordinateEpoch() == 2022.5
1605-
assert lyr.GetLayerDefn().GetFieldCount() == 1
1621+
minx, maxx, miny, maxy = lyr.GetExtent()
1622+
assert (minx, miny, maxx, maxy) == pytest.approx((-10, 40, 15, 50), abs=1e-3)
1623+
1624+
supported_srs_list = lyr.GetSupportedSRSList()
1625+
assert supported_srs_list is None
1626+
1627+
srs = lyr.GetSpatialRef()
1628+
assert srs
1629+
assert srs.GetAuthorityCode(None) == "4326"
1630+
assert srs.GetDataAxisToSRSAxisMapping() == [2, 1]
1631+
assert srs.GetCoordinateEpoch() == 2022.5
1632+
assert lyr.GetLayerDefn().GetFieldCount() == 1
16061633

16071634
handler = webserver.SequentialHandler()
16081635
# Coordinates must be in lat, lon order in the GeoJSON answer
@@ -1665,19 +1692,6 @@ def get_collections_handler():
16651692
assert ds
16661693
lyr = ds.GetLayer(0)
16671694

1668-
minx, maxx, miny, maxy = lyr.GetExtent()
1669-
assert (minx, miny, maxx, maxy) == pytest.approx(
1670-
(-611288.854779237, 4427761.561734099, 1525592.2813932528, 5620112.89047953),
1671-
abs=1e-3,
1672-
)
1673-
1674-
supported_srs_list = lyr.GetSupportedSRSList()
1675-
assert supported_srs_list
1676-
assert len(supported_srs_list) == 2
1677-
assert supported_srs_list[0].GetAuthorityCode(None) == "32631"
1678-
# Below doesn't work with early PROJ 6 versions
1679-
# assert supported_srs_list[1].GetAuthorityCode(None) == "CRS84"
1680-
16811695
def get_items_handler():
16821696
handler = webserver.SequentialHandler()
16831697
_add_dummy_root_and_api_pages(handler)
@@ -1699,9 +1713,22 @@ def get_items_handler():
16991713
return handler
17001714

17011715
with webserver.install_http_handler(get_items_handler()):
1702-
srs = lyr.GetSpatialRef()
1703-
assert srs
1704-
assert srs.GetAuthorityCode(None) == "32631"
1716+
minx, maxx, miny, maxy = lyr.GetExtent()
1717+
assert (minx, miny, maxx, maxy) == pytest.approx(
1718+
(-611288.854779237, 4427761.561734099, 1525592.2813932528, 5620112.89047953),
1719+
abs=1e-3,
1720+
)
1721+
1722+
supported_srs_list = lyr.GetSupportedSRSList()
1723+
assert supported_srs_list
1724+
assert len(supported_srs_list) == 2
1725+
assert supported_srs_list[0].GetAuthorityCode(None) == "32631"
1726+
# Below doesn't work with early PROJ 6 versions
1727+
# assert supported_srs_list[1].GetAuthorityCode(None) == "CRS84"
1728+
1729+
srs = lyr.GetSpatialRef()
1730+
assert srs
1731+
assert srs.GetAuthorityCode(None) == "32631"
17051732

17061733
json_info = gdal.VectorInfo(ds, format="json", featureCount=False)
17071734
assert "supportedSRSList" in json_info["layers"][0]["geometryFields"][0]

autotest/ogr/ogr_sql_sqlite.py

+2-2
Original file line numberDiff line numberDiff line change
@@ -2473,7 +2473,7 @@ def test_ogr_sql_sqlite_execute_sql_error_on_spatial_filter_mem_layer():
24732473
ds.CreateLayer("test")
24742474
geom = ogr.CreateGeometryFromWkt("POLYGON((0 0,0 1,1 1,1 0,0 0))")
24752475
with pytest.raises(
2476-
Exception, match="Cannot set spatial filter: no geometry field selected"
2476+
Exception, match="Cannot set spatial filter: no geometry field present in layer"
24772477
):
24782478
ds.ExecuteSQL("SELECT 1 FROM test", spatialFilter=geom, dialect="SQLITE")
24792479

@@ -2490,6 +2490,6 @@ def test_ogr_sql_sqlite_execute_sql_error_on_spatial_filter_shp_layer(tmp_vsimem
24902490
ds.CreateLayer("test")
24912491
geom = ogr.CreateGeometryFromWkt("POLYGON((0 0,0 1,1 1,1 0,0 0))")
24922492
with pytest.raises(
2493-
Exception, match="Cannot set spatial filter: no geometry field selected"
2493+
Exception, match="Cannot set spatial filter: no geometry field present in layer"
24942494
):
24952495
ds.ExecuteSQL("SELECT 1 FROM test", spatialFilter=geom, dialect="SQLITE")

autotest/ogr/ogr_vrt.py

+2-2
Original file line numberDiff line numberDiff line change
@@ -2381,9 +2381,9 @@ def test_ogr_vrt_33b(ogr_vrt_33, tmp_path):
23812381
ret = gdaltest.runexternal(
23822382
test_cli_utilities.get_test_ogrsf_path() + f" -ro {tmp_path}/ogr_vrt_33.vrt"
23832383
)
2384-
os.unlink(tmp_path / "ogr_vrt_33.vrt")
23852384

2386-
assert ret.find("INFO") != -1 and ret.find("ERROR") == -1
2385+
assert "INFO" in ret
2386+
assert "ERROR" not in ret
23872387

23882388

23892389
@pytest.mark.require_driver("CSV")

frmts/eeda/eedadataset.cpp

+6-9
Original file line numberDiff line numberDiff line change
@@ -140,13 +140,8 @@ class GDALEEDALayer final : public OGRLayer
140140
return -1;
141141
}
142142

143-
virtual void SetSpatialFilter(OGRGeometry *poGeom) CPL_OVERRIDE;
144-
145-
virtual void SetSpatialFilter(int iGeomField,
146-
OGRGeometry *poGeom) CPL_OVERRIDE
147-
{
148-
OGRLayer::SetSpatialFilter(iGeomField, poGeom);
149-
}
143+
virtual OGRErr ISetSpatialFilter(int iGeomField,
144+
const OGRGeometry *poGeom) override;
150145

151146
virtual OGRErr SetAttributeFilter(const char *) CPL_OVERRIDE;
152147

@@ -938,10 +933,11 @@ OGRErr GDALEEDALayer::SetAttributeFilter(const char *pszQuery)
938933
}
939934

940935
/************************************************************************/
941-
/* SetSpatialFilter() */
936+
/* ISetSpatialFilter() */
942937
/************************************************************************/
943938

944-
void GDALEEDALayer::SetSpatialFilter(OGRGeometry *poGeomIn)
939+
OGRErr GDALEEDALayer::ISetSpatialFilter(int /* iGeomField */,
940+
const OGRGeometry *poGeomIn)
945941
{
946942
if (poGeomIn)
947943
{
@@ -960,6 +956,7 @@ void GDALEEDALayer::SetSpatialFilter(OGRGeometry *poGeomIn)
960956
InstallFilter(poGeomIn);
961957

962958
ResetReading();
959+
return OGRERR_NONE;
963960
}
964961

965962
/************************************************************************/

0 commit comments

Comments
 (0)