Correctly zero init multiplanar textures#9700
Conversation
| ctx.device | ||
| .poll(wgpu_types::PollType::wait_indefinitely()) | ||
| .unwrap(); | ||
| assert!(buffer.get_mapped_range(..).unwrap().iter().all(|e| *e == 0)); |
There was a problem hiding this comment.
The other tests have a more sophisticated check that reports the error position, it might be worth creating a helper for that and using it here for the sake of anyone who ends up debugging this test.
I think it would be worth checking plane 0 as well, and maybe checking P010 as well. Between subsampling and channel count there are a bunch of ways to get this wrong.
There was a problem hiding this comment.
I forgot... I already added a few tests in zero_init for the cases where one plane is written. Between your test and those, we may have sufficient coverage. (#9753 added expect-fails for those, which will need to be removed in this PR.)
| let mut mip_size = texture_desc.mip_level_size(mip_level).unwrap(); | ||
| // Subsample for multi-planar formats (e.g. NV12/P010 chroma plane is half-res). | ||
| let (w_sub, h_sub) = texture_desc.format.subsampling_factors(aspect.to_plane()); | ||
| (mip_size.width, mip_size.height) = (mip_size.width / w_sub, mip_size.height / h_sub); |
There was a problem hiding this comment.
I suspect this won't be the only place we need to compute the size of a plane, so a plane_size helper would make sense.
There's also #8491, about calculating mip-level sizes for multi-planar textures, which may not be an important use case, but we should either handle it correctly or disallow it, and it might be that plane and mip level size calculation make sense to combine somehow. (The next two lines about rounding to block size might also be able to use the physical_size helper.)
2 participants
Connections
Closes #9688
Conflicts with #9753: resolves expect-fails added by that PR
Description
Previously these were not zero-initialized, now they are.
Testing
New test
Squash or Rebase?
Squash
Checklist
wgpumay be affected behaviorally.CHANGELOG.mdentries for the user-facing effects of this change are present.