-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Adding Two Dimensional Data Protocols for python wrapper (vtx & tex) #2154
Adding Two Dimensional Data Protocols for python wrapper (vtx & tex) #2154
Conversation
…s and get_texture_coordinates)
Hi @baptiste-mnh |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The pull request's logic is great, but there's a couple small things that could add some more polish.
BufData( void *ptr // Raw pointer | ||
, size_t count // Number of points | ||
, size_t dim // The number of floats inside a point | ||
) : BufData( ptr, sizeof(float), "@f", 2, std::vector<size_t> { count, dim }, std::vector<size_t> { sizeof(float)*dim, sizeof(float) }) { } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be nice if this wasn't hard coded to floats.
It should be enough to add the std::string &format
and size_t size
parameters from the above overload and use them instead of "@f"
and sizeof(float)
respectively.
@@ -397,6 +401,23 @@ PYBIND11_MODULE(NAME, m) { | |||
py::class_<rs2::points, rs2::frame> points(m, "points"); | |||
points.def(py::init<>()) | |||
.def(py::init<rs2::frame>()) | |||
.def("get_vertices_2d", [](rs2::points& self) -> BufData |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be cleaner to have a single get_vertices
function that takes an int parameter requesting the number of dimensions of the output than get_vertices
and get_vertices_2d
.
This function should default to 1 dimension on no input to remain backwards compatible.
); | ||
|
||
}, py::keep_alive<0, 1>()) | ||
.def("get_texture_coordinates_2d", [](rs2::points& self) -> BufData |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here as on get_vertices_2d
Thanks! |
Whatever you think looks most pythonic. |
Yes I approve this choice! |
This PR should get auto-merged once we make this weeks release and merge development into master |
Relating to this issue :
https://github.com/erleben/pySoRo#adding-two-dimensional-data-protocols
I add two function in the python wrapper to get the vertices and textures coordinate compatible with open3d to do real time visualisation.
The 2 new functions
The code to test :