Skip to content

cameraEffects: do the tone mapping operation before the display transfer function#1912

Open
illwieckz wants to merge 1 commit intofor-0.56.0/syncfrom
illwieckz/tonemap
Open

cameraEffects: do the tone mapping operation before the display transfer function#1912
illwieckz wants to merge 1 commit intofor-0.56.0/syncfrom
illwieckz/tonemap

Conversation

@illwieckz
Copy link
Member

Do the tone mapping operation before the display transfer function.

The author of the tone mapper we implement said it sould be done this way:

Linear Color HDR Render Output After Auto-Exposure Adjustment → Tonemap Linear Color HDR To Linear In Range of Display → Grain and Conversion via Display Transfer Function
-- Advanced Techniques and Optimization of HDR VDR Color Pipelines by Timothy Lottes (Game developers conference on March 14-18, 2016), presentation slides p. 4

Image

Other authorities on the topic said the same:

It is important to apply the conversion at the final stage of rendering (when the values are written to the display buffer for the last time), and not before. If post-processing is applied after gamma correction, post-processing effects will be computed in nonlinear space, which is incorrect and will often cause visible artifacts. So operations such as tone mapping [...] can be applied to images to adjust luminance balance, but gamma corection should always be done last.
-- Real-Time Rendering 3rd by Möller, Haines and Hoffman, p. 145
https://computergraphics.stackexchange.com/questions/5449/tone-mapping-gamma-correction

@illwieckz
Copy link
Member Author

We may re-evaluate the change of HDR Max from 8 to 2 and see if we need to revert it:

It looks like there was indeed a bug in our code modifying the render in an unexpected way, and that my diagnostic from #1865 was right:

@illwieckz
Copy link
Member Author

illwieckz commented Feb 25, 2026

We may re-evaluate the change of HDR Max from 8 to 2 and see if we need to revert it:

I've done some tests with very bright maps like Xoylent or maps with very bright parts like Parpax, and 2 looks good. We may reconsider it the day we have HDR lightmaps.

@slipher
Copy link
Member

slipher commented Feb 25, 2026

Hmm, indeed it was said to be for linear inputs: #1550 (comment)

@illwieckz
Copy link
Member Author

illwieckz commented Feb 25, 2026

When I rebased the sRGB branch over the tone mapping code, we all knew the tone mapper had to be set before the color conversion, and that's what I did first, then somehow we got collectively convinced about the other way in the Chat while all knowing it was wrong.

Well, I found the log:

2025-07-07 10:14:20 +02 <illwieckz> It looks like the only thing to take care about is that
                          tonemapping looks weird.
2025-07-07 10:14:45 +02 <illwieckz> It looks like that's the last remaining thing.

2025-07-08 11:15:45 +02 <.somaz> doing srgb transform before it is wrong then @illwieckz.
                          needs to happen afterwards. Then still wondering if sdl does gamma
                          correction as well. In other tech3 forks it sets the os gamma table,
                          which your fork seems to avoid (which is good in first place)

2025-07-09 13:10:56 +02 <_xreaperx_> The tonemapper creates the intended output from a given input,
                          if it's given the wrong input or the wrong output is expected it's not the
                          tonemapper's fault
2025-07-09 13:15:20 +02 <illwieckz> why would the input be wrong?
2025-07-09 14:31:14 +02 <illwieckz> Input
2025-07-09 14:31:15 +02 <illwieckz> [img]
2025-07-09 14:31:37 +02 <illwieckz> Output
2025-07-09 14:31:38 +02 <illwieckz> [img]
2025-07-09 14:34:21 +02 <illwieckz> Ah no, the tonemapper currently runs before `toSRGB()`,
                          as recommended by @.somaz, so that's not the input.
2025-07-09 14:34:32 +02 <illwieckz> Let's move it after.
2025-07-09 14:36:45 +02 <illwieckz> Input
2025-07-09 14:36:46 +02 <illwieckz> [img]
2025-07-09 14:36:55 +02 <illwieckz> Output
2025-07-09 14:36:55 +02 <illwieckz> [img]
2025-07-09 14:37:02 +02 <illwieckz> This is less bad.
2025-07-09 14:37:16 +02 <illwieckz> But then on sunny places or well-lit places, it looks unnatural.
2025-07-09 14:38:06 +02 <.somaz> most tonemappers I've seen expect linear input. srgb transform
                          should happen the very last in the transform chain

2025-07-09 14:39:49 +02 <illwieckz> When tonemapper is after `toSRGB()`
2025-07-09 14:39:49 +02 <illwieckz> Input
2025-07-09 14:39:51 +02 <illwieckz> [img]
2025-07-09 14:39:59 +02 <illwieckz> Output
2025-07-09 14:40:00 +02 <illwieckz> [img]
2025-07-09 14:40:25 +02 <illwieckz> Input
2025-07-09 14:40:26 +02 <illwieckz> [img]
2025-07-09 14:40:36 +02 <illwieckz> Output
2025-07-09 14:40:36 +02 <illwieckz> [img]
2025-07-09 14:41:13 +02 <illwieckz> In the last shots, ignore the stupid shadow above the lamp
                         light, that's a buggy side effect of q3map2 backsplah madness.
2025-07-09 14:44:47 +02 <illwieckz> @.somaz All I know is that our current tonemapper was written
                         when all we had were maps with colors computed without colorspace support.
2025-07-09 14:47:19 +02 <.somaz> Right, tonemapper is configured for gamma space input and not
                         linear. Well, then you need different tonemapper values for linear input
                         in this case 🙃
2025-07-09 14:50:49 +02 <illwieckz> Ok, so putting the conversion to sRGB before the tonemapper
                         is right.

We were just screwed by screenshots I guess, and some wrong assumptions have been made.

So we're back at the same place we were back in July of last year. When doing the tone mapper before the color conversion, things turn dark very quickly, which is somewhat avoided when doing the wrongness of moving the tone mapper after the color conversion, but then the high light look weird, etc.

At least we can stop doing the faulty thing of doing the tone mapping after the conversion, but that doesn't address the concern we had 7 months ago that something looks weird when the tone mapper is enabled. It still looks weird.

That thread was already wondering if we needed a different configuration for the tone mapper.

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.

2 participants