gazebosim / gazebosim/gz-rendering

Potential bug: Wrong value stuck in Ogre2GpuRays

Open
#521 1 comment 0 reactions 0 assignees View on GitHub
bug help wanted
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.