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

Shaders: Add support for transforming instanced mesh tangents #27128

Merged
merged 2 commits into from
Nov 7, 2023

Conversation

gkjohnson
Copy link
Collaborator

@gkjohnson gkjohnson commented Nov 6, 2023

Fix #27116

Description

  • Pulls the "transformedTangent" variable to the beginning of the shader block.
  • Ensure the batched geometry block modifies "transformedNormal" and "transformedTangent" rather than the "objectNormal" and "objectTangent" variants.
  • Ensure instanced geometry correct transform tangents.

@gkjohnson gkjohnson added this to the r159 milestone Nov 6, 2023
Copy link

github-actions bot commented Nov 6, 2023

📦 Bundle size

Full ESM build, minified and gzipped.

Filesize dev Filesize PR Diff
655 kB (162.2 kB) 655.1 kB (162.2 kB) +83 B

🌳 Bundle size after tree-shaking

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

Filesize dev Filesize PR Diff
447.6 kB (108.2 kB) 447.7 kB (108.2 kB) +83 B

@gkjohnson gkjohnson requested a review from Mugen87 November 6, 2023 09:43
Copy link
Collaborator

@Mugen87 Mugen87 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@WestLangley We are doing the right thing here, correct 😇 ? Tangents should be transformed by instancing/batching similar to normals.

@WestLangley
Copy link
Collaborator

@WestLangley We are doing the right thing here, correct 😇 ? Tangents should be transformed by instancing/batching similar to normals.

I don't think so. Transformed normals should remain orthogonal to the plane of the face; transformed tangents should remain within the plane of the face.

@gkjohnson
Copy link
Collaborator Author

gkjohnson commented Nov 7, 2023

I don't think so. Transformed normals should remain orthogonal to the plane of the face; transformed tangents should remain within the plane of the face.

Oops you're right 😅 The tangents still need to have the instance and batching matrices applied, though. I've updated the PR to remove the additional scaling logic only needed for normals.

@Mugen87 Mugen87 merged commit fa8539d into mrdoob:dev Nov 7, 2023
19 checks passed
@gkjohnson gkjohnson deleted the instanced-tangents-fixed branch November 7, 2023 09:33
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.

InstancedMesh: Tangents are not transformed by instance matrices
3 participants