Don't crash if directly set low level materials are used in sensors

[darksylinc]

Signed-off-by: Matias N. Goldberg [email protected]

🦟 Bug fix

No ticket was issued for this problem
This PR supersedes #586

Summary

The problem has been described in #586

This PR replaces it as it is applied to more sensors and comments explain why there can be nullptrs.

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.

[iche033]

looks good to me.

[darksylinc]

Please stand by before merging; I have one more fix incoming!

[darksylinc]

OK Done!

Just to explain this question from @chapulina

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.

The oldest code (before my PR that broke sensors) would just override any custom material with a standard one that does what's necessary by the sensor (e.g. for thermal, it draws the specified temperature on screen).

When @iche033 introduced custom (user provided) shaders, there was the problem that advanced vertex shaders could not be reproduced, thus custom geometry deformation could be handled by the standard material; and thus the sensor would see the right temperature but the wrong shape.

I fixed it by adding a PR which glues the custom vertex shader with a standard pixel shader. BUT, that only works with this new interface @iche033 added a while ago.

When it comes to low level materials internally defined (usually in material scripts) by Ignition (not the user defined ones); this gluing doesn't happen. It doesn't need to happen anyway because fortunately none of our VS are advanced enough (except for the point material, which can be manually workarounded).

[codecov]

Codecov Report

Merging #593 (c4cc18c) into main (f3c4003) will increase coverage by 0.00%.
The diff coverage is 64.00%.

@@           Coverage Diff           @@
##             main     #593   +/-   ##
=======================================
  Coverage   53.54%   53.55%           
=======================================
  Files         214      214           
  Lines       21304    21319   +15     
=======================================
+ Hits        11408    11417    +9     
- Misses       9896     9902    +6     

Impacted Files	Coverage Δ
ogre2/src/Ogre2ThermalCamera.cc	81.50% <40.00%> (-0.63%)	⬇️
ogre2/src/Ogre2GpuRays.cc	91.84% <80.00%> (-0.12%)	⬇️
ogre2/src/Ogre2MaterialSwitcher.cc	79.04% <80.00%> (-0.37%)	⬇️
ogre2/src/Ogre2SegmentationMaterialSwitcher.cc	64.77% <80.00%> (+0.03%)	⬆️
...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...c4cc18c. Read the comment docs.

[iche033]

latest changes look good to me.

@chapulina can you verify this works for you?

[chapulina]

Yup, works for me, thanks for the proper fix!