WebGLShadowMap: Support material.map with alphaTest

[WestLangley]

Fixes #24995.

[WestLangley]

@hybridherbst Can you please give it a try?

[hybridherbst]

Looking good, thank you!

Here's a transparent video being used as map:
https://user-images.githubusercontent.com/2693840/203875738-9fa6b9f5-83d5-4475-8919-4a02ec5bd320.mp4

[titansoftime]

Hey guys! It was recommended that I post this here from the forums.

https://discourse.threejs.org/t/r147-instancing-with-shadows/45423

It seems this PR has affected shadows of meshes that are created using instancing. Up through r146 for example, the below pattern worked:

mesh.customDepthMaterial = new THREE.MeshDepthMaterial( {

	depthPacking: THREE.RGBADepthPacking,
	map: map,
	alphaTest: 0.5

} );

mesh.customDepthMaterial.onBeforeCompile = shader => {

	shader.vertexShader = '#define DEPTH_PACKING 3201'+"\n"+MY_SHADERCHUNKS.instance_pars_vertex + "\n" + shader.vertexShader;

	shader.vertexShader = shader.vertexShader.replace('#include <begin_vertex>','#include <begin_vertex>'+MY_SHADERCHUNKS.instance_vertex);

	shader.fragmentShader = '#define DEPTH_PACKING 3201'+"\n" + shader.fragmentShader;

};

Now that depth material is ignored, this pattern has broke. What is the preferred method now to apply shadows to instances? If I comment out this PR, it goes back to working, but I prefer not to modify the base THREE code.

[WestLangley]

@titansoftime

I assume the issue is your custom depth material is not being honored.

I do not expect this PR is the cause of the problem you are facing. I think there is a more fundamental problem.

Please file a new PR and provide sufficient information to replicate the issue. A simple, live example would be helpful.

Also, please explain why a custom depth material is required in your use case.

[titansoftime]

@WestLangley

With due respect, I assure you this PR is causing customDepthMaterial to not be honored in this case. Reverting this specific PR resolves the issue and I will explain why.

Here result gets set to CustomDepthMaterial if exist:

const customMaterial = ( light.isPointLight === true ) ? object.customDistanceMaterial : object.customDepthMaterial;

if ( customMaterial !== undefined ) {

	result = customMaterial;

} else {

	result = ( light.isPointLight === true ) ? _distanceMaterial : _depthMaterial;

}

Overwriting result in the below following code block use to be skipped for objects with alphaTest with no AlphaMap as this was part of the condition:

( material.displacementMap && material.displacementScale !== 0 ) ||			
( material.alphaMap && material.alphaTest > 0 )  {

This did not meet conditions of my instanced objects with alphaTest and no alphaMap.

Now, with the new conditions from this PR, result set from customDepthMaterial gets overwritten and is no longer honored.

( material.displacementMap && material.displacementScale !== 0 ) ||			
( material.alphaMap && material.alphaTest > 0 ) ||
( material.map && material.alphaTest > 0 ) ) // ## Removing this solves as result does not get overwritten.
 {

		result = ...

I will work on an example, however setting one up with shadows and instancing will take me several hours.

Thank you for taking the time to look into this.

[WestLangley]

With due respect, I assure you this PR is causing customDepthMaterial to not be honored in this case.

There is nothing inherently wrong with this PR. There is a more fundamental problem that is causing a user's customDepthMaterial to be overwritten by a "variant".

three.js no longer has any examples requiring a customDepthMaterial, so this issue went unnoticed.

[titansoftime]

I showed where the overwrite happened and it is because the change in the conditional from this PR allows the result variable to be overwritten after being assigned from the customDepthMaterial.

Reverting only this PR which definitely overwrites the customDepthMaterial value result value brings back the prevoius functional behavior.

I will make a demo this week. If the plan is to ignore this and allow customDepthMaterial to be deprecated in these cases, PLEASE let me know so I do not waste my time.

[hybridherbst]

He didn't say this will go unnoticed, he just mentioned it's better to open a new issue and reference this PR.
This PR did not cause that issue, it uncovered it. Reverting it does not fix the underlying issue, just your particular case wasn't affected.

[titansoftime]

He didn't say this will go unnoticed, he just mentioned it's better to open a new issue and reference this PR. This PR did not cause that issue, it uncovered it. Reverting it does not fix the underlying issue, just your particular case wasn't affected.

I was asking "if", since this PR knowingly overwrites the depthMaterial in a way that may show possible deprecation. I have a lot to do and wanted to make sure my time is well spent. I was not meaning to be rude if that's the cause of your reply.

I am working to demonstrate this issue. It will just take some time.

[WestLangley]

@titansoftime If you have a valid use case that requires customDepthMaterial, I'm definitely interested in seeing it, so your efforts to create a live example are greatly appreciated. :-)

[titansoftime]

Sure. In the meantime, this was another user's use case (same thing, shadows on instanced meshes).

This thread points to now removed examples. I'm assuming the lambert material instance example they are referring to was the only instance example showing how to shadow instances. https://threejs.org/examples/?q=ins#webgl_buffergeometry_instancing_lambert

https://discourse.threejs.org/t/shadow-for-instances/7947

[titansoftime]

A live example of 147 not working with instanced shadows with alphaTest set on the material.

https://jsfiddle.net/titansoftime/q39foeLb/

You can switch between the live 147 (not working as neeeded) and 146 version of three.js. By swapping the sources at the top.

You can also toggle instancing via the instancing variable.

Here is it defaulted to 146:
https://jsfiddle.net/titansoftime/hncbvgf8/

[WestLangley]

@titansoftime The following works for me for your test case, but I would request @Mugen87 to confirm. I think if an app sets a customDepthMaterial, then the app should also be responsible for updating the uniforms.

+++ b/src/renderers/webgl/WebGLShadowMap.js
@@ -235,6 +235,7 @@ function WebGLShadowMap( _renderer, _objects, _capabilities ) {
                if ( customMaterial !== undefined ) {
 
                        result = customMaterial;
+                       return result;
 
                } else {
 

[titansoftime]

Your code solution if I understand it correctly seems reasonable to me, but I am unsure the far reaching consequences. If a customDepthMaterial is explicitly set, use it and do not override.

Preferably, shadows on meshes created via instancing would work like regular shadows in userland and do the magic behind the scenes without a weird customDepthMaterial. I don't even really know for sure what that even is, let alone updating related uniforms.

For now I will patch out this PR until the council of three has decided a permanent solution.