Potential bug: Wrong value stuck in Ogre2GpuRays

[darksylinc]

Environment

• OS Version: Ubuntu 18.04 LTS
• Source or binary build?
  main 5e19061 but I think it affects all of them

Description

I noticed Ogre2LaserRetroMaterialSwitcher performs the following:

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.

[darksylinc]

This bug is fixed in Fortress & Garden on Ogre2.
It should be backported to Citadel.

I don't know if the bug exists in Ogre1.