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

More efficient pointcloud creation #163

Merged
merged 12 commits into from
Mar 10, 2023
Merged

Conversation

paskino
Copy link
Contributor

@paskino paskino commented Nov 30, 2022

closes #162

@lauramurgatroyd
Copy link
Member

Please update the change log

Comment on lines +2926 to +2929
# instead of translating the point cloud so that point0 is in the point cloud
# we add point0 to the point cloud.
if hasattr(self, 'point0'):
pointCloud.SetPoint0(self.point0_world_coords)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we should do this. Because then the points aren't evenly spaced and the overlap size around point 0 will not be consistent with the rest of the cloud. E.g. it ends up looking like this:
image
And e.g. with an overlap of 0:
image
there's clearly overlap around point 0

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

my understanding is that we had approval from Catherine that this is fine? So re-opening the PR.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need to test this again and re-review

@lauramurgatroyd lauramurgatroyd marked this pull request as draft December 1, 2022 10:44
@lauramurgatroyd lauramurgatroyd marked this pull request as ready for review December 7, 2022 15:06
@lauramurgatroyd lauramurgatroyd self-requested a review January 10, 2023 15:40
Copy link
Member

@lauramurgatroyd lauramurgatroyd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please merge master and update change log

@lauramurgatroyd lauramurgatroyd added this to the v23.0.0 milestone Jan 10, 2023
@paskino paskino merged commit f9396bc into master Mar 10, 2023
@paskino paskino deleted the more_efficient_pointcloud_creation branch July 21, 2023 20:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Creation of pointcloud cycles many times over the points
2 participants