colmap / pycolmap

Python bindings for COLMAP
BSD 3-Clause "New" or "Revised" License
858 stars 125 forks source link

When should a class be bound with shared_ptr as its holder? #267

Closed nnop closed 3 months ago

nnop commented 3 months ago

I noticed some colmap classes were bound with shared_ptr holder type while others are not.

e.g.

py::class_<TrackElement, std::shared_ptr<TrackElement>> PyTrackElement(m, "TrackElement");

py::class_<Track, std::shared_ptr<Track>> PyTrack(m, "Track");

py::class_<Reconstruction, std::shared_ptr<Reconstruction>>(m, "Reconstruction")

If I want to bind a colmap class (e.g. IncrementalMapper), should I use shared_ptr instead of the default unique_ptr holder?

sarlinpe commented 3 months ago

We use shared_ptr for objects that are manipulated as such by COLMAP - mostly large ones. Given that we often share them across C++ and Python, unique_ptr is often not appropriate. Btw, @B1ueber2y should add bindings for the IncrementalMapper very soon. You're free to contribute though, but please send the PR to the COLMAP repo. Thank you!

nnop commented 3 months ago

Thanks @sarlinpe

I have managed to bind DatabaseCache and some IncrementalMapper methods for my use case. I found it's quite straight forward. Your binding codes are great for reference.