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

Make ShaderBall meshes double sided #95

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

cpichard
Copy link

When the doubleSided parameter is not set on the mesh, the mesh is by default single sided.
This PR ensure all the meshes in the StandardShaderBall are double sided, allowing renderers supporting sidedness to correctly render materials like glass.

@crydalch
Copy link
Contributor

Thanks for the PR @cpichard and sorry for the delayed response! This triggered some questions on our end, about the doubleSided property. There seems to be some reason to think it's not a hard-requirement, but just a suggestion? Karma interpreted the property that way, so it draws backfaces by default (i.e. the glass examples just work, as shown in the renders).

Anyway, it's spawned some internal investigations, and I think we'll be reaching out to the USD folks on this soon, just to get some clarity on the meaning/intent here. But I don't see a problem having this on by default. Thanks!

@cpichard
Copy link
Author

Hi @crydalch , thanks for the follow up !
We had the same questions internally as arnold-usd is using the default for sidedness which is single sided, and the shader ball isn't rendering properly in that case.
In a way it makes sense to make the reference shaderball geometry double sided for renderers that use the usd sideness default, as this is the original intent for physically based renderer.
Another solution, and the one we would also prefer, would be to have officially the doubleSided property true by default, this is something we look forward discussing with the USD group, let us know if we can team up on that.

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.

2 participants