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

Enable sRGB framebuffer #332

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

GranMinigun
Copy link
Contributor

@GranMinigun GranMinigun commented Oct 6, 2023

Some little changes to the textures, viewport, and framebuffer handling. Despite the name, this benefits any kind of display, because the affected part is gamma correction.

A long explanation can be read here; in short, linear colour interpolation (including blending) should happen in linear colour space to look correct. While the effect is most noticeable with linear-filtered output (which I'd imagine barely anyone uses), this should also affect LCD ghosting effect due to colour blending, and will be useful for other shaders that may land into the emulator in the future. Additionally, using OpenGL's built-in sRGB formats reduces GPU load, which is especially important for low-powered SBCs and such.

Currently a draft for demonstration purposes and possible suggestions. Unresolved issues are:

  • xBRZ shader doesn't work correctly (requires special handling, specifically a separate info texture) Fixed;
  • Last time I checked, Intel's driver on Windows wasn't respecting either sRGB textures or framebuffers, needs a confirmation with the latest driver, preferably on a pre-Xe iGPU;
  • Qt 6 currently forcefully disables sRGB framebuffer every frame for no apparent reason, online reports suggest it broke in 6.4, previous versions are unaffected, more door banging may be needed.
Before After
Serious Sam Advance uncorrected Serious Sam Advance gamma-correct
Pokemon Emerald uncorrected Pokemon Emerald gamma-correct

@fleroviux
Copy link
Member

I haven't had time to check this in detail yet, but I'm definitely interested in these changes. I have wanted to look into proper sRGB handling before but hadn't gotten around to it myself yet.

@GranMinigun
Copy link
Contributor Author

Looks like I've managed to make xBRZ behave. Just some cleanups, and it should be ready for review. The last two bullet points are still unresolved though. Testing is welcome.

@fleroviux
Copy link
Member

I just did some testing of this on Windows (with an Nvidia GPU) and so far the only thing I have noticed is that the combination of xBRZ + LCD ghosting appears to look different. Because the LCD ghosting is now done earlier in the post-processing stack, the xBRZ filter would try to to upscale the ghosting artifacts.

Old behavior:
image

New behavior:
image

(The length of the ghosting trail being different probably is due to different running velocity.)

I think this may have been the reason why I originally put the LCD ghosting pass to the end. Then again I'm not sure how likely someone is to use both LCD ghosting and xBRZ upscaling at the same time. Do you think it is necessary or makes sense to do the LCD ghosting before the upscaling though?

@GranMinigun
Copy link
Contributor Author

It's much cheaper that way. Note that all textures are now of the GBA screen resolution, and there's no recreating them at the output size. I agree that ghosting should happen after xBRZ pass, but that doesn't apply to other available setups. I'll see what I can do.

@fleroviux
Copy link
Member

I did notice that the textures now are at 240x160 resolution. I agree that this is more efficient in theory, but in practice I would think that none of the passes that we do would be close to stressing any competent GPU at full resolution.
If you can implement in a way where full resolution is only used when really needed, then I agree we that we should do that!

@GranMinigun
Copy link
Contributor Author

Looks like it's all functioning now as intended. See the latest pushed commit. If the overall flow there doesn't make you scream in terror, then I'll do another prettifying pass and (optionally) split changes not directly related to sRGB format handling into separate PR.

@fleroviux
Copy link
Member

Thanks fixing the issue. So far I couldn't spot any issues on Windows. But on macOS (using Qt 5; qt@5 from brew) I noticed that the color space is incorrect.
Bildschirmfoto 2023-11-18 um 14 56 43
(this is with color correction disabled)

I did not test this on macOS before, so it's likely that the implementation I tested before had the same issue already.

@GranMinigun
Copy link
Contributor Author

GranMinigun commented Nov 21, 2023

Looks exactly like the Qt 6 bug I mentioned in the first post (that is, unconditional disabling of sRGB framebuffer, which leads to darker output). That's a huge roadblock if that's happening with Qt 5 as well.

Update: found this bug report, seems to be the case you've hit.

@GranMinigun GranMinigun marked this pull request as draft November 21, 2023 13:24
@fleroviux
Copy link
Member

Regarding the Qt6 issue: in general it could make sense for us to get rid of QOpenGLWidget and instead create our own GL context on the native window. Maybe this would bypass the issue where Qt6 forcefully disables sRGB.
However regarding macOS it needs to be investigated if this issue is related to Qt or if the OpenGL implementation on macOS simply does not support sRGB textures.

@GranMinigun
Copy link
Contributor Author

macOS does support necessary functionality: sRGB texture formats extension became core (mandatory) in OpenGL 2.1 spec, sRGB framebuffer - in 3.0, and sRGB enablement was successfully done (not by me) in DOSBox Staging, which uses only SDL library for its window. macOS is limited to 2.1 compatibility profile, but exposes the framebuffer extension, and to 4.1 core profile. I'm fairly certain it's the linked issue.

Looking at the documentation, there's QOpenGLWindow class, which doesn't perform widgets composition. But the same docs discourage its usage due to unspecified possible problems. I'm not all that familiar with Qt, and have no idea what other Qt-based emulators do in their single-window configurations, so can't suggest anything.

@GranMinigun
Copy link
Contributor Author

Now it definitely works with Qt 6 on my Linux box. Still don't know whether Intel fixed their Windows driver for pre-Xe chips. That's my only major concern right now. I'm pretty sure macOS shouldn't have any problems. So, a bit of testing on various hardware, maybe formatting adjustments (if you spot anything), and it should be good to go.

Sharp bilinear shader already looks better. Colour correction shaders should run somewhat faster, as linear-to-sRGB pass is performed by specialized hardware. You might also notice that with colour correction enabled, dark tones are slightly darker. That's perfectly normal, those shaders were applying 2.2 gamma function instead of proper sRGB transfer function, which has a linear slope near zero.

@GranMinigun GranMinigun marked this pull request as ready for review March 17, 2024 00:29
Drop the output gamma correction, as that's handled by the driver for
sRGB-encoded framebuffer.
sRGB texture format is needed for the output to look right with
sRGB framebuffer enabled and no color correction shader applied.
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

2 participants