Fix initialization tracking for the destination of copyTextureToBuffer
#8099
+158
−69
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
andyleiserson
changed the title
copyTextureToBuffer
andyleiserson
force-pushed
the
more-buf-tex
branch
3 times, most recently
from
d79433f to
7aabc45
Compare
andyleiserson
commented
Aug 18, 2025
CHANGELOG.md
Outdated
| @@ -76,6 +76,7 @@ By @Vecvec in [#7913](https://github.com/gfx-rs/wgpu/pull/7913). | |||
| - The function you pass to `Device::on_uncaptured_error()` must now implement `Sync` in addition to `Send`, and be wrapped in `Arc` instead of `Box`. | |||
| In exchange for this, it is no longer possible for calling `wgpu` functions while in that callback to cause a deadlock (not that we encourage you to actually do that). | |||
| By @kpreid in [#8011](https://github.com/gfx-rs/wgpu/pull/8011). | |||
| - `wgpu` now tracks the initialization status of buffer memory correctly when `copy_texture_to_buffer` skips over padding space between rows or layers, or when the start/end of a texture-buffer transfer is not 4B aligned. By @andyleiserson in [#8099](https://github.com/gfx-rs/wgpu/pull/8099). | |||
There was a problem hiding this comment.
suggestion: It's odd to mention wgpu explicitly, because the premise of this CHANGELOG is that changes almost always apply to wgpu. It seems like we should elide that detail entirely; this would be consistent with other CHANGELOG items.
There was a problem hiding this comment.
I was perhaps too committed to complete sentences and avoiding passive voice. I've changed it to just say "Track the initialization status..."
Previously, the check was skipped if the copy was a single row, which is not correct. The check should be made whenever bytes_per_row is specified. It is permissible not to specify bytes_per_row if the copy is a single row, but if it is specified, it must be aligned. Also removes a redundant check of the `offset` alignment. Since the offset and bytesPerRow alignment checks are not part of "validating linear texture data", I chose to remove that instance of them. These checks are now in `validate_texture_buffer_copy`, which does not correspond 1:1 with the spec, but has a comment explaining how it does correspond.
andyleiserson
force-pushed
the
more-buf-tex
branch
from
7aabc45 to
0a834d2
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR is based on #8095, which is in turn based on #8085.
The main goal of this PR was to fix initialization tracking for buffers that are the destination of a copyT2B operation (#8097). This fixes some CTS tests whose intermittent failure was causing false positives with other changes. I believe it will fix #8021. There is a risk of performance regressions from this change. It would be more performant to use a shader to initialize only the padding areas instead of unnecessarily initializing the entire destination range, but this would be a more involved change.
It was easy to fix the problem with non-4B-aligned initializations (#7947) at the same time, so I did.
There is one additional tweak to the 256B alignment check
bytes_per_rowthat was needed to get theoffset_and_bytesPerRowtest fully passing, at least on Linux.Testing
Enables some CTS tests. A directed test might be reasonable too, but I wanted to go ahead and open this so I can see what CI thinks.
Squash or Rebase? Rebase (after merging #8085 and #8095)
Checklist
cargo fmt.taplo format.cargo clippy --tests. If applicable, add:--target wasm32-unknown-unknowncargo xtask testto run tests.CHANGELOG.mdentry.