'occludeSpecular' : no matching overloaded function found

[kungfooman]

I am still using playcanvas-gltf (or rather a modified version of that), but with the latest rewrites in the engine some shaders fail to compile:

One full error:

Failed to compile fragment shader:
ERROR: 0:486: 'occludeSpecular' : no matching overloaded function found
481:	    dAtten *= getLightDiffuse();
482:	    dDiffuseLight += dAtten * light0_color;
483:	    dSpecularLight += getLightSpecular() * dAtten * light0_color;
484:	
485:	
486:	    occludeSpecular();
487:	
488:	    gl_FragColor.rgb = combineColor();
489:	
490:	    gl_FragColor.rgb += dEmission;
491:	    gl_FragColor.rgb = addFog(gl_FragColor.rgb);

Different glTF models produce different errors:

Now I wonder if backwards-compatibility is abandoned or should the engine still support these shaders?

[yaustar]

Which version of the engine are you using? 1.54.1? What was the last version of the engine that the shader worked on?

[willeastcott]

Looks like this is triggered from adding a height map to a standard material (for parallax mapping)?

[slimbuck]

Hi @kungfooman, sorry about this. I took a quick look at the custom chunks in playcanvas-gltf and I don't think they are the problem. Can you provide a repro so we can investigate further?

[kungfooman]

Can you provide a repro so we can investigate further?

1. Download https://github.com/playcanvas/playcanvas-gltf/archive/refs/heads/main.zip
2. Extract it in web server of choice (lighttpd, Apache2, ...)
3. Open playcanvas-gltf-main/viewer/index.html
4. Change the lines:

        <script src='https://code.playcanvas.com/playcanvas-stable.js'></script>
        <script src='../src/playcanvas-anim.js'></script>
        <script src='../src/playcanvas-gltf.js' defer></script>

to:

        <script type="module">
          import * as pc from "http://127.0.0.1/playcanvas-engine/src/index.js";
          window.pc = pc;
        </script>
        <script src='../src/playcanvas-anim.js'></script>
        <script src='../src/playcanvas-gltf.js' defer></script>

(defer statement in last <script> is important)

Or alternatively just rewrite playcanvas-stable.js to playcanvas-latest.js or to your local build.

If you have DamagedHelmet glTF, that should be enough for testing it.

[slimbuck]

Ahh interesting. A week ago I found and fixed a bug in glb-parser.js which was setting occludeSpecular flag incorrectly here.

Looks like the bug originated in playcanvas-gltf.

In general I wonder how how the chunk system should handle invalid parameters like this...

[slimbuck]

The other issue that @willeastcott mentioned is heightmaps which no longer work, because I broken them. Will address this asap.

[slimbuck]

Fixed in https://github.com/playcanvas/engine/pull/4379

[slimbuck]

Marking fixed.

[kungfooman]

Thank you for the quick resolution, it works perfectly!

In general I wonder how how the chunk system should handle invalid parameters like this...

Since the error originated from options.occludeSpecular supposedly containing a number, but being "true", I think the closest solution is to simply do a simple type-check in standard-material.js in defineValueProp/setterFunc:

// define a simple value property (float, string etc)
const defineValueProp = (prop) => {
    const internalName = `_${prop.name}`;
    const dirtyShaderFunc = prop.dirtyShaderFunc || (() => true);

    const setterFunc = function (value) {
        const oldValue = this[internalName];
        // <newTypeCheck>
        console.log({internalName, oldValue, value});
        if (oldValue !== undefined) {
            if (typeof oldValue !== typeof value) {
                console.log(`Cannot change type of ${internalName} from ${oldValue} to ${value}`);
                return;
            }
        }
        // </newTypeCheck>
        if (oldValue !== value) {
            this._dirtyShader = this._dirtyShader || dirtyShaderFunc(oldValue, value);
            this[internalName] = value;
        }
    };

    definePropInternal(prop.name, () => prop.defaultValue, setterFunc, prop.getterFunc);
};

If then someone tries to set a wrong type, it will show this:

Then you can also go back to simply check if (options.occludeSpecular) because the type/value is "trustable".

I think this makes the code also more future safe after adding more SPECOCC_* enums and the if's don't need to be reconsidered/rewritten.

[mvaligursky]

assigning @GSterbrant as he's currently refactoring the options.

[GSterbrant]

This should've been fixed in #4900, where both the standard material options and the lit shader options have documented types and are also not anonymous classes.

[willeastcott]

So can we close this then?

[kungfooman]

Best is to test the playcanvas-gltf example procedure again.

I couldn't find any "type change checking" in my quick look over the PR at least so far.

[GSterbrant]

The options are documented in both the lit options and standard material options. We don't lock the types so you can still add members, but the members it has should show up in your syntax highlighter. They're also typed, so assigning a bad value should also trigger highlighting.

But perhaps you were thinking more that we should be more strict in asserting the types?

[kungfooman]

playcanvas-gltf e.g. has no typing/Intellisense enabled, but then again, it is also obsolete.

But perhaps you were thinking more that we should be more strict in asserting the types?

Right, my idea was just that once we know a property is a string for example, it shouldn't be allowed to assign a number / bool anymore?

It could be just in a Debug.call(() => { ... });

Right now this is just a could-be-nice-to-have-in-few-situations-that-may-or-may-not-occur for me, otherwise @slimbuck fixed the issue :100: :+1: Right now I don't have such situations luckily :sweat_smile: