-
Notifications
You must be signed in to change notification settings - Fork 498
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
sokol_gfx.h: add explicit depth / stencil formats #1124
base: master
Are you sure you want to change the base?
Conversation
case SG_PIXELFORMAT_RGBA32F: return DXGI_FORMAT_R32G32B32A32_FLOAT; | ||
case SG_PIXELFORMAT_DEPTH: return DXGI_FORMAT_R32_TYPELESS; | ||
case SG_PIXELFORMAT_DEPTH_STENCIL: return DXGI_FORMAT_R24G8_TYPELESS; | ||
case SG_PIXELFORMAT_DEPTH24PLUS: return DXGI_FORMAT_R24G8_TYPELESS; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I couldn't find a DirectX format that mapped to a 24-bit depth buffer with no stencil - so SG_PIXELFORMAT_DEPTH24PLUS
is equivalent to SG_PIXELFORMAT_DEPTH24PLUS_STENCIL8
. Is this the correct approach, or should this format not be supported at all on DirectX?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know either but will also try to find more info on that (and also test on Windows).
@@ -11693,6 +11768,10 @@ _SOKOL_PRIVATE MTLPixelFormat _sg_mtl_pixel_format(sg_pixel_format fmt) { | |||
case SG_PIXELFORMAT_RGBA32F: return MTLPixelFormatRGBA32Float; | |||
case SG_PIXELFORMAT_DEPTH: return MTLPixelFormatDepth32Float; | |||
case SG_PIXELFORMAT_DEPTH_STENCIL: return MTLPixelFormatDepth32Float_Stencil8; | |||
case SG_PIXELFORMAT_DEPTH24PLUS: return MTLPixelFormatDepth24Unorm_Stencil8; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is similar to the problem on DirectX, there is no format that maps directly to just a 24-bit depth buffer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
...I wonder if this is actually the reason why WebGPU calls it 'Depth24Plus'... as with D3D11, I'll check and test... if there's not pure Depth24 format I think it's ok to have the unused stencil buffer bits hanging around (it kinda makes sense because 24 bits are 3 bytes, so there would be 8 unused bits anyway).
5255953
to
f24e71d
Compare
Moving this into review now that the other formats have been added. See the comments above - I'm not sure how to handle |
Sorry, didn't get around yet to look into the PR. I'll try to make some time over the weekend :) I'll most like need to do a little research first for the other 3D backends. There's also always the option to split support for the different backends over multiple smaller PRs, might speed things up a bit :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I finally made time to look into this PR. Will do some tests in a local merge branch across D3D11, GL, Metal, WebGL and WebGPU and figure out what to do about those pure Depth24 formats.
case SG_PIXELFORMAT_RGBA32F: return DXGI_FORMAT_R32G32B32A32_FLOAT; | ||
case SG_PIXELFORMAT_DEPTH: return DXGI_FORMAT_R32_TYPELESS; | ||
case SG_PIXELFORMAT_DEPTH_STENCIL: return DXGI_FORMAT_R24G8_TYPELESS; | ||
case SG_PIXELFORMAT_DEPTH24PLUS: return DXGI_FORMAT_R24G8_TYPELESS; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know either but will also try to find more info on that (and also test on Windows).
@@ -11693,6 +11768,10 @@ _SOKOL_PRIVATE MTLPixelFormat _sg_mtl_pixel_format(sg_pixel_format fmt) { | |||
case SG_PIXELFORMAT_RGBA32F: return MTLPixelFormatRGBA32Float; | |||
case SG_PIXELFORMAT_DEPTH: return MTLPixelFormatDepth32Float; | |||
case SG_PIXELFORMAT_DEPTH_STENCIL: return MTLPixelFormatDepth32Float_Stencil8; | |||
case SG_PIXELFORMAT_DEPTH24PLUS: return MTLPixelFormatDepth24Unorm_Stencil8; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
...I wonder if this is actually the reason why WebGPU calls it 'Depth24Plus'... as with D3D11, I'll check and test... if there's not pure Depth24 format I think it's ok to have the unused stencil buffer bits hanging around (it kinda makes sense because 24 bits are 3 bytes, so there would be 8 unused bits anyway).
PS: don't worry about the merge conflict in CHANGELOG.md, I'll take care of that in my merge branch. |
Ok, some Metal info: The (I guess that's another reason why WebGPU calls those formats |
...and on D3D it's a bit more complicated in Dawn:
...same in the Vulkan backend, WebGPU's cross-platform features are very well researched, so when they do this there's must be a very good reason. TBH, I wonder if it's worth the trouble adding explicit The only advantage seems to be that on some platforms depth + stencil can be packed into 4 bytes (e.g. Depth24Stencil8) instead of taking 8 bytes (Depth32FloatStencil8). But for what is the stencil buffer actually useful these days? :) I would suggest:
...I guess I was at that point before already a couple of years ago, and that's why there's only the generic ...looking at the current sokol_gfx.h code,
...this different mapping would be the only good reason to have explicit pixel formats for |
@kcbanner ^^^ discuss :) |
Closes #1122.