Skip to content

Fix EXR encoder to write scaled values - #3195

Open
JimBobSquarePants wants to merge 1 commit into
release/4.1.xfrom
js/exr-encoder-scaled
Open

JimBobSquarePants wants to merge 1 commit into
release/4.1.xfrom
js/exr-encoder-scaled

Conversation

@JimBobSquarePants

Copy link
Copy Markdown
Member

Prerequisites

  • I have written a descriptive pull-request title
  • I have verified that there are no overlapping pull-requests open
  • I have verified that I am following the existing coding patterns and practice as demonstrated in the repository. These follow strict Stylecop rules 👮.
  • I have provided test coverage for my change (where applicable)

Description

Related to #3194

The EXR decoder and encoder did not agree. The decoder writes scaled values into the pixel format with FromAssociatedScaledVector4. The encoder read native values with ToAssociatedVector4().

For a pixel format whose native range is [0, 1], both values are equal, so the output was correct. For a pixel format with a different native range, the encoder wrote wrong values. For example, HalfVector4 maps the scaled range [0, 1] to the native range [-65504, 65504]. A scaled red value of 0.25 went to the file as -32752, and an opaque alpha went to the file as 65504.

The encoder now reads ToAssociatedScaledVector4() in the floating-point path and in the unsigned integer path. Rgba128.FromVector4 in the unsigned integer path also expects components in [0, 1].

I added a test that saves an Image<HalfVector4> as Half, Float, and UnsignedInt, loads the result as RgbaVector, and compares the scaled values. The test uses an alpha of 0.5, so the clamp in the decoder cannot hide a wrong alpha value.

This change does not add HDR support. Values outside [0, 1] still clamp in 4.x. I track that work in #3194 for v5.

The EXR decoder maps file values to scaled values, but the encoder read
native values. Pixel formats whose native range is not [0, 1], such as
HalfVector4, wrote wrong color and alpha values to the file.
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.

1 participant