Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
EXT_mesh_gpu_instancing #1691
EXT_mesh_gpu_instancing #1691
Changes from 1 commit
76587d9
de031ed
5d2dac5
ed5f6e6
a68dcf6
82b3adb
1b4f9c3
956b01d
62cf80a
2ed4903
a833742
8b8e39a
66fa030
332af44
133d28d
3857fa7
d53d9c8
b4854e9
ea4c776
c70f30e
615652a
8d8f4a0
eef3c13
900ac06
5984aa1
dbd7b63
e0a458b
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One option we should consider is enabling the KHR_instancing extension to (optionally) override the
node.mesh
property. For example:This simply provides exporters with some flexibility to control how/if fallback happens for viewers that don't recognize the extension:
mesh
override is omitted, clients that don't recognize the extension will render a single instance of the mesh with the node's transform.mesh
override is provided, it should reference a single instance of the mesh. The fallbacknode.mesh
value could then point to anything the exporter chooses – a merged mesh of all instances, a single point, or a textured plane saying "404 Not Found".KHR_instancing
extension is marked as required, clients that don't recognize the extension are expected to fail fast without attempting to proceed.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The spec should mention in which coordinate space those transforms are defined. Reasonable options are
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're only using local space at otoy. Maybe that could be handled by alternate 'TRANSFORM' attributes that include the space? i.e. TRANSFORM_LOCAL, TRANSFORM_SCENE, with maybe TRANSFORM defaulting to LOCAL?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TRANSFORM_SCENE
behavior can be achieved with a node positioned at the scene root. Supporting only local (with explicitly described interaction between JSON-stored local TRS and accessor-stored instanced TRS) seems enough to cover both cases from engine perspective.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these IDs implementation-dependent, with no spec-defined usage? If so, they shouldn't be included here. glTF 2.0 has an established workflow for supporting app-specific attribute semantics - they just start with an underscore.
Otherwise, the spec should be more sound and include at least:
In shaders, GPU instances have built-in IDs. Should this attribute be somehow related to them?