Opened 11 years ago

Closed 11 years ago

Last modified 11 years ago

#4407 closed defect (fixed)

the GML HUGE resolver fails to identify <Egde> within <Face>

Reported by: esseffe Owned by: warmerdam
Priority: normal Milestone: 1.9.0
Component: OGR_SF Version: svn-trunk
Severity: normal Keywords: GML HUGE
Cc: a.furieri@…


I discovered that the current implementation of the GML HUGE resolver was unable to correctly recognize any <Edge> item declared internally to some <Face>

the attached patch resolves this issue: and BTW implements a simpler and most standard method to retrieve <Node> coordinates

Bye, Sandro Furieri

Attachments (3) (2.3 KB ) - added by esseffe 11 years ago.
patch-edge-in-face-2.diff (12.7 KB ) - added by esseffe 11 years ago.
simplified_gmlHugeFileNodeCoords.patch (3.2 KB ) - added by Even Rouault 11 years ago.

Download all attachments as: .zip

Change History (7)

by esseffe, 11 years ago

Attachment: added

comment:1 by Even Rouault, 11 years ago

Several observations :

  • you should test the result of GML2OGRGeometry_XMLNode() for not being NULL. In case there's something that GML2OGRGeometry_XMLNode() doesn't like in the GML, it might return NULL
  • The poColl should be destroyed with delete after being used
  • I think you could directly wrap psNode inside a directedEdge and pass it to GML2OGRGeometry_XMLNode() (instead of wraping it again in a TopoCurve) which would return a linestring geometry.
  • The following seems to be broken :
+                    pItem->zNodeFrom = poLine->getY( 0 );
+                    pItem->zNodeTo = poLine->getX( iPoints - 1 );

I think it should be getZ(), right ?

comment:2 by esseffe, 11 years ago

Hi Even, thank a lot for your careful revision:

  • I've added a test to ignore the result of GML2OGRGeometry_XMLNode() when NULL
  • now poColl will be deleted when no longer required
  • I've corrected the two typos related to Z coords settings for <Node>

anyway, your suggestion about not using <TopoCurve> doesn't seem to be valid; I've performed several tests, and the unique way to actually get the linestring geometry seems to be the one to wrap the <Edge> in a <TopoCurve><directedEdge>; passing directly an <Egde> or a <directedEdge> doesn't return any linestring geometry at all.

please see: patch-edge-in-face-2.diff

bye, Sandro

by esseffe, 11 years ago

Attachment: patch-edge-in-face-2.diff added

comment:3 by Even Rouault, 11 years ago

Milestone: 1.9.0
Resolution: fixed
Status: newclosed

r23637 /trunk/gdal/ogr/ogrsf_frmts/gml/hugefileresolver.cpp: Fix the GML HUGE resolver so it can identify <Egde> within <Face> (#4407)

comment:4 by Even Rouault, 11 years ago

Attached simplified_gmlHugeFileNodeCoords.patch that avoids cloning the edge XML content and uses directly directedEdge as the wrapper level. The autotest still passes with it, but I haven't applied to avoid f*cking things at that point of the release process !

by Even Rouault, 11 years ago

Note: See TracTickets for help on using tickets.