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

Borrowed vs. owned geometries #4

Open
mgax opened this issue Jan 28, 2015 · 5 comments
Open

Borrowed vs. owned geometries #4

mgax opened this issue Jan 28, 2015 · 5 comments

Comments

@mgax
Copy link
Member

mgax commented Jan 28, 2015

The OGR C API has two ways of accessing geometries:

  • take a geometry pointer from a feature using OGR_F_GetGeometryRef, and you're not supposed to modify or deallocate it,
  • clone the above geometry, or create one from scratch e.g. using OGR_G_CreateFromWkt, and you can modify it, and you're responsible for deallocating; you can also pass ownership back to OGR by attaching it to a feature, using OGR_F_SetGeometryDirectly.

In both cases, you have a pointer to OGRGeometryH. We should support operations on both, e.g. transforming them to JSON or WKT, and it makes sense to share the implementation.

A naive approach would be to always copy geometries, and only work with owned geometries from Rust, but that seems wasteful. But I'm probably implementing this initially, to have a working API.

Another option is to create different types for owned and borrowed geometries and share the implementation by a common trait. I've experimented with this in the feature-geometry branch.

A third option is to have an owned geometry type, which wraps a borrowed geometry type, similar to String and str.

@georust/core, any input is much appreciated :)

@groteworld
Copy link
Member

What would be the duplication/harm/opinions in having separate sub-packages? one for native or borrowed.

@mgax
Copy link
Member Author

mgax commented Jan 29, 2015

What would be the duplication/harm/opinions in having separate sub-packages? one for native or borrowed.

You mean, having completely separate types, that share nothing? It would be duplicating the implementation of a bunch of methods (exporting to json/wkt/wkb, testing for overlap/containment/intersection, computing buffer/centroid/hull/area/etc), and not allowing the API user to use them interchangeably, even though in OGR they're actually the same type.

@sunng87
Copy link
Member

sunng87 commented Jan 29, 2015

For now we cannot avoid some package specific types. I think it's OK to keep these types and export them publicly. (Perhaps in a raw module?)

We only need to provide To/From traits and impls to convert them into our geo types for integration.

@frewsxcv
Copy link
Member

clone the above geometry, or create one from scratch e.g. using OGR_G_CreateFromWkt, and you can modify it, and you're responsible for deallocating; you can also pass ownership back to OGR by attaching it to a feature, using OGR_F_SetGeometryDirectly.

Admittedly I don't know too much about the OGR C API, but this approach sounds the most Rust-like

@metasim
Copy link
Contributor

metasim commented May 22, 2023

See also: #360

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

5 participants