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

NEW LIDAR Visualizer #18

Merged
merged 33 commits into from
Aug 15, 2024
Merged

NEW LIDAR Visualizer #18

merged 33 commits into from
Aug 15, 2024

Conversation

l00p3
Copy link
Contributor

@l00p3 l00p3 commented Aug 12, 2024

Following what we did for Kiss-ICP I want to propose here a polyscope integration also for LIDAR visualizer.
We have two kind of dataloader, one on which we can jump over:

Jumpable.webm

and one, like robags, where we cannot do it and only work on it sequentially:

NotJumpable.webm

In this case I added a disclaimer to explain why the jump and the backward steps are not possible.
To make it to work faster I also removed from all the dataloaders the dependency from open3D, because building every time a o3d.PointCloud object was really expensive and slow. Now it is actually too fast, but I can add something to control the speed of the visualization if you want.
Then I also better handled the "n-scans" an "jump" options, that were slightly broken before for some reason.
Finally, in the original version there is a problem with sequential datasets, like rosbags. You could still go back but the real result was just going forward, destroying completely the meaning of the "tqdm bar" that was actually going back. Then, also reaching the end was throwing an error.
In this update I solved all these problem.

Oh, also the keybindings are all there, included the controls for points size.

Copy link
Contributor

@nachovizzo nachovizzo left a comment

Choose a reason for hiding this comment

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

This is fantastic! thanks @l00p3 !

  1. For the non-jumpable ros, bagfiles is fine, it was always like this, I suggested shortening the message a bit on the GUI
  2. For the "too fast" we might add a time.sleep(delay) configurable through the GUI
  3. Is there any possibility of resetting the view point like before with R ?
  4. Is there any way to possibly reduce the point radius? Not the size (unless this is the same). With the new visualizer, even when having the most minor point, some clouds do render a bit ugly, especially when they are dense, for example

This PR

image

Before -- main

Smallest point size
image
Smallest point size +1
image

Smallest point size+2
image

Last but not least, do you know how to Rotate the view ? With open3D we can press shift for rotation (this is more like a newbie polyscope user question)

Great work!

src/lidar_visualizer/visualizer.py Outdated Show resolved Hide resolved
\ Outdated Show resolved Hide resolved
src/lidar_visualizer/visualizer.py Outdated Show resolved Hide resolved
l00p3 and others added 4 commits August 13, 2024 13:54
@l00p3
Copy link
Contributor Author

l00p3 commented Aug 13, 2024

The point size is exactly the point radius, probably the lower value in this visualizer was not small enough, I updated it, probably now it is better. These constants determine the values, they can be changed in case there is any problem:
FRAME_PTS_SIZE_STEP = 0.01
FRAME_PTS_SIZE_MIN = 0.001
FRAME_PTS_SIZE_MAX = 0.25

Also the button + keybinding to center the viewpoint is now there (sorry I cannot resist to use "C" instead of "R").

I will work on the sleep() on the loop.

@l00p3
Copy link
Contributor Author

l00p3 commented Aug 13, 2024

Oh, for the rotation like in Open3D I'm afraid there is nothing like that. If you think that it is really useful it can be implemented.

@l00p3
Copy link
Contributor Author

l00p3 commented Aug 13, 2024

Ok, now also the "slow down" option is there.

src/lidar_visualizer/visualizer.py Outdated Show resolved Hide resolved
@benemer
Copy link
Member

benemer commented Aug 13, 2024

The point size is exactly the point radius, probably the lower value in this visualizer was not small enough, I updated it, probably now it is better. These constants determine the values, they can be changed in case there is any problem: FRAME_PTS_SIZE_STEP = 0.01 FRAME_PTS_SIZE_MIN = 0.001 FRAME_PTS_SIZE_MAX = 0.25

Also the button + keybinding to center the viewpoint is now there (sorry I cannot resist to use "C" instead of "R").

I will work on the sleep() on the loop.

If you go down to point size 0.001, some points will no longer be visible. I would increase the minimum size a bit to avoid confusion of having a sparser scan.

Apart from this pretty awesome, the option to jump in the dataset is really really helpful. Thanks again Mr. Visualizer!

@nachovizzo nachovizzo self-requested a review August 14, 2024 09:23
@nachovizzo
Copy link
Contributor

this is fantastic @l00p3 thanks!!!

About rotating the camera... yes, this is something I immediately miss, as some data I can't visualize just because I can't move the camera around the cloud. But it's worth the upgrade.

Last annoying bit form my side, shall we increase the max point size now? It's super hard to me, a two-left-handed person to select the size with the mouse. Also the bigger point size feels TOO big now for me, I'd reduce the total range

min size

image

max size

image

@l00p3
Copy link
Contributor Author

l00p3 commented Aug 14, 2024

I tried to fix the point size problem. But still, in case you don't like them you can directly find the better configuration and update the three "constants":
FRAME_PTS_SIZE_STEP = 0.005
FRAME_PTS_SIZE_MIN = 0.02
FRAME_PTS_SIZE_MAX = 0.1
Probably it is faster, I have no access to the dataset that you are using, and the range should be found based on that.

For the camera rotation, I will work in the next days.
Thanks for appreciating the visualizer ❤️

src/lidar_visualizer/visualizer.py Outdated Show resolved Hide resolved
src/lidar_visualizer/visualizer.py Outdated Show resolved Hide resolved
@nachovizzo
Copy link
Contributor

@l00p3 I took the freedom to push a change in the point size, as I was not super convinced with the latest results; please do check if you agree with the changes! this is now good to merge for me

@l00p3
Copy link
Contributor Author

l00p3 commented Aug 15, 2024

Love it! Thanks!

@benemer
Copy link
Member

benemer commented Aug 15, 2024

If you go down to point size 0.001, some points will no longer be visible.

I still have this problem I mentioned above, have a look:
image
image

You can barely see the points, and not all will be rendered. This looks like a downsampled frame now. Of course I can zoom in, but it's super hard for a large scan. How about 0.005 min point size as a compromise @nachovizzo ?

@nachovizzo
Copy link
Contributor

@benemer done

@l00p3
Copy link
Contributor Author

l00p3 commented Aug 15, 2024

@saurabh1002 Now the speed level naming is fixed.

@l00p3 l00p3 merged commit beb5858 into main Aug 15, 2024
6 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.

4 participants