geos
geos copied to clipboard
Add classes for curved geometry types [WIP]
This PR adds curved geometry types to the GEOS Geometry
hierarchy and adds support to the WKTReader
for reading them. It is not ready to be merged; I'm just posting it at this stage in case anyone has feedback on the class hierarchy before I continue with implementation.
It makes the following additions to the class hierarchy:
- Adds a parent class
SimpleCurve
toLineString
. - Adds a parent class
Curve
toSimpleCurve
. - Adds a parent class
Surface
toPolygon
. - Adds class
CircularString
withSimpleCurve
as a parent - Adds class
CompoundCurve
withCurve
as a parent - Adds class
CurvePolygon
withSurface
as a parent - Adds class
MultiCurve
withGeometryCollection
as a parent - Adds class
MultiSurface
withGeometryCollection
as a parent
It turns out this is pretty close to what OGR does, except that OGR does Surface
--> CurvePolygon
--> Polygon
instead of having both CurvePolygon
and Polygon
as children of Surface
. While it is true that a Polygon
is a specialization of a CurvePolygon
, setting CurvePolygon
as the parent has a downside of making the result of dynamic_cast<CurvePolygon*>
less informative, and I didn't see an upside that would outweigh this annoyance. Similarly, I did not make MultiPolygon
a subclass of MultiSurface
, or MultiLineString
a subclass of MultiCurve
. But I'm receptive to the argument that it's simpler to just do things the same as OGR.
The QGIS class hierarchy looks to be the same as OGR's, except for the omission of SimpleCurve
.
Looks like you have a winner, @dbaston
a little helper for myself:
classDiagram
class GeometryCollection
class LineString
class SimpleCurve
class Curve
class Polygon
class Surface
class CircularString
class CompoundCurve
class CurvePolygon
class MultiCurve
class MultiSurface
SimpleCurve <|-- LineString
Curve <|-- SimpleCurve
Surface <|-- Polygon
SimpleCurve <|-- CircularString
Curve <|-- CompoundCurve
Surface <|-- CurvePolygon
GeometryCollection <|-- MultiCurve
GeometryCollection <|-- MultiSurface
From QGIS side, I don't expect any troubles from the difference, most code works on geometry level (and not individual geos class level), we this is pretty transparent.
@nyalldawson @lbartoletti @elpaso (as last contributors there) do you see any potential problem?
do you see any potential problem?
No, looks ok to me.
Glad to have curve support in GEOS! kudos @dbaston
Perhaps mark this as Draft?
I think this is ready for review. While large, I think it represents the minimum set of changes that can safely go into master:
- add curved types
- add WKT read/write support for use in testing
- add an implementation of
GetEnvelope
(since this is eagerly calculated, we can't have it just error out) - error out on curved types for all other operations accessible from the C API