Skip to content
This repository has been archived by the owner on Feb 25, 2021. It is now read-only.

Make the point cloud colored #238

Closed
wants to merge 18 commits into from
Closed

Make the point cloud colored #238

wants to merge 18 commits into from

Conversation

reunanen
Copy link
Contributor

@reunanen reunanen commented Feb 1, 2020

To address #149, keep color information related to point cloud points.

Mostly work by @vwvw – I'm just jumping in, because he's busy.

colored-point-cloud

reunanen
reunanen commented Feb 1, 2020
View reviewed changes
src/pangolin_viewer/viewer.cc
@@ -307,15 +309,17 @@ void viewer::draw_landmarks() {
return;
}

glPointSize(point_size_ * *menu_lm_size_);
glColor3fv(cs_.local_lm_.data());
glPointSize(point_size_ * *menu_lm_size_ * 2);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Now that there's no distinction in color, make local landmarks larger.

@shinsumicco
Copy link
Collaborator

shinsumicco commented Feb 20, 2020 •
edited
Loading

@reunanen
Sorry for the late reply, and thank you for the nice pull-request.
I have a question about processing time.
Is the consumed time for tracking of each frame increased after your implementation for this pull-req?
I'll check it ASAP, but we'd grateful if you could inspect it on your environment.
Thanks.

@reunanen
Copy link
Contributor Author

@shinsumicco I can't see any practical difference in the time required for tracking:

  • Before the change: 206 seconds
  • After the change: 206 seconds

(All else equal. Using the aist_living_lab_1 example video, --frame-skip 1, and the Pangolin viewer.)

I also profiled the CPU usage (using Visual Studio, not Google Perf Tools), and the lines changed in this PR were not really highlighted in any way.

@nickponline
Copy link

@reunanen Excited for this PR to be merged. Thanks for working on it.

@nickponline
Copy link

I don't think the color appear correctly in the Socket viewer though.

@reunanen
Copy link
Contributor Author

@nickponline You are right, the Socket viewer hasn't been touched. Want to contribute, if you are using it? Because I'm not.

@shinsumicco
Copy link
Collaborator 

* Before the change: 206 seconds
* After the change: 206 seconds

@reunanen Thank you for the report.
Before a review, could you merge the latest master branch because the recent commit in google-test had caused a compile issue (#252) ?

@reunanen
Copy link
Contributor Author

@shinsumicco Sure, merged.

@reunanen reunanen changed the base branch from master to develop April 11, 2020 07:55
@shinsumicco shinsumicco self-requested a review June 2, 2020 14:06
@shinsumicco shinsumicco self-assigned this Jun 2, 2020
@shinsumicco shinsumicco added the enhancement New feature or request label Jun 2, 2020
@ymd-stella ymd-stella mentioned this pull request Jan 30, 2021
Closed
5 tasks
@MikiyaShibuya MikiyaShibuya deleted the branch xdspacelab:develop February 25, 2021 06:32
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@vwvw vwvw vwvw left review comments

@shinsumicco shinsumicco Awaiting requested review from shinsumicco

Assignees

@shinsumicco shinsumicco

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@reunanen @shinsumicco @nickponline @vwvw @MikiyaShibuya