Skip to content

Conversation

@willeastcott
Copy link
Contributor

@willeastcott willeastcott commented Jan 3, 2026

Fixes #8328

image

Description

When a glTF node uses a matrix property (instead of separate translation, rotation, scale properties) that contains a negative scale component (creating a reflection/flip), the model was displayed incorrectly - appearing flipped or upside down.

Changes

  • Use Quat.setFromMat4() for rotation extraction instead of Mat4.getEulerAngles() when decomposing node matrices
  • Apply scaleSign to the X scale component to preserve negative determinant (mirrored) transforms

Technical Details

The root cause was that Mat4.getScale() returns only positive values (it computes vector lengths), and Mat4.getEulerAngles() doesn't coordinate with negative determinant handling. This could cause the flip to be absorbed into the rotation as an extra 180° rotation, or be lost entirely.

The fix uses Quat.setFromMat4() which already properly handles negative determinant matrices by normalizing the X axis before rotation extraction (see lines 589-597 in quat.js). The negative scale is then preserved by multiplying the X scale by Mat4.scaleSign, which returns -1 for mirrored matrices.

This approach is consistent with how the engine handles mirrored transforms elsewhere (e.g., batch manager, GraphNode._worldScaleSign).

Checklist

  • I have read the contributing guidelines
  • My code follows the project's coding standards
  • This PR focuses on a single change

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes incorrect rendering of glTF models when nodes use a matrix property containing negative scale components (reflections/flips). The issue was that the previous matrix decomposition approach using Mat4.getEulerAngles() didn't properly handle negative determinant matrices, causing models to appear flipped or inverted.

Key changes:

  • Switched from Euler angle extraction to quaternion-based rotation extraction for better handling of mirrored transforms
  • Applied scale sign correction to preserve negative determinant (mirrored) matrices

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@willeastcott
Copy link
Contributor Author

@mvaligursky I know this already has 1 approval but I think it's best if I get your approval too since you commented on the original issue. Let me know if you think this fix is correct.

@willeastcott willeastcott merged commit 9916af8 into main Jan 6, 2026
13 checks passed
@willeastcott willeastcott deleted the glb-mirror branch January 6, 2026 10:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Node with transform doesn't scale properly

4 participants