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

I/O readJSON() method cannot decode Data URIs #254

Closed
donmccurdy opened this issue May 19, 2021 · 1 comment · Fixed by #266
Closed

I/O readJSON() method cannot decode Data URIs #254

donmccurdy opened this issue May 19, 2021 · 1 comment · Fixed by #266
Labels
bug Something isn't working package:core
Milestone

Comments

@donmccurdy
Copy link
Owner

donmccurdy commented May 19, 2021

DracoMeshCompression doesn't handle Data URIs; in general there should probably be an abstraction for extensions that need to access binary data from a buffer before it's parsed into accessors:

Looking into this more, the Draco extension is probably doing the right thing — decoding Data URIs is handled in the I/O class, not the readers. Currently the read('path/to/file.gltf') methods will decode Data URIs, but readJSON does not. That'll be a small breaking change for WebIO unfortunately, since readJSON needs to become asynchronous to support this.

Perhaps it's worth thinking a little more about the sync/async differences between these methods. Currently NodeIO (1) uses sync for everything, and (2), can't load resources over the network. I think I'm OK with leaving (2) to the user but they shouldn't have to manually replace Data URIs. And (1) is not ideal for anything running on a web server.

@donmccurdy donmccurdy added bug Something isn't working package:extensions labels May 19, 2021
@donmccurdy donmccurdy added this to the v0.11 milestone May 19, 2021
@donmccurdy donmccurdy changed the title Unable to read Draco-compressed files with embedded Data URIs I/O readJSON() method cannot decode Data URIs. May 19, 2021
@donmccurdy donmccurdy changed the title I/O readJSON() method cannot decode Data URIs. I/O readJSON() method cannot decode Data URIs May 19, 2021
@donmccurdy donmccurdy modified the milestones: v0.11, Backlog May 19, 2021
@donmccurdy
Copy link
Owner Author

On further thought, not necessarily a breaking change since we can use BufferUtils.createBufferFromDataURI synchronously. The idea that NodeIO should become async is probably still worth doing at some point though...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working package:core
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant