neoforged
[RFC] Depth stencil format
Currently, neo patches RenderTarget to use GL_DEPTH32F_STENCIL8 if a mod enables the stencil. Is there a reason this format was chosen over the GL_DEPTH24F_STENCIL8 one? This addition precision might not be needed, and can cause some performance loss.
So in short, why is the format chosen, and does it need to be this one? This probably needs to be asked to mods make use of this feature (I only know of IE).
Quite interesting. I did some patch history digging to figure out the history of the stencil buffer format.
The oldest commit which adds the stencil bits is https://github.com/MinecraftForge/MinecraftForge/commit/ec755e09d07c0aa3ded91cd17812165961707064 from 2013 (see the commit message's linked PR and the PRs linked from there), which has the pixel format set to a depth of 24 bits and 8 stencil bits.
That format -- 24 depth, 8 stencil, as encoded by GL_DEPTH24_STENCIL8_EXT -- was kept all the way through Minecraft Forge (through various changes along the way, as OpenGL/LWJGL use changed and moved about, even a diversion where the constant GL_DEPTH_STENCIL_EXT was accidentally fed to it spawing #1032) up until 1.16.1.
In 1.16.1's patching cycle, specifically commit https://github.com/MinecraftForge/MinecraftForge/commit/ca2ed1ff7a4c74ad67bc0b84dc8d95488939cfa1, the patch was reintroduced to the Framebuffer class (where the stencil was setup before its eventual move to present-day Blaze3D's RenderTarget) but changed from GL_DEPTH24_STENCIL8_EXT[^old] to GL_DEPTH32F_STENCIL8[^new]. That change has persisted and gone unnoticed until now.
Why did the depth change? I can't say for sure, since that commit was more than 4 years ago and I'm not quite sure the porters of the time would remember that one change. My educated guess is the porters at the time sought to update the stencil constant from the old EXTPackedDepthStencil, as the extension was promoted to core in OpenGL 3.0 (as that class' javadocs state), and found GL_DEPTH32F_STENCIL8 first (on line 137, LWJGL 3.3.3) rather than the actual equivalent counterpart GL_DEPTH24_STENCIL8 (on line 325, LWJGL 3.3.3).
Therefore, I conclude it was an unintentional change, and if no pressing reason is found to keep it at the current value, we ought to revert it to the historical value of GL_DEPTH24_STENCIL8 (as soon as convenient) which I think would align with the stencil-disabled depth format.
[^old]: See old patch at https://github.com/MinecraftForge/MinecraftForge/commit/ca2ed1ff7a4c74ad67bc0b84dc8d95488939cfa1#diff-23ca8ccacfbf0fe62d86482aaf5aa954fccead2c694df5141636bcf4fc046386. [^new]: See new patch at https://github.com/MinecraftForge/MinecraftForge/commit/ca2ed1ff7a4c74ad67bc0b84dc8d95488939cfa1#diff-904d63fb952cd7a105f61b801368cfd91a22533d25ea7a2682c9605c008a7449R10
Is there anything that could break with decreasing the precision? If not, a PR can be spun up for 1.21
It could theoretically introduce z-fighting for mods that enable the stencil and relied on the 32-bit precision but I have my doubts...
Mojang now (1.21.5) uses a 32-bit depth format explicitly and we do not want to downgrade that to 24-bit.