gazebosim / gazebosim/gz-rendering
Potential bug: Wrong value stuck in Ogre2GpuRays
- Dominant language
- C++
- Stars
- 81
- Forks
- 90
- Avg merge
- 1d 13h
- Merged PRs (30d)
- 10
Description
## Environment
* OS Version: Ubuntu 18.04 LTS
* Source or binary build?
main 5e190619fddd15560ef4ef94558ae5e7fd46316d but I think it affects all of them
## Description
I noticed `Ogre2LaserRetroMaterialSwitcher` performs the following:
```cpp
if (!subItem->hasCustomParameter(this->customParamIdx))
{
// limit laser retro value to 2000 (as in gazebo)
if (retroValue > 2000.0)
{
retroValue = 2000.0;
}
float color = retroValue / 2000.0;
subItem->setCustomParameter(this->customParamIdx,
Ogre::Vector4(color, color, color, 1.0));
}
```
Basically: "If we haven't set the retroValue yet, set it".
However I see two potential issues with this:
- If the retro value can be changed at runtime (can it?) the change won't register unless Gazebo is recreating the associated Item
- `customParamIdx = 10u;` **_which is also shared with the thermal camera (same value)._** As a result, if an object is both in the Thermal Camera **and** in GpuRays, then the Thermal Camera will override the retroValue (because ThermalCamera never checks `hasCustomParameter( ... )`) and later `GpuRays` will not try to override anything.
**I see two possible solutions:**
1. Change `customParamIdx = 10u;` to one of the camera implementations (not recommended, this problem will reappear)
2. Get rid of the `if (!subItem->hasCustomParameter(this->customParamIdx))` check. It's no performance improvement, and this fix will get rid of the problem and any future clash.
## Steps to reproduce
This was found through manual examination, but I suppose it should be possible to repro by using GpuRays and Thermal Camera at the same time on the same object
## Other
It's possible both Ogre2 and Ogre1 backends are affected. I think the fix should be trivial to propagate to all branches without breaking ABI.
Contributor guide
Research direction
Start by locating Ogre2LaserRetroMaterialSwitcher and inspect how its custom parameter is assigned alongside the Thermal Camera implementation. Reproduce the interaction with GpuRays and Thermal Camera on the same object, then check whether the corresponding Ogre1 path is affected. Done means the retro value is not incorrectly retained or overridden, with behavior verified for the relevant backends.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- computer-graphics
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 42/100