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

Return submap ids in InsertionResult along with pointers #1413

Open
wants to merge 19 commits into
base: master
Choose a base branch
from

Conversation

ojura
Copy link
Contributor

@ojura ojura commented Sep 6, 2018

Returning just submap pointers in InsertionResult is not very useful for libcartographer users. You can't really tell which submap IDs changed when insertion happened. Therefore, have the pose graph return the assigned submap IDs just like it does for the inserted node ID.

Also, move GetSubmapData to the pose graph interface, so the user can query the pose graph just for these submap IDs (instead of having to use GetAllSubmapData).

@ojura ojura force-pushed the insertion-submap-ids branch 2 times, most recently from bfb64fc to 86d71fd Compare September 6, 2018 05:36
@ojura ojura changed the title Return submap ids in InsertionResult instead of pointers Return submap ids in InsertionResult along with pointers Sep 6, 2018
@ojura
Copy link
Contributor Author

ojura commented Dec 8, 2018

Ping?

@ojura
Copy link
Contributor Author

ojura commented Dec 17, 2018

@pifon2a since we're on a roll, if I could choose a PR review to get as a Christmas present :), it would be this one. I have encountered multiple times so far a need to know which submaps exactly changed with a recent insertion, and I think this would be a really useful extension of the API.

@pifon2a
Copy link
Contributor

pifon2a commented Dec 17, 2018

@ojura nice try :). PR review is not really a good Christmas present. I hope you get a normal present instead. Merry Christmas!

@ojura
Copy link
Contributor Author

ojura commented Dec 18, 2018

Damn :) Merry Christmas and happy new year to y'all as well!

@ojura
Copy link
Contributor Author

ojura commented Jan 18, 2019

Another month has passed, so here's another friendly ping for this PR.

@wohe
Copy link
Member

wohe commented Aug 26, 2020

@ojura Are you still interested in getting this merged? If so, please let me know and I will review. Also, it seems this PR is doing two completely separate things. Would it be possible to split off the GetSubmapData change to a separate PR? Should be faster to get it merged then. For the other part: can you give some context how it is used, for example the use case you have in mind?

@ojura
Copy link
Contributor Author

ojura commented Aug 27, 2020

Hi @wohe. One application is vertex coloring of scan matched point clouds according to the inserted submap ID (e.g. of the older one from the insertion pair). If you use that golden ratio HSV scheme, I think it’s a nice way to provide false coloring for checking the cloud quality.

Sure, I can split the PR.

@ojura
Copy link
Contributor Author

ojura commented Aug 27, 2020

@wohe here's a picture of coloring by submap ID:
image

wohe added a commit to wohe/cartographer that referenced this pull request Nov 20, 2020
Related to cartographer-project#1413.

Signed-off-by: Wolfgang Hess <whess@lyft.com>
wohe added a commit that referenced this pull request Nov 23, 2020
Related to #1413.

Signed-off-by: Wolfgang Hess <whess@lyft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants