WIP: feat(vk): built-in shaders from a folder with RX_VK_SHADER_DIR #33
No reviewers
Labels
No labels
accessibility
autorelease: pending
autorelease: tagged
bug
dependencies
documentation
duplicate
enhancement
github_actions
good first issue
help wanted
invalid
needs measurements
question
testing wanted
wontfix
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
MagicBeanDev/Longshot!33
Loading…
Reference in a new issue
No description provided.
Delete branch "dyst/Longshot:feat/builtin-shader-dir"
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?
Why
Trying an edit to one of the renderer's own shaders (the texture decoders, the upscalers, frame generation, the write-back diff) takes a renderer build today, because their SPIR-V is compiled in. With
RX_VK_SHADER_DIRset, the renderer takes each of them from a folder when the file is there, so an edit is a glslang run and a restart.What changed
renderer:
gpu/vk/vk_builtin_spv.{h,cpp}(new): one lookup,builtin_spv(name, words, bytes), behind theBUILTIN_SPV(ARRAY)andBUILTIN_SPV_ARGS(ARRAY)macros. WithRX_VK_SHADER_DIRunset it returns the compiled-in words before looking at anything else. Set, it takes each built-in shader from<folder>/<name>.spvwhen that file is SPIR-V (read_spv: a whole number of words, at least SPIR-V's header, and the magic number first), and logs it once:<name>is the header's (stem_of:clear_vert.spvforclear_vert_spv.h). A missing file means the compiled-in words, without a log line; a file that isn't SPIR-V is named in the log with the reason, and the compiled-in words are used. A file is read once and kept for the process, as the compiled-in words are.renderer: all 52 compiled-in arrays the renderer uses go through it, in
vk_draw.cpp,vk_framegen.cpp,vk_fsr3.cpp,vk_fsr3fi.cpp,vk_fsr3_inputs.cpp,vk_memory.cpp,vk_reactive.cpp,vk_temporal.cppandvk_texture.cpp. The two FSR 3 pass tables (PassInfo, thePASSmacro) carry each array's name for it inspv_name.renderer: not from a shared
longshot_env.txt: a folder of shader code is a_DIRswitch, soenv_line_allowedalready refuses it there. Only the environment the game is started with sets it. The*_spv.hheaders stay generated from the sources, as before; the folder is for iteration only.tests:
tests/test_builtin_spv.py(new, game-free): builds the lookup as C++ with a stand-in log and runs it with the folder unset, the file missing, a SPIR-V file, one cut short and one with the wrong first word, in a folder with a space in its name; checks that every compiled-in array ofgpu/vk/shadersandhost_shadersthatgpu/vkuses goes through the lookup (52 here; onmainit stops atvk_draw.cpp's seven); and thatRX_VK_SHADER_DIRisn't taken fromlongshot_env.txt.tests: the three fixtures that compile pieces of
vk_framegen.cpp(upscale_independent_test.cpp,framegen_resources_test.cpp,present_source_crop_test.cpp) defineBUILTIN_SPV_ARGSbeside their stand-in arrays, as the lookup is with the folder unset, so their pointer checks still hold.Flags and settings
RX_VK_SHADER_DIR<folder>/<name>.spvwhen that file is SPIR-V (logged once); a missing or bad file falls back to the compiled-in words. Not taken fromlongshot_env.txtMeasurements
Measured on the GitHub version (NotRexed/Hookshot#2878), not yet on Longshot: this PR stays WIP until it is.
Hardware: RTX 3080 (NVIDIA 621.14), Ryzen 9 3900X, 64 GB, Windows 11.
Scene: Kokiri Forest, from a save, headless through the control bridge; that branch built with the Builder's steps.
RX_VK_SHADER_DIR= a folder of the built-in shaders' own wordswb_diff_comp.spvtwo bytes too longastc_decoder_comp,bc_encoder_comp,block_linear_unswizzle_2d_comp,wb_diff_comp, each logged once with the same size as the compiled-in onewb_diff_comprefused, the compiled-in one usedThere is no column for GitHub
mainor for the switch unset: no such run is recorded. The refusal's log line:Testing
Tested: the runs under Measurements (on the GitHub version) and the game-free checks this description names. Checked but not measured: the two game runs above (fps ranges, no baseline).
Not tested: Linux (the lookup is standard C++, and paths are
fopen's, which is UTF-8 on Windows through the manifest). An edited shader: there is no glslang on the author's PC, so the files were the shipped words, and the run shows the path works, not a visible change.Needs other testers:
RX_VK_SHADER_DIRnaming a folder of the built-in shaders' own words, expecting each logged as taken from the folder, and a broken file falling back to the compiled-in one (request to open)Screenshots
Nothing on screen.
Risk and rollback
Unset,
builtin_spvreturns the compiled-in words before anything else, so a player without the switch runs the same shaders as before; the risk is in the 52 call sites now going through the lookup, where a wrong name or size would hand Vulkan the wrong words. Set, a valid but wrong SPIR-V file in the folder can break rendering: unsettingRX_VK_SHADER_DIRturns it off. Reverting the commit is enough.Reviewer notes
Carried over from NotRexed/Hookshot#2878 with the move to Longshot: the same change on Longshot's
main, with its rename applied. Its review and discussion are there.Checks run on the GitHub version: every check that reads the changed files (159), through
run_all.pyon Windows, rebased on GitHub'smainof that day: 157 pass;nx2/test_bc_storage_views.pyandtests/test_linux_driver_fault_guard.pyfail the same way onmainthere. Only those checks were run, not the whole suite. The checks and the game runs were done on the GitHub version; this branch is the same change with the rename applied (the same files and line counts). Forgejo runs no checks; they were not run again here. The dev-loop proposal's B2 (NotRexed/Hookshot#2636), the Vulkan side of whatRX_WGSL_OVERRIDEdoes for WebGPU.Issues
None on Longshot. See NotRexed/Hookshot#2636 (B2).
Checklist
For everyone
type(scope): description).main.python -I -B tests/run_all.pyrun and passing (any failure that also fails onmainis named above).third_party/untouched (a new folder with aPROVENANCE.mdinstead).If applicable
Assisted-by:/Co-Authored-By:trailers.docs/tested-configurations.mdfor each measured run.MKWVK_API_VERSIONbumped (gpu/vk/vk_api.h'sMkwVkHostorMkwVkRendererchanged).mainand passing here.tests/that reads changed source updated in this PR, not weakened.🤖 Generated with Claude Code
feat(vk): built-in shaders from a folder with RX_VK_SHADER_DIRto WIP: feat(vk): built-in shaders from a folder with RX_VK_SHADER_DIRBack to WIP under #48's process: this was carried over before the new template. Its description will be brought to the template and measured on Longshot (main against this PR, at named commits) when it's wanted for review; ask here if you'd like it sooner.
Description brought to Longshot's PR template (#48). Still WIP: its numbers are from the GitHub version, and it isn't rebased on current main; it gets measured on Longshot (main against this PR, at named commits) before it is marked ready.
Description moved to the template's Testing section and whole checklist (#72): what was tested, what wasn't, and what needs other testers (their test requests to open while this is WIP).
View command line instructions
Checkout
From your project repository, check out a new branch and test the changes.