gazebosim / gz-rendering

C++ library designed to provide an abstraction for different rendering engines. It offers unified APIs for creating 3D graphics applications.
https://gazebosim.org
Apache License 2.0
56 stars 51 forks source link

Potential bug: Wrong value stuck in Ogre2GpuRays #521

Open darksylinc opened 2 years ago

darksylinc commented 2 years ago

Environment

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:

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 commented 2 years ago

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.