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

Add support for double-sided materials #491

Merged
merged 10 commits into from
Sep 2, 2024
Merged

Add support for double-sided materials #491

merged 10 commits into from
Sep 2, 2024

Conversation

kring
Copy link
Member

@kring kring commented Aug 21, 2024

As reported here:
https://community.cesium.com/t/3d-tiles-model-appeared-with-face-orientation-flipped-backface-culling/34155

We previously weren't supporting glTF's doubleSided property. Now we are.

This was tricky because Unity controls this with a render-pipeline-dependent material property. I also had to add some nodes to our CesiumDefaultTilesetShader to flip the direction of the normal for back faces, as required by the glTF spec.

This is a draft because it's not quite working well in HDRP yet. For reasons I don't understand, HDRP defies all attempts to dynamically turn on double-sided rendering dynamically. I have some local changes where I turn it on via the UI, and then I'm able to turn it off dynamically. But I haven't committed that yet because it seems dodgy. Also, unlike the othe render pipelines, HDRP seems to automatically invert normals for back faces, which means my nodes that do that manually are now making the normals wrong again. I'm not sure yet the best way to deal with that.

Fixes #370
Fixes #486

It's working well in URP and the built-in pipeline, but HDRP is more
difficult.
@j9liu j9liu linked an issue Aug 21, 2024 that may be closed by this pull request
@kring
Copy link
Member Author

kring commented Aug 29, 2024

I had hoped to get this PR ready for final review so it can be included in next week's release, but HDRP and I still are not getting along. Will probably need to push it until next release.

@kring
Copy link
Member Author

kring commented Aug 29, 2024

Ok I think this is working well now. Double-sided rendering is working in all pipelines. The way it works is that the Shader and Material now have double-sided mode enabled. At runtime, we disable it if the glTF does not require it by setting some material parameters. This seems to work a lot better than the opposite. I'm not 100% sure why, but my theory is that double-sided rendering is a "feature" (at least in some RPs). If the shaders are compiled with support for that feature, then dynamically disabling it at runtime is fine. However, if the shaders are compiled without support for the feature at all, then enabling it dynamically is not possible.

I also fixed cartographic polygons in HDRP by enabling "alpha clipping".

@kring kring marked this pull request as ready for review August 29, 2024 23:57
@kring
Copy link
Member Author

kring commented Aug 29, 2024

I'm moving this into next week's release, but time is short so we'll move it back out if there are any concerns at all.

@kring kring added this to the September 2024 Release milestone Aug 29, 2024
@j9liu
Copy link
Contributor

j9liu commented Aug 30, 2024

Thanks @kring ! I verified that this was working (at least in URP) using the TextureSettingsTest sample model.

Just to sanity check, I noticed some seemingly unrelated edits to the material? Some of them seemed like autogenerated additions, but I wasn't sure about this one. Was this done by Unity too?

@kring
Copy link
Member Author

kring commented Sep 1, 2024

@j9liu I didn't purposely change that, so I can only imagine Unity change it itself for some reason. I reverted it back to "Auto" though, and everything still seems to be working well, so I've pushed up that change.

@kring
Copy link
Member Author

kring commented Sep 2, 2024

Merging this one for tomorrow's release, too.

@kring kring merged commit bfb6599 into main Sep 2, 2024
7 checks passed
@kring kring deleted the double-sided branch September 2, 2024 07:08
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.

CesiumPolygonRasterOverlay Cliping can not work when use HDRP Support double-sided materials
2 participants