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

Allow filtering "extras" in GLTFExporter #18445

Closed
garyo opened this issue Jan 21, 2020 · 6 comments
Closed

Allow filtering "extras" in GLTFExporter #18445

garyo opened this issue Jan 21, 2020 · 6 comments

Comments

@garyo
Copy link
Contributor

garyo commented Jan 21, 2020

I'm using three.js with a system that attaches all kinds of info to the three.js objects as userData properties. When I export, I'm getting those (large) properties as "extras" on the glTF nodes, which is as designed but definitely suboptimal, and breaks importers like Blender 2.8 (which expects "extras" to only have primitive types, not giant nested objects). I'd like to be able to filter out some or all of those props during the export process. (My scene is very large, so I'd like to avoid copying the whole thing and removing the props from the clone.)
I propose adding a new option, filterOutUserDataKeys: string[], defaulting to [], which would be used here:

gltfProperty.extras = json;
to filter the userData properties that get exported.
Something like this (untested):

const filtered = Object.keys(json)
  .filter(key => !options.filterOutUserDataKeys.includes(key))
  .reduce((obj, key) => {
    obj[key] = json[key];
    return obj;
  }, {});
gltfProperty.extras = filtered

Does that make sense? If there's a way to do this already, that would be even better.

@donmccurdy
Copy link
Collaborator

Is this sufficient? I'd rather avoid a userData-specific callback if we can.

const originalUserData = new Map();

// stash userData
scene.traverse((o) => {
  originalUserData.set(o, o.userData);
  const {a, b, c} = o.userData;
  o.userData = {a, b, c};
});

// export
exporter.parse( scene, ... );

// restore userData
scene.traverse((o) => {
  o.userData = originalUserData.get(o);
});

You can also do post-export modifications to the JSON, iterating over json.nodes and modifying each node's .extras property.

@garyo
Copy link
Contributor Author

garyo commented Jan 22, 2020

You're thinking it should be a callback rather than my simple option + filter because the json object at that point may not be a dict (i.e. it might be an array)? That's a good point -- for my use case it would be fine to only filter if the object has keys, but it wouldn't prevent a giant array from being exported, so you're right that it's an incomplete solution.
Postprocessing the JSON isn't a good option for me because the attached user data is quite large and sometimes circular, so avoiding serialization is pretty important for speed, memory size and correctness.
Your save-and-restore method seems to work fine for me, though (makes my .gltf file 28kB vs 164MB, wow!), so if you don't think this is worth pursuing I'll keep that workaround in my code.

@donmccurdy
Copy link
Collaborator

The root .userData object really should be an object and not an array or primitive. threejs defines it that way, and glTF strongly encourages the same. I'm not sure what you mean about a callback, though... I'm traversing the scene and not the JSON?

I'm hoping to avoid adding more options to GLTFExporter for things that could be done by processing the input data instead. There are, for better or worse, quite a lot of options that have been requested if we go down that road, and they eventually become difficult to maintain.

@takahirox
Copy link
Collaborator

takahirox commented Jan 24, 2020

Personally I'm interested in GLTFExporter extension mechanism you @garyo suggested at #11682 (comment) rather than adding new options. I agree with Don that I hope we can avoid a lot of options.

@garyo
Copy link
Contributor Author

garyo commented Jan 26, 2020

Extension mechanism would work fine for me as well.
BTW, here's my PR to the Blender glTF importer to allow it not to fail just in case some "bad" extras get in: KhronosGroup/glTF-Blender-IO#884 -- a simple failure case was when node.userData = { "a": [[1], 2] }.
In my use case though, it's best to fix it on the export side since my userDatas are huge and bloat the resulting .glTF files. Long story short: I'm happy with the save-userDatas workaround for now. Exporter extension would be great too.

@donmccurdy
Copy link
Collaborator

Sounds good, and thanks for that PR!

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

No branches or pull requests

3 participants