fix: use fixed-size Eigen vectors for plane-fit members (resolves #62)#81
Merged
Conversation
normal_, singular_values_, and pc_mean_ were declared as Eigen::VectorXf (dynamic-size). When a partial (non-360°) point cloud is processed — e.g. an OS1 128 Ouster pointcloud that doesn't start at azimuth 0 — the dynamic vectors are accessed at fixed indices before being sized, producing an immediate crash at startup. Switch all three to Eigen::Vector3f. Matches the SVD output (Vector3f from JacobiSVD<MatrixX3f>) and the existing per-index accesses (e.g. singular_values_.minCoeff(), pc_mean_(0..2), normal_(0..2)). Co-authored-by: MatteGombia <matteogombia@gmail.com> Closes #62
This was referenced May 9, 2026
TechRufy
pushed a commit
to ubm-driverless/patchwork-plusplus
that referenced
this pull request
May 13, 2026
Includes the partial-pointcloud crash fix from url-kaist#81 (Eigen::VectorXf → Eigen::Vector3f for normal_, singular_values_, pc_mean_).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Replaces
Eigen::VectorXfwithEigen::Vector3ffornormal_,singular_values_, andpc_mean_incpp/patchworkpp/include/patchwork/patchworkpp.h.The dynamic vectors are accessed at fixed indices (e.g.
pc_mean_(0),singular_values_.minCoeff()) before being sized, which crashes at startup when the input is a partial (non-360°) point cloud — for example, the OS1 128 Ouster pointcloud reported in #62 that does not start at azimuth 0.The fix matches the maintainer's own recommendation in #62 and the SVD output type (
JacobiSVD<MatrixX3f>returns aVector3ffor singular values andmatrixU().col(2)is aVector3f).Why this re-issues #72 instead of merging it
#72 had the correct fix but the commit was unsigned, and master now requires signed commits. This PR re-applies the same diff with
MatteGombiaas author and a signed committer, plus aCo-authored-by:trailer for full credit.#66 attempted the same fix but used
Vector2fforsingular_values_, which would silently change ground segmentation behavior becausesingular_values_.minCoeff()(used as the flatness measure atcpp/patchworkpp/src/patchworkpp.cpp:221) operates on the third (smallest) singular value — that's the one perpendicular to the fit plane.Test plan
data/000000.binCloses