FNA-XNA / FNA3D

FNA3D - 3D Graphics Library for FNA
http://fna-xna.github.io/
Other
287 stars 48 forks source link

Add support for sRGB textures, render targets and backbuffers #85

Closed kg closed 2 years ago

kg commented 3 years ago

Needs an FNA change to expose the new enum value but that's like two lines

kg commented 3 years ago

Can you explain what you mean by state shadowing toggle?

flibitijibibo commented 3 years ago

For example:

uint8_t srgbEnabled; /* Declared in the renderer */

uint8_t newSRGB; /* Determined in the function */
if (newSGRB != renderer->srgbEnabled)
{
    renderer->srgbEnabled = newSRGB;
    ToggleGLState(renderer, GL_FRAMEBUFFER_SRGB_EXT, renderer->srgbEnabled);
}
kg commented 3 years ago

Mostly style but OpenGL in particular needs some work.

It would be cool to have a small test as well, so I can throw together a Metal version of this.

Yeah it's hard to tell from looking at a scene whether sRGB is working, I did a lot of my checking in renderdoc. I should try to come up with a test that verifies we're getting sRGB blending and storage through readback or something

flibitijibibo commented 3 years ago

Converted to a draft for now - D3D11 and OpenGL have a lot of work before being considered ready. The Metal enum is thankfully just the one added enum to the array lookup so that one should be easy.

kg commented 3 years ago

Here for posterity: This PR turns out to be not so useful unless we also add SRGB versions of the block texture formats (DXT1/3/5). Going to figure out how hard that is to add...

kg commented 3 years ago

Added DXT5 SRGB variant, not sure it's worth adding 1/3 (I can do it if you want). Tested that in Vulkan and D3D11 but it seems to be wonky in OpenGL...

flibitijibibo commented 3 years ago

We can skip 1/3 I think.

D3D and Vulkan look a whole hell of a lot better now, OpenGL I'm still a bit worried about the backbuffer assumption (and it sounds like GL needs more stability fixes anyhow).

As a quick reminder we still need to incorporate buildfixes for Metal, it should just be adding a whopping 2 enums but maybe the block size stuff has changed that.

kg commented 3 years ago

I think the GL issues are actually just due to my use of block textures, because my block texture uploads are producing validator warnings in Vulkan. Hopefully once we get the validator warnings figured out it will indicate what I'm doing wrong and GL will turn out to be working... if it's not I'll keep digging in there.

flibitijibibo commented 2 years ago

Metal has been cut, so this should be rebased.

kg commented 2 years ago

I rebased + squished and then cleaned up the indentation after using .editorconfig to set my indent level to 8. Will do some testing locally to see if anything is weird.

kg commented 2 years ago

The OpenGL implementation broke since the last time I messed with it, so I need to figure out what's going on there. I got an INVALID_VALUE error once in VS that doesn't happen under Renderdoc, and it seems like the relevant sRGB flags get flipped at random while things are running. :(

D3D11 and Vulkan seem fine, though I'm having issues with one of my tests that have me a bit confused about what's going on with rendertargets...

kg commented 2 years ago

Anyone wanting to test this with FNA proper can use https://github.com/sq/FNA/tree/srgb. The rendertarget issues were just me forgetting to change the landmine if-statement in Texture2D that tricks me every time I mess with sRGB.

kg commented 2 years ago

The INVALID_VALUE in GL was from me using block textures, it doesn't happen if I switch them off and those work right in D3D11 and Vulkan. Something about what's expected for DXT block sizes in GL is weird - if someone else can run tests there that would be appreciated. I'm still seeing some weirdness where the framebuffer sRGB flag seems to get ignored sometimes in OpenGL and then it suddenly applies and sticks around.

kg commented 2 years ago

OK, cleanup should be done