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

Video: implement color correction to match the Wii/GC NTSC/PAL color spaces (and gamma) #11850

Merged
merged 3 commits into from Jun 23, 2023

Conversation

Filoppi
Copy link
Contributor

@Filoppi Filoppi commented May 28, 2023

This PR implements:

  • Color correction to match the NTSC and PAL color spaces that the GC and Wii targeted (this does NOT emulate actual features that CRT TVs had, it just users their color primaries)
  • Gamma correction to also match the NTSC/PAL gamma (instead of outputting in sRGB, which was never used by consoles)
  • HDR output (scRGB, 16 bit float)
  • Higher SDR output quality (R10G10B10A2 instead of R8G8B8A8, the alpha bits were wasted)

The advantages of having a "native" HDR implementation within Dolphin are the following:

  • We can sample textures in linear space correctly, so linear sampling works correctly and isn't influenced by gamma.
  • Allows Dolphin to add its own AutoHDR shader. This is especially relevant because the lighting of older games doesn't really play nice with Windows AutoHDR. Additionally, this implementation would be supported by Windows 10 and Linux.
  • Allows to display the whole EBU/PAL and NTSC-J color spaces, which are more extended than Rec.709.
  • It avoids loosing quality if doing multiple post process passes, like we might now do if we have color correction enabled.

All features are optional and disabled by default. Dolpin's default behaviour is unchanged (with the exception of outputting in a R10G10B10A2 if supported). Tooltips have been provided to explain all new features in the most user friendly way I could.

This has been carefully tested and there should be no regression in any of the different features that Dolphin
offers, like multisampling, stereo rendering, other post processes, etc etc.

Fixes: https://bugs.dolphin-emu.org/issues/8941

Credit to @EndlesslyFlowering and @Dogway for helping with the NTSC/PAL video "standards" information and conversion matrices.

Color Correction Settings:
Dolphin_eC9WZMcQeO

Untouched Dolphin output:
Untouched Dolphin output

Interpreted as NTSC-M:
Interpreted as NTSC-M

Interpreted as 2.35 gamma:
Interpreted as 2 35 gamma

Non gamma corrected sampling (low resolutition emulation into high res window):
Non gamma corrected sampling

Gamma corrected sampling (low resolutition emulation into high res window):
Gamma corrected sampling

HDR screenshots comparions (BT.2020 is needed to show all the colors from PAL and NTSC-J):
HDR Color Space Correction Comparison.zip
HDR Non Corrected.zip

Follow up PRs will add:

  • Different texture sampling (upscaling and downscaling) modes, similar to Cemu
  • My own AutoHDR shader, which has a set of different AutoHDR logics, given that the best one usually depends on the game.

@lextra2
Copy link

lextra2 commented May 28, 2023

How about doing this with LUTs?

You could have a menu somewhere in the dolphin settings, that lets you select LUTs from a folder.

That way, you could choose between HLG and PQ or whatever standards come out in the future or have people create their own ones.
The lut would run in 16bit so having the option to dither the output to (12bit or 10bit) would be nice. And since thats done via postprocessing, even 8bit panels would benefit from less banding.

@Filoppi
Copy link
Contributor Author

Filoppi commented May 28, 2023

How about doing this with LUTs?

Implementing a LUT would be much more complicated and the performance gain would be infinitively small (and the result worse).
I am exposing all the settings to qt right now, this isn't a full implementation yet, I will soon re-upload it.
It should already be very easy to implement pq output, as I have the linear_space_output which could further branch into pq_space_output, but for now that's not necessary. Dithering could also be implemented later if necessary.
I have also changed Vulkan and DX swapchains to use 10bit RGB buffers for SDR, so there's already gains there.

@lextra2
Copy link

lextra2 commented May 28, 2023

I am exposing all the settings to qt right now, this isn't a full implementation yet, I will soon re-upload it. It should already be very easy to implement pq output, as I have the linear_space_output which could further branch into pq_space_output, but for now that's not necessary.

Oh ok. Sounds great.

Dithering could also be implemented later if necessary. I have also changed Vulkan and DX swapchains to use 10bit RGB buffers for SDR, so there's already gains there.

Amazing, thanks a lot!

@sboukortt
Copy link

While native support for LUTs would allow for more flexible colour management, applying ReShade to Dolphin seems to work well enough for that.

@Filoppi Filoppi force-pushed the post_process_fixes branch 3 times, most recently from 2a53a9c to f781b02 Compare May 31, 2023 23:22
@Filoppi Filoppi marked this pull request as ready for review May 31, 2023 23:36

// Color.Correction

static constexpr float GFX_CC_GAME_GAMMA_MIN = 2.2f;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think these should go in here because this whole file is for settings. If these need to be source code wide I'd put them in Constants.h

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's some similar instances for min/max constants above, that's why I put them there.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those min/max values are actual settings though, not constants. Just felt a bit odd to me to have them in here. Will leave it up to someone else to decide, it's a small nit.

@TryTwo
Copy link
Contributor

TryTwo commented Jun 1, 2023

Does this affect efb copies?

https://bugs.dolphin-emu.org/issues/13128

@Filoppi
Copy link
Contributor Author

Filoppi commented Jun 1, 2023

Does this affect efb copies?

No, this is exclusively after the emulation, it only touches on post processing.

@AdmiralCurtiss
Copy link
Contributor

This is over 1000 lines of code in a single commit. Didn't you say you learned your lesson about big PRs?

@JMC47
Copy link
Contributor

JMC47 commented Jun 2, 2023

I actually do think the comparisons are favorable, but I can't actually review the code myself. Just as an onlooker I do think the adjusted colors are a bit nicer, at least on my monitors.

@AdmiralCurtiss
Copy link
Contributor

@iwubcode Can you review the video backend changes here?

@iwubcode
Copy link
Contributor

Please rebase when you have time.

@Filoppi
Copy link
Contributor Author

Filoppi commented Jun 18, 2023

Please rebase when you have time.

Done. Addressed all comments. Thanks for the review.

@iwubcode
Copy link
Contributor

iwubcode commented Jun 18, 2023

Enabling the acid metal post processing shader and then turning on the hdr setting (I'm not on a hdr display) gives me completely different colors, this surprises me. Is it expected?

Anaglyph and passive stereoscopic 3d are completely broken. I'm guessing that is due to the new pass. I will have to look into it in more detail later.

Additionally, I will try and find some time to test with a hdr display but it's kinda a pain for me. I can assume that has been heavily tested.

@AdmiralCurtiss
Copy link
Contributor

AdmiralCurtiss commented Jun 18, 2023

I belive the 'HDR' referred to in here has nothing to do with HDR displays, it just increases the bits per color that are available for intermediate calculations in the shader.

@iwubcode
Copy link
Contributor

iwubcode commented Jun 18, 2023

I belive the 'HDR' referred to in here has nothing to do with HDR displays, it just increases the bits per color that are available for intermediate calculations in the shader.

Hmm, ok. I assumed you had to have a HDR display to see those extra bits? I can say that whites are very blown out with it on. Maybe intentional?

ON (default settings)

Screenshot 2023-06-18 144324

OFF

Screenshot 2023-06-18 144352

.

All features are optional and disabled by default

@Filoppi - just fyi, it looks like HDR is enabled by default on my stand alone build

@Filoppi
Copy link
Contributor Author

Filoppi commented Jun 18, 2023

Enabling the acid metal post processing shader and then turning on the hdr setting (I'm not on a hdr display) gives me completely different colors, this surprises me. Is it expected?

Yes, scRGB HDR signals are in linear space, so post process effects end up working with different values. Most of them work fine.

// TODO: ideally we'd do the user selected post process pass in the intermediary buffer in linear

Ideally all post process effects would work in linear space, and then the signal is reconverted to the display gamma before the output, like this:
emulation output -> game gamma inverse -> linear -> post processes -> display gamma -> output

Anaglyph and passive stereoscopic 3d are completely broken. I'm guessing that is due to the new pass. I will have to look into it in more detail later.

Anaglyph and passive stereoscopic 3d are not broken as far as I can tell, I've put hours into testing them and comparing them and I've even added comments to explain some of the most confusing code. I just tried again and they work fine. I've tried every 3D mode with every backend.

I belive the 'HDR' referred to in here has nothing to do with HDR displays

HDR is actual "HDR" as in it's a scRGB float16 buffer, which allows for values beyond 0-1 and thus colors beyond Rec.709.
That's required to show the whole PAL and NTSC-J color spaces, and also allows for Dolphin's own implementation of AutoHDR which I already have but didn't want to include in this PR (it's just a post process anyway).

just fyi, it looks like HDR is enabled by default on my stand alone build

I just fixed that, my mistake.
HDR is overblown because GFX_CC_HDR_PAPER_WHITE_NITS defaults to 200, to roughly match the brightness an SDR image would have on an SDR screen.
If you don't have an HDR screen, any color beyond a value of 1 will be clipped, so to display HDR with the same brightness as SDR, you need to set it to 80 nits (min) in the Color Correction settings. 200 is a widely used default nits target used for SDR->HDR conversions.

Filoppi and others added 2 commits June 19, 2023 01:34
…ces (and gamma) that GC and Wii targeted.

To further increase the accuracy of the post process phase, I've added (scRGB) HDR support, which is necessary
to fully display the PAL and NTSC-J color spaces, and also to improve the quality of post process texture samplings and
do them in linear space instead of gamma space (which is very important when playing at low resolutions).
For SDR, the quality is also slightly increased, at least if any post process runs, as the buffer is now
R10G10B10A2 (on Vulkan, DX11 and DX12) if supported; previously it was R8G8B8A8 but the alpha bits were wasted.

Gamma correction is arguably the most important thing as Dolphin on Windows outputted in "sRGB" (implicitly)
as that's what Windows expects by default, though sRGB gamma is very different from the gamma commonly used
by video standards dating to the pre HDR era (roughly gamma 2.35).

Additionally, the addition of HDR support (which is pretty straight forward and minimal), added support for
our own custom AutoHDR shaders, which would allow us to achieve decent looking HDR in Dolphin games without
having to use SpecialK or Windows 11 AutoHDR. Both of which don't necessarily play nice with older games
with strongly different and simpler lighting. HDR should also be supported in Linux.
Development of my own AutoHDR shader is almost complete and will come next.

This has been carefully tested and there should be no regression in any of the different features that Dolphin
offers, like multisampling, stereo rendering, other post processes, etc etc.

Fixes: https://bugs.dolphin-emu.org/issues/8941

Co-authored-by: EndlesslyFlowering <EndlesslyFlowering@protonmail.com>
Co-authored-by: Dogway <lin_ares@hotmail.com>
@iwubcode
Copy link
Contributor

Anaglyph and passive stereoscopic 3d are not broken as far as I can tell, I've put hours into testing them and comparing them and I've even added comments to explain some of the most confusing code. I just tried again and they work fine. I've tried every 3D mode with every backend.

Yeah, tried again on latest fresh build and it works fine. I don't know what happened, it wasn't showing at all later. No big deal.

Yes, scRGB HDR signals are in linear space, so post process effects end up working with different values

Ah yeah, we did discuss that. Thank you for the explanation, no concerns then with how it is.

200 is a widely used default nits target used for SDR->HDR conversions.

Does that depend at all on your display? I know some higher end displays have a higher nit count.

@Filoppi
Copy link
Contributor Author

Filoppi commented Jun 19, 2023

The nits multiplier is just a setting (paper white). It's not really related to your monitor peak brightness, but it should be left below it, though nobody would try to reach the monitor peak brightness as average brightness because it would be blinding.

The shaders probably broke, i had that happen many times during testing. If you change the 3D settings while new shaders are building the game might stop rendering until you clean the shader cache manually because dolphin thinks it built them but underneath they are busted. At least that's what it seemed to me. Unrelated to this PR.

Copy link
Contributor

@iwubcode iwubcode left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't test the HDR bits but code LGTM and I ran a number of tests with and without post processing shaders on various games.

@AdmiralCurtiss
Copy link
Contributor

I'll just trust you that this is good.

@AdmiralCurtiss AdmiralCurtiss merged commit 02909bd into dolphin-emu:master Jun 23, 2023
14 checks passed
@Miksel12
Copy link
Contributor

Nice feature. I do think the color correction config window looks very out of place as there is no padding between the widgets and the windows border. Which is different from the rest of the UI.

@Filoppi
Copy link
Contributor Author

Filoppi commented Jun 28, 2023

Nice feature. I do think the color correction config window looks very out of place as there is no padding between the widgets and the windows border. Which is different from the rest of the UI.

Understandable. I think I'm happy with how it is at the moment, anybody is welcome to improve the widget :). In fact, somebody could port the menu to the Android GUI?

@mbc07
Copy link
Contributor

mbc07 commented Jun 28, 2023

I'll try to improve the Qt GUI a little, at least to make it consistent with the existing config windows. Won't promise anything regarding the Android GUI, though, I'm already in development hell trying to ship a feature for it...

@Linx-ESP
Copy link

Linx-ESP commented Jul 22, 2023

Aside from sRGB gamma curve, would BT.1886 be another good option?. Since it does target a more CRT to LCD panel translation and is not equal as a 2,2/2,4/etc curve.

Does the Game to Linear Gamma already account for that?
Would BT.1886 use as Game to Linear instead of Linear to Display?

@Linx-ESP
Copy link

You may be interested in testing with the new Windows Advanced Color Management
Forced via registry in a SDR only display, HDR Paper White Nits: 80 and enabling HDR postprocessing doesn't cause any issues.
Hopefully this allows for features in relation to the color correction done in Windows from the scRGB output that are mentioned in the microsoft documentation.

@EndlesslyFlowering
Copy link
Contributor

EndlesslyFlowering commented Jul 22, 2023

Aside from sRGB gamma curve, would BT.1886 be another good option?. Since it does target a more CRT to LCD panel translation and is not equal as a 2,2/2,4/etc curve.

Does the Game to Linear Gamma already account for that? Would BT.1886 use as Game to Linear instead of Linear to Display?

BT.1886 with a black point of 0 is just pure power gamma 2.4.
If the black point of the display is known you can apply BT.1886 to the linear buffer, then unapply it by using pure power 2.4 and then apply the display gamma (iirc).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet