Double threshold light ramps ignore t1 and l1

[Juha Jeronen]

As described in this thread on the forums:

https://www.panda3d.org/forums/viewtopic.php?f=4&t=7439

double threshold light ramps (LightRampAttrib.makeDoubleThreshold) currently ignore the t1 and l1 parameters, and behave as single threshold light ramps. This is current at least as of Panda3D 1.8.1.

Looking at the source code, I think this is actually a bug in panda/src/pgraph/lightRampAttrib.cxx and panda/src/pgraphnodes/shaderGenerator.cxx.

In lightRampAttrib.cxx of Panda3D 1.8.1, the function LightRampAttrib::make_double_threshold() says

---8<---8<---8<---
    CPT(RenderAttrib) LightRampAttrib::
    make_double_threshold(PN_stdfloat thresh0, PN_stdfloat val0, PN_stdfloat thresh1, PN_stdfloat val1) {
      LightRampAttrib *attrib = new LightRampAttrib();
      attrib->_mode = LRT_single_threshold;
      attrib->_threshold[0] = thresh0;
      attrib->_level[0] = val0;
      attrib->_threshold[1] = thresh1;
      attrib->_level[1] = val1;
      return return_new(attrib);
    }
---8<---8<---8<---

Note that the attrib mode is set as LRT_single_threshold. However, for double threshold mode, shaderGenerator.cxx expects LRT_double_threshold. This is why the parameters for the second threshold are getting ignored.

Hence, to fix this,

attrib->_mode = LRT_single_threshold;

should be changed to

attrib->_mode = LRT_double_threshold;

The rest is ok as-is.

Additionally, reading through ShaderGenerator::synthesize_shader(), there is a related problem, which would cause double threshold light ramps to fail even if the above is fixed. The shader generator actually expects three levels when an LRT_double_threshold light ramp is active (quoting from shaderGenerator.cxx):

---8<---8<---8<---
          case LightRampAttrib::LRT_double_threshold:
            {
              PN_stdfloat t0 = light_ramp->get_threshold(0);
              PN_stdfloat t1 = light_ramp->get_threshold(1);
              PN_stdfloat l0 = light_ramp->get_level(0);
              PN_stdfloat l1 = light_ramp->get_level(1);
              PN_stdfloat l2 = light_ramp->get_level(2);
              text << "\t // Double-threshold light ramp\n";
              text << "\t float lr_in = dot(tot_diffuse.rgb, float3(0.33,0.34,0.33));\n";
              text << "\t float lr_out = " << l0 << "\n";
              text << "\t if (lr_in > " << t0 << ") lr_out=" << l1 << ";\n";
              text << "\t if (lr_in > " << t1 << ") lr_out=" << l2 << ";\n";
              text << "\t tot_diffuse = tot_diffuse * (lr_out / lr_in);\n";
              break;
            }
---8<---8<---8<---

LightRampAttrib only has two levels available (according to lightRampAttrib.h and lightRampAttrib.I - see the array allocations and the functions LightRampAttrib::get_level(), LightRampAttrib::get_threshold()). Retrieving the level for an out-of-range index returns 0.0 (see LightRampAttrib::get_level() in lightRampAttrib.I).

Hence, with the code quoted here, the light level will drop to black when the input is above t1.

To fix this, I propose the following, adapting the approach of the single threshold mode (where the "low" value is always 0.0):

---8<---8<---8<---
          case LightRampAttrib::LRT_double_threshold:
            {
              PN_stdfloat t0 = light_ramp->get_threshold(0);
              PN_stdfloat t1 = light_ramp->get_threshold(1);
              PN_stdfloat l0 = light_ramp->get_level(0);
              PN_stdfloat l1 = light_ramp->get_level(1);
              text << "\t // Double-threshold light ramp\n";
              text << "\t float lr_in = dot(tot_diffuse.rgb, float3(0.33,0.34,0.33));\n";
              text << "\t float lr_out = 0.0\n";
              text << "\t if (lr_in > " << t0 << ") lr_out=" << l0 << ";\n";
              text << "\t if (lr_in > " << t1 << ") lr_out=" << l1 << ";\n";
              text << "\t tot_diffuse = tot_diffuse * (lr_out / lr_in);\n";
              break;
            }
---8<---8<---8<---

The only changes are in the levels: 0.0 for less than t0, l0 for between t0 and t1, and l1 for greater than t1. This version matches the Python API documentation for makeDoubleThreshold() (see http://www.panda3d.org/reference/1.8.1/python/classpanda3d.core.LightRampAttrib.php ), and is consistent with the behaviour of makeSingleThreshold().

I can prepare a proper patch if necessary.

[Juha Jeronen (juha-jeronen)]

Furthermore, the line

text << "\t float lr_out = " << l0 << "\n";

as also its proposed replacement

text << "\t float lr_out = 0.0\n";

is missing the final semicolon before the \n in the generated code. It should be:

text << "\t float lr_out = 0.0;\n";

After applying these modifications to my local copy of panda sources, the double light ramp shading started working.

[Juha Jeronen (juha-jeronen)]

Attached is a patch containing the required changes against 1.8.1.

[Juha Jeronen (juha-jeronen)]

To test this patch, take samples/Cartoon-Shader/Tut-Cartoon-Basic.py from the Panda distribution, and change the LightRampAttrib to

tempnode = NodePath(PandaNode("temp node"))
tempnode.setAttrib(LightRampAttrib.makeDoubleThreshold(thresh0=0.3, lev0=0.2, thresh1=0.6, lev1=1.0))
tempnode.setShaderAuto()
base.cam.node().setInitialState(tempnode.getState())

Turn the view using the mouse control, and examine the dragon; there should be three different lighting levels visible instead of the original two.

The values in the example are chosen such that the three lighting levels should be easily distinguishable.

[rdb (rdb)]

Thanks!