Skip to content

Return submap ids in InsertionResult along with pointers#1413

Open
ojura wants to merge 19 commits into
cartographer-project:masterfrom
larics:insertion-submap-ids
Open

Return submap ids in InsertionResult along with pointers#1413
ojura wants to merge 19 commits into
cartographer-project:masterfrom
larics:insertion-submap-ids

Conversation

@ojura

@ojura ojura commented Sep 6, 2018

Copy link
Copy Markdown
Contributor

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 force-pushed the insertion-submap-ids branch from 86d71fd to f0b2f1c Compare September 6, 2018 05:39
@ojura ojura force-pushed the insertion-submap-ids branch from 11b8b53 to b757831 Compare September 6, 2018 05:53
@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

ojura commented Dec 8, 2018

Copy link
Copy Markdown
Contributor Author

Ping?

@ojura

ojura commented Dec 17, 2018

Copy link
Copy Markdown
Contributor Author

@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

pifon2a commented Dec 17, 2018

Copy link
Copy Markdown
Contributor

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

@ojura

ojura commented Dec 18, 2018

Copy link
Copy Markdown
Contributor Author

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

@ojura

ojura commented Jan 18, 2019

Copy link
Copy Markdown
Contributor Author

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

@wohe

wohe commented Aug 26, 2020

Copy link
Copy Markdown
Member

@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

ojura commented Aug 27, 2020

Copy link
Copy Markdown
Contributor Author

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

ojura commented Aug 27, 2020

Copy link
Copy Markdown
Contributor Author

@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.

4 participants