Provide an API to create a Geometry object from an owned OGRGeometryH

[thomas001]

Currently not all of the C Geometry API is wrapped, so users will need to go to gdal-sys and raw unsafe C calls from time to time. This is fine, wrapping all of GDAL is a huge undertaking. So right now I have code like this:

let p = gdal_sys::OGR_G_MakeValid(geom.c_geometry());
let v = Geometry::lazy_feature_geometry();
v.set_c_geometry(p);
v

As you can see creating a Geometry back from the raw C pointer is cumbersome and it leaks memory since v doesn't own the C geometry. I am asking for a function like Geometry::from_owned_c_geometry which will own the passed C object. So the above code could become

unsafe { 
  Geometry::from_owned_c_geometry(gdal_sys::OGR_G_MakeValid(geom.c_geometry()))
}

Please consider implementing it in public API. There seems to be a crate local function that almost does this, so maybe just change it to public .

[metasim]

@thomas001 Is OGR_G_MakeValid just one of many calls you're wanting to make, or would the more convenient approach be to add something like Geometry::make_valid instead?

In the future, I wonder if we would we regret letting an internal hack leak out (to never be put back in the box), and wish we'd just go ahead and implement the desired functionality? Not sure what the scope is of what you need, but feel like it's worth asking the question.

[thomas001]

@metasim Okay here is my brain dump on this topic, sorry if it is a bit unstructured.

There are a few functions that I missed, MakeValid is one of them, but also some set operation (Union, Difference, ...) or things like Polygonize seem to be not wrapped.

Yes, implementing the missing functions would be one way to go forward here (for a simple case like MakeValid I could also send a PR).

The more general question is: if you agree that not all functionality of GDAL is wrapped in this crate, should we offer users at least a supported way to go back and forth with gdal-sys which is more of a complete (since generated) wrapper?

Finally, I think there are some hackish functions already out there. There is at least set_c_geometry which is just not taking ownership of the C geometry.

So maybe two possible directions:

• C interop is supported and common, then my ask seems valid
• C interop is considered a hack and users should only interact using safe rust API, then
  • missing functions should be added (i am willing so send PR since I use this crate quite a bit)
  • maybe: unsafe functions should be deprecated
  • but: it might becomea cat&mouse game with upstream GDAL development to keep every user happy

[lnicola]

I think we should wrap more of the GDAL methods, but also offer a good interop story. Unfortunately, I'm not convinced that the current API is a good one, cf. #365.

[metasim]

See also: https://github.com/georust/gdal/discussions/360