MagicBeanDev/Hookshot

Archived

Watch

Fork 0

fix(vk): count BC textures by blocks in the texture budget #2624

Closed

mooki000 wants to merge 1 commit from mooki000/fix/vk-block-texture-budget into main

mooki000 commented

2026-11-12 03:10:43 +00:00

(Migrated from github.com)

What this is for: stop the texture budget from charging BC1/BC4 images about twice their compressed size, and make the estimate taken before an image is created use whole compressed blocks too, following hckrman101's review of #2288. What is measured: game-free runs of the production accounting with GCC and Clang: 4,845 cases plus the original 182 helper cases pass, and 3,040 of them fail on main. One pair of game runs (main and the change, Kokiri Forest, RTX 4090) shows no fault and no difference in behaviour, because the budget is never reached there. Not measured: eviction in play, any frame-rate effect, AMD, Windows, or a long session on a card with little VRAM.

Problem

The review's follow-up on BC was:

texel is 1 byte for every BC format, but BC1/BC4 are 8 bytes per 4x4 block (0.5 B/texel), so they count 2x until the x.pad floor (which only raises, never lowers).

create() summed the mip chain's texels and charged every BC format one byte a texel. BC1 and BC4 have eight-byte blocks, the other BC formats sixteen-byte blocks, and every level needs its extent rounded up to whole 4x4 blocks, the 1x1 tail included. The allocator floor covers an underestimate but cannot undo an overestimate, and the inflated count goes into total_bytes, which the eviction sweep compares with the budget.

The review also pointed at the separate estimate taken before creation (bytes_at): an ASTC image the GPU samples itself was still treated as at least one byte a texel, four times the size of 8x8 blocks and nine times that of 12x12 blocks. That estimate decides whether the top levels of a very large image are dropped and whether an array is made sparse.

Last, the ASTC test mixed decimal MB with binary MiB. Its example is 3,410.0 MiB by the old texel count and 53.8 MiB of blocks; comment and output now agree.

What changed

gpu/vk/vk_texture.cpp: texture_level_units is used for native BC and native ASTC in both the estimate before creation and the final count. The helper already rounds each level to whole blocks; no allocation and no further pass over the mip chain is added.
ASTC that is decoded and re-encoded as BC3 counts its own 4x4, 16-byte output blocks, not the source ASTC's block size. ASTC kept as RGBA8 still counts four-byte texels (the image's format decides, not the source's).
Unchanged: the allocator-size floor, a sparse image's count by what is bound, the layer and sample factors, uncompressed and widened images, and all of the eviction policy.
tests/test_native_astc_texture_count.py: the helper's cases stay, and the old checks of the source's text are replaced by compiling and running the production estimate and final count: all 16 BC formats, all 14 ASTC block sizes, partial blocks, 1x1 tails, skipped top levels, arrays, volumes and samples, decoded and re-encoded ASTC, widening, sparse images and the allocator floor.
tests/test_low_vram_texture_mips.py: its cut-out fixture now brings the real format predicates and texture_level_units along, which the changed estimate needs to compile. Every configuration and assertion is kept.
docs/development.md, the RX_VK_VRAM_MB row: what a texture counts, that the log prints MiB as MB, and the default as the code computes it (70% of what the driver allows the process, 45% where that is 8.5 GB or less, at most 6 GB; on an integrated GPU less, by the system's RAM: 1 GB with 8 GB of RAM, 2 with 12, 2.5 with 16, 3.5 with 24, 4 with 32, up to 6 above that).

The count changes; the image data, formats, uploads and compression do not. A smaller count is not less VRAM allocated.

How to try it

No new switch. Compare the log's textures N MB (budget B MB) and textures retired ...: budget N lines in one scene with the same budget settings. Game-free:

python -I -B tests/test_native_astc_texture_count.py --cxx g++
python -I -B tests/test_native_astc_texture_count.py --cxx clang++
python -I -B tests/test_native_astc_texture_count.py --cxx g++ --source /path/to/main/gpu/vk/vk_texture.cpp

How it was tested

Arch Linux, Core i9-13900K, GCC 16.2.1 and Clang 23.1.3. Branch 883c3072, one commit on main 0ec31ff6 (rebased from 1c08cde1 on 8568959d: at that base main's own vk_texture.cpp did not compile on Linux, std::min and std::max with mixed integer types in the integrated-GPU budget, which is why this pull request's first AppImage job failed; main has since fixed it).

On this head, both compilers:

texture accounting: 4845 cases pass; 16 BC formats, 14 ASTC block sizes, decoded/re-encoded ASTC, uncompressed/widened, layers/depth/samples, sparse and allocation floor
native ASTC texture count: 182 cases pass (the 1365 x 1365 x 90 ASTC 8x8 array: 3410.0 MiB counted before, 53.8 MiB of blocks)
PASS 24 mip-policy configurations

The negative checks compile and run rather than fail on a missing string:

Source the test runs against	GCC	Clang
This branch	4,845 pass	4,845 pass
main (as of `de866017`)	3,040 fail	3,040 fail
The branch with only the old estimate put back	2,508 fail	2,508 fail
The branch with only the old final count put back	1,140 fail	1,140 fail

The last two rows were run on the same code change one rebase earlier (a comment and the documentation row changed since); they show that neither half of the fix hides a missing check of the other.

The whole tests/run_all.py suite was run once on the earlier base (fce53453): 447 passed, 3 failed, 20 skipped. The three: the mip-policy fixture, which did not compile without the helpers and is repaired here (above); nx2/test_depth_sample_types.py, whose Clang process was killed by SIGKILL (the cause was not established; the machine was heavily loaded) and which passes when run alone; and test_env_on.py, main's HOS_FPS_LOG failure of that day, since fixed by #2575.

In the game (RTX 4090, NVIDIA 615.78.08, headless, a save in Kokiri Forest with camera turns, 170 s each, one after the other): main fce53453 and a build of the same base with this change's vk_texture.cpp (the code is the same as on this branch; one comment differs).

Build	Run	Fatal, crash, device lost, driver lines	Textures counted at their highest / budget	Textures retired for the budget
main	170.6 s, ended by the timer	0	4022 / 6144 MiB	0
this change	170.6 s, ended by the timer	0	3978 / 6144 MiB	0

The pictures of both runs show the forest and the HUD as they should be. Neither run reached its budget, so this pair shows that the change does no harm in an ordinary scene on a large card. It does not show an eviction that comes later, and it is not a timing.

What to expect in play

Reasoned, not measured: with BC1/BC4 no longer counted double, the fixed texture budget is reached later, not earlier. The allocator floor stays, and the rules that react to the driver's own figures still use what the driver reports, so this does not postpone every eviction. A smaller count can also put a BC texture the GPU drew under one of the existing keep-size thresholds; those rules are unchanged. Tiny and partial mip levels that the old formula undercounted now count a whole block, which the allocator floor usually covered already.

Not tested

Windows, AMD, the Steam Deck, or an Intel run with natively sampled ASTC for this follow-up.
Long play, or a card with 4 to 8 GB where the budget is reached.
A real oversized compressed array that crosses the sparse or the drop-top-levels threshold; the estimate itself is covered by the test.
Frame rate, frame times or stutter.

Risks

Unlike #2288, this changes the count on NVIDIA and AMD too. An undercount would put eviction off too long; the per-format arithmetic in the test, the tiny-level cases and the allocator floor guard against that. Correcting the estimate before creation can keep a large compressed image whole where the old estimate dropped its top levels or made it sparse. That is intended, but how it behaves at those thresholds needs other workloads and hardware.

Overlaps

#2338 and #2368 add rows next to the RX_VK_VRAM_MB row in docs/development.md: keep their rows and this description.
#2557 (draft) changes the set-up of tests/test_low_vram_texture_mips.py beside this change's two imports: keep both.
#2561 moves docs/development.md in its rewrite of the docs: the row's new wording should move with it.
In vk_texture.cpp the overlap scan of 2026-11-11 found no conflict with an open pull request.

No separate issue: this is the follow-up the review of #2288 asked for.

Assisted-by: GPT-6 Astra
Co-Authored-By: GPT-6 Astra [email protected]

**What this is for:** stop the texture budget from charging BC1/BC4 images about twice their compressed size, and make the estimate taken before an image is created use whole compressed blocks too, following [hckrman101's review of #2288](https://git.magicbean.dev/MagicBeanDev/Hookshot/pulls/2288). **What is measured:** game-free runs of the production accounting with GCC and Clang: 4,845 cases plus the original 182 helper cases pass, and 3,040 of them fail on main. One pair of game runs (main and the change, Kokiri Forest, RTX 4090) shows no fault and no difference in behaviour, because the budget is never reached there. **Not measured:** eviction in play, any frame-rate effect, AMD, Windows, or a long session on a card with little VRAM. ## Problem The review's follow-up on BC was: > `texel` is 1 byte for every BC format, but BC1/BC4 are 8 bytes per 4x4 block (0.5 B/texel), so they count 2x until the `x.pad` floor (which only raises, never lowers). `create()` summed the mip chain's texels and charged every BC format one byte a texel. BC1 and BC4 have eight-byte blocks, the other BC formats sixteen-byte blocks, and every level needs its extent rounded up to whole 4x4 blocks, the 1x1 tail included. The allocator floor covers an underestimate but cannot undo an overestimate, and the inflated count goes into `total_bytes`, which the eviction sweep compares with the budget. The review also pointed at the separate estimate taken before creation (`bytes_at`): an ASTC image the GPU samples itself was still treated as at least one byte a texel, four times the size of 8x8 blocks and nine times that of 12x12 blocks. That estimate decides whether the top levels of a very large image are dropped and whether an array is made sparse. Last, the ASTC test mixed decimal MB with binary MiB. Its example is 3,410.0 MiB by the old texel count and 53.8 MiB of blocks; comment and output now agree. ## What changed - `gpu/vk/vk_texture.cpp`: `texture_level_units` is used for native BC and native ASTC in both the estimate before creation and the final count. The helper already rounds each level to whole blocks; no allocation and no further pass over the mip chain is added. - ASTC that is decoded and re-encoded as BC3 counts its own 4x4, 16-byte output blocks, not the source ASTC's block size. ASTC kept as RGBA8 still counts four-byte texels (the image's format decides, not the source's). - Unchanged: the allocator-size floor, a sparse image's count by what is bound, the layer and sample factors, uncompressed and widened images, and all of the eviction policy. - `tests/test_native_astc_texture_count.py`: the helper's cases stay, and the old checks of the source's text are replaced by compiling and running the production estimate and final count: all 16 BC formats, all 14 ASTC block sizes, partial blocks, 1x1 tails, skipped top levels, arrays, volumes and samples, decoded and re-encoded ASTC, widening, sparse images and the allocator floor. - `tests/test_low_vram_texture_mips.py`: its cut-out fixture now brings the real format predicates and `texture_level_units` along, which the changed estimate needs to compile. Every configuration and assertion is kept. - `docs/development.md`, the `RX_VK_VRAM_MB` row: what a texture counts, that the log prints MiB as `MB`, and the default as the code computes it (70% of what the driver allows the process, 45% where that is 8.5 GB or less, at most 6 GB; on an integrated GPU less, by the system's RAM: 1 GB with 8 GB of RAM, 2 with 12, 2.5 with 16, 3.5 with 24, 4 with 32, up to 6 above that). The count changes; the image data, formats, uploads and compression do not. A smaller count is not less VRAM allocated. ## How to try it No new switch. Compare the log's `textures N MB (budget B MB)` and `textures retired ...: budget N` lines in one scene with the same budget settings. Game-free: ```sh python -I -B tests/test_native_astc_texture_count.py --cxx g++ python -I -B tests/test_native_astc_texture_count.py --cxx clang++ python -I -B tests/test_native_astc_texture_count.py --cxx g++ --source /path/to/main/gpu/vk/vk_texture.cpp ``` ## How it was tested Arch Linux, Core i9-13900K, GCC 16.2.1 and Clang 23.1.3. Branch `883c3072`, one commit on main `0ec31ff6` (rebased from `1c08cde1` on `8568959d`: at that base main's own `vk_texture.cpp` did not compile on Linux, `std::min` and `std::max` with mixed integer types in the integrated-GPU budget, which is why this pull request's first AppImage job failed; main has since fixed it). On this head, both compilers: ```text texture accounting: 4845 cases pass; 16 BC formats, 14 ASTC block sizes, decoded/re-encoded ASTC, uncompressed/widened, layers/depth/samples, sparse and allocation floor native ASTC texture count: 182 cases pass (the 1365 x 1365 x 90 ASTC 8x8 array: 3410.0 MiB counted before, 53.8 MiB of blocks) PASS 24 mip-policy configurations ``` The negative checks compile and run rather than fail on a missing string: | Source the test runs against | GCC | Clang | |---|---:|---:| | This branch | 4,845 pass | 4,845 pass | | main (as of `de866017`) | 3,040 fail | 3,040 fail | | The branch with only the old estimate put back | 2,508 fail | 2,508 fail | | The branch with only the old final count put back | 1,140 fail | 1,140 fail | The last two rows were run on the same code change one rebase earlier (a comment and the documentation row changed since); they show that neither half of the fix hides a missing check of the other. The whole `tests/run_all.py` suite was run once on the earlier base (`fce53453`): 447 passed, 3 failed, 20 skipped. The three: the mip-policy fixture, which did not compile without the helpers and is repaired here (above); `nx2/test_depth_sample_types.py`, whose Clang process was killed by SIGKILL (the cause was not established; the machine was heavily loaded) and which passes when run alone; and `test_env_on.py`, main's `HOS_FPS_LOG` failure of that day, since fixed by #2575. **In the game** (RTX 4090, NVIDIA 615.78.08, headless, a save in Kokiri Forest with camera turns, 170 s each, one after the other): main `fce53453` and a build of the same base with this change's `vk_texture.cpp` (the code is the same as on this branch; one comment differs). | Build | Run | Fatal, crash, device lost, driver lines | Textures counted at their highest / budget | Textures retired for the budget | |---|---|---|---|---| | main | 170.6 s, ended by the timer | 0 | 4022 / 6144 MiB | 0 | | this change | 170.6 s, ended by the timer | 0 | 3978 / 6144 MiB | 0 | The pictures of both runs show the forest and the HUD as they should be. Neither run reached its budget, so this pair shows that the change does no harm in an ordinary scene on a large card. It does not show an eviction that comes later, and it is not a timing. ## What to expect in play Reasoned, not measured: with BC1/BC4 no longer counted double, the fixed texture budget is reached later, not earlier. The allocator floor stays, and the rules that react to the driver's own figures still use what the driver reports, so this does not postpone every eviction. A smaller count can also put a BC texture the GPU drew under one of the existing keep-size thresholds; those rules are unchanged. Tiny and partial mip levels that the old formula undercounted now count a whole block, which the allocator floor usually covered already. ## Not tested - Windows, AMD, the Steam Deck, or an Intel run with natively sampled ASTC for this follow-up. - Long play, or a card with 4 to 8 GB where the budget is reached. - A real oversized compressed array that crosses the sparse or the drop-top-levels threshold; the estimate itself is covered by the test. - Frame rate, frame times or stutter. ## Risks Unlike #2288, this changes the count on NVIDIA and AMD too. An undercount would put eviction off too long; the per-format arithmetic in the test, the tiny-level cases and the allocator floor guard against that. Correcting the estimate before creation can keep a large compressed image whole where the old estimate dropped its top levels or made it sparse. That is intended, but how it behaves at those thresholds needs other workloads and hardware. ## Overlaps - #2338 and #2368 add rows next to the `RX_VK_VRAM_MB` row in `docs/development.md`: keep their rows and this description. - #2557 (draft) changes the set-up of `tests/test_low_vram_texture_mips.py` beside this change's two imports: keep both. - #2561 moves `docs/development.md` in its rewrite of the docs: the row's new wording should move with it. - In `vk_texture.cpp` the overlap scan of 2026-11-11 found no conflict with an open pull request. No separate issue: this is the follow-up the review of #2288 asked for. Assisted-by: GPT-6 Astra Co-Authored-By: GPT-6 Astra <[email protected]>

hckrman101 (Migrated from github.com) approved these changes

2026-11-12 04:29:51 +00:00

hckrman101 (Migrated from github.com) left a comment

Thanks, this is the BC follow-up I asked for in #2288 and the math checks out.

I went through it format by format against nvn_fmt_bpb (host/nvn_layout.c), which is where bpb/bw/bh come from: DXT1 and RGTC1 (BC1/BC4) give 8 bytes per 4x4 block; DXT3/DXT5, RGTC2 and BPTC (BC2/3/5/6H/7) give 16. Both the estimate (bytes_at) and the final count now go through texture_level_units, which rounds every level up to whole blocks, so 13x13 counts as 4x4 blocks and the 1x1 tail as one block. 3D levels are blocked in x/y only, which is right for BC. Re-encoded ASTC counts as 4x4 blocks of 16 bytes no matter the source block size, and the same holds for the BC7 variant. Decoded RGBA8 and the widened formats are unchanged.

What players will see: on NVIDIA/AMD the game's ASTC still goes through the decoded path, so the RGBA8 count is unchanged and BC3 only changes at partial-block tails (the allocator floor already covered those). Native BC1/BC4 drop from twice their size to their real size. Since x.pad still raises the count to allocation_size, a texture never counts for less than VMA gave it, so this can't push eviction past real use, and the integrated-GPU budget only gets an honest count to compare against. Low-VRAM cards reach the budget a little later where BC1/BC4 are involved, but the driver-figure rules (the 90%/97% squeeze, compaction) are untouched.

Two behaviour changes worth knowing about, neither blocking:

bytes_at against the 1536 MB RX_VK_TEX_MAX_MB cap: a big native BC1 or native ASTC (8x8 and up, Intel) array that used to go sparse or lose its top levels can now stay whole. That's correct now that the number is right. For the 1365x1365x90 arrays it makes no difference: they were under the cap before and still are.
The keep thresholds in the eviction sweep (4 / 64 / 256 MB on tex->bytes, for textures the GPU wrote) now see BC at its real size, as the description says. That's fine.

Tests: the new fixture runs the production estimate and final count rather than matching strings, the expected values are computed independently, and test_low_vram_texture_mips.py now pulls in the real helpers. Both pass in CI on Linux and Windows. The red jobs are main's known failures plus the ten fixtures #2648 repaired (now on main), none of them this PR's tests. A merge of main should turn most of it green.

One nit on the docs row, inline. Approving.

Thanks, this is the BC follow-up I asked for in #2288 and the math checks out. I went through it format by format against `nvn_fmt_bpb` (host/nvn_layout.c), which is where `bpb`/`bw`/`bh` come from: DXT1 and RGTC1 (BC1/BC4) give 8 bytes per 4x4 block; DXT3/DXT5, RGTC2 and BPTC (BC2/3/5/6H/7) give 16. Both the estimate (`bytes_at`) and the final count now go through `texture_level_units`, which rounds every level up to whole blocks, so 13x13 counts as 4x4 blocks and the 1x1 tail as one block. 3D levels are blocked in x/y only, which is right for BC. Re-encoded ASTC counts as 4x4 blocks of 16 bytes no matter the source block size, and the same holds for the BC7 variant. Decoded RGBA8 and the widened formats are unchanged. What players will see: on NVIDIA/AMD the game's ASTC still goes through the decoded path, so the RGBA8 count is unchanged and BC3 only changes at partial-block tails (the allocator floor already covered those). Native BC1/BC4 drop from twice their size to their real size. Since `x.pad` still raises the count to `allocation_size`, a texture never counts for less than VMA gave it, so this can't push eviction past real use, and the integrated-GPU budget only gets an honest count to compare against. Low-VRAM cards reach the budget a little later where BC1/BC4 are involved, but the driver-figure rules (the 90%/97% squeeze, compaction) are untouched. Two behaviour changes worth knowing about, neither blocking: - `bytes_at` against the 1536 MB `RX_VK_TEX_MAX_MB` cap: a big native BC1 or native ASTC (8x8 and up, Intel) array that used to go sparse or lose its top levels can now stay whole. That's correct now that the number is right. For the 1365x1365x90 arrays it makes no difference: they were under the cap before and still are. - The keep thresholds in the eviction sweep (4 / 64 / 256 MB on `tex->bytes`, for textures the GPU wrote) now see BC at its real size, as the description says. That's fine. Tests: the new fixture runs the production estimate and final count rather than matching strings, the expected values are computed independently, and `test_low_vram_texture_mips.py` now pulls in the real helpers. Both pass in CI on Linux and Windows. The red jobs are main's known failures plus the ten fixtures #2648 repaired (now on main), none of them this PR's tests. A merge of main should turn most of it green. One nit on the docs row, inline. Approving.

docs/development.md

Nit: by the code, an 8 GB-RAM integrated GPU gets 1 GB, not 1.5 GB. reserved is at least 8 GB, so available comes out 0, the cap gives 0, and the 1 GB floor wins. 12 GB gives 2 GB, 16 GB gives 2.5, 24 GB gives 3.5, 32 GB gives 4, and above that up to 6 GB. Something like "1 GB with 8 GB of RAM up to 4 GB with 32 GB (6 GB above)" would match.

Nit: by the code, an 8 GB-RAM integrated GPU gets 1 GB, not 1.5 GB. `reserved` is at least 8 GB, so `available` comes out 0, the cap gives 0, and the 1 GB floor wins. 12 GB gives 2 GB, 16 GB gives 2.5, 24 GB gives 3.5, 32 GB gives 4, and above that up to 6 GB. Something like "1 GB with 8 GB of RAM up to 4 GB with 32 GB (6 GB above)" would match.

mooki000 (Migrated from github.com) reviewed

2026-11-12 04:33:25 +00:00

docs/development.md

You are right, I read the caps and missed that reserved leaves nothing to cap at 8 GB. Corrected in 883c3072 to what the code gives: 1 GB with 8 GB of RAM, 2 GB with 12, 2.5 GB with 16, 3.5 GB with 24, 4 GB with 32, and up to 6 GB above that. The same push rebases over #2651, which added the row above this one.

You are right, I read the caps and missed that `reserved` leaves nothing to cap at 8 GB. Corrected in `883c3072` to what the code gives: 1 GB with 8 GB of RAM, 2 GB with 12, 2.5 GB with 16, 3.5 GB with 24, 4 GB with 32, and up to 6 GB above that. The same push rebases over #2651, which added the row above this one.

github-sync commented

2026-11-12 12:37:19 +00:00

Owner

@klikso commented on GitHub · 2026-11-12 12:28 UTC · original

A data point from the case the description lists as not measured (Windows, a card short of VRAM, in play), which turned out inconclusive.

Windows 11, RTX 3060 Ti 8 GB (driver 32.0.16.2114), Ryzen 5 5600X, 2560x1440 DLSS. main ba005ca4 with #2711, #2731, #2685 and #2709 merged locally; with and without this PR on top (two builds, so the arms could not be alternated: base, this PR, base again). Castle Town Market child, 30 s of the camera turning after a settle, where the texture budget is reached all the time (budget ~1.5-2.1 GB, 2.3-2.9 GB of textures).

	fps mean	frames over 50 ms in 30 s	GPU a frame
base, first set	46.8 / 49.7 / 49.2	17 / 5 / 10	11.3 ms
this PR	32.6 / 37.8 / 42.3	112 / 68 / 44	14.7 / 14.1 / 11.7 ms
base, rebuilt after	40.8 / 43.8 / 42.4	56 / 42 / 53	12.2 / 11.9 / 11.9 ms

The same base build drifted from ~48 to ~42 fps between the two sets, so the Market on this card moves too much between launches to call this PR's lower numbers a regression. Evictions per run were about the same in all nine (26-30 thousand textures retired); this PR's runs had fewer compactions (2-7 against 7-9). No fault, no texture in system memory. I'd treat it as "no improvement seen here" rather than evidence either way.

> [@klikso](https://github.com/klikso) commented on GitHub · 2026-11-12 12:28 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2624#issuecomment-6823607768) A data point from the case the description lists as not measured (Windows, a card short of VRAM, in play), which turned out inconclusive. Windows 11, RTX 3060 Ti 8 GB (driver 32.0.16.2114), Ryzen 5 5600X, 2560x1440 DLSS. `main` ba005ca4 with #2711, #2731, #2685 and #2709 merged locally; with and without this PR on top (two builds, so the arms could not be alternated: base, this PR, base again). Castle Town Market child, 30 s of the camera turning after a settle, where the texture budget is reached all the time (budget ~1.5-2.1 GB, 2.3-2.9 GB of textures). | | fps mean | frames over 50 ms in 30 s | GPU a frame | |---|---|---|---| | base, first set | 46.8 / 49.7 / 49.2 | 17 / 5 / 10 | 11.3 ms | | this PR | 32.6 / 37.8 / 42.3 | 112 / 68 / 44 | 14.7 / 14.1 / 11.7 ms | | base, rebuilt after | 40.8 / 43.8 / 42.4 | 56 / 42 / 53 | 12.2 / 11.9 / 11.9 ms | The same base build drifted from ~48 to ~42 fps between the two sets, so the Market on this card moves too much between launches to call this PR's lower numbers a regression. Evictions per run were about the same in all nine (26-30 thousand textures retired); this PR's runs had fewer compactions (2-7 against 7-9). No fault, no texture in system memory. I'd treat it as "no improvement seen here" rather than evidence either way.

github-sync commented

2026-11-12 12:37:19 +00:00

Owner

@mooki000 commented on GitHub · 2026-11-12 12:32 UTC · original

@klikso thank you, that is exactly the case the description leaves open, and your three sets are enough to take seriously even with the drift: the runs with this change are the lowest of the three.

Please hold the merge until this is settled. I am measuring it now on this PC under an 8 GB memory model (a 24 GB card held to a small allowance, as #2690 and #2711 were measured), in the Market with the camera turning: this change against its own base, the two builds alternated in one hold, plus a control with memory to spare. The reason it could be real: counting BC textures at their true, smaller size lets the same budget admit more of them, and that can change when the squeeze and compaction start even though the driver's allowance is untouched. The result and what follows from it (unchanged, a fix on top, or a draft until a real 8 GB card has run it) go here.

> [@mooki000](https://github.com/mooki000) commented on GitHub · 2026-11-12 12:32 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2624#issuecomment-6823701590) @klikso thank you, that is exactly the case the description leaves open, and your three sets are enough to take seriously even with the drift: the runs with this change are the lowest of the three. **Please hold the merge until this is settled.** I am measuring it now on this PC under an 8 GB memory model (a 24 GB card held to a small allowance, as #2690 and #2711 were measured), in the Market with the camera turning: this change against its own base, the two builds alternated in one hold, plus a control with memory to spare. The reason it could be real: counting BC textures at their true, smaller size lets the same budget admit more of them, and that can change when the squeeze and compaction start even though the driver's allowance is untouched. The result and what follows from it (unchanged, a fix on top, or a draft until a real 8 GB card has run it) go here.

github-sync referenced this pull request

2026-11-12 13:40:20 +00:00

perf(vk): import game memory in 4 MB chunks, not 16, so VRAM mirrors fit the game's pools #2685

github-sync referenced this pull request

2026-11-12 13:40:28 +00:00

fix(vk): make the blocks at the start in the pool the textures are made in (#2690) #2711

github-sync commented

2026-11-12 14:43:30 +00:00

Owner

@Joey-Magnus reviewed on GitHub · 2026-11-12 14:33 UTC · original

I merged this into current main (it merges cleanly) and ran test_native_astc_texture_count, test_low_vram_texture_mips, test_texture_staging_publication, test_ui_texture_upload and the other texture tests on Linux. All pass. The one red job is test_nightly_two_runs_at_once_end_at_the_newer_commit in test_release_scripts, which flakes on main too and has nothing to do with this PR. hckrman101's docs nit about the 1 GB minimum with 8 GB of RAM is fixed in the current head.
The arithmetic matches nvn_fmt_bpb, and x.pad still floors the count at the allocation. The open question is the one klikso raised and you're measuring. With the budget number unchanged, the cache now keeps up to twice as much real BC1/BC4 before it evicts. On an 8 GB card that moves more of the work onto the driver-figure rules (the 90%/97% squeeze and compaction). His three Market sets put this PR lowest, though the base drifted between them. Since you asked to hold the merge, I'll wait for your alternated runs. If they show a drop, lowering the default budget for under 8.5 GB to match the honest count would be one way to keep both.

> [@Joey-Magnus](https://github.com/Joey-Magnus) reviewed on GitHub · 2026-11-12 14:33 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2624#pullrequestreview-5876223118) I merged this into current main (it merges cleanly) and ran test_native_astc_texture_count, test_low_vram_texture_mips, test_texture_staging_publication, test_ui_texture_upload and the other texture tests on Linux. All pass. The one red job is test_nightly_two_runs_at_once_end_at_the_newer_commit in test_release_scripts, which flakes on main too and has nothing to do with this PR. hckrman101's docs nit about the 1 GB minimum with 8 GB of RAM is fixed in the current head. The arithmetic matches nvn_fmt_bpb, and x.pad still floors the count at the allocation. The open question is the one klikso raised and you're measuring. With the budget number unchanged, the cache now keeps up to twice as much real BC1/BC4 before it evicts. On an 8 GB card that moves more of the work onto the driver-figure rules (the 90%/97% squeeze and compaction). His three Market sets put this PR lowest, though the base drifted between them. Since you asked to hold the merge, I'll wait for your alternated runs. If they show a drop, lowering the default budget for under 8.5 GB to match the honest count would be one way to keep both.

github-sync commented

2026-11-12 14:43:30 +00:00

Owner

@mooki000 commented on GitHub · 2026-11-12 14:34 UTC · original

Thanks. The alternated runs are queued on this PC: this pull request against its base in one session, Castle Town Market, with the card held to an 8 GB allowance, so the base cannot drift between sets. I will post them here. If they show a drop, I will try your suggestion (a lower default budget below 8.5 GB that matches the honest count) and measure that as a third column before asking for the hold to be lifted.

> [@mooki000](https://github.com/mooki000) commented on GitHub · 2026-11-12 14:34 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2624#issuecomment-6826509563) Thanks. The alternated runs are queued on this PC: this pull request against its base in one session, Castle Town Market, with the card held to an 8 GB allowance, so the base cannot drift between sets. I will post them here. If they show a drop, I will try your suggestion (a lower default budget below 8.5 GB that matches the honest count) and measure that as a third column before asking for the hold to be lifted.

github-sync referenced this pull request

2026-11-12 15:49:16 +00:00

[Tracking] VRAM and RAM fill up over play time: fps fades, textures degrade or turn rainbow, a restart fixes it (mostly 4-8 GB cards) #993

github-sync referenced this pull request from a commit

2026-11-12 16:54:32 +00:00

fix(vk): count BC textures by blocks in the texture budget (#2624)

~~github-sync referenced this pull request 2026-11-12 16:54:48 +00:00~~

chore(main): release 2.2.0-rc.1 #2071

github-sync closed this pull request

2026-11-12 16:55:20 +00:00

github-sync commented

2026-11-12 16:55:21 +00:00

Owner

@mooki000 commented on GitHub · 2026-11-12 16:10 UTC · original

@klikso @Joey-Magnus: the alternated pressure runs show no difference this PC can resolve, not proof that the Windows result was just drift. Against the PR's own parent, mean frame time rose 1.88%, beside 1.83% spread between the base repeats; the ranges are narrowly separated, so this is not an equivalence claim.

Linux/NVIDIA 615.78.08, RTX 4090 24 GB, i9-13900K; child Market south entrance, camera turning, 2560×1440, fixed 67% render scale, DLSS K, frame generation off, cap 240. Prepared caches and unmeasured warm-ups; A/B/A/B in one exclusive hold, at least 60 s measured per launch after settling.

Build	Mean / median frame ms	FPS	GPU ms/frame	Frames >50 ms
A: parent `ee29dd55`	13.352 / 12.327; 13.597 / 12.756	74.89 / 73.54	6.89 / 7.14	0 / 0
B: #2624 `da33e6cc`	13.765 / 12.771; 13.692 / 12.709	72.65 / 73.04	7.24 / 7.03	0 / 0

1% lows: A 37.4 / 36.8 fps, B 35.9 / 35.6. Both B runs sit behind both A runs, by about the size of A's own spread: small and at the edge of what this PC can resolve, not nothing.

Model: RX_VK_TEST_ALLOWED_MB=7000, RX_VK_IMAGE_PREALLOC=9, plus RX_VK_MIRROR_MB=2048 to match an 8-GB heap's mirror limit. Every sampled pressure line exceeded the texture budget. Compaction starts per whole 110-s launch were A 14/17, B 16/19, not fewer on B; budget retirements A 17314/17220, B 17252/17358. No fault or logged system-memory image fallback.
With the card's full memory (no model) the three accepted runs show no difference: A 86.4 fps, B 87.3 and 86.4. That control is incomplete: one A arm was rejected because other work peaked at 19.26 CPU cores; its raw result and the three accepted arms are retained. System-wide swap-in was also recorded, so the small pressure gap is not isolated from all host activity.
Your PR set was lowest (32.6–42.3 fps) against the first base (46.8–49.7) and rebuilt base (40.8–43.8), with more long frames. I am not dismissing that: your base drifted, and your ba005ca4 + #2711/#2731/#2685/#2709 stack is not this isolated parent/head pair. Our standing point, camera speed and explicitly fixed DLSS settings may differ too.
The model exercises renderer eviction/squeeze/compaction, not a real 8-GB heap or Windows residency/paging/allocation costs. The real driver allowance was still 22,473–22,488 MiB here. When driver pressure sets the budget, the smaller texture total also lowers that target; these runs do not show extra residency caused by the corrected count.

So the hold stays, for one check this PC cannot do: an alternated pair on a real 8 GB card on Windows. Many players have such cards, and the pair takes about 20 minutes. @klikso, you have the card: could you run it?

A ee29dd55b593577ae0a7a6e2f871848ee5e176b4 (this pull request's parent) and B da33e6cc77309603063e038609e1e7c826a15ce4, built into two folders from the same game files, nothing else merged in.
The same save and spot at the Market, 1440p, a fixed render scale (67% here), DLSS K or whatever you normally use as long as it is fixed, frame generation off, a 240 cap, no mods; the allowance, preallocation, mirror and texture-budget overrides left unset, since the real card sets them.
Prepare and warm both, then A, B, A, B without rebuilding: 60 s to settle, then 120 s of the same camera turn each. Frame times from RX_VK_FRAME_LOG, the full logs, the driver version, and the dedicated and shared GPU memory if you can record them.

If B is behind A by more than the two A runs differ, I will try Joey-Magnus's lower default budget below 8.5 GB as the next step and measure all three here and ask for the same pair again. If it is not, I lift the hold. I am also putting this in the test-request queue so anyone with an 8 GB card can take it.

No code changed from these results. The arithmetic of the count is right; what is open is only what it costs on a real small card.

> [@mooki000](https://github.com/mooki000) commented on GitHub · 2026-11-12 16:10 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2624#issuecomment-6828948324) @klikso @Joey-Magnus: the alternated pressure runs show **no difference this PC can resolve**, not proof that the Windows result was just drift. Against the PR's own parent, mean frame time rose **1.88%**, beside **1.83%** spread between the base repeats; the ranges are narrowly separated, so this is not an equivalence claim. Linux/NVIDIA 615.78.08, RTX 4090 24 GB, i9-13900K; child Market south entrance, camera turning, 2560×1440, fixed 67% render scale, DLSS K, frame generation off, cap 240. Prepared caches and unmeasured warm-ups; A/B/A/B in one exclusive hold, at least 60 s measured per launch after settling. | Build | Mean / median frame ms | FPS | GPU ms/frame | Frames >50 ms | |---|---|---|---|---| | A: parent `ee29dd55` | 13.352 / 12.327; 13.597 / 12.756 | 74.89 / 73.54 | 6.89 / 7.14 | 0 / 0 | | B: #2624 `da33e6cc` | 13.765 / 12.771; 13.692 / 12.709 | 72.65 / 73.04 | 7.24 / 7.03 | 0 / 0 | 1% lows: A 37.4 / 36.8 fps, B 35.9 / 35.6. Both B runs sit behind both A runs, by about the size of A's own spread: small and at the edge of what this PC can resolve, not nothing. Model: `RX_VK_TEST_ALLOWED_MB=7000`, `RX_VK_IMAGE_PREALLOC=9`, plus `RX_VK_MIRROR_MB=2048` to match an 8-GB heap's mirror limit. Every sampled pressure line exceeded the texture budget. Compaction starts per whole 110-s launch were A **14/17**, B **16/19**, not fewer on B; budget retirements A **17314/17220**, B **17252/17358**. No fault or logged system-memory image fallback. With the card's full memory (no model) the three accepted runs show no difference: A 86.4 fps, B 87.3 and 86.4. That control is **incomplete**: one A arm was rejected because other work peaked at **19.26 CPU cores**; its raw result and the three accepted arms are retained. System-wide swap-in was also recorded, so the small pressure gap is not isolated from all host activity. Your PR set was lowest (**32.6–42.3 fps**) against the first base (**46.8–49.7**) and rebuilt base (**40.8–43.8**), with more long frames. I am not dismissing that: your base drifted, and your `ba005ca4` + #2711/#2731/#2685/#2709 stack is not this isolated parent/head pair. Our standing point, camera speed and explicitly fixed DLSS settings may differ too. The model exercises renderer eviction/squeeze/compaction, **not a real 8-GB heap or Windows residency/paging/allocation costs**. The real driver allowance was still **22,473–22,488 MiB** here. When driver pressure sets the budget, the smaller texture total also lowers that target; these runs do not show extra residency caused by the corrected count. **So the hold stays, for one check this PC cannot do: an alternated pair on a real 8 GB card on Windows.** Many players have such cards, and the pair takes about 20 minutes. @klikso, you have the card: could you run it? - A `ee29dd55b593577ae0a7a6e2f871848ee5e176b4` (this pull request's parent) and B `da33e6cc77309603063e038609e1e7c826a15ce4`, built into two folders from the same game files, nothing else merged in. - The same save and spot at the Market, 1440p, a fixed render scale (67% here), DLSS K or whatever you normally use as long as it is fixed, frame generation off, a 240 cap, no mods; the allowance, preallocation, mirror and texture-budget overrides left unset, since the real card sets them. - Prepare and warm both, then A, B, A, B without rebuilding: 60 s to settle, then 120 s of the same camera turn each. Frame times from `RX_VK_FRAME_LOG`, the full logs, the driver version, and the dedicated and shared GPU memory if you can record them. If B is behind A by more than the two A runs differ, I will try Joey-Magnus's lower default budget below 8.5 GB as the next step and measure all three here and ask for the same pair again. If it is not, I lift the hold. I am also putting this in the test-request queue so anyone with an 8 GB card can take it. No code changed from these results. The arithmetic of the count is right; what is open is only what it costs on a real small card.

github-sync commented

2026-11-12 16:55:21 +00:00

Owner

@hckrman101 approved on GitHub · 2026-11-12 16:35 UTC · original

Re-review. The code is the same as the commit I approved; this push rebased onto main and fixed the docs nit, and the row now matches what the integrated-GPU budget gives. Thanks.

I rechecked the block math on the rebased file. The image's own format decides. Decoded ASTC is an RGBA8 image, so it stays at 4 bytes a texel. ASTC re-encoded to BC3 or BC7 counts 4x4 blocks of 16 bytes. Native BC1/BC4 count 8 bytes a block, BC2/3/5/6H/7 16, and native ASTC 16 per bw x bh block. Each level is rounded up to whole blocks on its own, so 2x2 and 1x1 tails count one block each, and a level left out (x.skip) shifts the extents first. Depth isn't blocked, which is right for 2D-block formats. Still approving.

On klikso's Market numbers and the hold. I don't think this change explains that drop:

In those runs the budget was 1.5-2.1 GB with 2.3-2.9 GB of textures, and it moved between runs, which a fixed budget doesn't do. So the driver-figure rules set the line: the hold and its reserve, 90% and 97%. Each of those is "the count minus the excess the driver measures", and that excess is real VRAM whatever the count says. So the driver's figure, not how BC is counted, decides how much stays resident. If anything, with BC1/BC4 at their real size, taking N MB off the count now frees about N MB, where before it freed only half that for those textures. Joey-Magnus's point (the same number admits up to twice the real BC1/BC4) holds when the fixed budget is the line that binds. That only happens when nothing is short, so it's harmless there.
The change is small in this game. On NVIDIA/AMD the game's ASTC takes the decoded path, which is unchanged, so only native BC moves. Your Kokiri pair counted 4022 against 3978 MiB at the peak, about 1%.

That agrees with your alternated 8 GB model: +1.9% beside 1.8% of spread between the A runs, with the same retirement and compaction counts. klikso's base itself went from about 48 to 42 fps between sets, on a stack that also had #2711, #2731, #2685 and #2709.

For one cheap number on the size, look at the log's 600-present textures: decoded ASTC ... other N MB line in your unpressured control runs, A against B at the same spot. Native BC sits in "other", so the difference is about as much as this PR can move. A real 8 GB Windows pair from klikso is still welcome. Apart from that, the review needs nothing more. I also wouldn't lower the default for cards under 8.5 GB to make up for this unless that pair shows a drop: under pressure, the default isn't the line that binds.

Overlaps: none in the code. #2711 (blocks made at the start in the textures' pool, and the 97% rule waiting while the pool is down to them) and #2826 (the 90%/97% rules evict their excess, not an eighth) change how the driver's excess becomes a target, at the other end of vk_texture.cpp. They read the total but not how a texture is counted, so the three can land in any order. #2826 adds a row right after the RX_VK_VRAM_MB row in docs/development.md, so whichever lands second may need a small merge there.

CI: the one red job is test_release_scripts' test_nightly_two_runs_at_once_end_at_the_newer_commit, which fails on main too, as Joey-Magnus said. It merges cleanly.

> [@hckrman101](https://github.com/hckrman101) approved on GitHub · 2026-11-12 16:35 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2624#pullrequestreview-5878247891) Re-review. The code is the same as the commit I approved; this push rebased onto main and fixed the docs nit, and the row now matches what the integrated-GPU budget gives. Thanks. I rechecked the block math on the rebased file. The image's own format decides. Decoded ASTC is an RGBA8 image, so it stays at 4 bytes a texel. ASTC re-encoded to BC3 or BC7 counts 4x4 blocks of 16 bytes. Native BC1/BC4 count 8 bytes a block, BC2/3/5/6H/7 16, and native ASTC 16 per bw x bh block. Each level is rounded up to whole blocks on its own, so 2x2 and 1x1 tails count one block each, and a level left out (`x.skip`) shifts the extents first. Depth isn't blocked, which is right for 2D-block formats. Still approving. **On klikso's Market numbers and the hold.** I don't think this change explains that drop: - In those runs the budget was 1.5-2.1 GB with 2.3-2.9 GB of textures, and it moved between runs, which a fixed budget doesn't do. So the driver-figure rules set the line: the hold and its reserve, 90% and 97%. Each of those is "the count minus the excess the driver measures", and that excess is real VRAM whatever the count says. So the driver's figure, not how BC is counted, decides how much stays resident. If anything, with BC1/BC4 at their real size, taking N MB off the count now frees about N MB, where before it freed only half that for those textures. Joey-Magnus's point (the same number admits up to twice the real BC1/BC4) holds when the fixed budget is the line that binds. That only happens when nothing is short, so it's harmless there. - The change is small in this game. On NVIDIA/AMD the game's ASTC takes the decoded path, which is unchanged, so only native BC moves. Your Kokiri pair counted 4022 against 3978 MiB at the peak, about 1%. That agrees with your alternated 8 GB model: +1.9% beside 1.8% of spread between the A runs, with the same retirement and compaction counts. klikso's base itself went from about 48 to 42 fps between sets, on a stack that also had #2711, #2731, #2685 and #2709. For one cheap number on the size, look at the log's 600-present `textures: decoded ASTC ... other N MB` line in your unpressured control runs, A against B at the same spot. Native BC sits in "other", so the difference is about as much as this PR can move. A real 8 GB Windows pair from klikso is still welcome. Apart from that, the review needs nothing more. I also wouldn't lower the default for cards under 8.5 GB to make up for this unless that pair shows a drop: under pressure, the default isn't the line that binds. **Overlaps:** none in the code. #2711 (blocks made at the start in the textures' pool, and the 97% rule waiting while the pool is down to them) and #2826 (the 90%/97% rules evict their excess, not an eighth) change how the driver's excess becomes a target, at the other end of `vk_texture.cpp`. They read the total but not how a texture is counted, so the three can land in any order. #2826 adds a row right after the `RX_VK_VRAM_MB` row in `docs/development.md`, so whichever lands second may need a small merge there. **CI:** the one red job is `test_release_scripts`' `test_nightly_two_runs_at_once_end_at_the_newer_commit`, which fails on main too, as Joey-Magnus said. It merges cleanly.

github-sync commented

2026-11-12 16:55:21 +00:00

Owner

@mooki000 commented on GitHub · 2026-11-12 16:37 UTC · original

Thanks for the re-review and for working through why the count is not what set the line in those runs. I am lifting the hold: this can merge.

The number you suggested, from the unpressured control runs here (Market, the same spot, the log's textures: line once the scene has settled):

	decoded ASTC	other
A, parent `ee29dd55` (two runs)	2503 MB	564 MB
B, this pull request (two runs)	2505 to 2510 MB	431 MB

So at the Market the honest count is 133 MB lower, about 3% of what the textures count there: that is as much as this change can move in that scene. Together with the alternated runs under the 8 GB model (about 2%, at the edge of what this PC resolves) and klikso's note that his runs were on the harder model-L case with other pull requests stacked, I see no reason left to keep it waiting.

Test request T-TBJL stays open for an alternated pair on a real 8 GB card on Windows (recipe in my comment above): still welcome, as you say. If such a pair shows a real drop I will take it up at once, starting with where the line is set under pressure rather than the default budget.

> [@mooki000](https://github.com/mooki000) commented on GitHub · 2026-11-12 16:37 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2624#issuecomment-6829658861) Thanks for the re-review and for working through why the count is not what set the line in those runs. **I am lifting the hold: this can merge.** The number you suggested, from the unpressured control runs here (Market, the same spot, the log's `textures:` line once the scene has settled): | | decoded ASTC | other | |---|---|---| | A, parent `ee29dd55` (two runs) | 2503 MB | 564 MB | | B, this pull request (two runs) | 2505 to 2510 MB | 431 MB | So at the Market the honest count is 133 MB lower, about 3% of what the textures count there: that is as much as this change can move in that scene. Together with the alternated runs under the 8 GB model (about 2%, at the edge of what this PC resolves) and klikso's note that his runs were on the harder model-L case with other pull requests stacked, I see no reason left to keep it waiting. Test request T-TBJL stays open for an alternated pair on a real 8 GB card on Windows (recipe in my comment above): still welcome, as you say. If such a pair shows a real drop I will take it up at once, starting with where the line is set under pressure rather than the default budget.

github-sync referenced this pull request

2026-11-12 17:57:58 +00:00

fix(vk): keep the textures the GPU drew that the game samples again after finalizing them (a cached shadow map) #2368

github-sync referenced this pull request

2026-11-12 17:59:39 +00:00

perf(vk): an import mostly outside every game pool is trimmed back to its live parts #2825

Commenting is not possible because the repository is archived.

No reviewers

hckrman101