Skip to content

Commit f7a765d

Browse files
elpasoelpaso
authored
[GML] Support CityGML3 Shell (#12789)
Fix #12769 Also, expose SKIP_RESOLVE_ELEMS as an open option
1 parent b622b56 commit f7a765d

File tree

7 files changed

+175
-49
lines changed

7 files changed

+175
-49
lines changed

‎autotest/ogr/ogr_gml.py‎

Lines changed: 138 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1277,21 +1277,33 @@ def test_ogr_gml_35(tmp_path):
12771277

12781278

12791279
@pytest.mark.parametrize("GML_FACE_HOLE_NEGATIVE", ("NO", "YES"))
1280-
def test_ogr_gml_36(tmp_path, GML_FACE_HOLE_NEGATIVE):
1280+
@pytest.mark.parametrize("skip_resolve_as_open_option", (True, False))
1281+
def test_ogr_gml_36(tmp_path, GML_FACE_HOLE_NEGATIVE, skip_resolve_as_open_option):
12811282

12821283
if GML_FACE_HOLE_NEGATIVE == "NO":
12831284
if not ogrtest.have_geos():
12841285
pytest.skip("GEOS not available")
12851286

12861287
shutil.copy("data/gml/GmlTopo-sample.xml", tmp_path)
12871288

1288-
with gdal.config_options(
1289-
{
1290-
"GML_SKIP_RESOLVE_ELEMS": "NONE",
1291-
"GML_FACE_HOLE_NEGATIVE": GML_FACE_HOLE_NEGATIVE,
1292-
}
1293-
):
1294-
ds = ogr.Open(tmp_path / "GmlTopo-sample.xml")
1289+
if skip_resolve_as_open_option:
1290+
with gdal.config_options(
1291+
{
1292+
"GML_FACE_HOLE_NEGATIVE": GML_FACE_HOLE_NEGATIVE,
1293+
}
1294+
):
1295+
ds = gdal.OpenEx(
1296+
tmp_path / "GmlTopo-sample.xml",
1297+
open_options=["SKIP_RESOLVE_ELEMS=NONE"],
1298+
)
1299+
else:
1300+
with gdal.config_options(
1301+
{
1302+
"GML_SKIP_RESOLVE_ELEMS": "NONE",
1303+
"GML_FACE_HOLE_NEGATIVE": GML_FACE_HOLE_NEGATIVE,
1304+
}
1305+
):
1306+
ds = ogr.Open(tmp_path / "GmlTopo-sample.xml")
12951307
assert gdal.GetLastErrorMsg() == "", "did not expect error"
12961308

12971309
lyr = ds.GetLayerByName("Suolo")
@@ -1318,8 +1330,9 @@ def test_ogr_gml_36(tmp_path, GML_FACE_HOLE_NEGATIVE):
13181330

13191331

13201332
@pytest.mark.parametrize("resolver", ("HUGE", "NONE"))
1333+
@pytest.mark.parametrize("skip_resolve_as_open_option", (True, False))
13211334
@pytest.mark.require_geos
1322-
def test_ogr_gml_38(tmp_path, resolver):
1335+
def test_ogr_gml_38(tmp_path, resolver, skip_resolve_as_open_option):
13231336

13241337
if resolver == "HUGE" and ogr.GetDriverByName("SQLite") is None:
13251338
pytest.skip("Requires SQLite support")
@@ -1329,8 +1342,15 @@ def test_ogr_gml_38(tmp_path, resolver):
13291342
tmp_path,
13301343
)
13311344

1332-
with gdal.config_option("GML_SKIP_RESOLVE_ELEMS", resolver):
1333-
ds = ogr.Open(tmp_path / "sample_gml_face_hole_negative_no.xml")
1345+
if skip_resolve_as_open_option:
1346+
ds = gdal.OpenEx(
1347+
tmp_path / "sample_gml_face_hole_negative_no.xml",
1348+
open_options=[f"SKIP_RESOLVE_ELEMS={resolver}"],
1349+
)
1350+
else:
1351+
with gdal.config_option("GML_SKIP_RESOLVE_ELEMS", resolver):
1352+
ds = ogr.Open(tmp_path / "sample_gml_face_hole_negative_no.xml")
1353+
13341354
gdal.SetConfigOption("GML_FACE_HOLE_NEGATIVE", None)
13351355

13361356
if resolver == "HUGE":
@@ -1353,7 +1373,10 @@ def test_ogr_gml_38(tmp_path, resolver):
13531373

13541374
@pytest.mark.require_driver("SQLite")
13551375
@pytest.mark.require_geos
1356-
def test_ogr_gml_huge_resolver_same_nested_property_name(tmp_path):
1376+
@pytest.mark.parametrize("skip_resolve_as_open_option", (True, False))
1377+
def test_ogr_gml_huge_resolver_same_nested_property_name(
1378+
tmp_path, skip_resolve_as_open_option
1379+
):
13571380

13581381
shutil.copy(
13591382
"data/gml/same_nested_property_name.gml",
@@ -1370,9 +1393,16 @@ def check_ds(ds):
13701393
ds = ogr.Open(tmp_path / "same_nested_property_name.gml")
13711394
check_ds(ds)
13721395

1373-
with gdal.config_option("GML_SKIP_RESOLVE_ELEMS", "HUGE"):
1374-
ds = ogr.Open(tmp_path / "same_nested_property_name.gml")
1375-
check_ds(ds)
1396+
if skip_resolve_as_open_option:
1397+
ds = gdal.OpenEx(
1398+
tmp_path / "same_nested_property_name.gml",
1399+
open_options=["SKIP_RESOLVE_ELEMS=HUGE"],
1400+
)
1401+
else:
1402+
with gdal.config_option("GML_SKIP_RESOLVE_ELEMS", "HUGE"):
1403+
ds = ogr.Open(tmp_path / "same_nested_property_name.gml")
1404+
1405+
check_ds(ds)
13761406

13771407

13781408
###############################################################################
@@ -5027,6 +5057,7 @@ def test_ogr_gml_write_error(tmp_vsimem):
50275057
ds.Close()
50285058

50295059

5060+
###############################################################################
50305061
# Test open option SKIP_CORRUPTED_FEATURES
50315062

50325063

@@ -5052,27 +5083,27 @@ def test_ogr_gml_skip_corrupted_features(tmp_vsimem):
50525083
<gml:featureMember>
50535084
<ogr:solids fid="solids.1">
50545085
<boundary>
5055-
<ClosureSurface>
5056-
<lod2MultiSurface>
5057-
<gml:MultiSurface>
5058-
<gml:surfaceMember>
5059-
<gml:Polygon gml:id="UUID_da310e81-cd4a-4167-94a0-3ea04f39ebd0">
5060-
<gml:exterior>
5061-
<gml:LinearRing>
5062-
<gml:posList srsDimension="3">646102.957 5667459.848 146.668 646102.957 5667459.848 142.56 646107.611 5667459.649 142.56 646107.611 5667459.649 146.696 646102.957 5667459.848 146.668</gml:posList>
5063-
</gml:LinearRing>
5064-
</gml:exterior>
5065-
</gml:Polygon>
5066-
</gml:surfaceMember>
5067-
</gml:MultiSurface>
5068-
</lod2MultiSurface>
5069-
</ClosureSurface>
5070-
</boundary>
5086+
<ClosureSurface>
5087+
<lod2MultiSurface>
5088+
<gml:MultiSurface>
5089+
<gml:surfaceMember>
5090+
<gml:Polygon gml:id="UUID_da310e81-cd4a-4167-94a0-3ea04f39ebd0">
5091+
<gml:exterior>
5092+
<gml:LinearRing>
5093+
<gml:posList srsDimension="3">646102.957 5667459.848 146.668 646102.957 5667459.848 142.56 646107.611 5667459.649 142.56 646107.611 5667459.649 146.696 646102.957 5667459.848 146.668</gml:posList>
5094+
</gml:LinearRing>
5095+
</gml:exterior>
5096+
</gml:Polygon>
5097+
</gml:surfaceMember>
5098+
</gml:MultiSurface>
5099+
</lod2MultiSurface>
5100+
</ClosureSurface>
5101+
</boundary>
50715102
<gml:Solid>
50725103
<gml:exterior>
5073-
<gml:Shell>
5104+
<gml:XXXX>
50745105
<gml:surfaceMember xlink:href="#UUID_da310e81-cd4a-4167-94a0-3ea04f39ebd0"/>
5075-
</gml:Shell>
5106+
</gml:XXXX>
50765107
</gml:exterior>
50775108
</gml:Solid>
50785109
</ogr:solids>
@@ -5109,3 +5140,77 @@ def test_ogr_gml_skip_corrupted_features(tmp_vsimem):
51095140
ds = gdal.OpenEx(filename, open_options=["SKIP_CORRUPTED_FEATURES=NO"])
51105141
layer = ds.GetLayer(0)
51115142
layer.GetNextFeature()
5143+
5144+
5145+
###############################################################################
5146+
# Test CityGML3 for issue GH #12769
5147+
5148+
5149+
@pytest.mark.parametrize("skip_resolve_as_open_option", (True, False))
5150+
def test_ogr_gml_citygml3(tmp_vsimem, skip_resolve_as_open_option):
5151+
"""Test for issue GH #12769"""
5152+
5153+
filename = str(tmp_vsimem / "test.gml")
5154+
gdal.FileFromMemBuffer(
5155+
filename,
5156+
"""<?xml version="1.0" encoding="utf-8" ?>
5157+
<CityModel>
5158+
<ogr:FeatureCollection
5159+
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
5160+
xsi:schemaLocation=""
5161+
xmlns:ogr="http://ogr.maptools.org/"
5162+
xmlns:gml="http://www.opengis.net/gml">
5163+
<gml:boundedBy>
5164+
<gml:Box>
5165+
<gml:coord><gml:X>0</gml:X><gml:Y>-5</gml:Y></gml:coord>
5166+
<gml:coord><gml:X>8</gml:X><gml:Y>3</gml:Y></gml:coord>
5167+
</gml:Box>
5168+
</gml:boundedBy>
5169+
<gml:featureMember>
5170+
<ogr:solids fid="solids.1">
5171+
<lod2Solid>
5172+
<gml:Solid>
5173+
<gml:exterior>
5174+
<gml:Shell>
5175+
<gml:surfaceMember xlink:href="#UUID_da310e81-cd4a-4167-94a0-3ea04f39ebd0"/>
5176+
</gml:Shell>
5177+
</gml:exterior>
5178+
</gml:Solid>
5179+
</lod2Solid>
5180+
<boundary>
5181+
<ClosureSurface>
5182+
<lod2MultiSurface>
5183+
<gml:MultiSurface>
5184+
<gml:surfaceMember>
5185+
<gml:Polygon gml:id="UUID_da310e81-cd4a-4167-94a0-3ea04f39ebd0">
5186+
<gml:exterior>
5187+
<gml:LinearRing>
5188+
<gml:posList srsDimension="3">0 0 0 1 1 1 1 0 2 0 0 0</gml:posList>
5189+
</gml:LinearRing>
5190+
</gml:exterior>
5191+
</gml:Polygon>
5192+
</gml:surfaceMember>
5193+
</gml:MultiSurface>
5194+
</lod2MultiSurface>
5195+
</ClosureSurface>
5196+
</boundary>
5197+
</ogr:solids>
5198+
</gml:featureMember>
5199+
</ogr:FeatureCollection>
5200+
</CityModel>""",
5201+
)
5202+
5203+
if skip_resolve_as_open_option:
5204+
ds = gdal.OpenEx(filename, open_options=["SKIP_RESOLVE_ELEMS=NONE"])
5205+
else:
5206+
with gdal.config_option("GML_SKIP_RESOLVE_ELEMS", "NONE"):
5207+
ds = ogr.Open(filename)
5208+
5209+
layer = ds.GetLayer(0)
5210+
assert layer.GetGeometryColumn() == "lod2Solid"
5211+
f = layer.GetNextFeature()
5212+
assert f is not None
5213+
assert (
5214+
f.GetGeometryRef().ExportToWkt()
5215+
== "POLYHEDRALSURFACE Z (((0 0 0,1 1 1,1 0 2,0 0 0)))"
5216+
)

‎doc/source/drivers/vector/gml.rst‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -313,6 +313,7 @@ will be also interpreted as being potential non-linear geometries, and
313313
corresponding OGR geometry type will be used for the layer geometry
314314
type.
315315

316+
316317
gml:xlink resolving
317318
-------------------
318319

@@ -614,6 +615,12 @@ The following open options are supported:
614615
coordinates will be always swapped regarding the order they appear in
615616
the GML, and when it set to NO, they will be kept in the same order.
616617

618+
- .. oo:: SKIP_RESOLVE_ELEMS
619+
:choices: NONE, ALL, HUGE, <list>
620+
:default: ALL
621+
622+
Control the gml:xlink resolving. See `gml:xlink resolving`_.
623+
617624
- .. oo:: READ_MODE
618625
:choices: AUTO, STANDARD, SEQUENTIAL_LAYERS, INTERLEAVED_LAYERS
619626
:default: AUTO

‎ogr/gml2ogrgeometry.cpp‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2161,6 +2161,7 @@ GML2OGRGeometry_XMLNode_Internal(const CPLXMLNode *psNode, const char *pszId,
21612161
/* --------------------------------------------------------------------- */
21622162
if (EQUAL(pszBaseGeometry, "MultiPolygon") ||
21632163
EQUAL(pszBaseGeometry, "MultiSurface") ||
2164+
EQUAL(pszBaseGeometry, "Shell") || // CityGML 3 uses this
21642165
EQUAL(pszBaseGeometry, "CompositeSurface"))
21652166
{
21662167
std::unique_ptr<OGRMultiSurface> poMS =
@@ -2268,6 +2269,8 @@ GML2OGRGeometry_XMLNode_Internal(const CPLXMLNode *psNode, const char *pszId,
22682269
(EQUAL(pszMemberElement, "Surface") ||
22692270
EQUAL(pszMemberElement, "Polygon") ||
22702271
EQUAL(pszMemberElement, "PolygonPatch") ||
2272+
EQUAL(pszMemberElement,
2273+
"Shell") || // CityGML 3 uses this
22712274
EQUAL(pszMemberElement, "CompositeSurface")))
22722275
{
22732276
auto poGeom = GML2OGRGeometry_XMLNode_Internal(
@@ -3645,7 +3648,8 @@ GML2OGRGeometry_XMLNode_Internal(const CPLXMLNode *psNode, const char *pszId,
36453648
return std::make_unique<OGRPolyhedralSurface>();
36463649
}
36473650

3648-
if (EQUAL(BareGMLElement(psChild->pszValue), "CompositeSurface"))
3651+
if (EQUAL(BareGMLElement(psChild->pszValue), "CompositeSurface") ||
3652+
EQUAL(BareGMLElement(psChild->pszValue), "Shell"))
36493653
{
36503654
auto poPS = std::make_unique<OGRPolyhedralSurface>();
36513655

@@ -3840,7 +3844,7 @@ OGRGeometryH OGR_G_CreateFromGMLTree(const CPLXMLNode *psTree)
38403844
* The following GML3 elements are parsed : Surface,
38413845
* MultiSurface, PolygonPatch, Triangle, Rectangle, Curve, MultiCurve,
38423846
* CompositeCurve, LineStringSegment, Arc, Circle, CompositeSurface,
3843-
* OrientableSurface, Solid, Tin, TriangulatedSurface.
3847+
* Shell, OrientableSurface, Solid, Tin, TriangulatedSurface.
38443848
*
38453849
* Arc and Circle elements are returned as curves by default. Stroking to
38463850
* linestrings can be done with

‎ogr/ogrgeometryfactory.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2242,7 +2242,7 @@ OGRGeometry *OGRGeometryFactory::organizePolygons(OGRGeometry **papoPolygons,
22422242
* The following GML3 elements are parsed : Surface,
22432243
* MultiSurface, PolygonPatch, Triangle, Rectangle, Curve, MultiCurve,
22442244
* LineStringSegment, Arc, Circle, CompositeSurface, OrientableSurface, Solid,
2245-
* Tin, TriangulatedSurface.
2245+
* Shell, Tin, TriangulatedSurface.
22462246
*
22472247
* Arc and Circle elements are returned as curves by default. Stroking to
22482248
* linestrings can be done with

‎ogr/ogrsf_frmts/gml/gmlhandler.cpp‎

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -426,6 +426,7 @@ static const char *const apszGMLGeometryElements[] = {
426426
"BoundingBox", /* ows:BoundingBox */
427427
"CompositeCurve",
428428
"CompositeSurface",
429+
"Shell", /* CityGML 3 */
429430
"Curve",
430431
"GeometryCollection", /* OGR < 1.8.0 bug... */
431432
"LineString",
@@ -1210,7 +1211,6 @@ OGRErr GMLHandler::startElementFeatureAttribute(const char *pszName,
12101211
poClass->GetGeometryPropertyCount();
12111212
}
12121213
}
1213-
12141214
else
12151215
{
12161216
if (!poClass->IsSchemaLocked() &&
@@ -1247,13 +1247,16 @@ OGRErr GMLHandler::startElementFeatureAttribute(const char *pszName,
12471247
return startElementGeometry(pszName, nLenName, attr);
12481248
}
12491249
}
1250-
else if (nLenName == 9 && strcmp(pszName, "boundedBy") == 0 &&
1251-
// We ignore the UseBBOX() flag for CityGML, since CityGML
1252-
// has elements like bldg:boundedBy, which are not a simple
1253-
// rectangular bbox. This is needed to read properly
1254-
// autotest/ogr/data/gml/citygml_lod2_713_5322.xml
1255-
// (this is a workaround of not being namespace aware)
1256-
(eAppSchemaType == APPSCHEMA_CITYGML || m_poReader->UseBBOX()))
1250+
else if (
1251+
(nLenName == 9 || nLenName == 8) &&
1252+
(strcmp(pszName, "boundedBy") == 0 ||
1253+
strcmp(pszName, "boundary") == 0) &&
1254+
// We ignore the UseBBOX() flag for CityGML, since CityGML
1255+
// has elements like bldg:boundedBy or 'boundary', which are not a simple
1256+
// rectangular bbox. This is needed to read properly
1257+
// autotest/ogr/data/gml/citygml_lod2_713_5322.xml
1258+
// (this is a workaround of not being namespace aware)
1259+
(eAppSchemaType == APPSCHEMA_CITYGML || m_poReader->UseBBOX()))
12571260
{
12581261
m_inBoundedByDepth = m_nDepth;
12591262

‎ogr/ogrsf_frmts/gml/ogrgmldatasource.cpp‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -779,8 +779,10 @@ bool OGRGMLDataSource::Open(GDALOpenInfo *poOpenInfo)
779779
}
780780
}
781781

782-
const char *pszSkipOption =
783-
CPLGetConfigOption("GML_SKIP_RESOLVE_ELEMS", "ALL");
782+
const char *pszSkipOption = CSLFetchNameValueDef(
783+
poOpenInfo->papszOpenOptions, "SKIP_RESOLVE_ELEMS",
784+
CPLGetConfigOption("GML_SKIP_RESOLVE_ELEMS", "ALL"));
785+
784786
char **papszSkip = nullptr;
785787
if (EQUAL(pszSkipOption, "ALL"))
786788
bResolve = false;

‎ogr/ogrsf_frmts/gml/ogrgmldriver.cpp‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -214,9 +214,14 @@ void RegisterOGRGML()
214214
"to read schema for imports along with includes or not' default='NO'/>"
215215
" <Option name='SKIP_CORRUPTED_FEATURES' type='boolean' "
216216
"description='Whether to skip features that cannot be parsed instead "
217-
"of "
218-
"failing' "
219-
"default='NO'/>"
217+
"of failing' default='NO'/>"
218+
" <Option name='SKIP_RESOLVE_ELEMS' type='string' "
219+
"description='Configure xlink element resolution. Set to NONE to "
220+
"resolve all elements, set to ALL to skip all xlink elements, "
221+
"set to HUGE to store linked elements in a temporary SQLite DB, "
222+
"set to a comma separated list of names of specific elements to be "
223+
"skipped.' "
224+
"default='ALL'/>"
220225
"</OpenOptionList>");
221226

222227
poDriver->SetMetadataItem(

0 commit comments

Comments
 (0)