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.

Sign up for GitHub

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

Jump to bottom

Negative transformation scale #1697

Closed
JoshKlint opened this issue Oct 28, 2019 · 7 comments
Closed

Negative transformation scale #1697

JoshKlint opened this issue Oct 28, 2019 · 7 comments

Comments

@JoshKlint
Copy link

JoshKlint commented Oct 28, 2019 •
edited
Loading

https://github.com/KhronosGroup/glTF/tree/master/specification/2.0#transformations

Implementation Note: If the determinant of the transform is a negative value, the winding order of the mesh triangle faces should be reversed. This supports negative scales for mirroring geometry.

According to what I am reading, the indice order should be reversed if a node has a negative scale in its transformation. This makes little sense, as meshes can potentially be shared across different nodes, some of which may be reversed and some not. Face direction should be a per-mesh property, not a per-node property. Am I misunderstanding this?
@JoshKlint
Copy link
Author

JoshKlint commented Oct 29, 2019 •
edited
Loading

I think I get it. At some point someone was asked "what happens if the scale inverts the faces" and this was the answer. It is true that you could save some VRAM if you had a separate mesh object that included the triangle order and a "mesh data" object that was shared between meshes that contains the vertex arrays, and thus instance the mesh with two different winding orders, but it seems like a really obscure optimization.

I think this decision is covering one mistake with another one.

@donmccurdy
Copy link
Contributor

I think this implementation note should be removed. Negative scales will invert single-sided faces, and that's OK.

@bghgary
Copy link
Contributor

bghgary commented Oct 29, 2019

This implementation note is there to support mirrored geometry. Both Unity and Unreal do this.

For the history, see #971 (comment) and the corresponding PR #985.

@JoshKlint
Copy link
Author

Well, at least the correct behavior is explicitly stated. Thanks for the info.

@JoshKlint
Copy link
Author

JoshKlint commented Oct 30, 2019 •
edited
Loading

I think this implementation note should be removed. Negative scales will invert single-sided faces, and that's OK.

Now that it's in use, this is actually critical to follow. I have encountered GLTF files that use a negative scale on one axis and if you don't follow this rule the faces will be inverted from what you would expect.

@bghgary
Copy link
Contributor

bghgary commented Oct 30, 2019 •
edited
Loading

See https://github.com/KhronosGroup/glTF-Asset-Generator/blob/master/Output/Positive/Node_NegativeScale/README.md for a set of test cases if you are implementing a loader.

@donmccurdy
Copy link
Contributor

I see, thanks – looks like this was added to three.js as well, in r89, and I'd missed it. mrdoob/three.js#12787.

@JoshKlint to your original question...

... meshes can potentially be shared across different nodes, some of which may be reversed and some not. Face direction should be a per-mesh property, not a per-node property. Am I misunderstanding this?

...the same mesh can be can be reused, uploaded to the GPU once, and rendered with different states of gl.frontFace for example. So reversed face direction does not really interfere with reuse of the mesh in practice.

@donmccurdy donmccurdy mentioned this issue Nov 8, 2019
Merged
@lyuma lyuma mentioned this issue Oct 11, 2021
Open
@robtfm robtfm mentioned this issue Jun 16, 2023
Merged
github-merge-queue bot pushed a commit to bevyengine/bevy that referenced this issue Sep 18, 2023
@robtfm @mockersf
invert face culling for negatively scaled gltf nodes (#8859)
28060f3 
# Objective

according to
[khronos](KhronosGroup/glTF#1697), gltf nodes
with inverted scales should invert the winding order of the mesh data.
this is to allow negative scale to be used for mirrored geometry.

## Solution

in the gltf loader, create a separate material with `cull_mode` set to
`Face::Front` when the node scale is negative.

note/alternatives:
this applies for nodes where the scale is negative at gltf import time.
that seems like enough for the mentioned use case of mirrored geometry.
it doesn't help when scales dynamically go negative at runtime, but you
can always set double sided in that case.

i don't think there's any practical difference between using front-face
culling and setting a clockwise winding order explicitly, but winding
order is supported by wgpu so we could add the field to
StandardMaterial/StandardMaterialKey and set it directly on the pipeline
descriptor if there's a reason to. it wouldn't help with dynamic scale
adjustments anyway, and would still require a separate material.

fixes #4738, probably fixes #7901.

---------

Co-authored-by: François <[email protected]>
rdrpenguin04 pushed a commit to rdrpenguin04/bevy that referenced this issue Jan 9, 2024
@robtfm @mockersf
invert face culling for negatively scaled gltf nodes (bevyengine#8859)
628661d 
# Objective

according to
[khronos](KhronosGroup/glTF#1697), gltf nodes
with inverted scales should invert the winding order of the mesh data.
this is to allow negative scale to be used for mirrored geometry.

## Solution

in the gltf loader, create a separate material with `cull_mode` set to
`Face::Front` when the node scale is negative.

note/alternatives:
this applies for nodes where the scale is negative at gltf import time.
that seems like enough for the mentioned use case of mirrored geometry.
it doesn't help when scales dynamically go negative at runtime, but you
can always set double sided in that case.

i don't think there's any practical difference between using front-face
culling and setting a clockwise winding order explicitly, but winding
order is supported by wgpu so we could add the field to
StandardMaterial/StandardMaterialKey and set it directly on the pipeline
descriptor if there's a reason to. it wouldn't help with dynamic scale
adjustments anyway, and would still require a separate material.

fixes bevyengine#4738, probably fixes bevyengine#7901.

---------

Co-authored-by: François <[email protected]>
dekirisu pushed a commit to dekirisu/bevy_gltf_trait that referenced this issue Jul 7, 2024
@robtfm @mockersf
invert face culling for negatively scaled gltf nodes (#8859)
f7cc7ae 
# Objective

according to
[khronos](KhronosGroup/glTF#1697), gltf nodes
with inverted scales should invert the winding order of the mesh data.
this is to allow negative scale to be used for mirrored geometry.

## Solution

in the gltf loader, create a separate material with `cull_mode` set to
`Face::Front` when the node scale is negative.

note/alternatives:
this applies for nodes where the scale is negative at gltf import time.
that seems like enough for the mentioned use case of mirrored geometry.
it doesn't help when scales dynamically go negative at runtime, but you
can always set double sided in that case.

i don't think there's any practical difference between using front-face
culling and setting a clockwise winding order explicitly, but winding
order is supported by wgpu so we could add the field to
StandardMaterial/StandardMaterialKey and set it directly on the pipeline
descriptor if there's a reason to. it wouldn't help with dynamic scale
adjustments anyway, and would still require a separate material.

fixes #4738, probably fixes #7901.

---------

Co-authored-by: François <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@donmccurdy @JoshKlint @bghgary