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 single channel luminance - RedFormat #23007

Merged
merged 2 commits into from
Dec 15, 2021

Conversation

sciecode
Copy link
Contributor

@sciecode sciecode commented Dec 12, 2021

Addresses #20195 (comment)

I noticed a difference between our implementation and OpenEXR's in trying to decompress a block only if input-data is actually smaller than the expected data-size. This PR standardizes this issue and makes equivalent to OpenEXR implementation.

It also initially includes support for luminance - single channel files HalfFloatType / FloatType.
Luminance images are treated as RedFormat - WebGL2 only context.

Updated textureData header info to include specification flags, to help identify if problematic files contain lack of supported features like Deep-Image, Multi-Part or Tiled image files.

image

I largely tested these changes, but would appreciate further testing - @Mugen87, @WestLangley

--

Currently we fail to support .setDataType( THREE.UnsignedByteType ) on Luminance maps.

setDataType is properly followed even in cases where there is an underlying mismatch between input-type and output-types.
( e.g like a float32b input file transformed into a half16b texture ). When we set the output-type to UnsignedByteType on a RGB/RGBA files we also encode the output-texture as RGBEEncoding and transform the data appropriately.

However transforming a luminance map of 32/16 type to an uint8 type is not yet specified, should I explicitly allow that transformation? If so, should I then change the texture format to LuminanceFormat in order to support WebGL1 contexts?

Signed-off-by: Guilherme Avila <guilhermebav@gmail.com>
Signed-off-by: Guilherme Avila <guilhermebav@gmail.com>
@Mugen87
Copy link
Collaborator

Mugen87 commented Dec 13, 2021

However transforming a luminance map of 32/16 type to an uint8 type is not yet specified, should I explicitly allow that transformation?

I think at some point the encoding to RGBE should be removed from EXRLoader and half float used instead. With WebGL 2 we can rely on half float, see #22998.

Yes, half float textures allocate more memory however it's possible to use THREE.LinearFilter and it simplifies the code in the loader and shader.

@Mugen87
Copy link
Collaborator

Mugen87 commented Dec 13, 2021

The luminance texture on my computer loads now as expected 👍 .

@sciecode sciecode marked this pull request as ready for review December 13, 2021 15:08
@sciecode
Copy link
Contributor Author

sciecode commented Dec 13, 2021

However transforming a luminance map of 32/16 type to an uint8 type is not yet specified, should I explicitly allow that transformation?

I think at some point the encoding to RGBE should be removed from EXRLoader and half float used instead. With WebGL 2 we can rely on half float, see #22998.

Loader already specifies HalfFloatType as the default output type.

Yes, half float textures allocate more memory however it's possible to use THREE.LinearFilter and it simplifies the code in the loader and shader.

Yeah I can definitely see that being an issue. In that case, this PR should be ready to merge, in case we don't find any testing problems.

@WestLangley WestLangley added this to the r136 milestone Dec 14, 2021
@mrdoob mrdoob merged commit ab528ac into mrdoob:dev Dec 15, 2021
@mrdoob
Copy link
Owner

mrdoob commented Dec 15, 2021

Thanks!

@sciecode sciecode deleted the exrloader-channels branch December 15, 2021 15:45
gero3 pushed a commit to gero3/three.js that referenced this pull request Dec 16, 2021
* EXRLoader: support single channel luminance - RedFormat encoding

Signed-off-by: Guilherme Avila <guilhermebav@gmail.com>

* clean up

Signed-off-by: Guilherme Avila <guilhermebav@gmail.com>
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.

None yet

4 participants