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

[Merged by Bors] - expose extras from gltf nodes #2154

Closed
wants to merge 3 commits into from

Conversation

mockersf
Copy link
Member

fixes #2153

expose the extras field value as a string

@mockersf mockersf added A-Assets Load files from disk to use for things like images, models, and sounds C-Enhancement A new feature labels May 12, 2021
@cart
Copy link
Member

cart commented May 17, 2021

Lazy questions:

  • How often are "extras" defined in gltf files
  • Is gltf_node.extras() None when gltf extras aren't defined in the file? I'm assuming the answer is yes. I just want to sanity check that we aren't allocating a bunch of empty strings per-node/entity each time we spawn a GLTF scene.

@NathanSWard
Copy link
Contributor

I just want to sanity check that we aren't allocating a bunch of empty strings per-node/entity each time we spawn a GLTF scene.

Depending on this, we could always put this behind a feature flag (disabled by default) to avoid the extra allocations. 🤷

@mockersf
Copy link
Member Author

  • How often are "extras" defined in gltf files

I don't know, I didn't saw yet in a model I used but it seems a nice place to put some metadata that I put in code until now

  • Is gltf_node.extras() None when gltf extras aren't defined in the file? I'm assuming the answer is yes. I just want to sanity check that we aren't allocating a bunch of empty strings per-node/entity each time we spawn a GLTF scene.

Yup, just checked with FlightHelmet, no extras added

@cart
Copy link
Member

cart commented May 18, 2021

Just checked out a complicated gltf from my game High Hat and extras weren't defined.

@cart
Copy link
Member

cart commented May 18, 2021

It looks like blender doesn't include extras by default. I think validating that other popular 3d packages dont include extras unless the user defines them is probably the "complete" solution here, but I understand that this hard to verify. Can we just "randomly" sample a few free gltf assets from some "model sharing" website and see if they include the field?

@mockersf
Copy link
Member Author

mockersf commented May 18, 2021

I didn't find files with those in my random sample of files on my drive that I collected here and there (kenney's assets and a few other free downloads), and it seems only babylon.js supports this out of the box (but that's easy as json support is native in js...)

On the other hand I don't think it add much code or complexity, but it does add an extra component with a string that the user will have to deserialize... not the nicest UX

Also it's part of the spec: https://github.com/KhronosGroup/glTF/tree/master/specification/2.0#extras

I think Maya can support extras if user defines custom attributes https://doc.babylonjs.com/extensions/Exporters/Maya_to_glTF#custom-attributes

@cart cart added the S-Pre-Relicense This PR was made before Bevy added the Apache license. Cannot be merged or used for other work label Jul 23, 2021
@mockersf mockersf removed the S-Pre-Relicense This PR was made before Bevy added the Apache license. Cannot be merged or used for other work label Jul 24, 2021
@quinnyo
Copy link

quinnyo commented Sep 15, 2021

I would like to see this feature in bevy. I often find situations/projects where I want to be able to specify some metadata in a 3D package (Blender) and have it exported with the scene/model. GLTF extras is a solution to this problem.

Some points:

  • extras can be included with many(/most/all?) gltf object types -- not just nodes
  • I think it would be acceptable to only support / expect the extras data type to be a json object
  • the gltf spec recommends using the object data type as a best practice
  • Blender specific:

@nicopap
Copy link
Contributor

nicopap commented Jan 23, 2022

I can confirm this could be very helpful. Currently working around the limitation with a trait that just pushes components on nodes with specific names. my skin hook impl

mockersf and others added 3 commits April 7, 2022 22:57
Co-authored-by: Nathan Ward <43621845+NathanSWard@users.noreply.github.com>
@cart
Copy link
Member

cart commented Apr 7, 2022

We've been steadily increasing the number of "per entity string allocations" when instancing GLTF scenes. I think we should prioritize "proper string cows" in the very near future to keep this behavior in check.

@cart
Copy link
Member

cart commented Apr 7, 2022

bors r+

bors bot pushed a commit that referenced this pull request Apr 7, 2022
fixes #2153 

expose the `extras` field value as a string
@bors bors bot changed the title expose extras from gltf nodes [Merged by Bors] - expose extras from gltf nodes Apr 7, 2022
@bors bors bot closed this Apr 7, 2022
aevyrie pushed a commit to aevyrie/bevy that referenced this pull request Jun 7, 2022
fixes bevyengine#2153 

expose the `extras` field value as a string
ItsDoot pushed a commit to ItsDoot/bevy that referenced this pull request Feb 1, 2023
fixes bevyengine#2153 

expose the `extras` field value as a string
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Assets Load files from disk to use for things like images, models, and sounds C-Enhancement A new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

gltf extras component
7 participants