-
-
Notifications
You must be signed in to change notification settings - Fork 4.6k
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
[Visualizer] Fix crashes and exceptions in updatePointCloud()
#4017
[Visualizer] Fix crashes and exceptions in updatePointCloud()
#4017
Conversation
Perhaps merge the test with the code. And if it fixes the test, looks like you get 1 week worth of beer (or the drink of your choice) |
I just added them in seperate PRs. so we can see it actually fail now, before the fix gets merged. |
Please keep in mind that merging a failing test into master makes little sense. If the CI fails there, that'd confirm that the test is correct. After that, please consider moving those tests here and we'll merge once the CI turns green. |
840dc6e
to
8607dc0
Compare
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.
LGTM.
Let's also close #4016 in favor of another issue/PR to track the issue of the tests not being compiled/run?
CI is green. @larshg Please squash to have only 2 commits (test + mitigation). Ping someone after that so we can merge it. |
d34c374
to
8d4cc69
Compare
Merging because no content changed, and CI was green before |
updatePointCloud()
Setting the Cells count more explicitly seems to do the trick.
vertices->SetCells (nr_points, cells);
Failed to correctly identify that we changed its array/count. Maybe, one should look into this another day,
Closes #3452