Material: Added callback to identify customized programs

[Oletus]

In case onBeforeCompile has conditional statements in it, the parameter values affecting the shader code can be returned from customProgramCode so the shader program is correctly fetched from cache or recompiled as needed.

Fixed #19377.

[Oletus]

We've found this API to be very useful for more complex customizations of materials, though we're not quite sure of the naming - maybe "programHash" would be more understandable than "programCode"?

Also suggestions on how to make an example for this would be welcome. Maybe we could change the existing webgl_materials_modified example so that it switches between two different materials using the same onBeforeCompile function but with different parameters?

[pboyer]

This PR provides a solution to a major gotcha when extending materials with onBeforeCompile. This gotcha is noted in one of the top Google search results on using this API (from @pailhead):

No matter what you put in your userData.customChunk the WebGLRenderer would not care, it would see the same material and retreive some instance from the cache (most likely the first one it compiled). It calls onBeforeCompile.toString() for hashing, and all it would ever get is the same generic body :(

https://medium.com/@pailhead011/extending-three-js-materials-with-glsl-78ea7bbb9270

[pailhead]

@pboyer

Related:

#13192

Unfortunately (IMO), it hasn't been accepted as a bug, but rather as design.

[pailhead]

@Oletus

Related PRs for modified webgl material examples:

Chunk replacements. You can provide a chunk replacement to be consumed by the material when the time comes. Ie. you're not bound to modify the materials when render() encounters them, you just fire and forget. As soon as you say myMaterial.chunks.myChunk = myChunk you're done, you don't care when compilation happens.

#14206

onBeforeCompile example PR:

#14166

And another onBeforeCompile example:

#14174

[Oletus]

@Mugen87 @mrdoob Can I get feedback on this change? Do you have some other direction in mind for customizing shader programs?

[Mugen87]

I've struggled a bit with the name of customProgramCode () however I can't think of a better one. (maybe onCustomProgramCode() but only if @mrdoob wants to retain the on* syntax for such hooks).

[Oletus]

I think "customProgramHash" or "customShaderHash" might be more understandable, even if an actual hash algorithm isn't necessarily involved. Or maybe "customProgramCacheKey"?

If we do change it I'd like to go and change the core to also use "programHash" or "programCacheKey" instead of "programCode" for consistency. "programCode" can be a bit confusing since it sounds like it might refer to the shader source code rather than a key used for caching the compiled program.

[Mugen87]

"programCode" can be a bit confusing since it sounds like it might refer to the shader source code rather than a key used for caching the compiled program.

True. Related #16674

[Oletus]

Updated the naming since #17748 was merged, and added documentation and an example. @Mugen87 @mrdoob could this be merged?

[Mugen87]

I suggest to undo the changes in webgl_materials_modified in order to keep it as simple as possible. Your code example in the documentation seems sufficient to me.

[pailhead]

Is there any chance that #14231 could be revisited?

IIRC the biggest pro of onBeforeCompile was that it was only 3 lines of code (although, truth be told we're at 6 now :) ) But if we are fixing #13192 (or adding the feature) onBeforeCompile is slowly growing.

I was never convinced that these kind of operations need to be called asynchronously in the onBeforeCompile.

It seems like onBeforeCompile will break cloning even further if it introduces more callbacks that aren't cloned?

[Oletus]

Force-pushed to remove the commit that contained the example changes. @mrdoob this should be ready to merge! :)

[Oletus]

Rebased this patch on top of the latest dev. @mrdoob do you think that this patch is ok? @Mugen87 approved the earlier version quite a while ago but not sure if you noticed it.

[mrdoob]

In case onBeforeCompile has conditional statements in it, the parameter values affecting the shader code can be returned from customProgramCode so the shader program is correctly fetched from cache or recompiled as needed.

Do you mind elaborating a bit? I'm having a hard time to understand what the problem is.
The example being added to the documentation also doesn't seem easy to understand 🤔

[pailhead]

@mrdoob

I believe it's describing this issue:
#13192

There's a jsfiddle there. In short, given N materials with the same onBeforeCompile but with conditional logic in there, like a switch (condition) {} you won't get N different materials to render, but one, and arbitrary at that.

[Oletus]

@mrdoob @pailhead described the issue fairly well. Without this change three.js is only able to cache one material shader per unique onBeforeCompile callback. With this change several material shaders can be cached in case they have the same onBeforeCompile callback but that callback is run with different parameters (from a closure for example) for each material.

Earlier this PR included an example but I was asked to remove that. Should I reintroduce the example code to make this easier to understand?

[DusanBosnjakKodiak]

described the issue fairly well.

It's not an issue unfortunately, but a feature, by design. So to be more specific, the request is can we change the behavior of onBeforeCompile to be deterministic.

Earlier this PR included an example but I was asked to remove that. Should I reintroduce the example code to make this easier to understand?

It is also something that might be unnecessary since NodeMaterial is pretty far along. I think you no longer need to bother with onBeforeCompile and GLSL, you can just make nodes and hook them up and have them compile to GLSL on every user's machine.

I fought this battle for years, don't take it to personally if the PR is not merged despite being approved :)

(btw it's me, @pailhead, just lazy to log in)

[mrdoob]

Thanks!

[pboyer]

Woohoo!

[titansoftime]

THANK YOU! I spent 7 hours straight trying to debug my code and eventually realized it was related to three.js material cache. So happy to find this.