MagicBeanDev/Longshot

Watch

Fork 98

fix(vk): keep a copy into BC3 to whole blocks or the level's edge #672

Open

jcabarca wants to merge 1 commit from jcabarca/Longshot:fix/vk-bc-copy-extent into main

jcabarca commented

2026-11-15 03:32:28 +00:00

First-time contributor

Type: fix · Risk: low · Tested on: Windows, AMD Radeon RX 9070 XT · Needs: none

Why

A copy from an ASTC 6x6 or 10x10 texture into a BC3 texture copied the encoded blocks in with the region's own size. When that size is not whole 4x4 blocks and the region stops before the level's edge, the copy breaks VUID-vkCmdCopyBufferToImage-dstImage-00207 / 00208, and what a driver does with it is undefined (blocks written in the wrong place, or the copy dropped).

How to reproduce

Seen on: not seen in the game: found by reading texture_copy_into_bc() in gpu/vk/vk_texture.cpp; it can happen whenever a game copies a region of an ASTC 6x6 or 10x10 texture whose size is not a multiple of 4 into the middle of a BC3 texture
Settings and switches: defaults
Steps: game-free: python -I -B tests/test_mem_trim.py on main (copy the test from this PR); it reads texture_copy_into_bc() in vk_texture.cpp
Expected: a region the BC3 copy cannot write as whole blocks is not copied with an invalid extent
Actual: on main it is encoded and copied in with its own extent; the test fails

What changed

texture_copy_into_bc() now returns false, before it takes the BC scratch, for a region into a BC3 texture that is not resized when its width is not a multiple of 4 and does not reach the level's right edge, or its height is not a multiple of 4 and does not reach the level's bottom edge. A false there already means "not done": the caller (vk_xfer.cpp, the copy into a BC3-kept texture) then skips the copy with the warning "texture copy into or out of a BC3-kept ASTC texture it cannot make: skipped", as it does for every other case this function refuses (offsets not on a block, a region bigger than the scratch, a resized destination that is not a whole level). The code comment's "left to the copy's other paths" means that caller.

vk: texture_copy_into_bc() in gpu/vk/vk_texture.cpp: if (!dst->bc_resized && (((w & 3) && dx + w != ow) || ((h & 3) && dy + h != oh))) return false; before bc_scratch_layout(). Resized BC3 destinations already take only whole levels (checked just above) and are unchanged.
tests: tests/test_mem_trim.py (extended): the check of texture_copy_into_bc() now also needs this guard, before bc_scratch_layout(.

The bug, step by step:

texture_copy_into_bc() copies a w x h region of an ASTC texture into a BC3 texture at dx, dy: it copies the region into the scratch, encodes it to BC3 blocks in a buffer (bc_encode_into), then copies the buffer into the BC3 image.
It already needs dx and dy on a 4x4 block ((dx & 3) || (dy & 3) returns false), but not w and h.
ASTC 6x6 and 10x10 regions are often 6, 10, 30... texels wide: not whole 4x4 blocks.
bc_encode_into copies the blocks with VkBufferImageCopy{..., {dx, dy, 0}, {w, h, 1}}. With w not a multiple of 4 and dx + w short of the level's width (or the same for h), that copy breaks the two rules below.

The rules, from Khronos validusage.json:

VUID-vkCmdCopyBufferToImage-dstImage-00207: "For each element of pRegions, if VkCopyCommandTransformInfoQCOM::transform is equal to VK_SURFACE_TRANSFORM_IDENTITY_BIT_KHR, the sum of imageOffset.x and extent.width does not equal the width of the subresource specified by imageSubresource, extent.width must be a multiple of the texel block extent width of the VkFormat of dstImage"
VUID-vkCmdCopyBufferToImage-dstImage-00208: "For each element of pRegions, if VkCopyCommandTransformInfoQCOM::transform is equal to VK_SURFACE_TRANSFORM_IDENTITY_BIT_KHR, and the sum of imageOffset.y and extent.height does not equal the height of the subresource specified by imageSubresource, extent.height must be a multiple of the texel block extent height of the VkFormat of dstImage"

The copy that breaks them, in bc_encode_into, unchanged:

const VkBufferImageCopy c{0, 0, 0, {VK_IMAGE_ASPECT_COLOR_BIT, level, layer, n}, {dx, dy, 0}, {w, h, 1}};
vkCmdCopyBufferToImage(cmd, bc_out, dst.image, VK_IMAGE_LAYOUT_GENERAL, 1, &c);

Before:

	}
	if (!bc_scratch_layout(cmd, dst->bc_resized)) return false;

After:

	}
	// (the blocks are copied in with the region's extent: whole 4x4 blocks, or reaching the level's edge. A region of
	// another size, from an ASTC 6x6 or 10x10 source, is left to the copy's other paths)
	if (!dst->bc_resized && (((w & 3) && dx + w != ow) || ((h & 3) && dy + h != oh))) return false;
	if (!bc_scratch_layout(cmd, dst->bc_resized)) return false;

Start reading at bool texture_copy_into_bc( in vk_texture.cpp, then bc_encode_into near the top of the file.

Measurements

Verdict: it fixes the bug: tests/test_mem_trim.py fails on main (a BC3 copy region that is not whole blocks and does not reach the level's edge is copied in) and passes with this PR, in both interleaved runs. No speed change is expected: one comparison before the copy; a refused region is skipped, as other refused regions already are.

Hardware: AMD Radeon RX 9070 XT, Windows; the test itself is game-free (it reads source, it does not use the GPU); Python 3.12
Scene: none, game-free
Method: tests/test_mem_trim.py from this PR against each side's source (a checkout of each commit, the PR's test copied into main's), 2 runs each, interleaved (main, PR, main, PR); the result is pass/fail, so the median is the result both runs gave
How to reproduce: python -I -B tests/test_mem_trim.py in a checkout of each side (copy the test into main's)

	Before: main @ `82886e89`	After: this PR @ `5ed443b9`
`tests/test_mem_trim.py`, run 1	fails	passes
`tests/test_mem_trim.py`, run 2	fails	passes
Median	fails	passes

run 1, main @ 82886e89:
a BC3 copy region that is not whole blocks and does not reach the level's edge is copied in
exit 1
run 1, this PR @ 5ed443b9:
ok
exit 0
run 2, main @ 82886e89:
a BC3 copy region that is not whole blocks and does not reach the level's edge is copied in
exit 1
run 2, this PR @ 5ed443b9:
ok
exit 0

What the test checks (the new part):

In the body of bool texture_copy_into_bc(, the guard if (!dst->bc_resized && (((w & 3) && dx + w != ow) || ((h & 3) && dy + h != oh))) return false; must be present.
It must come before bc_scratch_layout(, so a refused region takes no scratch and records no copy.
The test's other checks of this file and of vk_memory.cpp are unchanged and still pass.

Flags and settings

None.

Testing

Tested: on this branch, Windows, Python 3.12: test_mem_trim passes (fails on main), and the other checks that read the BC3 copy and upload pass: test_bc_upload_retry, test_astc_bc_policy, test_parallel_init_order.

Built and played: this commit builds on Windows (llvm-mingw back end) and on Linux (longshot_vk.so, without and with AMD FSR 4). Played on Windows, AMD Radeon RX 9070 XT: the game runs normally.

Not tested: such a copy in the game: not checked in the game (it needs an ASTC 6x6 or 10x10 region whose size is not a multiple of 4 copied into the middle of a BC3 texture). Linux was built, not run.

Must be tested: None: the change is the same on every GPU and OS, and the game-free test covers it.

Risks

What could break: a region refused here is no longer copied at all (the caller skips it with its warning), so those texels of the BC3 texture keep what they had. Before, the same region was copied with an extent the rules forbid, so what landed there was undefined anyway. Copies whose size is whole blocks or that reach the level's edge, and copies into resized BC3 textures, are unchanged.

Turning it off: revert the commit.

Compatibility: Nothing changes.

Checklist

Everyone

One topic.
The title is a Conventional Commit (type(scope): description) of the status line's type.
Why gives the reason plainly and links the issues that fit (the tracker searched). (no issue fits)
Measurements has a verdict, or one line saying why there is nothing to measure.
Testing names what was tested, what wasn't, and what must still be tested, where and how.
Rebased on current main.
python -I -B tests/run_all.py passes (any failure that also fails on main is named). (the checks that read the changed code, on this PR, Windows: all pass)
No game files, dumps, build output or generated files.
third_party/ untouched (a new folder with a PROVENANCE.md instead).
The license is OK: GPL-3.0-or-later, nothing incompatible and nothing decompiled.

When it applies

AI assistance recorded with Assisted-by: / Co-Authored-By: trailers.
A game-free test added for the fix, failing on main and passing here. (tests/test_mem_trim.py extended)
A check under tests/ that reads changed source updated in this PR, not weakened. (tests/test_mem_trim.py: one check added, none removed)
Host code builds on both Windows (llvm-mingw) and Linux, with UTF-8 paths. (built on Windows with llvm-mingw and on Linux, longshot_vk.so without and with AMD FSR 4; no OS-specific code or paths)

**Type:** fix · **Risk:** low · **Tested on:** Windows, AMD Radeon RX 9070 XT · **Needs:** none ## Why A copy from an ASTC 6x6 or 10x10 texture into a BC3 texture copied the encoded blocks in with the region's own size. When that size is not whole 4x4 blocks and the region stops before the level's edge, the copy breaks VUID-vkCmdCopyBufferToImage-dstImage-00207 / 00208, and what a driver does with it is undefined (blocks written in the wrong place, or the copy dropped). ## How to reproduce **Seen on:** not seen in the game: found by reading `texture_copy_into_bc()` in `gpu/vk/vk_texture.cpp`; it can happen whenever a game copies a region of an ASTC 6x6 or 10x10 texture whose size is not a multiple of 4 into the middle of a BC3 texture **Settings and switches:** defaults **Steps:** game-free: `python -I -B tests/test_mem_trim.py` on main (copy the test from this PR); it reads `texture_copy_into_bc()` in `vk_texture.cpp` **Expected:** a region the BC3 copy cannot write as whole blocks is not copied with an invalid extent **Actual:** on main it is encoded and copied in with its own extent; the test fails ## What changed `texture_copy_into_bc()` now returns `false`, before it takes the BC scratch, for a region into a BC3 texture that is not resized when its width is not a multiple of 4 and does not reach the level's right edge, or its height is not a multiple of 4 and does not reach the level's bottom edge. A `false` there already means "not done": the caller (`vk_xfer.cpp`, the copy into a BC3-kept texture) then skips the copy with the warning "texture copy into or out of a BC3-kept ASTC texture it cannot make: skipped", as it does for every other case this function refuses (offsets not on a block, a region bigger than the scratch, a resized destination that is not a whole level). The code comment's "left to the copy's other paths" means that caller. - vk: `texture_copy_into_bc()` in `gpu/vk/vk_texture.cpp`: `if (!dst->bc_resized && (((w & 3) && dx + w != ow) || ((h & 3) && dy + h != oh))) return false;` before `bc_scratch_layout()`. Resized BC3 destinations already take only whole levels (checked just above) and are unchanged. - tests: `tests/test_mem_trim.py` (extended): the check of `texture_copy_into_bc()` now also needs this guard, before `bc_scratch_layout(`. The bug, step by step: 1. `texture_copy_into_bc()` copies a `w` x `h` region of an ASTC texture into a BC3 texture at `dx`, `dy`: it copies the region into the scratch, encodes it to BC3 blocks in a buffer (`bc_encode_into`), then copies the buffer into the BC3 image. 2. It already needs `dx` and `dy` on a 4x4 block (`(dx & 3) || (dy & 3)` returns `false`), but not `w` and `h`. 3. ASTC 6x6 and 10x10 regions are often 6, 10, 30... texels wide: not whole 4x4 blocks. 4. `bc_encode_into` copies the blocks with `VkBufferImageCopy{..., {dx, dy, 0}, {w, h, 1}}`. With `w` not a multiple of 4 and `dx + w` short of the level's width (or the same for `h`), that copy breaks the two rules below. The rules, from Khronos `validusage.json`: - VUID-vkCmdCopyBufferToImage-dstImage-00207: "For each element of pRegions, if VkCopyCommandTransformInfoQCOM::transform is equal to VK_SURFACE_TRANSFORM_IDENTITY_BIT_KHR, the sum of imageOffset.x and extent.width does not equal the width of the subresource specified by imageSubresource, extent.width must be a multiple of the texel block extent width of the VkFormat of dstImage" - VUID-vkCmdCopyBufferToImage-dstImage-00208: "For each element of pRegions, if VkCopyCommandTransformInfoQCOM::transform is equal to VK_SURFACE_TRANSFORM_IDENTITY_BIT_KHR, and the sum of imageOffset.y and extent.height does not equal the height of the subresource specified by imageSubresource, extent.height must be a multiple of the texel block extent height of the VkFormat of dstImage" The copy that breaks them, in `bc_encode_into`, unchanged: ```cpp const VkBufferImageCopy c{0, 0, 0, {VK_IMAGE_ASPECT_COLOR_BIT, level, layer, n}, {dx, dy, 0}, {w, h, 1}}; vkCmdCopyBufferToImage(cmd, bc_out, dst.image, VK_IMAGE_LAYOUT_GENERAL, 1, &c); ``` Before: ```cpp } if (!bc_scratch_layout(cmd, dst->bc_resized)) return false; ``` After: ```cpp } // (the blocks are copied in with the region's extent: whole 4x4 blocks, or reaching the level's edge. A region of // another size, from an ASTC 6x6 or 10x10 source, is left to the copy's other paths) if (!dst->bc_resized && (((w & 3) && dx + w != ow) || ((h & 3) && dy + h != oh))) return false; if (!bc_scratch_layout(cmd, dst->bc_resized)) return false; ``` Start reading at `bool texture_copy_into_bc(` in `vk_texture.cpp`, then `bc_encode_into` near the top of the file. ## Measurements **Verdict:** it fixes the bug: `tests/test_mem_trim.py` fails on main (a BC3 copy region that is not whole blocks and does not reach the level's edge is copied in) and passes with this PR, in both interleaved runs. No speed change is expected: one comparison before the copy; a refused region is skipped, as other refused regions already are. Hardware: AMD Radeon RX 9070 XT, Windows; the test itself is game-free (it reads source, it does not use the GPU); Python 3.12 Scene: none, game-free Method: `tests/test_mem_trim.py` from this PR against each side's source (a checkout of each commit, the PR's test copied into main's), 2 runs each, interleaved (main, PR, main, PR); the result is pass/fail, so the median is the result both runs gave How to reproduce: `python -I -B tests/test_mem_trim.py` in a checkout of each side (copy the test into main's) | | Before: main @ 82886e89 | After: this PR @ 5ed443b9 | |---|---|---| | `tests/test_mem_trim.py`, run 1 | fails | passes | | `tests/test_mem_trim.py`, run 2 | fails | passes | | Median | fails | passes | ``` run 1, main @ 82886e89: a BC3 copy region that is not whole blocks and does not reach the level's edge is copied in exit 1 run 1, this PR @ 5ed443b9: ok exit 0 run 2, main @ 82886e89: a BC3 copy region that is not whole blocks and does not reach the level's edge is copied in exit 1 run 2, this PR @ 5ed443b9: ok exit 0 ``` What the test checks (the new part): - In the body of `bool texture_copy_into_bc(`, the guard `if (!dst->bc_resized && (((w & 3) && dx + w != ow) || ((h & 3) && dy + h != oh))) return false;` must be present. - It must come before `bc_scratch_layout(`, so a refused region takes no scratch and records no copy. - The test's other checks of this file and of `vk_memory.cpp` are unchanged and still pass. ### Flags and settings None. ## Testing **Tested:** on this branch, Windows, Python 3.12: `test_mem_trim` passes (fails on main), and the other checks that read the BC3 copy and upload pass: `test_bc_upload_retry`, `test_astc_bc_policy`, `test_parallel_init_order`. **Built and played:** this commit builds on Windows (llvm-mingw back end) and on Linux (`longshot_vk.so`, without and with AMD FSR 4). Played on Windows, AMD Radeon RX 9070 XT: the game runs normally. **Not tested:** such a copy in the game: not checked in the game (it needs an ASTC 6x6 or 10x10 region whose size is not a multiple of 4 copied into the middle of a BC3 texture). Linux was built, not run. **Must be tested:** None: the change is the same on every GPU and OS, and the game-free test covers it. ## Risks **What could break:** a region refused here is no longer copied at all (the caller skips it with its warning), so those texels of the BC3 texture keep what they had. Before, the same region was copied with an extent the rules forbid, so what landed there was undefined anyway. Copies whose size is whole blocks or that reach the level's edge, and copies into resized BC3 textures, are unchanged. **Turning it off:** revert the commit. **Compatibility:** Nothing changes. ## Checklist ### Everyone - [x] One topic. - [x] The title is a Conventional Commit (`type(scope): description`) of the status line's type. - [x] Why gives the reason plainly and links the issues that fit (the tracker searched). (no issue fits) - [x] Measurements has a verdict, or one line saying why there is nothing to measure. - [x] Testing names what was tested, what wasn't, and what must still be tested, where and how. - [x] Rebased on current `main`. - [x] `python -I -B tests/run_all.py` passes (any failure that also fails on `main` is named). (the checks that read the changed code, on this PR, Windows: all pass) - [x] No game files, dumps, build output or generated files. - [x] `third_party/` untouched (a new folder with a `PROVENANCE.md` instead). - [x] The license is OK: GPL-3.0-or-later, nothing incompatible and nothing decompiled. ### When it applies - [x] AI assistance recorded with `Assisted-by:` / `Co-Authored-By:` trailers. - [x] A game-free test added for the fix, failing on `main` and passing here. (`tests/test_mem_trim.py` extended) - [x] A check under `tests/` that reads changed source updated in this PR, not weakened. (`tests/test_mem_trim.py`: one check added, none removed) - [x] Host code builds on both Windows (llvm-mingw) and Linux, with UTF-8 paths. (built on Windows with llvm-mingw and on Linux, `longshot_vk.so` without and with AMD FSR 4; no OS-specific code or paths)

jcabarca added 1 commit

2026-11-15 03:32:28 +00:00

fix(vk): keep a copy into BC3 to whole blocks or the level's edge

CI / Conventional Commit title (pull_request) Blocked by required conditions

Details

CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Blocked by required conditions

Details

CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Blocked by required conditions

Details

CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Blocked by required conditions

Details

CI / Workflow lint (actionlint) (pull_request) Blocked by required conditions

Details

Third-party parts / As their notes say (pull_request) Blocked by required conditions

Details

Third-party parts / The same as upstream (pull_request) Blocked by required conditions

Details

PR description / Description follows the template (pull_request_target) Successful in 3s

Details

Website / Website on Cloudflare (pull_request_target) Has been skipped

Details

5ed443b98d

A BC3 copy region from an ASTC 6x6 or 10x10 source that is not whole 4x4 blocks and does not reach the level's edge broke VUID-vkCmdCopyBufferToImage-dstImage-00207/00208; it is now left to the copy's other paths. test_mem_trim checks the guard.

Assisted-by: GitHub Copilot

jcabarca changed title from ~~WIP: fix(vk): keep a copy into BC3 to whole blocks or the level's edge~~ to fix(vk): keep a copy into BC3 to whole blocks or the level's edge

2026-11-15 03:45:12 +00:00