search

ros-industrial-attic / yak_ros

Example ROS frontend node for the Yak TSDF package
Apache License 2.0
48 stars 22 forks source link

Remove clipped points instead of shifting them #28

Closed LorenzBung closed 4 years ago

LorenzBung commented 4 years ago

Instead of shifting the points outside the visible area to the borders, shouldn't we ignore those points and continue in the loop? Also, is it possible that pixel_pos_x and pixel_pos_y get negative values (e.g. wrong camera matrix)? If so, maybe catching this error and notifying the user is useful.

I'm sorry if I just misunderstood the code.

schornakj commented 4 years ago

I think that your proposed solution looks good. @mpowelson contributed the onReceivedPointCloud function so he can probably provide more insight here.

mpowelson commented 4 years ago

It looks good to me. I imagine we did this because we were always converting a point cloud that came from a depth image back to a depth image. So the points were just barely outside the limit due to some roundoff. But throwing them away is safer. I would think that negative could happen if the camera matrix is slightly wrong (or maybe if you are incorporating point clouds from another camera using this virtual camera). I'd say we could throw those out too.

LorenzBung commented 4 years ago

I thought about that, maybe it'd be helpful to throw a warning if we throw out points with negative values. The user might be unaware that his camera matrix is not correct.

mpowelson commented 4 years ago

That sounds fine to me.

schornakj commented 4 years ago

@LorenzBung Let us know when this is ready to merge

LorenzBung commented 4 years ago

@schornakj Should be ready, if nobody has any complaints?

mpowelson commented 4 years ago

@schornakj I see you thumbs upped this. I do not have merge rights, so you'll have to merge this.

schornakj commented 4 years ago

Sorry, thought I'd already merged it!