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

stb_image_write.h: Radiance .hdr saved incorrectly when source exceeds Rec709 color space #1677

Open
AnotherCommander opened this issue Aug 26, 2024 · 2 comments

Comments

@AnotherCommander
Copy link

Describe the bug
The Radiance .hdr format supported by stbi is RGB9_E5 . This means that no negative numbers can be used when encoding an image. When working in the scRGB color space, this implies that any non-Rec709 colors, (i.e. colors with negative R,G or B component values) cannot be represented.

Attempting to save to an .hdr file using stb_image_write when the source contains DCI-P3 or BT2020 colors will result in a broken .hdr. Broken here means that any non-Rec709 colors appear with completely wrong hues and the result is unusable.

To Reproduce
Steps to reproduce the behavior:

  1. Download and build the example exr2rgbe from https://github.com/syoyo/tinyexr. It is a very simple program to build and uses tinyexr and stb_image_write to do its read/write operations.
  2. Get an .exr image (lets call it inputimg.exr and be sure that it contains colors in the DCI-P3/BT2020 spaces. Place it in the exr2rgbe folder together with the executable you created at step 1.
  3. Run exr2rgbe inputimg.exr ouputimg.hdr
  4. Now try to open the resulting .hdr. You will immediately notice that it is wrong.

Expected behavior
The resulting .hdr should look very close to the source on screen. It is expected that DCI-P3 and/or BT2020 colors will be lost during write since the format does not allow for negative numbers, but at least it won't be blatantly wrong.

Screenshots
I have already uploaded my results in a zip file available from here for those who can't be bothered to follow the steps to reproduce: https://drive.google.com/file/d/15fF3VWsKjHNrgzhYg-Ce3WrofQBCIwnp/view?usp=sharing

Some comments:
You will note that the zip linked above contains three files: my original .exr, the bad result I get from stb_image_write.h v1.16 and one more file, which is the .hdr fixed to display properly all colors in Rec709. The fix is actually very simple: make sure that the rgb components of the buffers being written are clamped to 0.0f. The proposed patch for the fix is this:

--- 638,656 ----

  static void stbiw__linear_to_rgbe(unsigned char *rgbe, float *linear)
  {
!    int exponent, i;
     float maxcomp = stbiw__max(linear[0], stbiw__max(linear[1], linear[2]));

     if (maxcomp < 1e-32f) {
        rgbe[0] = rgbe[1] = rgbe[2] = rgbe[3] = 0;
     } else {
        float normalize = (float) frexp(maxcomp, &exponent) * 256.0f/maxcomp;
+
+         // ensure that any non-Rec709 colors are clamped to Rec709
+         for (i = 0; i < 3; i++)
+         {
+                 if (linear[i] < 0.0f)  linear[i] = 0.0f;
+         }

        rgbe[0] = (unsigned char)(linear[0] * normalize);
        rgbe[1] = (unsigned char)(linear[1] * normalize);
***************
@nothings
Copy link
Owner

Serious question, which is better, "fixing" it or adding documentation "it's the client's job to make sure the colors lie in the correct colorspace", given that we can't actually fix it, just clamp it.

@rygorous
Copy link
Collaborator

Given that we literally produce complete garbage when values are negative, clamping seems the minimum.

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

6 participants
@AnotherCommander @rygorous @nothings and others