autoshader:undefined variable "tot_diffuse" (w/ light ramps)

Hi all,

An error, an investigation and an idea.

I’m getting the following error in the console when using the autoshader in Panda3D 1.8.1 with light ramping enabled:

:gobj(error): created-shader: (60) : error C1008: undefined variable "tot_diffuse"
:gobj(error): created-shader: (62) : error C1008: undefined variable "tot_diffuse"
:gobj(error): created-shader: (62) : error C1008: undefined variable "tot_diffuse"
:gobj(error): Shader encountered an error.
:gobj(error): created-shader: (70) : error C1008: undefined variable "tot_diffuse"
:gobj(error): created-shader: (72) : error C1008: undefined variable "tot_diffuse"
:gobj(error): created-shader: (72) : error C1008: undefined variable "tot_diffuse"
:gobj(error): Shader encountered an error.

Considering the possible code paths in ShaderGenerator::synthesize_shader() in panda/src/pgraphnodes/shaderGenerator.cxx, it seems there is indeed a case where “tot_diffuse” may get used without being defined, indeed related to light ramps. Consider the following:

  • “tot_diffuse” is defined if:
    _separate_ambient_diffuse && _have_diffuse
    _have_ambient || _have_diffuse

  • “tot_diffuse” is used if:
    !(_separate_ambient_diffuse && _have_ambient) && _have_diffuse

The _auto_ramp_on case does not care about _have_diffuse, _separate_ambient_diffuse and _have_ambient. Hence, if a light ramp is applied, it may occur that tot_diffuse is referred to (by the light ramp code) without being defined.

The combination of conditions suggests that the error will be triggered if a material has specular, but no ambient or diffuse. While this is unusual, it would be nice not to generate errors even in corner cases :slight_smile:

Now, how to fix this?

Currently, the light ramping code only affects the diffuse lighting. I think that the most consistent solution would be to change the ramping to affect all lighting types (diffuse, specular, ambient), and each of those only if the relevant variable (tot_diffuse, tot_specular, tot_ambient) is defined (which is easy to track in the code that generates the definitions).

This would make the calculations for all lighting types consistent, and notably, quantization of specular highlights would be very useful for rendering anime style hair. Currently, if light ramping is enabled, having specular highlights in the materials (in my opinion) breaks the cartoon look.

I’m basically posting to ask, what do others (especially more experienced members of the community) think? Is this the way to fix the error, or should it be fixed in some other way?

And how difficult is it to compile the Panda source in Linux? I think the required modifications are pretty trivial, if I can just get a working development tree set up :slight_smile:

Thanks for the detective work! It is clear from the code that LightRampAttrib is supposed to only affect the diffuse component. We can’t change this to affect other components without breaking existing code that relies on the old behaviour. If you would like it to affect all components, then we should make that a separate feature; ie. a setDiffuseOnly(False) flag or something akin to that.

Once in a while, we find these little corner cases that the ShaderGenerator author simply overlooked. This is one of them. The proper fix is probably to just replace the first occurrence of “if(_auto_ramp_on)” with “if (_auto_ramp_on && _have_diffuse)”.

The most tricky part about compiling Panda3D for Linux tends to be installing all of the thirdparty dependencies, which isn’t really hard if you know what to look for.

Np. Trying to use Panda3D in a hobby project, and happened to get this error. :slight_smile:

Ah, true! Good point on backward compatibility.

I won’t promise anything, but if my free time allows, I might be interested in experimenting with adding this. At least specular quantization would be useful as an option.

About ambient quantization, on second thought I’m less sure, and will really have to test. Currently the ambient light level is a nice tuning parameter for adjusting the cartoon look. If it was quantized, it might actually become less useful. But maybe as a separate option.

I also have some other ideas for the cartoon shader that I’d like to try:

  • Short gradients between the shades for an antialiased look. Currently, the edges between light and shadow look very sharp because of the strict quantization. A simple solution to make the edges smoother would be to exploit the fact that the original lighting values are usually continuous. Hence, if we add a linear interpolation (in brightness space) transitioning the light level over the threshold value(s), the edges between light and dark should become less pixellated, while retaining the overall quantized look. This should be easy to implement (but if the length of the gradient needs to be customizable, then I need to learn a bit about how to add new parameters to the LightRampAttrib API).

  • Tut-Cartoon-Advanced has a cutoff parameter that would be nice to have in the CommonFilters version. This should only require integrating the custom cutoff code from the sample program into the existing CommonFilters infra, so it shouldn’t be too hard (and should be a good chance to learn).

  • The cartoon ink filter could be improved to take into account the distance from the camera when drawing the edges, in order to keep the perceived line thickness (approximately) constant. This would be especially useful in settings where the camera distance changes dynamically. The first idea that comes to mind is to use z-buffer data to affect the separation value. It seems the “depth” texture (used by the ssao filter) already contains this data.

Ok. I will try this.

Ok… and done.

Not exactly new to this game, but setting up a dev tree always takes a few hours :stuck_out_tongue:

Running “python makepanda/ --everything --verbose” gave a pretty good idea what to install, and the manual: … cense_info
helped with identifying the more exotic ones. Installing and using apt-file (as you suggested at Compiling on Kubuntu 12.04 ) to find the correct packages was also a good idea.

Just in case anyone else wants to set up a Panda3D dev tree in Ubuntu 12.04 (Precise Pangolin), at least the following packages are needed:

libeigen3-dev libavformat-dev libavcodec-dev libavutil-dev libswscale-dev libfreetype6-dev libgles1-mesa-dev libgles2-mesa-dev libegl1-mesa-dev libosmesa6-dev libode-dev libopenal-dev libopencv-dev libcv-dev libhighgui-dev libtiff4-dev libgtk2.0-dev libjpeg-dev libpng-dev libxrandr-dev libxxf86dga-dev libxcursor-dev bison flex libboost-python-dev

And since it’s not in the repos, get libRocket from here (thanks to JörnS!):

As for Bullet, as was explained by enn0x here:

the version of Bullet needs to be 2.80. The newer 2.81 won’t do, because the interface has changed. Bullet 2.80 has not been packaged for precise (12.04), but it seems possible to use the packages for quantal (12.10), available here: … bullet2.80 … bullet-dev
Just download and sudo dpkg -i these two.

After installing all that, “python makepanda/ --everything --nofftw --verbose --installer” should complete without errors. (There was a problem linking pview when fftw was enabled, and since I don’t really need this library, I just chose to disable it.)

I installed the generated .deb (first uninstalling the downloaded official one) and at least the sample programs in /usr/share/panda3d run fine.

On a final side note, it seems that the automatic dependency checking in makepanda in 1.8.1 won’t catch if libcv-dev (cxcore.h) and libhighgui-dev (highgui.h) are missing; installing just libopencv-dev makes the dependency check pass, but the compile will fail midway unless also libcv-dev and libhighgui-dev are installed. Maybe cxcore.h and highgui.h should be added to the dependency checker (unless already there in the latest source)?

Now, to find some more time to do some coding :slight_smile:

Tested. Sure enough, changing



    if(_auto_ramp_on && (  (_separate_ambient_diffuse && _have_diffuse) ||
                          (!_separate_ambient_diffuse && (_have_ambient
                                                       || _have_diffuse)) ) ) {

(to match the conditions when “tot_diffuse” is defined; I’m sure some logical simplification could be applied) fixes the error.

This issue is now in the bug tracker:

along with a minimal patch that fixes the problem, along the lines of what rdb suggested here.

Talking to myself to keep related things linked…

The development ideas have been tested, and there is a new thread about them at: