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

EXRLoader: Support additional attribute types #19106

Merged
merged 1 commit into from
Apr 11, 2020

Conversation

WestLangley
Copy link
Collaborator

Fixes #19105.

/ping @sciecode I am concerned that parseValue() does not appear to differentiate between int and Uint. But I think this PR OK for now.

@WestLangley WestLangley added this to the r116 milestone Apr 10, 2020
@sciecode
Copy link
Contributor

sciecode commented Apr 10, 2020

Yeah, parseValue should be using a Int32 not Uint32. It's not a breaking bug, but we can fix it later just to make sure it is correct.

Should be as simple as creating parseInt32 function and using in the correct places, in this case Rational and Int header values.

But PR is good, we can merge 👍

As a side note, I've created a utility function to help loaders parse data, unfortunately it's on hold until we drop examples/js support, ideally we want to remove all this type parsing from inside the loader implementation. https://github.com/sciecode/three.js/blob/dev-datautils/examples/jsm/utils/DataParser.js

@mrdoob mrdoob merged commit 11739d9 into mrdoob:dev Apr 11, 2020
@mrdoob
Copy link
Owner

mrdoob commented Apr 11, 2020

Thanks!

@WestLangley WestLangley deleted the dev_exrloader_parse branch April 11, 2020 02:49
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

Successfully merging this pull request may close these issues.

EXRLoader - unsupported type: rational
3 participants