-
Notifications
You must be signed in to change notification settings - Fork 1.1k
[dx12,vulkan] Add support for P010 texture format #8086
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
Conversation
I've filed #8088 for the cargo-deny failure. |
3bfcb71
to
acf19d4
Compare
wgpu-hal/src/dx12/adapter.rs
Outdated
hr.is_ok() | ||
&& p010_info | ||
.Support1 | ||
.contains(Direct3D12::D3D12_FORMAT_SUPPORT1_TEXTURE2D) |
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.
We should probably check shader load and sample as well.
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.
Genuine question as I don't know the answer. We don't sample or load directly from this texture, but via R16 and Rg16 views. So is this handled by the Feature::TEXTURE_FORMAT_16BIT_NORM
check? Or do we have to specifically check for this format too? Would checking this perhaps return false even if sampling/loading from R16/Rg16 is actually supported? (I will test this)
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.
That's a good question, and for D3D12, I don't know how to easily find the answer. I saw load and sample are supported (if the format is) in the table here: https://learn.microsoft.com/en-us/windows/win32/direct3ddxgi/format-support-for-direct3d-11-0-feature-level-hardware#dxgi_format_p010v-104 but I don't know for sure if they are needed since as you mentioned, we never bind views of that format to the shader.
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.
Okay, I'll manually test whether my PC supports says it supports loads and samples witht his format. If it does, I'll add the check. If it does not, but given we know it works when loading and sampling from the views, then I'll leave the check out. Sound good?
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 think NV12/P010 formats be used directly as view formats, so I assume the requirements are the intersection of capabilities of the NV12/P010 formats and the capabilities of the view format.
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.
Both CI and my desktop report that they support shader load and sample for P010. so let's include the check. have force pushed an update. LMK if you're happy for me to land once CI comes back green
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.
Looks good!
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.
Besides the one comment, it looks good!
P010 is a 4:2:0 chroma subsampled planar format, similar to NV12. Each component uses 16 bits of storage, of which only the high 10 bits are used. On DX12 this maps to DXGI_FORMAT_P010, and on Vulkan this maps to G10X6_B10X6R10X6_2PLANE_420_UNORM_3PACK16. The existing "nv12" gpu test module has been renamed to "planar_texture", and a new test P010_TEXTURE_CREATION_SAMPLING has been added similar to the existing NV12_TEXTURE_CREATION_SAMPLING. The remaining tests in this module have been converted to validation tests, and now test both NV12 and P010 formats.
acf19d4
to
e774295
Compare
P010 is a 4:2:0 chroma subsampled planar format, similar to NV12. Each component uses 16 bits of storage, of which only the high 10 bits are used. On DX12 this maps to DXGI_FORMAT_P010, and on Vulkan this maps to G10X6_B10X6R10X6_2PLANE_420_UNORM_3PACK16.
The existing "nv12" gpu test module has been renamed to "planar_texture", and a new test P010_TEXTURE_CREATION_SAMPLING has been added similar to the existing NV12_TEXTURE_CREATION_SAMPLING. The remaining tests in this module have been converted to validation tests, and now test both NV12 and P010 formats.
Testing
Added GPU and validation tests. Have manually tested it works on Direct X in my firefox external textures branch
Squash or Rebase?
Squash
Checklist
cargo fmt
.taplo format
.cargo clippy --tests
. If applicable, add:--target wasm32-unknown-unknown
cargo xtask test
to run tests.CHANGELOG.md
entry.