Respect vertex shader from low level materials in sensor passes (#544)

[darksylinc]

🦟 Bug fix

Fixes #544

Summary

It implements the first option (clone the material with same VS but different PS).

However I could NOT test this PR because I had no test nor sample to test it.

That is the reason this PR is marked as draft. I would appreciate a way to play with this (i.e. a low level material entity being drawn in all sensors).

The lack of tests is tracked by #543

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.

[codecov]

Codecov Report

Merging #578 (722f794) into main (11197df) will decrease coverage by 0.03%.
The diff coverage is 27.05%.

@@            Coverage Diff             @@
##             main     #578      +/-   ##
==========================================
- Coverage   53.12%   53.08%   -0.04%     
==========================================
  Files         214      214              
  Lines       21229    21299      +70     
==========================================
+ Hits        11278    11307      +29     
- Misses       9951     9992      +41     

Impacted Files	Coverage Δ
ogre2/src/Ogre2GpuRays.cc	90.89% <0.00%> (-0.70%)	⬇️
ogre2/src/Ogre2MaterialSwitcher.cc	72.54% <0.00%> (-3.74%)	⬇️
ogre2/src/Ogre2SegmentationMaterialSwitcher.cc	60.69% <0.00%> (-1.81%)	⬇️
ogre2/src/Ogre2ThermalCamera.cc	80.39% <0.00%> (-1.80%)	⬇️
ogre2/src/Ogre2Material.cc	62.91% <41.81%> (-0.24%)	⬇️
ogre2/src/Ogre2RenderEngine.cc	78.50% <0.00%> (+0.43%)	⬆️

---

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 11197df...722f794. Read the comment docs.

[iche033]

I am testing this with this integration test (ShaderSelection). Just copy and paste to test/integration/camera.cc, build and run it: ./<install_path>/bin/INTEGRATION_camera

I'm getting a segfault pointing to this line in Ogre2SegmentationMaterialSwitcher.cc. Looks like it's not able to find the material file with _solid name suffix. Interestingly if I comment out the the thermal and gpu lidar cameras, segmentation camera works by itself. I confirmed this with ign-gazebo running this world file. If I only have the segmentation camera in the world I see that the sphere deforms in the camera view (yay!) but if I add a thermal camera or gpu lidar, it crashes with similar backtrace.

[darksylinc]

Thanks for the test! I accidentally removed two lines I shouldn't have, thus Thermal camera wasn't restoring the original material once it was done.

The code failed because the last camera was trying to look for "material_name_solid_solid", since Thermal didn't restore ""material_name_solid" ->"material_name"

[darksylinc]

It works!!

!

[iche033]

nice, confirmed that it works now!

[srmainwaring]

@iche033 and @darksylinc, unfortunately this PR breaks a number of the examples when using Metal. They include

• actor_animation
• custom_shaders_uniforms
• lidar_visual
• mesh_viewer

It also prevents the gazebo GUI from running on macOS (i.e. ign gazebo -g will error out).

The Ogre log message is a similar in all cases:

Shader _ign_scene::Material(65519)_solid_fs compiled successfully.
WARNING: Pixel Shader '_ign_scene::Material(65519)_solid_fs' without shader_reflection_pair_hint. If this is intentional, use build_parameters_from_reflection false to hide this warning.

[darksylinc]

The code definitely handles the reflection hint, but perhaps it doesn't run or runs too late?

[srmainwaring]

The code definitely handles the reflection hint, but perhaps it doesn't run or runs too late?

It may be running too late. The error is emitted here:

https://github.com/ignitionrobotics/ign-rendering/blob/23955a98b0b5082b6f6a7ea1ed3a3e84e1c39cfd/ogre2/src/Ogre2Material.cc#L869-L870

before the reflection_pair_hint is set. I'll see if I can either move the reflection hint earlier or defer setting the constant inColor.

[darksylinc]

Ahhh!! Of course!

That call can only be done in Ogre2Material::SetVertexShader, when shader_reflection_pair_hint is set for ogreSolidColorShader

[srmainwaring]

Yep - just verified for the mesh_viewer example. Moved that block to Ogre2Material::SetVertexShader and seems ok. I'll check the other examples and the main gazebo calls now...

Ok - looks good to me with that change (I've been messing about with a local copy of the wave shader which is why that looks different)