Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

webgl_shadowmap_pointlight: customDistanceMaterial is no longer required #25091

Merged
merged 1 commit into from
Dec 7, 2022

Conversation

WestLangley
Copy link
Collaborator

@WestLangley WestLangley commented Dec 7, 2022

Related: #25000

What would be nice is an example where a custom distance material, or depth material, is required.

@WestLangley WestLangley marked this pull request as ready for review December 7, 2022 01:46
@WestLangley WestLangley added this to the r148 milestone Dec 7, 2022
@mrdoob mrdoob merged commit ae6f291 into mrdoob:dev Dec 7, 2022
@mrdoob
Copy link
Owner

mrdoob commented Dec 7, 2022

What would be nice is an example where a custom distance material, or depth material, is required.

If you can you think of and describe an example that would need a custom distance material the community can help creating it.

Repository owner deleted a comment from linxi6 Dec 7, 2022
@WestLangley
Copy link
Collaborator Author

I am having a hard time thinking of one... We now handle the common use cases automatically. :-)

@mrdoob
Copy link
Owner

mrdoob commented Dec 7, 2022

@titansoftime any ideas?

@WestLangley WestLangley deleted the dev-custom_distance_material branch December 7, 2022 06:10
@titansoftime
Copy link
Contributor

titansoftime commented Dec 7, 2022

@titansoftime any ideas?

I did not think shadows on instanced meshes with materials with alphaTest would be so niche =[

If you can show me an alternate method to achieve this without customDepthMaterial, I would be more than happy to implement.

Here is how your examples showed how to do it previously:

Working r146: https://jsfiddle.net/titansoftime/hncbvgf8/

Not working r147: https://jsfiddle.net/titansoftime/q39foeLb/

This comment shows another use case from the forums:
#25000 (comment)

@WestLangley
Copy link
Collaborator Author

@titansoftime What is the reason you are you not using InstancedMesh?

@titansoftime
Copy link
Contributor

I built my solution prior to the release of InstancedMesh. I have had no need to change my solution as mine works great until r147.

Are shadows using instancing with alphaTest no longer expected to work correctly with instancing unless InstanceMesh is used?

@WestLangley
Copy link
Collaborator Author

@titansoftime Hopefully we can modify r148 to accommodate your use case. But, I also expect InstancedMesh will work in your use case -- unless you are doing something I am unaware of. Would you be willing to try it?

@titansoftime
Copy link
Contributor

@titansoftime Hopefully we can modify r148 to accommodate your use case. But, I also expect InstancedMesh will work in your use case -- unless you are doing something I am unaware of. Would you be willing to try it?

Yes, if you believe that is the correct way to do things in three.js, I will work on a implementing a new instancing workflow.

@titansoftime
Copy link
Contributor

titansoftime commented Dec 13, 2022

I have done a test with InstancedMesh. It works quite well. It is nice not having to deal with customDepthMaterial. In addition my multi-material meshes with one part alphaTest the other not (trees), now have correct shadows and respect the alpha of the foliage. Very nice.

Ideally this would be the same behavior with meshes created manually using instancedGeometry. However, InstancedMesh does simplify my workflow, so this works for me.

I wanted to also mention that this works with CSM as well which I was a little worried about.

@titansoftime
Copy link
Contributor

titansoftime commented Dec 13, 2022

Hmm, I am noticing a rather significant performance decrease though. I have not seen my app drop below 60fps since I got a 2070S some years ago. Now dips to low 50's. I need to investigate more what is causing this while using InstanceMesh vs. modified vertex shaders and having to use customDepthMaterial for shadows.

Edit:
TL;DR: Things are working (and now looking) correctly now I think, but a side effect is more draw calls / polygons from shadows than previous method which yielded odd shadows in some cases.

I think this performance hit is in at least part due to increased draw calls from shadows basically now casting correctly on objects that were created in more complex ways (multiple geometries merged and then using array material (one with alphaTest, the other not, then instanced).

I "believe" the customDepthMaterial method was ultimately making an incorrect shadow from a single material (foliage did not correct), and now a proper shadow is being made but now requires multiple (two in this case) draw calls for the shadow instead of one with customDepthMaterial.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants