Opened 21 months ago

Closed 4 weeks ago

#34406 closed New feature (fixed)

Add support for curved geometries in GeoDjango

Reported by: Fabien Le Frapper Owned by: David Smith
Component: GIS Version: dev
Severity: Normal Keywords: geodjango gdal
Cc: Anthony Ricaud, Claude Paroz Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Fabien Le Frapper)

I tried ingesting curved geometries in a GeometryField in GeoDjango.

At first I encountered some errors as these are not officially supported in GeoDjango, but I noticed that gdal is able to go from a geometry to another using converters :

I successfully ingested the following geometries for a project :

  • 9: "CompoundCurve"
  • 10: "CurvePolygon"
  • 11: "MultiCurve"
  • 12: "MultiSurface"

Below is a code snippet I used, subclassing OGRGeometry and OGRGeomType in order to bypass existing GeoDjango validation to add support for more geometries :

  • It mainly adds keys for new geometries and classes inheriting for OGRGeometry to handle these in GeoDjango
  • It shows how we could make these geometries backward compatible (from curved geometries to more standard geometries) using a simple call to existing gdal methods

Is there a reason why it is not supported at the moment in GeoDjango ?
Would you consider a pull request adding support for these geometries ?

I could suggest a patch based on the snippet above, but I am not sure how to treat the transform part of it.
Should we keep this part ?
It seems that Postgis can handle these polygons https://postgis.net/docs/using_postgis_dbmanagement.html#CircularString


Here is the full snippet

from django.contrib.gis.gdal.geometries import GEO_CLASSES, OGRGeometry
from django.contrib.gis.gdal.geomtype import OGRGeomType
from django.contrib.gis.gdal.libgdal import lgdal
from django.contrib.gis.gdal.prototypes import ds as capi
from django.contrib.gis.gdal.prototypes import geom as geom_api


class ExtendedOGRGeometry(OGRGeometry):
    def __init__(self, geom_input, srs=None):
        try:
            super().__init__(geom_input, srs)
        except KeyError:
            if (
                not isinstance(geom_input, self.ptr_type)
                and self.geom_type.num not in gdal_transform.keys()
            ):
                raise
                
             self.__class__ = EXTENDED_GEO_CLASSES[self.geom_type.num]
            
    @property
    def geom_type(self):
        "Return the Type for this Geometry."
        return ExtendedOGRGeomType(geom_api.get_geom_type(self.ptr))

class CurvePolygon(ExtendedOGRGeometry):
    pass

class CompoundCurve(ExtendedOGRGeometry):
    pass

class MultiSurface(ExtendedOGRGeometry):
    pass

class MultiCurve(ExtendedOGRGeometry):
    pass

EXTENDED_GEO_CLASSES = {
    **GEO_CLASSES,
    9: CompoundCurve,
    10: CurvePolygon,
    11: MultiCurve,
    12: MultiSurface,
}

class ExtendedOGRGeomType(OGRGeomType):
    # Copy paste of original types dictionnary from GeoDjango implementation
    # https://github.com/django/django/blob/main/django/contrib/gis/gdal/geomtype.py#L9
    _types = {
        0: "Unknown",
        1: "Point",
        2: "LineString",
        3: "Polygon",
        4: "MultiPoint",
        5: "MultiLineString",
        6: "MultiPolygon",
        7: "GeometryCollection",
        100: "None",
        101: "LinearRing",
        102: "PointZ",
        1 + OGRGeomType.wkb25bit: "Point25D",
        2 + OGRGeomType.wkb25bit: "LineString25D",
        3 + OGRGeomType.wkb25bit: "Polygon25D",
        4 + OGRGeomType.wkb25bit: "MultiPoint25D",
        5 + OGRGeomType.wkb25bit: "MultiLineString25D",
        6 + OGRGeomType.wkb25bit: "MultiPolygon25D",
        7 + OGRGeomType.wkb25bit: "GeometryCollection25D",
        # Extended geometry types
        9: "CompoundCurve",
        10: "CurvePolygon",
        11: "MultiCurve",
        12: "MultiSurface",
    }
 

# New bindings to existing GDAL methods
force_to_polygon = geom_output(lgdal.OGR_G_ForceToPolygon, [c_void_p])
force_to_multi_polygon = geom_output(lgdal.OGR_G_ForceToMultiPolygon, [c_void_p])
force_to_line = geom_output(lgdal.OGR_G_ForceToLineString, [c_void_p])
force_to_multi_line = geom_output(lgdal.OGR_G_ForceToMultiLineString, [c_void_p])

# The functions below need to be called on the corresponding geometry type 
# before saving it in the database to prevent errors in geodjango. 
gdal_transform = {
    9: force_to_line,
    10: force_to_polygon,
    11: force_to_multi_line,
    12: force_to_multi_polygon,
}

def ingest_curved_geometry(geom_ptr):
    transform = gdal_transform[geom.geom_type.num]
    geom = ExtendedOGRGeometry(transform(geom_api.clone_geom(geom_ptr)))

    # FIXME: for a yet unknown reason, the initial SRID is not kept when using ExtendedOGRGeometry 
    geom.srid = 3857
    geom.transform(4326)

Change History (20)

comment:1 by Claude Paroz, 21 months ago

At first, I'd say that we are open to extend OGR recognition of those types. However, I wonder if the missing GEOS type counterpart may prevent proper usage of those new types in GeoDjango. At the very least, we can recognize these new types and error out with a proper error message if we don't support them in the framework. Accepting on that base.

comment:2 by Claude Paroz, 21 months ago

Triage Stage: UnreviewedAccepted

in reply to:  1 comment:3 by Fabien Le Frapper, 21 months ago

Thanks for your reply.
Do we have existing geodjango features that are gdal-only ?
Would a gdal-only contribution erroring in case of GEOS be accepted ?
Edit : sorry this is my first time in the Django bug tracker, I just noticed that you updated the ticket status. I will work on a PR then.

Last edited 21 months ago by Fabien Le Frapper (previous) (diff)

comment:4 by Fabien Le Frapper, 21 months ago

Owner: changed from nobody to Fabien Le Frapper
Status: newassigned

comment:5 by Fabien Le Frapper, 21 months ago

Description: modified (diff)

comment:6 by Fabien Le Frapper, 21 months ago

Description: modified (diff)

comment:7 by Anthony Ricaud, 21 months ago

Cc: Anthony Ricaud added

comment:8 by David Smith, 10 months ago

Here is a link to GDAL's RFC for "Curve geometries" RFC 49

This required the following C API changes. I think we should make a decision on each of these (should Django implement them) as part of this ticket.

Last edited 4 weeks ago by David Smith (previous) (diff)

comment:9 by David Smith, 10 months ago

Owner: changed from Fabien Le Frapper to David Smith

comment:10 by David Smith, 8 months ago

Has patch: set

comment:11 by Natalia Bidart, 7 months ago

Needs documentation: set
Patch needs improvement: set
Version: 4.1dev

Setting patch flags per latest reviews from Claude and myself.

comment:12 by David Smith, 5 months ago

Needs documentation: unset
Patch needs improvement: unset

comment:13 by David Smith, 5 months ago

Just looking at some of the other C API changes that were made in GDAL at the time this was implemented. These two items I suggest that we don't implement.

  • void OGRSetNonLinearGeometriesEnabledFlag(int bFlag)
  • int OGRGetNonLinearGeometriesEnabledFlag()

The RFC says:

If they don't want to test the geometry type and explicitly calling the conversion function, they can call OGRSetNonLinearGeometriesEnabledFlag(FALSE) (the default value is TRUE, i.e. non-linear geometries can be returned). In which case, they will be transformed into their closest linear geometry, by doing linear approximation, with OGR_G_ForceTo().

Given PR#18007 proposes to implement the geometry curve checking (via has_curve) and the linear conversion method I think that probably gives us enough coverage here.

comment:14 by Natalia Bidart, 6 weeks ago

Patch needs improvement: set

Setting to patch needs improvement until next week where I'll review answers from David (so this is no longer listed in the patch needing review list).

comment:15 by Natalia Bidart, 4 weeks ago

Patch needs improvement: unset

comment:16 by Natalia Bidart, 4 weeks ago

Has patch: unset

comment:17 by GitHub <noreply@…>, 4 weeks ago

In 04adff9f:

Refs #34406 -- Added support for GDAL curved geometries.

Co-authored-by: Fabien Le Frapper <contact@…>

comment:18 by Natalia Bidart, 4 weeks ago

Cc: Claude Paroz added

Claude, David: I have merged the latest PR from David and now I wonder what's left to do for this ticket? Could either of you, please, summarize next steps for future contributors?

comment:19 by Claude Paroz, 4 weeks ago

Thanks for the patch and the merge!

I'm not sure we should go further with this issue. Maybe OGR_G_Value? David?

comment:20 by David Smith, 4 weeks ago

Resolution: fixed
Status: assignedclosed

I think we can close this issue. We could always consider OGR_G_Value as part of #35058

Note: See TracTickets for help on using tickets.
Back to Top