Some lighting issues interiour

[hbeni]

• Yoke / Yoke cable
• Hobbs meter
• Davtron Clock

Always bright in fgfs next

[hbeni]

Played with the clock; the source of this is the material animation:

• if i add the lcd object to the material animation (and remove the other definition) it is bright too.
• if i add all the objects to the lighning animation and remove the material animation, all of the instrument is lit by the knob setting.
• if i remove the material definition alltogether, it looks "ok" again.... (tested with interoiur shadows etc)

[hbeni]

For the hobbs meter disabling the material animation properties also helps.

But for the yoke, and its cable, I'm out of luck.

@TheFGFSEagle Do you have any idea, by chance?

[hbeni]

(made a PR for the branch for the experimental animatoin fix)

[TheFGFSEagle]

Uh-oh, I think me introducing expressions for the material animation caused this ! I'll try to find a fix (either C++ or aircraft side) later.

[hbeni]

So... my commits are probably not the proper fix?

[hbeni]

Thanks for investigating!

[TheFGFSEagle]

So... my commits are probably not the proper fix?

Most probably not - you can try reverting SimGear commits 1cd4cb5ea2e5ad65cec8988b91ec51a7705c89cc and 51149bf5954176a4c9add634931fb4adb967dcd7 - if it works then, you can do nothing about it and I need to fix it on the C++ side ! :)

[TheFGFSEagle]

Most interesting is if reverting only the latter commit fixes the problem.

[hbeni]

trying, stand by

[hbeni]

confirming, it helped. I reverted both commits.

[TheFGFSEagle]

and if you only revert the latter of the two ?

[TheFGFSEagle]

this is important.

[hbeni]

On it

[TheFGFSEagle]

I looked at the animation code for the yoke cables and the hobbs - it could probably be fixed by adding

<emission>
  <red>0</red>
  <green>0</green>
  <blue>0</blue>
</expression>

but it needs to be fixed on the C++ side because it breaks other aircraft and possibly scenery objects as well.

[hbeni]

Confirm, 51149bf5954176a4c9add634931fb4adb967dcd7 is the commit causing the issue.

• reverting 1cd4cb5ea2e5ad65cec8988b91ec51a7705c89cc results in all black yokes and the clock/hobbs meter in "OK" mode
• reverting 51149bf5954176a4c9add634931fb4adb967dcd7 (got a merge conflict i resolved to the variant with getFloatValue()) results in OK behaviour

So i think your Reasoning is right, 51149bf5954176a4c9add634931fb4adb967dcd7 is flawed.

Most specifically the change here:

<<<<<<< HEAD
    float red = 1.0, green = 1.0, blue = 1.0, factor = 1.0, offset = 0.0;
    if (redex)
      red = redex->getValue();
    if (greenex)
      green = greenex->getValue();
    if (blueex)
      blue = blueex->getValue();
    if (factorex)
      factor = factorex->getValue();
    if (offsetex)
      offset = offsetex->getValue();
    v[0] = SGMiscf::clip(red * factor + offset, 0, 1);
    v[1] = SGMiscf::clip(green * factor + offset, 0, 1);
    v[2] = SGMiscf::clip(blue * factor + offset, 0, 1);
=======
    if (red_prop)
      red = red_prop->getFloatValue();
    if (green_prop)
      green = green_prop->getFloatValue();
    if (blue_prop)
      blue = blue_prop->getFloatValue();
    if (factor_prop)
      factor = factor_prop->getFloatValue();
    if (offset_prop)
      offset = offset_prop->getFloatValue();
    v[0] = SGMiscf::clip(red*factor + offset, 0, 1);
    v[1] = SGMiscf::clip(green*factor + offset, 0, 1);
    v[2] = SGMiscf::clip(blue*factor + offset, 0, 1);
>>>>>>> parent of 51149bf5 (Support expressions in material animations)

[TheFGFSEagle]

But it is needed for the instruments not to be all black … I'll try to find a solution as soon as I get some time to debug this ! :)

[TheFGFSEagle]

Could you go back to current next then apply the attached patch and tell me if everything is ok again ?

diff --git a/simgear/scene/model/SGMaterialAnimation.cxx b/simgear/scene/model/SGMaterialAnimation.cxx
index 8d654733..70f0fe6f 100644
--- a/simgear/scene/model/SGMaterialAnimation.cxx
+++ b/simgear/scene/model/SGMaterialAnimation.cxx
@@ -46,6 +46,11 @@ struct ColorSpec {

   ColorSpec(const SGPropertyNode* configNode, SGPropertyNode* modelRoot)
   {
+    redex = new SGConstExpression<double>(-1.0);
+    greenex = new SGConstExpression<double>(-1.0);
+    blueex = new SGConstExpression<double>(-1.0);
+    factorex = new SGConstExpression<double>(1.0);
+    offsetex = new SGConstExpression<double>(0.0);
     if (!configNode)
       return;

@@ -136,16 +141,10 @@ struct ColorSpec {
     return redex->getValue() >= 0 || greenex->getValue() >= 0 || blueex->getValue() >= 0;
   }
   bool live() {
-    return !(
-      (redex && redex->isConst()) &&
-      (greenex && greenex->isConst()) &&
-      (blueex &&blueex->isConst()) &&
-      (factorex && factorex->isConst()) &&
-      (offsetex && offsetex->isConst())
-    );
+    return !(redex->isConst() && greenex->isConst() && blueex->isConst() && factorex->isConst() && offsetex->isConst());
   }
   SGVec4f &rgba() {
-    float red = 1.0, green = 1.0, blue = 1.0, factor = 1.0, offset = 0.0;
+    float red = -1.0, green = -1.0, blue = -1.0, factor = 1.0, offset = 0.0;
     if (redex)
       red = redex->getValue();
     if (greenex)

[hbeni]

Confirm it helped! Looks all good now again.

[TheFGFSEagle]

Great ! :) I'll submit the patch to the mailing list for merging then ! :)

[TheFGFSEagle]

Done, I think you can close this issue now ! :D

[TheFGFSEagle]

My patch has just been committed to the SimGear repo ! :)

[hbeni]

Confirm, its working :)

Thanks!