Repository navigation
fix(vk): count natively sampled ASTC textures by their blocks in the texture budget - #2288
Conversation
…texture budget On a GPU that samples ASTC itself (Intel's), create() counted each texel of an ASTC image as the block's 16 bytes, 16 (4x4) to 144 (12x12) times the image's size. A UHD 770 (Mesa 26.2.5) counted 17608 MB of textures against an integrated GPU's 2560 MB budget in Kokiri Forest while every allocation of the process was 4766 MB, so the budget's eviction ran at every frame. Such an image's level chain is now counted in blocks (texture_level_units); every other image is counted as before. tests/test_native_astc_texture_count.py runs texture_level_units on block and texel cases and checks that create() counts through it for native ASTC only. Assisted-by: Claude Opus 5.5 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
hckrman101
left a comment
There was a problem hiding this comment.
Thanks, this is a clean fix and the numbers in the description make the case well.
I checked the math: for a natively sampled ASTC image the level chain is now ceil(w_l / bw) * ceil(h_l / bh) * d units of 16 bytes, which is what Vulkan says a level of a block-compressed image takes (mip extent first, then rounded up to whole blocks), times layers. That holds for all 14 block sizes; the test covers each of them, part blocks and 1x1 levels. bw/bh come from nvn_block_size in texture_get, so they are the real block dims for these formats, and every other image (including decoded ASTC, which keeps astc ? 4 per texel) goes through the 1x1 path and counts exactly as before. On NVIDIA/AMD nothing changes.
Budget/eviction: x.pad still raises the count to allocation_size, so for these images bytes + pad is now about the allocation instead of 16 to 144 times it, and total_bytes goes back to meaning what the budget assumes. Removal (tex.bytes + tex.pad), the eviction sweep's pending_bytes and compaction's moved_bytes all use the stored value, so they stay consistent. The gpu_written size thresholds in the sweep don't apply to sampled ASTC. This doesn't touch the lines #2198 or #2368 change, and it should help both: the compaction hold's held_budget(budget, total_bytes, ...) in #2198 would otherwise see the inflated total on Intel.
Nits, none blocking:
- The pre-creation estimate that picks top levels to leave out (
bytes_atincreate,texel = astc ? 4 : std::max(1, bpb / std::max(1, bw * bh))) still counts native ASTC per texel at 1 byte, which is exact for 4x4 but 4x too big for 8x8 and 9x for 12x12. It only matters pastRX_VK_TEX_MAX_MB(1536 MB) or for the sparse decision, so it's fine to leave, buttexture_level_unitswould fit there too if you want the two to agree. - See the inline notes for a follow-up idea on BC and a unit mix-up in the test comment.
CI: only the PR title check has run; the build and game-free regression workflows are waiting for a maintainer to approve the run. The new test declares RUN_ALL_ARGS, so run_all picks it up. Approving on the code; it should go in once those workflows are green. Looking forward to the RTX 4090 run you mentioned.
| for (uint32_t l = 0; l < x.levels; l++) | ||
| px += (uint64_t)std::max(1u, x.width >> l) * std::max(1u, x.height >> l) * std::max(1u, x.depth >> l); | ||
| px += texture_level_units(x.width >> l, x.height >> l, x.depth >> l, astc_blocks ? (uint32_t)bw : 1u, astc_blocks ? (uint32_t)bh : 1u); | ||
| const uint32_t texel = x.bc || is_bc(x.format) ? 1 : astc ? 4 : std::max(1, from ? (from == 3 ? 4 : from == 6 ? 8 : 16) : bpb); |
There was a problem hiding this comment.
Not for this PR: BC gets the same kind of over-count at a smaller scale. 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). Counting BC by blocks through texture_level_units with bpb would fix that the same way, if you feel like a follow-up.
| assert int(o) == max(1, w) * max(1, h) * max(1, d) # (unchanged for everything but native ASTC) | ||
|
|
||
| # The array the UHD 770's log named: 1365 x 1365, 90 layers, 11 levels, ASTC 8 x 8. Counted as texels at 16 bytes, | ||
| # 3410 MB; its blocks are 56 MB |
There was a problem hiding this comment.
Nit: 56365920 bytes is 56 MB decimal but 53.75 MiB, and the print below shifts by 20, so the output says 53 while this comment says 56 (and the PR body says 54). 3410 is MiB too. Maybe say MiB here, or 54 MB, so they agree.
|
@hckrman101 a heads-up, mine again: this pull request's test does not compile on Windows, so |
|
The owed RTX 4090 check has finished. Both logs say Arch Linux, NVIDIA 615.78.08, headless, same save and scripted camera input, 170 s limits, Numbers below come from each run's highest texture-count log line. Units are MiB (the log prints “MB”, but shifts byte counts by 20):
Both runs reached their time limits without a fatal/crash/device-loss report or kernel driver-fault line. The live texture populations and system loads differ, so these peaks are not an equal-workload memory or frame-rate comparison; I am not attributing the difference to this patch. This closes the owed NVIDIA-path run for the merged ASTC fix. It does not test the later proposed BC block-counting/pre-creation-estimate followup, Windows, or AMD. |
… marks scanf deprecated (#2397) **What this is for:** main's game-free checks pass on Windows again. The test that came with my #2288 does not compile there, so `Python source checks (windows-2022, part 1/2)` fails on every run since that merge. **What is measured:** the test with g++ and clang++ on Linux, on this branch. **Not measured:** Windows on my side: I have no Windows machine, so this pull request's own windows-2022 job is the proof, and I will confirm its result here. ## Problem - `tests/test_native_astc_texture_count.py` compiles a small fixture with `-Wall -Wextra -Werror` and reads its cases with `std::scanf`. With clang and the Windows CRT's headers `scanf` is marked deprecated, which `-Werror` makes an error: ``` fixture.cpp:13:17: error: 'scanf' is deprecated: This function or variable may be unsafe. Consider using scanf_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. ``` - #2288's workflows had not run before it was merged (they waited for an approval), and I had run the test on Linux only. It first showed in a later pull request's run: [#2289's windows-2022 part 1/2](https://github.com/NotRexed/Hookshot/actions/runs/43065144550/job/130677975993), 179 passed, 1 failed, 13 skipped. ## What changed - `tests/test_native_astc_texture_count.py`: the fixture defines `_CRT_SECURE_NO_WARNINGS` ahead of its includes, as the other test sources that use these functions do (`tests/capture_dimensions_test.cpp`, `tests/png_publication_test.cpp` and others). One line; no assertion, case or source file changes. ## How it was tested Arch Linux, g++ 16.2.1 and clang++ 23.1.3, branch head `fc0157a1` on main `696d85b0`: - `python -I -B tests/test_native_astc_texture_count.py --cxx g++` and `--cxx clang++`: `native ASTC texture count: 182 cases pass`, as before the change. ## Not tested - Windows here. The define is the one the compiler's message names, and the one main's other tests use for the same deprecation. ## Risks - Tests only. Fixes #2396. Assisted-by: Claude Opus 5.5 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
|
Thanks. #2624 counts native BC textures by their blocks, uses |
Follow hckrman101's review of NotRexed#2288. Count native BC/ASTC at each mip's whole blocks, including part blocks and 1x1 levels, and use the same unit helper in the pre-creation estimate. Re-encoded BC3 uses its own 4x4, 16-byte blocks rather than source ASTC dimensions. Keep the allocation floor and sparse accounting, and use MiB consistently in the test. Exercise the production estimate and final count: 4845 cases plus the 182 existing unit cases pass with GCC and Clang. The base fails 3040; reverting only the estimate fails 2508 and only the final count 1140. Assisted-by: GPT-6 Astra Co-Authored-By: GPT-6 Astra <noreply@openai.com>
Follow hckrman101's review of NotRexed#2288. Count native BC/ASTC at each mip's whole blocks, including part blocks and 1x1 levels, and use the same unit helper in the pre-creation estimate. Re-encoded BC3 uses its own 4x4, 16-byte blocks rather than source ASTC dimensions. Keep the allocation floor and sparse accounting, and use MiB consistently in the test. Exercise the production estimate and final count: 4845 cases plus the 182 existing unit cases pass with GCC and Clang. The base fails 3040; reverting only the estimate fails 2508 and only the final count 1140. Assisted-by: GPT-6 Astra Co-Authored-By: GPT-6 Astra <noreply@openai.com>
Follow hckrman101's review of NotRexed#2288. Count native BC/ASTC at each mip's whole blocks, including part blocks and 1x1 levels, and use the same unit helper in the pre-creation estimate. Re-encoded BC3 uses its own 4x4, 16-byte blocks rather than source ASTC dimensions. Keep the allocation floor and sparse accounting, and use MiB consistently in the test. Exercise the production estimate and final count: 4845 cases plus the 182 existing unit cases pass with GCC and Clang. The base fails 3040; reverting only the estimate fails 2508 and only the final count 1140. Assisted-by: GPT-6 Astra Co-Authored-By: GPT-6 Astra <noreply@openai.com>
Follow hckrman101's review of NotRexed#2288. Count native BC/ASTC at each mip's whole blocks, including part blocks and 1x1 levels, and use the same unit helper in the pre-creation estimate. Re-encoded BC3 uses its own 4x4, 16-byte blocks rather than source ASTC dimensions. Keep the allocation floor and sparse accounting, and use MiB consistently in the test. Exercise the production estimate and final count: 4845 cases plus the 182 existing unit cases pass with GCC and Clang. The base fails 3040; reverting only the estimate fails 2508 and only the final count 1140. Assisted-by: GPT-6 Astra Co-Authored-By: GPT-6 Astra <noreply@openai.com> (cherry picked from commit da33e6c)
Intel GPUs sample ASTC textures directly, and the texture budget counts each of those textures 16 to 144 times too big. On an Intel UHD 770 the count in Kokiri Forest reached 17.6 GB against the integrated GPU's 2.5 GB budget, while the whole process held 4.8 GB of GPU memory, so the cache kept evicting textures it had room for.
Fixes #2333.
What changed
gpu/vk/vk_texture.cppcreate: the budget counts an image as its level chain's units times the bytes of one unit. An ASTC image the GPU samples natively keeps its blocks (vk: ASTC sampled natively:astcis false,is_astc(x.format)). For those the unit was the texel, but its size was the block's 16 bytes (bpb). A 4x4-block texture counted 16 times its size, a 12x12 one 144 times.texture_level_unitsnow counts such a level in blocks (ceil(w / bw) * ceil(h / bh)), and every other image in texels, as before. The floor at the allocation's size (x.pad) is unchanged.tests/test_native_astc_texture_count.py(new): extractstexture_level_unitsand runs 182 cases: every ASTC block size, part blocks, levels down to 1x1, and 1x1 units for other images, which must equal the old count. It also checks thatcreatecounts through it for native ASTC only. Without the change it fails, because the helper isn't there.GPUs that decode ASTC on upload (NVIDIA and AMD:
vk: ASTC not sampled by this GPU: blocks are decoded on upload) take the 1x1 path, so their counts don't change.How it was tested
Intel UHD 770 (Raptor Lake-S GT1, no mesh shaders), Mesa 26.2.5 ANV on i915, Arch Linux, i9-13900K, 93 GB RAM. The game ran headless, capped at 32 GB. Same save in Kokiri Forest, the same scripted input (load the save, then camera turns), the same shader caches, 8 minutes each. The fix was tested as a cherry-pick (defddca0) onto main 872cb68, where those caches were made. Rebased onto 49f75dc, the commit is the same apart from line numbers. Numbers come from
hookshot_log.txt(VRAM: ... textures N MB (budget B MB), T textures):VRAM: ... used)On main the count jumps 3410 MB for one 1365x1365 array of 90 layers, ASTC 8x8, whose blocks are 54 MB. With the change the count stays under the budget, the cache keeps about 20% more textures, and the process's allocations stay where they were. The log still says
vk: ASTC sampled natively. No errors, no device lost, nothing from i915 in the kernel log. The picture after loading is the same as main's, which is wrong on this GPU for an unrelated reason (the upscaler's input; a separate PR). 9.4 fps against 8.4, GPU-bound at about 100 ms a frame and at different load, so I make no speed claim.Checks:
python -I -B tests/run_all.pyon the branch: 362 passed, 3 failed, 10 skipped. The failures aretest_updater_asset_digest.py(needs the .NET 8 SDK; only .NET 10 here),test_wayland_libdecor_hint.py(clang 23's-Wunused-but-set-globalunder-Werror), which both fail the same way on main here, andnx2/test_texture_phi_dominance.py, which timed out under load and passes alone.test_native_astc_texture_count.pypasses with g++ and clang++.RTX 4090 (NVIDIA 615.78.08): its log says
vk: ASTC not sampled by this GPU: blocks are decoded on upload, so its textures take the 1x1 path, whose count the test checks is the old one. A run of the branch on it is queued behind other work and not finished; I'll add it here.Not tested
Windows (Intel's own driver), Intel Arc, and other Intel iGPUs. Mesa's other drivers that sample ASTC natively (none on this machine). AMD. Other areas of the game. How much the extra textures held change frame pacing on Intel: the UHD 770 is GPU-bound at about 100 ms a frame in this scene, so I made no speed claim.
Refs #1001 and #999 (the Intel trackers). The over-count is the same on any GPU that reports ASTC as sampled natively; I could only run it on the UHD 770.
Assisted-by: Claude Opus 5.5
Co-Authored-By: Claude Opus 5.5 noreply@anthropic.com