fix(vk): upload ASTC textures re-encoded as BC3 over several submissions (#900 rebased) #1130
Open
metroid98 wants to merge 7 commits from
metroid98/fix/bc3-multi-submission-rebase into main
pull from: metroid98/fix/bc3-multi-submission-rebase
merge into: MagicBeanDev:main
MagicBeanDev:main
MagicBeanDev:github/pr/2893
MagicBeanDev:github/pr/2890
MagicBeanDev:github/pr/2887
MagicBeanDev:github/pr/2878
MagicBeanDev:github/pr/2875
MagicBeanDev:github/pr/2874
MagicBeanDev:github/pr/2873
MagicBeanDev:github/pr/2871
MagicBeanDev:github/pr/2756
MagicBeanDev:github/pr/2897
MagicBeanDev:github/pr/2896
MagicBeanDev:github/pr/2718
MagicBeanDev:github/pr/2842
MagicBeanDev:github/pr/2691
MagicBeanDev:github/pr/2895
MagicBeanDev:github/pr/2743
MagicBeanDev:github/pr/2765
MagicBeanDev:github/pr/2708
MagicBeanDev:github/pr/2732
MagicBeanDev:github/pr/2755
MagicBeanDev:github/pr/2817
MagicBeanDev:github/pr/2876
MagicBeanDev:release-please--branches--main
MagicBeanDev:release-please--branches--main--release-notes
MagicBeanDev:github/pr/2778
MagicBeanDev:github/pr/2888
MagicBeanDev:github/pr/2866
MagicBeanDev:github/pr/2877
MagicBeanDev:github/pr/2800
MagicBeanDev:github/pr/2829
MagicBeanDev:github/pr/2850
MagicBeanDev:github/pr/2889
MagicBeanDev:github/pr/2882
MagicBeanDev:github/pr/2860
MagicBeanDev:github/pr/2849
MagicBeanDev:github/pr/2886
MagicBeanDev:github/pr/2794
MagicBeanDev:github/pr/2840
MagicBeanDev:github/pr/2830
MagicBeanDev:github/pr/2834
MagicBeanDev:github/pr/2719
MagicBeanDev:github/pr/2742
MagicBeanDev:github/pr/2741
MagicBeanDev:github/pr/2820
MagicBeanDev:github/pr/2771
MagicBeanDev:github/pr/2859
MagicBeanDev:github/pr/2858
MagicBeanDev:github/pr/2823
MagicBeanDev:github/pr/2757
MagicBeanDev:github/pr/2769
MagicBeanDev:github/pr/2844
MagicBeanDev:github/pr/2843
MagicBeanDev:github/pr/2673
MagicBeanDev:github/pr/2855
MagicBeanDev:github/pr/2739
MagicBeanDev:github/pr/2807
MagicBeanDev:github/pr/2851
MagicBeanDev:github/pr/2789
MagicBeanDev:github/pr/2814
MagicBeanDev:github/pr/2818
MagicBeanDev:github/pr/2774
MagicBeanDev:github/pr/2772
MagicBeanDev:github/pr/2750
MagicBeanDev:github/pr/2786
MagicBeanDev:github/pr/2785
MagicBeanDev:github/pr/2782
MagicBeanDev:github/pr/2779
MagicBeanDev:github/pr/2775
MagicBeanDev:github/pr/2706
MagicBeanDev:github/pr/2766
MagicBeanDev:github/pr/2747
MagicBeanDev:github/pr/2759
MagicBeanDev:github/pr/2745
MagicBeanDev:github/pr/2740
MagicBeanDev:github/pr/2733
MagicBeanDev:github/pr/2725
MagicBeanDev:github/pr/2727
MagicBeanDev:github/pr/2728
MagicBeanDev:github/pr/2729
MagicBeanDev:github/pr/2721
MagicBeanDev:github/pr/2722
MagicBeanDev:github/pr/2715
MagicBeanDev:github/pr/2714
MagicBeanDev:github/pr/2707
MagicBeanDev:github/pr/2703
MagicBeanDev:github/pr/2705
MagicBeanDev:github/pr/2697
MagicBeanDev:github/pr/2679
MagicBeanDev:github/pr/2676
MagicBeanDev:github/pr/2669
MagicBeanDev:docs/what-hookshot-connects-to
MagicBeanDev:fix/hdr-encode-test
MagicBeanDev:fix/main-tests
MagicBeanDev:fix/play-cache-files
MagicBeanDev:perf/game-barrier-order
MagicBeanDev:feat/heap-lazy
MagicBeanDev:feat/texture-detail
MagicBeanDev:perf/astc-opaque-bc1
MagicBeanDev:fix/fs-dir-lock
MagicBeanDev:fix/framegen-native-kept
MagicBeanDev:fix/update-no-downgrade
MagicBeanDev:port/k10-gcn-f16-mirrors
MagicBeanDev:fix/framegen-mult-cap
MagicBeanDev:chore/release-as-2.1.1
MagicBeanDev:perf/pcache-load-switch
MagicBeanDev:fix/prepare-progress-log
MagicBeanDev:fix/update-closes-stale-game
MagicBeanDev:fix/hdr-scaled-sampled
MagicBeanDev:perf/first-start-preset
MagicBeanDev:perf/zero-upload
MagicBeanDev:fix/vcore-escape-2s
MagicBeanDev:test/fake-gh-atomic
MagicBeanDev:perf/batch-idle-flush
MagicBeanDev:test/finish-stub-a
MagicBeanDev:chore/record-2.1.0
MagicBeanDev:fix/main-red-tests
MagicBeanDev:fix/test-display-startup-driver-cache
MagicBeanDev:feat/mesh-shader-mode
MagicBeanDev:fix/menu-probes-at-startup
MagicBeanDev:fix/fps-counter-classic
MagicBeanDev:fix/test-log-batch-flake
MagicBeanDev:fix/narrow-screen-hud
MagicBeanDev:ci/prereleases
MagicBeanDev:release/v2.0.15
MagicBeanDev:hotfix/v2.0.15
MagicBeanDev:fix/present-ahead-replay
MagicBeanDev:release/v2.0.13
MagicBeanDev:fix/bracket-paths-2013
MagicBeanDev:fix/hdr-overlays-2013
MagicBeanDev:fix/fps-counter-scale-2013
MagicBeanDev:fix/stick-seen-at-rest
MagicBeanDev:hotfix/menu-fits-screen
MagicBeanDev:fix/f1-always-opens-menu
MagicBeanDev:fix/ini-keep-lines
MagicBeanDev:claude/cranky-pike-7cf247
MagicBeanDev:perf/texture-answer-flat
MagicBeanDev:fix/stamp-test-modules
MagicBeanDev:fix/appimage-fonts
MagicBeanDev:fix/display-awake
MagicBeanDev:release/v2.0.12
MagicBeanDev:fix/discord-windows
MagicBeanDev:rc/v2.0.12-tests
MagicBeanDev:fix/dlss-keep-feature
MagicBeanDev:rc/v2.0.12
MagicBeanDev:button-icons
No reviewers
Labels
Clear labels
accessibility
Barrier affecting people with disabilities
autorelease: pending
autorelease: tagged
bug
Something isn't working
dependencies
Pull requests that update a dependency file
documentation
Improvements or additions to documentation
duplicate
This issue or pull request already exists
enhancement
New feature or request
github_actions
Pull requests that update GitHub Actions code
good first issue
Good for newcomers
help wanted
Extra attention is needed
invalid
This doesn't seem right
question
Further information is requested
testing wanted
wontfix
This will not be worked on
No labels
accessibility
autorelease: pending
autorelease: tagged
bug
dependencies
documentation
duplicate
enhancement
github_actions
good first issue
help wanted
invalid
question
testing wanted
wontfix
Milestone
Clear milestone
No items
No milestone
Assignees
Clear assignees
No assignees
2 participants
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".
No due date set.
Dependencies
No dependencies set
Reference
MagicBeanDev/Hookshot!1130
Loading…
Reference in a new issue
No description provided.
Delete branch "metroid98/fix/bc3-multi-submission-rebase"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
This is #900 by @ZurielAtr, rebased onto current main because it was closed only for conflicts ("please rebase on current main and reopen"). Their commit is kept as is (authorship preserved); the conflict resolution is part of that cherry-picked commit. The original description and hardware test (RX 7700S, Windows 11, Adrenalin 26.10.1: Castle Town to Hyrule Field reset the driver 3 out of 3 times before, no reset after) are in #900.
It belongs to the AMD-on-Windows tracker #996 ("device lost after minutes of play"). It has no
Fixesline, because the original did not have one.What was resolved
Two conflicts, nothing else changed:
tests/run_all.py: kept main's three newerARGSentries and addedtests/test_bc_parts.py. The original workflow-file registration is not needed, since the runner discovers tests.gpu/vk/vk_texture.cpp: the#includelist (kept main's includes plusbc_parts.h), and the layer-run loop inupload_astc_gpu. Since #900, main made BC3 uploads one layer a run by default (RX_VK_BC_LAYERS, default 1, to stop the dark lines on Hyrule Field's terrain). The loop now takes its run length from that samerunvalue, passed tobc_parts::run_endinstead ofkBcLayers, so main's behaviour is kept and only the per-recording budget (RX_VK_BC_PART_MTEXELS) is added.tests/test_bc_parts.py's source checks still match the resolved files (I compared the needles by reading).Not run
Nothing was compiled or run locally: this was code-only work and no tests, game or build were executed. The behaviour is the original author's tested one; only the merge was done here. It needs a build,
tests/run_all.py, and ideally a repeat of the Castle Town to Hyrule Field test on AMD, with the one-layer-a-run default in place.AI-assisted (Claude Code) on behalf of metroid98.
🤖 Generated with Claude Code
https://claude.ai/code/session_015DmSMGJSY9hHswALy4eqtQ
Tested on NVIDIA (RTX 3070 8 GB, Windows 11), merged onto v2.0.8-rc.5, with a scripted run: load the Hyrule Castle autosave, turn the camera.
BC3 texture … 1365x1365 x90: 89 of 90 layers left for the next recordings, 20 such lines.RX_VK_FRAME_LOG), two runs each, from the save's confirm screen into play:So on this card the castle load stalls about 1.2 s longer. That is perhaps the price of spreading the 90-layer arrays over several recordings. Gating the per-recording budget to AMD, or raising it on NVIDIA, might avoid it.
Thanks @Nitnek, that's exactly the data this needed. Pushed
b7f5525: the per-recording budget now defaults to AMD only (dev.props.vendorID == 0x1002, the same checkvk_draw.cppuses). Other GPUs default to 0, meaning all at once as before, so NVIDIA should be back to rc.5's load hitch.RX_VK_BC_PART_MTEXELSstill overrides it on any GPU. Not built locally. A re-run of your castle script on the 3070 would confirm the hitch is back to ~0.9 s.(AI-assisted via Claude Code on my behalf.)
Hi! This PR has merge conflicts with
mainnow (a lot has landed today). Could you mergemainin or rebase and resolve them? Happy to give it a proper review once it's up to date.Merged
mainin (onlytests/run_all.pyconflicted: kept bothtest_output_resolution.pyandtest_bc_parts.py). GitHub now reports it as mergeable. Thanks for the heads-up. (AI-assisted via Claude Code on my behalf.)Test result from an AMD card, since this PR defaults to AMD only and the one test above was on NVIDIA (where it is off): on an RX 7800 XT the default of 16 M texels a recording showed large blocks of wrong colour in the sky, and they went away with
RX_VK_BC_PART_MTEXELS=0. One observation, so please take it as a lead and not a verdict.Setup: RX 7800 XT 16 GB, Ryzen 7 5800X3D, Windows 11, driver 0x8001a0 as Vulkan reports it, v2.0.11 with this PR merged by hand on top (see the caveat), 2560x1440, frame generation x2, normal play. The log line for the split path appears (
BC3 texture 0x109538a600 1365x1365 x90: 87 of 90 layers left for the next recordings (1 so far), then 84, 81, 78 ... up to the 20 lines the log prints).What I saw, about 11 minutes after start, standing in Castle Town below the ruined castle: big square blocks, black, purple, yellow and red, over the sky (and over the top-left corner behind the HUD), with a stepped outline like wrong BC blocks. Two screenshots 0.7 s apart were identical (the fairy and the flames moved, the blocks did not), so it is a texture that stayed wrong, not an interpolated frame.
I then set only
RX_VK_BC_PART_MTEXELS=0(everything at once, as before) and restarted; the sky was clean in the same place. I have only about 2 minutes of play since, so I will repeat it for a longer session before calling it confirmed.Caveats:
RX_VK_LOG_MADE/RX_VK_LOG_UPLOADSthe next time it happens.If there is a place where a deferred layer can be skipped (a texture used again, or made again, before
texture_bc_continuegets to it), that would fit what I saw. Happy to test a build on this card.Thanks @lucasmazzoni45-del, that's an important lead, since AMD is exactly where this is on by default. I've moved the PR back to draft until it's understood: as you say, with
RX_VK_BC_PART_MTEXELS=0this PR does nothing, so "clean with 0, blocks with 16" points at the split path. That could be a deferred layer run that is never re-uploaded or goes stale, or the BC scratch being reused across recordings.I can't reproduce this without an AMD card. If a longer run confirms it, the most useful data would be whether the blocks appear only after a texture had
... layers left for the next recordingslines, and whether they persist across a scene reload. @ZurielAtr, since it's your change, you may see the cause faster than I can.(AI-assisted via Claude Code on my behalf.)
Thanks @metroid98 for moving it to draft and for the questions. What my log from the session with the blocks can and cannot answer:
... layers left for the next recordingslines: only one is printed,0x109538a600 1365x1365 x90. The line is only logged for the first 20 deferrals and then every 200th, and nothing after the 20th was printed, so there were fewer than 200 deferrals in the whole session, but I cannot tell which textures deferrals 21 and up were.RX_VK_BC_PART_MTEXELS=0; the game was a new process by then.What would settle it is one more log line when a deferred texture is finished or dropped (the
bc_pendingerase in the texture's destroy path), with the texture's address, so a "left" line can be matched to an "all layers in" line. I can add that to my build and re-run with the default of 16, if that is useful to you; I would rather not guess which texture it was.Also, my build is v2.0.11 + this PR + AMD changes of mine, so it may not behave exactly like the PR on stock main.
Correction to my earlier report here, so nobody chases the wrong thing: square blocks came back while the split was off. With
RX_VK_BC_PART_MTEXELS=0(the log has nolayers left for the next recordingsline at all in this session) I saw them again about 5 minutes after start.They do not look like the first ones, though. The first were opaque black, purple, yellow and red blocks over the sky of Castle Town. These are translucent squares, mostly greyer than the glow around them, in a grid over a very bright light shaft in a forest area (RX 7800 XT, v2.0.11, same machine). That second kind looks like #1117 (blocky light halos around bright spots, the few NEAREST samplers), not like a texture that was left half uploaded.
So what I can say now:
I would not treat "clean with 0" as a confirmation of anything yet. Sorry for the noise.
Thanks for the careful correction, @lucasmazzoni45-del. That makes the translucent grid look like #1117 rather than this PR. I'll keep this as a draft until someone gets a longer AMD run with the default 16 M budget and either sees the opaque sky blocks again or doesn't, then mark it ready or dig in. (AI-assisted via Claude Code on my behalf.)
Follow-up with a useful detail: the place where I first saw the opaque black/purple/yellow/red blocks (the sky of Castle Town, below the ruined castle) is clean again with
RX_VK_BC_PART_MTEXELS=0. I went back to that spot in this session and the sky is fine there.The translucent squares I mentioned in my correction are somewhere else, in a strong light shaft in the Kakariko graveyard, and I think they are #1117's blocky glow and not this PR. So the two things are separate, and the Castle Town result is consistent with my first report: the blocks there went away when the split was turned off.
Still one observation on one machine, and a few minutes in the same spot, not a long run, so I would still call it a lead. Today's log has no deferral line at all with the setting at 0; the earlier session with the default had 20 printed ones (the only texture shown was the 1365x1365 x90 array, at the load).
Thanks @lucasmazzoni45-del, that makes the split path the suspect for the opaque blocks. I've read through it for a cause; here's what I found:
destroy_later, which drops the texture's pending entries, so a recycledTexture*can't receive another texture's layers.8487ee8: withRX_VK_PARALLEL> 1, segment workers pushed tobc_pending(a plain vector) while the replay thread drained it. A deferred run could also land in another segment that the queue reaches before the image's first layout change, which is the hazard theseg_initcomment describes. Segment workers now upload all at once. WasRX_VK_PARALLELset in your session? If not, this isn't your cause.N of 90 layers leftcounting down to the end, or not, would show which.Still a draft. Not built or run here (no AMD card).
(AI-assisted via Claude Code on my behalf.)
Thanks @metroid98 for reading through it. Answers:
RX_VK_PARALLEL: no. It is not inhookshot_env.txtin that session or in any of my earlier copies of the file, and not in the user or machine environment. TheParallel...lines in that log are the game's ownParallelDecompThreadmessages, not the parallel replay. So the replay was serial and thebc_pendingrace in8487ee8is not my cause.images from a pool of 128 MB blocks (memory type 0), 30 made at the start, so 30 blocks (3.75 GB) were allocated up front, against the default of about 12 on this card (10% of VRAM). Your point about the deferred layers showing old BC blocks from another texture fits that; I cannot say whether the default would show them too.N of 90 layers leftlines printed in that session are only the first 20 (the log stops printing after 20), so the countdown to the end is not in my log either; the 20 lines are all at the load (present ~2280), the blocks were on screen at present ~15960.What I can do next without a rebuild: a longer run with the split on at the default 16 and the preallocation at the default as well, to separate "split path" from "split path with a big preallocated pool". If I rebuild, I would add a log line when a deferred texture finishes, with its address. Tell me which of the two is more useful to you; I will report here either way.
Thanks, that settles parallel replay. The gap you found (deferrals at present ~2280, blocks at ~15960) is the key question: did that array's continuation ever finish? Pushed
f494015, which adds the line you suggested:vk: BC3 texture …: its last N deferred layers are in (K finished so far)when a texture's deferred layers are all uploaded, and… destroyed with deferred layers still to comewhen one is dropped. If your next run with the default 16 shows the blocks, three cases separate cleanly:Running with the default pool size at the same time, as you planned, would separate the pool question as well. (AI-assisted via Claude Code on my behalf; not built here.)
Thanks @metroid98 for carrying this, and @lucasmazzoni45-del for the careful runs. Reading the current head (
f494015), I found two holes in the split path that would explain opaque blocks. Both are fixed in one commit on top off494015: 6f76660 on ZurielAtr:fix/bc3-parts-fill.Deferred layers showed stale image memory. Until
texture_bc_continuereached them, a new image's deferred layers held whatever its memory held before. With pooled image memory (VMA blocks, and more so with a large preallocated pool), that is other textures' old BC3 blocks: opaque black, purple, yellow and red squares with BC-block edges. My original comment ("a frame or two of a scene change") only covered how long, not what was shown. Now, when a new image defers layers, they are filled in the same recording with a VRAM-to-VRAMvkCmdCopyImageof the last layer encoded (every level). There is no decode or encode, so the long submission does not come back. A texture that was uploaded before keeps its old layers meanwhile.A failed continuation dropped its layers for good, and the new line called it finished. If
upload_astc_gpureturnsfalseinsidetexture_bc_continue(no staging arena, no scratch), nothing went back onbc_pending. Sobc_pending.size() == before, and the log said "its last N deferred layers are in", while those layers kept the stale memory until the texture was made again. That would also fit blocks that do not move between screenshots 0.7 s apart. The entry now goes back on the list (up to 8 tries), "in" is logged only when it recorded, and giving up is logged as such.tests/test_bc_parts.pycovers both:fill_source/retry_continue, plus source checks that fail without either fix.Tested on RX 7700S 8 GB, Windows 11, Adrenalin 26.10.1, v2.0.11 + this PR + the fix, default budget (16 M texels): one session of about 5.5 minutes in play, including the spot where I used to see blocks in the sky. Seven textures deferred layers (the two 1365x1365 x90 terrain arrays, twice each, plus three small arrays); all seven logged
filled from layer 0and thenits last N deferred layers are in. None wascould not be uploaded, there was no device lost, and no opaque blocks in the sky.@metroid98, feel free to cherry-pick it onto this branch (
git fetch https://github.com/ZurielAtr/Hookshot fix/bc3-parts-fill && git cherry-pick 6f76660).AI-assisted (Claude Code) on my behalf.
Thanks @ZurielAtr, both holes are real. The second is in the log line I added: it would have called a failed continuation "in". I fast-forwarded this PR to your
6f76660, so it's now the head here, with your authorship kept. The fill is a VRAM copy inside the same recording, so it shouldn't bring back the long submission. @lucasmazzoni45-del, a run of this head with the default 16 would now be the real test: no opaque blocks at all, plus the newfilled from layerandlast … are inlines. Still a draft until then. (AI-assisted via Claude Code on my behalf; not built here.)Thanks @ZurielAtr and @metroid98, both holes read right to me, and the first one describes what I saw (opaque black, purple, yellow and red squares with BC-block edges, not moving between two screenshots).
I will run the head you pushed (
6f76660) tomorrow on the RX 7800 XT / Windows 11, with the defaults you asked for: the split at 16 and the image pool at its default size, noRX_VK_BC_PART_MTEXELSand noRX_VK_IMAGE_PREALLOC. I will go through Castle Town below the ruined castle (where I first saw the blocks), out to Hyrule Field and back, and report here whether any opaque blocks show up, plus the newfilled from layerandlast ... are inlines from the log.I will also look at what it does to performance on this card: frame times and the replay
submit/memory syncfigures from the log, compared with a session of the same build with the split off. I will keep that apart from the blocks result, and I will say how long each session was so the numbers are not over-read.Thanks @metroid98 and @ZurielAtr! Tomorrow I'll run the current head
(
6f76660) with the default 16, both with my larger pool and with thedefault pool size, and report whether the opaque blocks are gone and
whether the
filled from layer/last … are inlines show up.On Tue, Nov 10, 2026 12:04 AM, metroid98 @.***> wrote:
Small change of plans: I'm setting everything up for testing right now
instead of tomorrow. I'll run the current head (
6f76660) with the default16 and report back here with the log (opaque blocks or not, plus the
filled from layer/last … are inlines).On Tue, Nov 10, 2026 12:04 AM, metroid98 @.***> wrote:
Starting the test of
6f76660now. Build: current main (0828863) with this head merged on top (one conflict, only in the test list in tests/run_all.py), no patches of mine, stock settings: the split at the default 16, no RX_VK_BC_PART_MTEXELS, no RX_VK_IMAGE_PREALLOC. RX 7800 XT / Windows 11.The first start after the swap redoes the shaders and pipelines, so that session will not count for performance. After that I will walk Castle Town below the ruined castle, out to Hyrule Field and back, look for opaque blocks, and post the
filled from layer/last … are inlines. Then a second session with my larger pool, and one with the split off for the frame-time comparison. Results here, with the length of each session.Result of
6f76660on RX 7800 XT / Windows 11: the split runs and the loading is fast; I cannot yet say the opaque blocks are gone for good.Build and setup: current main (
0828863) with this head merged (one conflict, only in the test list intests/run_all.py), no other patches. RX 7800 XT 16 GB, Ryzen 7 5800X3D, 2560x1440 fullscreen, frame generation x2, frame pacing on, emptyhookshot_env.txt(so the split is at the default 16, noRX_VK_BC_PART_MTEXELS, noRX_VK_IMAGE_PREALLOC). Two mods were active (a fix for 3 damaged textures and a UI mod). Played with a save: Ganon's Castle area as adult Link, Zora's River path, Zora's Domain (frozen) and back. About 27 minutes so far, one session, normal play and not a scripted route. Castle Town and Hyrule Field are not done yet.What the log shows:
deferred layers filled from layer N until they are in, 7its last N deferred layers are in(so every fill finished), and 21N of 90 layers left for the next recordings. All of them are two BC3 1365x1365 x90 textures (the terrain tiles). One fill at the first load (about 0.5 min after the start), then 2 at about 6 minutes, 2 at about 17 minutes and 2 about half a minute later.did not complete in time, no error. 60 fps for the game; after the first minute: mean 57.7 fps over 780 five-second samples, 63 of them under 50 fps, lowest 24.1.pipelines 707.5), the ones at the start are the boot, and one of 499.9 ms (draw 201, descriptors 136, texture lookups 128) comes right after the log line where two of these textures were filled from layer 0/1 at once. The later fills (about 17 min) have no slow submit of 40 ms or more. So at least once a fill coincides with a half-second hitch; I cannot say if the fill is the cause (the same second also has the first sight of new textures).What I saw playing (my own words, no measurement):
I will do Castle Town below the ruined castle and Hyrule Field next, and a session with the split off for the frame-time comparison, and post those here.
Correction to my run above: at Ganon's Castle I also saw, briefly, black
opaque blocks in the sky. This is separate from the white glow I mentioned
(#1004), which was a different thing. The black blocks went away after I
left the area and came back.
So this session was not fully clean: the blocks can still appear on head
6f76660with the default split, but they did not persist after re-enteringthe area. I don't have the exact time in the log, so I can't tie them to a
specific deferred fill yet.
Log lines from the
6f76660session I reported above (RX 7800 XT, main0828863+ this head, split at the default 16, emptyhookshot_env.txt, about 27 minutes of play). Only the lines about the split path; I checked the whole log forcould not be uploaded,destroyed with deferred,device lostanddid not complete in time: none appear.What it shows: 7 fills, 7 "last N ... are in", so every fill finished, and only the two terrain arrays (0x10a228a600, 0x10a5b02c00) were ever deferred. Because the 20-line limit stops the "left" lines after the first fill, I cannot tell from the log how long the later fills took.
What it does not show: the black blocks in the sky at Ganon's Castle. I did not note the time, so I cannot say which fill they were near. If it is useful, next time I will write down the clock time when they appear, and can run with
RX_VK_LOG_MADE/RX_VK_LOG_UPLOADSon. The full log is 6.4 MB; I can send the part around a given time on request.Two things from reading the code of the merged head (
release/v2.0.12+6f76660) that may be worth a look for the opaque sky blocks. Neither is confirmed; I have no log or capture that ties them to the blocks.1. Staging memory that is not cleared before the GPU decodes it (lead, unverified). In
upload_astc_gpu(vk_texture.cpp~883-885) the copy into the staging slot is skipped whenat >= totaland cut withn = min(tail, total - at). The rest of that layer's slot then keeps whatever the recycled staging arena held (staging(), ~382, does not clear it), and the GPU decodes it. That would give random coloured blocks. It only matters if a layer can reach those branches; I do not know whether it can for the sky. A cheap check: log whenat >= totalorn < tail; the fix would be to zero the tail.2. A test that separates the deferred path from the rest:
RX_VK_BC_PART_MTEXELS=0turns the split off. If the blocks vanish with it, the deferral is involved;RX_VK_IMAGE_PREALLOC=0andRX_VK_CAPTURE_KEEP_OLD=1(my local pool and capture edits) rule those out. Earlier today on this card: the blocks were gone with the split off in Castle Town, but I saw them again briefly at Ganon's Castle with the default 16 on6f76660, and could not tie them to a fill.What I did check and found no defect in: the fill copy and its barriers, and
bc_pendingentries (destroy_latererases them, so no danglingTexture*). I did not review the race-related parts ofvk_replay.cppfrom this PR.Also: this is the same sky I could not identify (the only deferred textures in my log are two 1365x1365 x90 BC3 arrays). If someone knows which texture the Castle Town / Ganon sky is, I can log that one.
A small thing I hit when merging this PR's head (
6f76660) intorelease/v2.0.12(046e37f), in case it is not on the branch yet:tests/test_parallel_rb_refresh.pyno longer compiles. The fixturetests/parallel_rb_refresh_test.cppincludesrb_texture.incfrom the texture code, and the newthread_local uint64_t bc_part_texels(vk_texture.cpp~854, used at ~911 and ~944) is not declared in the fixture. Adding one line beside the other thread-locals fixes it for me:With that,
tests/run_all.py(llvm-mingw, Windows) gave 207 passed, 3 failed, 30 skipped; the 3 failures are toolchain ones on my side (test_fps_overlay_order,test_fpx_stickycould not find the C++ standard headers,test_precompile_compute_guardcould not findwindows.h), not this PR. I only ran it in that merge, not on the PR branch alone.Heads-up: this conflicts with main now (a lot landed today). Could you merge main into the branch (or rebase) and resolve the conflicts? Thanks!
The Python source check failures are caused by this PR. tests/test_parallel_rb_refresh.py and tests/test_parallel_init_order.py cut texture code out of gpu/vk/vk_texture.cpp into their fixtures. The new
bc_part_texels = 0;at vk_texture.cpp:1938 (and the use at :1950) now ends up in those fixtures without thethread_localdeclaration at :766, so both fail with "use of undeclared identifier 'bc_part_texels'". I reproduced test_parallel_rb_refresh.py locally onb4fed0c, and @lucasmazzoni45-del reported the same thing. The fixtures need the declaration added, or the reset moved out of the functions they cut. Loosening the text the tests match on would not be a fix.The branch also conflicts with main again in vk_texture.cpp and tests/run_all.py. Main now discovers tests and reads each test's arguments from RUN_ALL_ARGS in the test file, so the run_all.py line can simply go.
The 16 M-texel split is the default on AMD, and the RX 7800 XT run on
6f76660still showed black blocks at Ganon's Castle. I'd keep this in draft until a long, clean AMD run.WIP: fix(vk): upload ASTC textures re-encoded as BC3 over several submissions (#900 rebased)to fix(vk): upload ASTC textures re-encoded as BC3 over several submissions (#900 rebased)I rebased this onto current main on a branch in my fork: fix/pr1130-bc3-multisubmit. main is a single commit now, so the PR's net diff is applied as one commit.
vk_texture.cpp(kept main'sastc_failreasons plus the PR'sastc_layer_sizeand run loop) andtests/run_all.py(main's version;RUN_ALL_ARGSadded totest_bc_parts.py).tests/parallel_rb_refresh_test.cppandtests/parallel_init_order_test.cppnow declarethread_local uint64_t bc_part_texels; the text the tests match on is unchanged.test_parallel_rb_refreshandtest_parallel_init_order(with the Vulkan headers) pass.Full suite on Windows (llvm-mingw, .NET 8): the failures are the same ones that fail on main here.
Still open: the AMD sky blocks at Ganon's Castle. A read of the code found no confirmed cause; a long AMD run at the default 16 M budget (with and without the large pool preallocation) is still needed.
🤖 Generated with Claude Code
Follow-up: while reading the deferred BC3 path I found a separate defect that could explain the colored sky blocks, though it is not confirmed as the cause.
texture_touch(gpu/vk/vk_texture.cpp, thebc_retry_frame > frame_noearly return and the end of the function after a failed upload) returns a BC3 texture whose image was never written, and nothing invk_draw.cppcheckstex->uploaded, so a draw can sample uninitialised memory for up to 30 frames. The code comment says "not drawn as noise" but nothing enforced it. A fix (returnnullptrlikeupload_deferdoes, and don't cache that null intexture_view_of, otherwise the retry never runs) is on top of this PR's branch in my fork: fix/bc3-unuploaded-not-sampled. Source-level test only (tests/test_bc_upload_retry.py); no GPU run. In the log with the blocks, the suspected texture was last used 482 presents before them and there is no "upload failed" line, so a run at Ganon's Castle withRX_VK_LOG_UPLOADSis still needed to tell.🤖 Generated with Claude Code
One more fix for the split path, on ZurielAtr:fix/bc3-parts-fill: d1074fa "stage only the BC3 layers a recording takes", on top of
6f76660.What was wrong.
upload_astc_gpucopied every requested layer into staging and only then encoded the runs that fit the recording's budget. A continuation asks for all the layers still to come, so each one staged them all again: about 56 MB copied from guest memory to encode 2 or 3 of a 1365 x 1365 x 90 array's layers, about 1.3 GB per array over its recordings. On my RX 7700S (8 GB, Windows 11, Adrenalin 26.10.1, v2.0.11 + this PR +6f76660, default 16 M budget), every reload of Hyrule Field's two terrain arrays cost 1.1-1.2 s, and one frame took 10.7 s (descriptors 9.4 s, staging arenas up to 32 x 64 MB).The fix. The split is decided before staging (
bc_parts::fit_layers): only the runs this recording takes are copied, and the rest goes onbc_pendingwith nothing staged. Per array that is about 56 MB of copies in all instead of about 1.3 GB. The fill of a new image's deferred layers and the retry of a failed continuation are unchanged, and the "left for the next recordings" line now also says how much was staged.tests/test_bc_partschecks that each layer is staged once, and that the split comes before the staging.This may also be behind two things reported here, though I have not measured either: @Nitnek's ~1.2 s longer castle load on the RTX 3070 (since
b7f5525the budget is off on NVIDIA by default anyway), and the 500 ms submit @lucasmazzoni45-del saw right after two arrays were filled.Applying it.
b4fed0c):git fetch https://github.com/ZurielAtr/Hookshot fix/bc3-parts-fill && git cherry-pick d1074faapplies cleanly.vk_texture.cpp, atbc_scratch_layout. Keep main'sastc_fail = "no memory for the BC3 scratch"and drop the old per-runlayer_texels/runblock, whichfit_layersnow replaces above the staging. After that it also applies cleanly onto fix/bc3-unuploaded-not-sampled.bc_part_texelsthat Lucas added is still needed.About the sky blocks at Ganon's Castle. One data point that may save some searching. On the same machine (v2.0.11 + this PR +
6f76660), I saw purple and black blocks in the sky on Death Mountain Trail and took an F9 dump. The blocks first appear in the cloud draw, program1a7482c0ec9991ea. It reads three 128 x 72 x 96 3D volumes that compute dispatches1e2fcee33a0c7209and27ee9883e2a20323write every frame (fog / volumetric light). That session's deferred BC3 layers had all logged "are in" early on, so that kind of sky block did not come from this PR. The same program also shows up in the log asa draw (program 1a7482c0ec9991ea) samples the colour target it writes (… read undefined where it writes), and it still does on v2.1.0. The dump kept only layer 0 of each volume, which was clean, while the sky samples far layers, so I could not go further. If the Ganon blocks are in that pass too, they are a separate bug.AI-assisted (Claude Code) on my behalf.