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

BatchedMesh, InstancedMesh: add support for negative scaled, back side rendering #30342

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

Conversation

gkjohnson
Copy link
Collaborator

Related issue: #30327

Description

Multiplies the normal by the sign of the determinant of the instance / batch matrix so the normal is facing the right direction in cases where the matrix has a negative scale.

Unfortunately I don't think there's a lot that can be done in terms of automatically fixing the triangle winding order when using a negatively-scaled matrix but this lets the user use double sided rendering if desired.

@gkjohnson gkjohnson requested a review from WestLangley January 17, 2025 03:08
@gkjohnson gkjohnson added this to the r173 milestone Jan 17, 2025
Copy link

📦 Bundle size

Full ESM build, minified and gzipped.

Before After Diff
WebGL 339.53
79.09
339.58
79.11
+56 B
+20 B
WebGPU 493.59
137.22
493.59
137.22
+0 B
+0 B
WebGPU Nodes 493.05
137.12
493.05
137.12
+0 B
+0 B

🌳 Bundle size after tree-shaking

Minimal build including a renderer, camera, empty scene, and dependencies.

Before After Diff
WebGL 465.41
112.16
465.46
112.17
+56 B
+14 B
WebGPU 564.12
152.88
564.12
152.88
+0 B
+0 B
WebGPU Nodes 519.5
142.46
519.5
142.46
+0 B
+0 B

@WestLangley
Copy link
Collaborator

WestLangley commented Jan 17, 2025

I am inclined to think that we should not attempt to support mixing positive and negatively-scaled instance/batch matrices in the same draw call -- unless the implementation works in every use case.

Instead, just say negative scales in the instance/batch matrix are not supported.

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