NeoForge
NeoForge copied to clipboard
Unnecessary `useCombinedDepthStencilAttachment` client config value
The config value can be found here.
The only place it used in NeoForge is here.
To me to is strange that the user should be able to configure this. There is no case where changing the config value can resolve a hardware incompatibility. The config also affects all framebuffers. It would seem more appropriate to be able to configure this per-framebuffer as a parameter to enableStencil
.
However, even that is not necessary. A GL documentation page for glFramebufferTexture2D
here says the following.
Attaching a level of a texture to
GL_DEPTH_STENCIL_ATTACHMENT
is equivalent to attaching that level to both theGL_DEPTH_ATTACHMENT
and theGL_STENCIL_ATTACHMENT
attachment points simultaneously.
Therefore, the config actually does nothing, as both code paths have the same result. In my opinion, the config value should be removed and the separate attachment approach should always be used.
This was added by @gigaherz in https://github.com/neoforged/NeoForge/commit/8e5138da2757c1ac71d104180bd0984fbcfaffb7
I don't see a related PR, so he'll have to comment on why it exists.
I can't remember all the details about this, but there was some reason why the combined code path was crashing some GPUs. I'll try to find discord logs later.
The conversation happened on a private channel. Link: https://discord.com/channels/313125603924639766/584953713060347914/728399383112187906
It appears that it actively errored in my PC, and separating the attachments fixed the error. I do not know what reason there was for it, only that separating the attachment fixed the error.
@gigaherz @PepperCode1 So what is the stance? Should we remove the config and stuff and see if it errors at all for anyone?