Fix Queue::write_texture, Fix DX12 write_texture_subset_2d and re-enable the test. #4990

dtzxporter · 2024-01-04T22:29:47Z

Queue::write_texture calculated the rows_per_image incorrectly for block compressed textures. This causes the DX12 backend to fail when uploading a block compressed image. Enabled the write_texture_subset_2d test now that it passes.

Connections
Closes #3992
Closes #3072

Description
Queue::write_texture fails for block compressed textures on DX12.

Testing
Enabled the write_texture_subset_2d test now that it passes.

Checklist

Run cargo fmt.
Run cargo clippy. If applicable, add:
- --target wasm32-unknown-unknown
- --target wasm32-unknown-emscripten
Run cargo xtask test to run tests.
Add change to CHANGELOG.md. See simple instructions inside file.

cwfitzgerald · 2024-01-04T22:43:43Z

We had a PR very similar that was abandoned - does this solve the same problem? #3992 (issue #3072)

Either way would it be possible to write a small test for this case?

dtzxporter · 2024-01-04T22:49:14Z

I believe that the root cause is a bad calculation here:

(Should also be height_blocks) as rows_per_image doc says it's in blocks.
https://github.com/gfx-rs/wgpu/blob/trunk/wgpu-core/src/device/queue.rs#L739C13-L743

It just happens that the DX12 backend used rows_per_image and the vulkan backend doesn't seem to, so this leads to the crash. I'm happy to fix whatever here, and or accept a fix to both changes to make the DX12 backend match the other backends.

cwfitzgerald · 2024-01-04T23:02:28Z

Definitely whatever you're willing to do - definitely the more fixed the merrier!

Seems to be a CI failure on DX12 though.

Fix row calculation in `Queue::write_texture`. The D3D12_SUBRESOURCE_FOOTPRINT was calculated incorrectly by multiplying the height by the block count. DirectX automatically takes format into account because it's passed into the footprint. Only the block width is necessary to compute the pitch or linear size. This prevents a crash when using `Queue::write_texture` with a block compressed texture.

dtzxporter · 2024-01-04T23:56:22Z

All tests should pass now, and I enabled the write_texture_subset_2d test now that it passes on DX12!

…a54af3aa12d. r=webgpu-reviewers,supply-chain-reviewers,ErichDonGubler Changelog * #4927 BGL Weak Pointer Deduplication Pool By cwfitzgerald in gfx-rs/wgpu#4927 * #4958 Change examples page menu on smaller screens By Dinnerbone in gfx-rs/wgpu#4958 * #4950 Bump anyhow from 1.0.77 to 1.0.78 By dependabot[bot] in gfx-rs/wgpu#4950 * #4957 Disable Linux Tests By cwfitzgerald in gfx-rs/wgpu#4957 * #4960 Fix incorrect ConfigureSurfaceError::TooLarge message By Dinnerbone in gfx-rs/wgpu#4960 * #4935 Add `cfg_aliases` to `wgpu` By daxpedda in gfx-rs/wgpu#4935 * #4939 hello_compute: check for missing command-line args By vilcans in gfx-rs/wgpu#4939 * #4948 Bump winit from 0.29.6 to 0.29.8 By dependabot[bot] in gfx-rs/wgpu#4948 * #4944 Fix xtask wasm-bindgen install By rukai in gfx-rs/wgpu#4944 * #4858 [glsl-in] fix swizzle in global const context By teoxoy in gfx-rs/wgpu#4858 * #4968 [gl] fix RGBA8 format capabilities By teoxoy in gfx-rs/wgpu#4968 * #4947 Avoid allocating during queue submit By udoprog in gfx-rs/wgpu#4947 * #4965 Bump serde from 1.0.193 to 1.0.194 By dependabot[bot] in gfx-rs/wgpu#4965 * #4975 Fix Hang in Multithreaded Compute Test By cwfitzgerald in gfx-rs/wgpu#4975 * #4966 Bump anyhow from 1.0.78 to 1.0.79 By dependabot[bot] in gfx-rs/wgpu#4966 * #4978 Bump thiserror from 1.0.52 to 1.0.56 By dependabot[bot] in gfx-rs/wgpu#4978 * #4979 Bump syn from 2.0.46 to 2.0.47 By dependabot[bot] in gfx-rs/wgpu#4979 * #4977 Use Custom Mesa for Building By cwfitzgerald in gfx-rs/wgpu#4977 * #4981 Bump serde_json from 1.0.108 to 1.0.110 By dependabot[bot] in gfx-rs/wgpu#4981 * #4959 wgpu-hal: Fix Mesa version check for version with suffix containing `.` By ids1024 in gfx-rs/wgpu#4959 * #4976 Shorten Lock Lifetimes By cwfitzgerald in gfx-rs/wgpu#4976 * #4980 Pin DXC and Vulkan SDK version By cwfitzgerald in gfx-rs/wgpu#4980 * #4974 gles: use already existing debug__fn private capabilty instead of checking extensions By valaphee in gfx-rs/wgpu#4974 * #4987 Remove Mac CI By cwfitzgerald in gfx-rs/wgpu#4987 * #4990 Fix Queue::write_texture, Fix DX12 write_texture_subset_2d and re-enable the test. By dtzxporter in gfx-rs/wgpu#4990 * #4994 Bump syn from 2.0.47 to 2.0.48 By dependabot[bot] in gfx-rs/wgpu#4994 * #4993 Bump serde_json from 1.0.110 to 1.0.111 By dependabot[bot] in gfx-rs/wgpu#4993 * #4992 Bump gpu-allocator from 0.24.0 to 0.25.0 By dependabot[bot] in gfx-rs/wgpu#4992 * #4995 Add Verbosity Flags to wgpu-info By cwfitzgerald in gfx-rs/wgpu#4995 * #4996 Dependency Update Rollup By cwfitzgerald in gfx-rs/wgpu#4996 * #4954 Put raw texture access behind snatch guards By nical in gfx-rs/wgpu#4954 * #4954 Put raw texture access behind snatch guards By nical in gfx-rs/wgpu#4954 * #4969 Texture snatching By nical in gfx-rs/wgpu#4969 * #4969 Texture snatching By nical in gfx-rs/wgpu#4969 Differential Revision: https://phabricator.services.mozilla.com/D197786

…a54af3aa12d. r=webgpu-reviewers,supply-chain-reviewers,ErichDonGubler Changelog * #4927 BGL Weak Pointer Deduplication Pool By cwfitzgerald in gfx-rs/wgpu#4927 * #4958 Change examples page menu on smaller screens By Dinnerbone in gfx-rs/wgpu#4958 * #4950 Bump anyhow from 1.0.77 to 1.0.78 By dependabot[bot] in gfx-rs/wgpu#4950 * #4957 Disable Linux Tests By cwfitzgerald in gfx-rs/wgpu#4957 * #4960 Fix incorrect ConfigureSurfaceError::TooLarge message By Dinnerbone in gfx-rs/wgpu#4960 * #4935 Add `cfg_aliases` to `wgpu` By daxpedda in gfx-rs/wgpu#4935 * #4939 hello_compute: check for missing command-line args By vilcans in gfx-rs/wgpu#4939 * #4948 Bump winit from 0.29.6 to 0.29.8 By dependabot[bot] in gfx-rs/wgpu#4948 * #4944 Fix xtask wasm-bindgen install By rukai in gfx-rs/wgpu#4944 * #4858 [glsl-in] fix swizzle in global const context By teoxoy in gfx-rs/wgpu#4858 * #4968 [gl] fix RGBA8 format capabilities By teoxoy in gfx-rs/wgpu#4968 * #4947 Avoid allocating during queue submit By udoprog in gfx-rs/wgpu#4947 * #4965 Bump serde from 1.0.193 to 1.0.194 By dependabot[bot] in gfx-rs/wgpu#4965 * #4975 Fix Hang in Multithreaded Compute Test By cwfitzgerald in gfx-rs/wgpu#4975 * #4966 Bump anyhow from 1.0.78 to 1.0.79 By dependabot[bot] in gfx-rs/wgpu#4966 * #4978 Bump thiserror from 1.0.52 to 1.0.56 By dependabot[bot] in gfx-rs/wgpu#4978 * #4979 Bump syn from 2.0.46 to 2.0.47 By dependabot[bot] in gfx-rs/wgpu#4979 * #4977 Use Custom Mesa for Building By cwfitzgerald in gfx-rs/wgpu#4977 * #4981 Bump serde_json from 1.0.108 to 1.0.110 By dependabot[bot] in gfx-rs/wgpu#4981 * #4959 wgpu-hal: Fix Mesa version check for version with suffix containing `.` By ids1024 in gfx-rs/wgpu#4959 * #4976 Shorten Lock Lifetimes By cwfitzgerald in gfx-rs/wgpu#4976 * #4980 Pin DXC and Vulkan SDK version By cwfitzgerald in gfx-rs/wgpu#4980 * #4974 gles: use already existing debug__fn private capabilty instead of checking extensions By valaphee in gfx-rs/wgpu#4974 * #4987 Remove Mac CI By cwfitzgerald in gfx-rs/wgpu#4987 * #4990 Fix Queue::write_texture, Fix DX12 write_texture_subset_2d and re-enable the test. By dtzxporter in gfx-rs/wgpu#4990 * #4994 Bump syn from 2.0.47 to 2.0.48 By dependabot[bot] in gfx-rs/wgpu#4994 * #4993 Bump serde_json from 1.0.110 to 1.0.111 By dependabot[bot] in gfx-rs/wgpu#4993 * #4992 Bump gpu-allocator from 0.24.0 to 0.25.0 By dependabot[bot] in gfx-rs/wgpu#4992 * #4995 Add Verbosity Flags to wgpu-info By cwfitzgerald in gfx-rs/wgpu#4995 * #4996 Dependency Update Rollup By cwfitzgerald in gfx-rs/wgpu#4996 * #4954 Put raw texture access behind snatch guards By nical in gfx-rs/wgpu#4954 * #4954 Put raw texture access behind snatch guards By nical in gfx-rs/wgpu#4954 * #4969 Texture snatching By nical in gfx-rs/wgpu#4969 * #4969 Texture snatching By nical in gfx-rs/wgpu#4969 Differential Revision: https://phabricator.services.mozilla.com/D197786 UltraBlame original commit: 17af24495b14543eed7f3cfd1434a961d879f47e

dtzxporter requested a review from a team as a code owner January 4, 2024 22:29

dtzxporter force-pushed the fix-dx12-copy-texture-resource branch from 3459d64 to a3577f2 Compare January 4, 2024 23:23

dtzxporter changed the title ~~The D3D12_SUBRESOURCE_FOOTPRINT is calculated incorrectly for block compressed textures.~~ Fix Queue::write_texture, Fix DX12 write_texture_subset_2d and re-enable the test. Jan 4, 2024

dtzxporter added 2 commits January 4, 2024 18:52

Enable write_texture_subset_2d test for DX12 now that it passes.

4e75e5f

dtzxporter force-pushed the fix-dx12-copy-texture-resource branch from a3577f2 to 4e75e5f Compare January 4, 2024 23:54

cwfitzgerald approved these changes Jan 5, 2024

View reviewed changes

cwfitzgerald merged commit 00cf05c into gfx-rs:trunk Jan 5, 2024
26 checks passed

dtzxporter deleted the fix-dx12-copy-texture-resource branch January 5, 2024 00:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix Queue::write_texture, Fix DX12 write_texture_subset_2d and re-enable the test. #4990

Fix Queue::write_texture, Fix DX12 write_texture_subset_2d and re-enable the test. #4990

dtzxporter commented Jan 4, 2024 •

edited by cwfitzgerald

Loading

cwfitzgerald commented Jan 4, 2024

dtzxporter commented Jan 4, 2024

cwfitzgerald commented Jan 4, 2024

dtzxporter commented Jan 4, 2024

Fix Queue::write_texture, Fix DX12 write_texture_subset_2d and re-enable the test. #4990

Fix Queue::write_texture, Fix DX12 write_texture_subset_2d and re-enable the test. #4990

Conversation

dtzxporter commented Jan 4, 2024 • edited by cwfitzgerald Loading

cwfitzgerald commented Jan 4, 2024

dtzxporter commented Jan 4, 2024

cwfitzgerald commented Jan 4, 2024

dtzxporter commented Jan 4, 2024

dtzxporter commented Jan 4, 2024 •

edited by cwfitzgerald

Loading