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

fix potential segfault due to pointer reference and add ability to set max_norm explicitly #3

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

deeptoaster
Copy link

@deeptoaster deeptoaster commented Jul 31, 2023

Two changes:

  1. In LineSegment3D.hpp, m_cloud is a reference to a shared pointer, so it doesn't count towards the shared pointer's reference count. This means that if the original cloud as passed into the contructor goes out of scope before the LineSegment3D does so, the shared pointer thinks it's no longer used and calls its destructor. This leads to a segfault with any further operations on the LineSegment3D.

  2. In HoughTransform.ipp, a use case I've come across a couple times now is that sometimes we know that the origin of a line falls within a certain distance from an initial point—that is, I'm only interested in searching for lines that start there. Allowing maxNorm to be set means it's possible to greatly reduce the time taken to find such lines, instead of finding all lines (which would take many orders of magnitude longer) and filtering them down.

    The new parameter defaults to 0, which maintains the original behavior by using the entire point cloud space. The reason I made it a function parameter rather than a member of Param is because restricting the origin search space is a per-operation configuration, rather than one that applies to the entire point cloud. (The workflow I'm using is to instantiate a single HoughTransform object, then use it multiple times on different initial points.)

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

Successfully merging this pull request may close these issues.

1 participant