-
Notifications
You must be signed in to change notification settings - Fork 51
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
Fix objects using shaders when there is a lidar in the scene (ogre2) #575
Conversation
Signed-off-by: Ian Chen <[email protected]>
Codecov Report
@@ Coverage Diff @@
## ign-rendering6 #575 +/- ##
==================================================
- Coverage 54.36% 54.35% -0.01%
==================================================
Files 198 198
Lines 20157 20170 +13
==================================================
+ Hits 10958 10964 +6
- Misses 9199 9206 +7
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made two style suggestions, I tested locally and it works great
ogre2/src/Ogre2GpuRays.cc
Outdated
@@ -324,12 +337,24 @@ void Ogre2LaserRetroMaterialSwitcher::cameraPostRenderScene( | |||
subItem->setDatablock(it.second); | |||
} | |||
|
|||
auto lIt = laserRetroMaterialMap.find(this); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use a variable name more descriptive
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
está hecho a6c13de
ogre2/src/Ogre2MaterialSwitcher.cc
Outdated
if (technique && !technique->isDepthWriteEnabled() && | ||
!technique->isDepthCheckEnabled()) | ||
subItem->setMaterial(this->plainOverlayMaterial); | ||
else | ||
subItem->setMaterial(this->plainMaterial); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (technique && !technique->isDepthWriteEnabled() && | |
!technique->isDepthCheckEnabled()) | |
subItem->setMaterial(this->plainOverlayMaterial); | |
else | |
subItem->setMaterial(this->plainMaterial); | |
if (technique && !technique->isDepthWriteEnabled() && | |
!technique->isDepthCheckEnabled()) | |
{ | |
subItem->setMaterial(this->plainOverlayMaterial); | |
} | |
else | |
{ | |
subItem->setMaterial(this->plainMaterial); | |
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Signed-off-by: Ian Chen <[email protected]>
Signed-off-by: Ian Chen [email protected]
🦟 Bug fix
Summary
The gpu ray sensor sets up generates retro values by switching materials. The probably is that it incorrectly restores shader materials causing objects using custom shaders to render incorrectly after first update.
Applied a similar fix as as #533. Updated integration test should which fail without this fix.
On way to reproduce the issue is to run this shader_param_gpu_lidar.sdf world with ign-gazebo:
Without this fix, the red sphere and waves (which use custom shaders) will appear black in the
Image Display
plugin.Checklist
codecheck
passed (See contributing)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.