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: optimization "createPlaneSegments" algorithm stage #45

Merged
merged 7 commits into from
Oct 22, 2023

Conversation

LevDenisov
Copy link
Collaborator

No description provided.

@@ -73,6 +74,7 @@ class PlaneExtractor::Impl {
int32_t image_height_;
int32_t image_width_;
Eigen::MatrixXi labels_map_;
std::vector<std::vector<Eigen::Index>> neighbours;
Copy link
Collaborator

Choose a reason for hiding this comment

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

  1. Why do we need precomputed neighbours
  2. Why is it a field of PlaneExtractor? It doesn't make sense for me.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

To get rid of numerous calls to the getNeighbours function, in which we look at neighboring cells every time and initialize a vector with them, when processing each image

unassigned_mask[i] = false;
--remaining_planar_cells;
}
for (auto& v : cells_to_merge) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Using reference for primitive type (e.g. int) is more expensive than using its copy. It's better to remove &

}
++stacked_cell_id;
}
for (auto& v : cells_to_merge) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same. Remove &

@achains achains self-requested a review October 20, 2023 20:18
@achains achains self-requested a review October 20, 2023 20:24
@@ -15,6 +15,7 @@
*/
#include "cell_grid.h"

#include <iostream>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove iostream

@achains achains merged commit 0fd2cfa into prime-slam:stable Oct 22, 2023
15 checks passed
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.

2 participants