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

make box-mode point cloud voxels render lighter on top than bottom #1058

Conversation

codebot
Copy link
Member

@codebot codebot commented Sep 7, 2023

Previously, the GLSL 1.2 shader for box-mode rendering of point clouds resulted in the top of the voxels rendering darker than the bottoms of the voxels. I think it looks nicer the other way around (top lighter than bottom), since most scenes are lit from the top.

(The GLSL 1.2 program is being used because of #851 ; the GLSL 1.5 shaders calculate lightness differently.)

Strangely, this GLSL 1.2 shader program structure has been in rviz for at least 11 years (according to git-blame). Am I just using it incorrectly, or have the tops of voxels always been darker than the bottoms of voxels, when rendering point clouds as boxes?

It may be important to note that I'm using a machine with an integrated (Intel) GPU. I don't have access to a machine with a discrete GPU to test the behavior in that case, since I assume a different shader program is selected when a GPU is present. It would be interesting to know if the same behavior happens with a GPU.

To see the effect:

  1. start a point cloud streaming from anything
  2. add a point cloud visualizer to rviz
  3. select "Boxes" style
  4. rotate camera around to view the scene from above and below

@ahcorde
Copy link
Contributor

ahcorde commented Sep 7, 2023

can you share a gif ? I tried with my discrete GPU, but I can't see any difference

@codebot
Copy link
Member Author

codebot commented Sep 7, 2023

That's interesting! I guess the discrete GPU is doing the "real" lighting calculation, rather than the "fake-lighting" shading used in glsl120/nogp/box.frag. That explains why this issue has been in the code for so long, since I assume that most people using RViz heavily for point-cloud visualization have a discrete GPU in their machine.

I attached two GIFs of a stereo camera looking at a tabletop, reduced using the PCL voxel_grid filter and rendered in Boxes mode in rviz.

The first GIF shows the current behavior on "integrated GPU" machines, where the top of the voxels is darker than the bottom. This works fine, but just looks kind of weird because most scenes in the real world are lit from above.

The second GIF shows what it looks like using this PR, with the top-side brighter in the fake-lighting shader.

(Please ignore the dithering artifacts due to heavy compression in the GIFs, but at least they show the relative brightness)

top_darker-opt top_brighter-opt

Copy link
Contributor

@ahcorde ahcorde left a comment

Choose a reason for hiding this comment

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

I tried with a flat surface and it makes sense

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status

@codebot
Copy link
Member Author

codebot commented Sep 7, 2023

Not sure what happened with those CI jobs. I don't think those failures were related to this tiny change (?)

@clalancette
Copy link
Contributor

Not sure what happened with those CI jobs. I don't think those failures were related to this tiny change (?)

No, all of CI is having problems at the moment. We have a solution, just need for CI to come back so we can merge.

@ahcorde
Copy link
Contributor

ahcorde commented Sep 7, 2023

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status

@clalancette
Copy link
Contributor

So CI is still failing here because the repos file used needs to be updated to use the branches as specified in ros2/ros2#1479 .

@ahcorde
Copy link
Contributor

ahcorde commented Sep 7, 2023

Thank you @clalancette

Let's try again

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status

@codebot codebot merged commit f0b3053 into ros2:rolling Sep 7, 2023
2 checks passed
@codebot codebot deleted the codebot/glsl120_point_cloud_box_lightness_top_bottom branch September 7, 2023 18:56
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.

None yet

3 participants