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

basic sRGB sampling with SG_PIXELFORMAT_SRGB8A8 format #758

Merged
merged 1 commit into from
Jan 26, 2023

Conversation

allcreater
Copy link
Contributor

SG_PIXELFORMAT_SRGB8A8 texture format is introduced

This is the first separate part towards full sRGB support, as specified in #562.

The SRGB8A8 format is a non-linear RGB format with linear alpha. This format is the most universal and the most supported sRGB format, with the exception of the compressed DXT family. DXT family is out of scope of this pull request but could be implemented separately. The proposed implementation is supposed to work on all backends except WebGL1 and WGPU

@floooh
Copy link
Owner

floooh commented Dec 15, 2022

Thanks for the PR! I'm currently on vacation and do some emulator coding (only do some very simple drive-by PR merges here and there), but this one I want to test properly :) I'll try to take care of this when I'm back from vacation (January). If nothing has happened mid January though, please 'at' me :)

@allcreater
Copy link
Contributor Author

@floooh so sorry to bother you, but I wanted to remind you of the request, looks like mid-January is already over :)

Also during this time, another of my requests appeared, it would be great if you can take a look at it too :)

@floooh
Copy link
Owner

floooh commented Jan 23, 2023

Yep, delayed but not forgotten ;) (also saw your other PR, but didn't form enough of an opinion yet to provide useful feedback)

@floooh
Copy link
Owner

floooh commented Jan 25, 2023

Going to look into this now...

@floooh
Copy link
Owner

floooh commented Jan 25, 2023

Hmm, interesting. I've added the SRGBA8 pixel format to the pixelformats-sapp sample, and with the GL backend I can see a clear difference, while with the Metal backend, RGBA8 and SRGBA8 look absolutely identical 🤔

GL:
Screenshot 2023-01-25 at 19 18 21

Metal:
Screenshot 2023-01-25 at 19 19 05

this is on macOS ...I'll check the other platforms too...

...the difference only exists for render targets though...

PS: WebGL2 (Chrome on macOS): looks identical, unlike GL(!) and like Metal. At least it's consistent across browsers (Chrome, Safari and Firefox):

Screenshot 2023-01-25 at 19 31 05

OpenGL on Linux is consistent with OpenGL on Mac (looks different).

On Windows, OpenGL is consistent with GL on macOS and Linux, and D3D11 is consistent with Metal and WebGL2.

I'll look into this issue tomorrow and decide whether I'll add a fix or merge the PR as is, I seem to remember that GL framebuffer objects need to be made 'sRGB aware', maybe it's easy to fix (and in the long distant past I also had weird problems with sRGB framebuffers on GL, but hopefully it's more robust nowadays).

@floooh
Copy link
Owner

floooh commented Jan 26, 2023

Ok, I can get the same behaviour in GL as in D3D11 and Metal by enabling GL_FRAMEBUFFER_SRGB for offscreen rendertargets, but disabling it for the default framebuffer (I guess that NSOpenGLView creates an SRGB frambuffer, while the D3D11 and Metal probably create a non-SRGB framebuffer).

I'll enforce that behaviour for in the sokol-gfx GL backend, to properly fix this, an update in sokol_app.h would be needed later (basically an init param for sokol_app.h whether the default framebuffer should be in SRGB format or not).

I'll add a fix to the sokol-gfx GL backend in my merge branch, test on Linux and Windows, and then merge.

PS: I looked around a bit in the GLFW source code, and on macOS it's not possible to configure the SRGB behaviour (it's always enabled), while on other platforms, there are extensions to get SRGB framebuffers. This will be a bit tricky to resolve because sokol-gfx needs to know whether the default framebuffer is SRGB-enabled or not in order to do the right thing.

@floooh floooh merged commit 082d2c6 into floooh:master Jan 26, 2023
@floooh
Copy link
Owner

floooh commented Jan 26, 2023

Ok merged! Many thanks for the PR!

@allcreater allcreater deleted the pixelformat_srgba8_nowgpu branch January 27, 2023 17:17
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