Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Why does createlinearring create a wkbLineString #314

Closed
rafaqz opened this issue Aug 4, 2022 · 4 comments
Closed

Why does createlinearring create a wkbLineString #314

rafaqz opened this issue Aug 4, 2022 · 4 comments

Comments

@rafaqz
Copy link
Collaborator

rafaqz commented Aug 4, 2022

See: https://github.com/yeesian/ArchGDAL.jl/blob/master/test/test_geometry.jl#L292

I'm trying to fix type stability of geometry creation for performance (some large gains from small changes) but createlinearring is a strange wart to special-case.

Why does it return a wkbLineString? as the comment says, it seems very odd.

@visr
Copy link
Collaborator

visr commented Aug 4, 2022

I don't really understand. LINEARRING is a non-standard extension of WKT that several projects support. It basically is a closed LINESTRING. The OGRwkbGeometryType has both wkbLineString and wkbLinearRing. GDAL seems to support it with a createlinearring function that creates a LINEARRING on toWKT. But why not use wkbLinearRing as a geometry type? Perhaps because LINESTRING is the more general type, which you can also close by having the first and last coordinate equal?

@rafaqz
Copy link
Collaborator Author

rafaqz commented Aug 4, 2022

Yeah seems like something like that. So the question is really in what cases does GDAL actually return wkbLinearRing form ogr_g_getgeometrytype, if ever?

Hard to make things type stable without knowing that. This is a pretty commmon path too - getting rings from a polygon - so it would be good to make it faster.

@visr
Copy link
Collaborator

visr commented Aug 4, 2022

It seems never, based on this comment:

wkbLinearRing = 101,    /**< non-standard, just for createGeometry() */

From https://github.com/OSGeo/gdal/blob/v3.5.1/ogr/ogr_core.h#L401

@rafaqz
Copy link
Collaborator Author

rafaqz commented Aug 4, 2022

Ok great. We can lock in wkbLineString as the type parameter.

Edit: but I guess we still need to handle passing wkbLinearRing but getting back a wkbLineString. So a line or two of special casing for createlinearring.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants