Fix crash on material switcher when there are points

[chapulina]

🦟 Bug fix

Summary

Since #578, simulations where we use point markers are crashing when the user clicks on the scene. I traced it back to a null pointer access. This fixes the issue for me, but I don't know if there are unintended consequences.

CC @darksylinc

Checklist

• Signed all commits for DCO
• Added tests
• Updated documentation (as needed)
• Updated migration guide (as needed)
• codecheck passed (See contributing)
• All tests passed (See test coverage)
• While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸

[darksylinc]

Hi!

Your fix should revert code to the old behavior thus any "unintended consequence" is basically just the same as before my PR.

What I would be interested is in knowing how to repro, to identify & understand why it is happening in the first place.

[codecov]

Codecov Report

Merging #586 (3c94f9d) into main (f3c4003) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main     #586   +/-   ##
=======================================
  Coverage   53.54%   53.55%           
=======================================
  Files         214      214           
  Lines       21304    21304           
=======================================
+ Hits        11408    11409    +1     
+ Misses       9896     9895    -1     

Impacted Files	Coverage Δ
ogre2/src/Ogre2MaterialSwitcher.cc	79.41% <100.00%> (ø)
...e/ignition/rendering/base/BaseGaussianNoisePass.hh	100.00% <0.00%> (+3.33%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f3c4003...3c94f9d. Read the comment docs.

[iche033]

I think I was able to reproduce this in the camera integration test. I updated the ShaderSelection test to include a LidarVisual which uses point rendering and it crashes with backtrace pointing to that line. Here is the test:
https://github.com/ignitionrobotics/ign-rendering/blob/lowLevelMat_points/test/integration/camera.cc

The shader used for point rendering is this one here:
https://github.com/ignitionrobotics/ign-rendering/blob/lowLevelMat_points/ogre2/src/media/materials/programs/GLSL/point_vs.glsl

[chapulina]

Your fix should revert code to the old behavior thus any "unintended consequence" is basically just the same as before my PR.

I don't understand why that would be the case. All this PR checks is for a null pointer access. My guess is that we're trying to get the material for the points but there's none.

If one thing, I think this PR means that points aren't handled properly on sensor passes, but it shouldn't affect any other items which do have materials. So the current choice is either not to handle points well on sensors, or a crash. I'd like to get this PR in ASAP to fix the crash.

---

Thanks for the reproduction steps, @iche033 .

[iche033]

The problem is that the point cloud low level material is manually created here https://github.com/ignitionrobotics/ign-rendering/blob/ign-rendering6/ogre2/src/Ogre2LidarVisual.cc#L171 and so does not have a _solid version. The resulting behavior is that it will be rendered incorrectly in the selection buffer camera used for mouse picking and also other camera sensors. In the ign-gazebo's use case, the sensors should not see point cloud rendering so it should be fine. If mouse picking starts acting wierd especially with 3d lidar points that fill the whole scene, we will probably need to set visibility masks / flags so that the selection camera does not see the points.

I'll get this PR in first to fix the crash.

[iche033]

oh just saw #593. I'll take a look at that PR

[chapulina]

Superseded by #593