gboisse / gfx

A minimalist and easy to use graphics API.
MIT License
498 stars 35 forks source link

Add support for changing depth function #78

Closed maoliver-amd closed 1 year ago

maoliver-amd commented 1 year ago

Adds gfxDrawStateSetDepthFunction to support setting alternate depth comparison functions on draw states.

Since textures and drawstates are independent there is no way to know which is the correct value to clear depth textures. So an optional parameter was added during texture creation that can be used to set the clear value.

gboisse commented 1 year ago

Mmh I think I'd rather we set the clear color (optionally) when creating the texture, since otherwise we'll be spamming the console output with warnings, what do you think?

Also I think it should be a float[4] (for depth textures, we'd simply take the first value in the array).

maoliver-amd commented 1 year ago

It didn't seem immediately obvious how to associate a clear value with a texture (apart from add another member). I also wasnt sure if there was some usecase wherein a texture might be cleared to different values at different points during rendering a frame. Either way fell free to change it however you feel works best. Im not sure what you mean by spamming the console though

gboisse commented 1 year ago

Ah sorry, yea so in D3D12 you have to supply the clear color at creation time. You're free to pick another one at "clear time" afterwards, although that'll get picked up by the debug layer, which will print out some "performance warning" message every single time such a clear happens, therefore spamming the console output.

That's the reason why the clear color hasn't been exposed so far... (explicit APIs are such a PITA!)

maoliver-amd commented 1 year ago

Ahh, OK then, thats a problem. Ignoring the last commit on this PRs branch just hard changes the default depth test to inverted-Z. If we want a configurable way to do it then im open to options. Hardcoding to inverted is the easiest, adding a preprocessor define to toggle default behaviour is the next easiest. Allowing it to be changed via runtime api will need some way to set the clear value on textures during creation. I cant find any way to retrieve what the clear value was using d3d api which means it would need to be manually stored. Adding a float4 member variable to all gfxTextures seems heavy, I have seen engines use an enum flag instead that has some basic variations (clear to 0, clear to 1 etc.) thats a bit simpler

maoliver-amd commented 1 year ago

Changed so that the clear value is passed during texture creation and is held internally in texture objects

gboisse commented 1 year ago

Clear value logic was pretty broken as the user-supplied value (or lack of) was never set to the internal texture storage. Fixed it in: https://github.com/gboisse/gfx/commit/eb587ccfb60a8a4f0d70df0b1bf801e4eb593e17.

maoliver-amd commented 1 year ago

Clear value logic was pretty broken as the user-supplied value (or lack of) was never set to the internal texture storage. Fixed it in: eb587cc.

Hmm, not sure what happened here, I thought I had tested everything.