DX12: Align copies b/w textures and buffers when `D3D12_FEATURE_DATA_D3D12_OPTIONS13.UnrestrictedBufferTextureCopyPitchSupported` is `false`
Connections
- Likely resolves #5285.
- A case we need to handle in order to fix #4150.
Testing
- TODO: Will prove that this works even when forced to align. This won't definitively prove that we're fixing cases that need this, but it will prove that we're not breaking anything with intermediate buffers.
Squash or Rebase?
rebase plz
Checklist
- [ ] If this contains user-facing changes, add a
CHANGELOG.mdentry.
I'm...honestly not sure why this is failing on the wgpu_gpu::regression::issue_6827::test_scatter test entry in CI. I think I need some help. Marking as ready for review; perhaps @teoxoy or @cwfitzgerald could help me out?
The test is failing because of:
Panic: GraphicsCommandList::close failed: The parameter is incorrect. (0x80070057)
I'd run the test locally with the validation layers enabled to see if there is more info. I've seen the error come up in cases where we didn't uphold some D3D12 invariant while encoding.
@teoxoy and I spent some time enabling debug layers, and even forcing the same version of WARP that CI is using in the test that's failing with only a WARP adapter. We're not able to reproduce this locally on either of our machines. 😓
I wonder if this is a windows 10 runtime thing. There's something going wrong in the runtime on Win10, but Win11's runtime deals with it fine. We would need to investigate setting up the Agility SDK
Got the CI issue resolved! I was definitely doing something wrong. 👉🏻👈🏻 From community chat:
@ErichDonGubler:
I just paired with Teo, and we actually FIGGERED IT OUT!
@teoxoy and I sideloaded a bunch of DLLs from an ancient Windows 10 computer I had, which happened to have a WARP adapter that did not support low-alignment buffer-to-texture copies.
...turns out I accidentally swapped the alignment requirement branches. 😅
We had to work around:
- Not having the right DLLs (because new versions have lower alignment needs) and stealing them from an old machine I happened to have around.
- Crashing when trying to compile indirect buffer validation shaders (because old versions?)
- Feature unification working against us when trying to use WGPU_VALIDATION_INDIRECT_CALL=0
- Erich being dumb and necessitating debugging in the first place
It was an hours-long adventure, lemmetellyawhat.
@teoxoy:
It was fun :)
@cwfitzgerald:
oh boy!
that sounds like an adventure indeed
Doing some branch pushes to check that webgpu:api,operation,command_buffer,image_copy:undefined_params:* fails before and after, and hopefully that it catches the bug @teoxoy found with region offsets being swapped on the tex-to-buf path. 🤞🏻
Alright, so:
-
https://github.com/erichdongubler-mozilla/wgpu/actions/runs/18361970647/job/52307306029 is based on
trunk, and has 4 failures, including some in2dcopies and3dcopies. -
https://github.com/erichdongubler-mozilla/wgpu/actions/runs/18361971738/job/52307309175 is based on this PR's contents before the fix Teo identified for offsets in tex-to-buf. There are still
2dfailures here, but fewer. -
https://github.com/gfx-rs/wgpu/actions/runs/18362103668/job/52307694538?pr=7721 implements Teo's fix, and confirms that things get greener. Yay! Unfortunately, there are still breakages with
3dcopies, so this whole thing isn't passing yet, but I'd say this is a nice confirmation that we're fixing something.
I'll figure out the exact set of CTS tests to use tomorrow. If nothing else, I can use a (verbosely requested) subset of webgpu:api,operation,command_buffer,image_copy:undefined_params:*.
I added baseline expectations (plus failures) for webgpu:api,operation,command_buffer,image_copy:undefined_params:* to our CTS list with 932567d242135677e779e06303f48bca8f46c007. I have amended the current tip of the PR to remove fails-if(dx12) from some of the 2d variants of the added tests, thus proving that we fix something with this PR. Nice!
Oh, is a backport of this to the 25/26/27 series, and corresponding patch releases feasible? 👀 It certainly would be appreciated IMHO!
@torokati44: Two things:
- For optional fixes like this, we generally expect the community to handle backporting effort (which should be light here, since you can omit the test coverage). So, if you're willing, then we look forward to a PR from you against
v26andv27. 😄 - In our maintainers' meeting today, we know we're willing to field backports to v26 (esp. for Bevy) and v27. Was there a reason you wanted to backport to v25, specifically?
@ErichDonGubler:
-
Alright, I'll see what I can do w.r.t backporting PRs.
-
No, not really, anymore, since https://github.com/ruffle-rs/ruffle/pull/21730 got merged (but not yet https://github.com/ruffle-rs/ruffle/pull/21895)