nba-emu / NanoBoyAdvance

A cycle-accurate Nintendo Game Boy Advance emulator.
GNU General Public License v3.0
955 stars 53 forks source link

Enable sRGB framebuffer #332

Open GranMinigun opened 9 months ago

GranMinigun commented 9 months ago

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:

Before After
Serious Sam Advance uncorrected Serious Sam Advance gamma-correct
Pokemon Emerald uncorrected Pokemon Emerald gamma-correct
fleroviux commented 9 months ago

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 commented 8 months ago

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 commented 8 months ago

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 commented 8 months ago

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 commented 8 months ago

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 commented 7 months ago

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 commented 7 months ago

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 commented 7 months ago

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.

fleroviux commented 7 months ago

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 commented 7 months ago

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 commented 3 months ago

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.