feat: make_game.py --fetch-fsr-runtime fetches AMD's FSR 4 runtime #1866
No reviewers
Labels
No labels
accessibility
autorelease: pending
autorelease: tagged
bug
dependencies
documentation
duplicate
enhancement
github_actions
good first issue
help wanted
invalid
question
testing wanted
wontfix
No milestone
No assignees
1 participant
No due date set.
Dependencies
No dependencies set
Reference
MagicBeanDev/Hookshot!1866
Loading…
Reference in a new issue
No description provided.
Delete branch "feat/fsr4-fetch-runtime"
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?
#1008 leaves AMD's DLLs to the player; this is an opt-in way to have the build fetch them. With --fsr --fetch-fsr-runtime (Windows), scripts/ffx_sdk.py downloads amd_fidelityfx_loader_vk.dll, amd_fidelityfx_upscaler_vk.dll and AMD's license from AMD's FidelityFX SDK repository at the v2.4.0 tag, checks each file against a pinned SHA-256 (a file that does not match is deleted, not used), and make_game.py puts them in the game folder, where vk_fsr4.cpp looks. A download cut off is tried again. --fsr alone still downloads nothing. HOOKSHOT_FFX_SDK keeps them between builds (default work/ffx-sdk);
python -I scripts/ffx_sdk.py --game <folder>does the same for a game built already.tests/test_ffx_sdk.py covers the file table, the SHA-256 check, the retry and make_game's placing, without the network. The script was also run against AMD's repository on Windows: the three files fetched match their SHA-256s.
Assisted-by: Claude Opus 5.5
Doesn't nvidia auto pull when
--dlssis on? Should we do same?Reviewed at
45e9efe. The download itself looks right: it comes from AMD's own repo (GPUOpen-LibrariesAndSDKs/FidelityFX-SDK) over HTTPS. It's pinned to a commit, not a branch, and that commit is what tagv2.4.0points at. Each file goes to.part, is checked against a pinned SHA-256 and is only moved into place if it matches. A mismatch is deleted and stops the build, and only network errors are retried. There's no archive to unpack, so no zip-slip risk. The two DLLs insignedbin/at that commit are real files (39 KB and 36 MB), not LFS pointers. The names and the folder match whatvk_fsr4.cpplooks for on main:kLoaderat L68, and the game folder inlibrary_dirs()at L129-145. The progress lines printed are plain lines, so they don't get in the way of the--prepareprotocol, and a failure goes throughfail(). The test runs without the network, andrun_all.pypicks it up without aRUN_ALL_ARGS.On the question above ("doesn't nvidia auto pull when
--dlssis on?"). That's half right, and the half that's right is the part this PR is missing:make_game.py, DLSS is not pulled automatically. There's no--dlssflag. DLSS gets built in whenHOOKSHOT_NGX_SDKholds the SDK (ngx_built(), L1222). The SDK is downloaded only with--fetch-ngx-sdkplus--accept-nvidia-license(L1278-1280, and L1347-1348if getattr(a, "fetch_ngx_sdk", False): fetch_ngx_sdk(a)).fetch_ngx_sdk()stops if the licence flag is missing (L1211-1212). So a separate--fetch-fsr-runtimeflag matches how DLSS works on the command line, and keeps--fsr downloads nothingtrue (thefsr_wanted()docstring, and the comment above--fsr).MainWindow.xaml.csL710-722: the tick stays only if you accept). Then each build adds--fetch-ngx-sdk --accept-nvidia-license(L812; the Linux/Python builder does the same inwindow.pyL1411-1412). The FSR tick adds only--fsr(L705), and this PR doesn't touch the Builder. So a Builder user who ticks FSR 4 still gets no DLLs.So I'd keep the flag as it is and copy the Builder side of DLSS: on Windows, have
FsrTickadd--fetch-fsr-runtime, with AMD's licence shown once when the box is ticked (asDlssLicensedoes). That can be a follow-up PR, but it's what makes this useful to most players. Fetching just because--fsris passed would break a promise the code makes and wouldn't match DLSS, so I wouldn't do that. Both fetchers already pin the same way:ngx_sdk.pyhas a commitCOMMITplus a SHA-256 for every file, and so does this one.Smaller points, inline:
docs/license.mdat v2.4.0 says about end users getting and using the signed DLLs? If it has terms to accept, it should probably follow NVIDIA's pattern (--accept-amd-license, or a prompt). I haven't downloaded the file, so this is a question, not a finding.install()replaces DLLs the player already put in the game folder.Overlap: #1831 (super build 2/4) reworks how the FSR 4 loader is found, with
RX_VK_FFX_DIRinstead of main'sRX_VK_FSR_DIR. If it merges, the folder and env var named here and in the readme would need to follow it. #1834 also editsmake_game.py, so there may be a text conflict.On Linux,
--fsr --fetch-fsr-runtimedoes nothing and says nothing (here, andplace_fsr_runtimereturns None). One printed line would help, something like "--fetch-fsr-runtime: AMD publishes this runtime for Windows only; on Linux see the readme (amd_fidelityfx_upscaler_vk.so)". Then someone who passed the flag doesn't assume it worked.@ -0,0 +42,4 @@RUNTIME = [f[2] for f in FILES]DLLS = [f[2] for f in FILES if f[2].endswith(".dll")]NOTICE = """\ngx_sdk.pyrefuses to download until NVIDIA's licence is accepted (--accept-nvidia-licenseor a typed yes, and the Builder asks first). This one only prints a notice. Does AMD'sdocs/license.mdat v2.4.0 have terms for using the signed DLLs (it's what gets saved asLICENSE-AMD-FSR-SDK.md)? If it does, the same accept step would be safer, and the readme could link the licence at the pinned commit the way the DLSS notice links NVIDIA's (LICENSE_URL).@ -0,0 +117,4 @@for _, digest, name in FILES:dst = os.path.join(game_ffx, name)if not (os.path.isfile(dst) and sha256(dst) == digest):shutil.copyfile(os.path.join(folder, name), dst + ".part")If the game folder already has
amd_fidelityfx_*_vk.dllwith a different hash, for example a newer SDK the player dropped in by hand as the readme says, this quietly replaces it with the pinned v2.4.0 copy. That also happens on every later--update --fetch-fsr-runtime. Sincevk_fsr4.cpptakes the newest FSR 4 the loader lists, that can be a downgrade. I'd leave a non-matching file alone and print a line, or at least say what was replaced.Thanks for the thorough review! Pushed
d78d2ee:LICENSE_URL), as the DLSS notice links NVIDIA's. The licence file is still saved beside the DLLs. Happy to add--accept-amd-licenseif you'd rather match NVIDIA's flow anyway.--fetch-fsr-runtimenow prints "AMD publishes this runtime for Windows only; on Linux see the readme (amd_fidelityfx_upscaler_vk.so)".amd_fidelityfx_*_vk.dllthat isn't v2.4.0's,install()leaves both DLLs alone (so loader and upscaler are never mixed), and the build prints what it kept. Same on--update. Added a test for it.Agreed on keeping the flag separate from
--fsr. I can do the Builder side (FsrTick adding--fetch-fsr-runtimeon Windows, with AMD's licence shown once on tick) as a follow-up PR. I'll also follow #1831'sRX_VK_FFX_DIRif it lands first.Following up on my approval: we'd like DLSS and FSR 4 to work the same way, so the Builder side is better in this PR than in a follow-up. Here's what I'd suggest, mirroring the DLSS flow on main:
make_game.py
--accept-amd-licensenext to--fetch-fsr-runtime, with the same pattern asfetch_ngx_sdk():--fetch-fsr-runtimewithout it fails with "AMD FSR 4 needs AMD's license accepted first (--accept-amd-license)." AMD's licence doesn't strictly require an accept step, but one rule for both vendors is simpler to explain, and it makes sure nobody downloads AMD's files without seeing the terms.LICENSE-AMD-FSR-SDK.mdbeside the DLLs, as you have now.Builder (
builder/HookshotBuilder/MainWindow.xaml.cs)FsrLicensetext next toDlssLicense, with the same shape: whose it is, what gets downloaded (FidelityFX SDK v2.4.0, the two DLLs), the licence link at the pinned commit, where the files go, and "Do you accept AMD's license?"FsrChanged, the same asDlssChanged: ticking FSR on Windows shows it, and the tick stays only on Yes.Run, add--fetch-fsr-runtime --accept-amd-licensewhen FSR is ticked on Windows, as thedlsspath does with the NGX flags.--update: keep the runtime the way DLSS does (line ~1228). If the game folder has AMD's DLLs and the saved licence file is there, the licence counts as accepted already.builder/py/hookshot_builder/window.py) and the Tk one have no FSR tick today, so they can stay as they are. If one gets an FSR tick later, it should follow the same pattern.Linux
Tests
tests/test_ffx_sdk.py(ortest_make_game_ngx.py's twin):--fetch-fsr-runtimewithout--accept-amd-licensefails before any download.With that, the two work the same everywhere: the tick, then the licence prompt, then the flag pair, then the fetch with checksums. The command line also needs the explicit accept for both.
Thanks, done in
841aeb5(main merged in first, no conflicts):make_game.py / ffx_sdk.py
--accept-amd-license next to --fetch-fsr-runtime. Without it, fetch_fsr_runtime() fails before any download with "AMD FSR 4 needs AMD's license accepted first (--accept-amd-license)." scripts/ffx_sdk.py takes the same flag, or a typed yes when run by hand (an accepted() like ngx_sdk.py's), and downloads nothing otherwise. The notice and LICENSE-AMD-FSR-SDK.md beside the DLLs stay.
Builder
FsrLicense next to DlssLicense, with the same shape: whose it is, what gets downloaded (FidelityFX SDK v2.4.0, the two DLLs), the license at the pinned commit, where the files go, and "Do you accept AMD's license?"
FsrChanged (wired in the XAML): the tick stays only on Yes.
Run(fsr:) adds --fsr --fetch-fsr-runtime --accept-amd-license. AMD's runtime is kept in the builder's own ffx-sdk folder (Storage.FfxSdk, passed as HOOKSHOT_FFX_SDK through Runner, the way NgxSdk is).
--update: fetches again when the saved tick had the license accepted, or when the folder has the .hookshot_fsr mark plus AMD's DLL and LICENSE-AMD-FSR-SDK.md beside it, as DLSS does around L1228.
One difference from DLSS: an FSR tick saved before this change never saw AMD's license. So acceptance is saved on its own (FsrLicense in builder-settings.json). An old tick still builds with --fsr as before (the player's own DLLs) and fetches nothing until it is ticked again.
The Qt and Tk builders are unchanged (no FSR tick there).
Tests
tests/test_ffx_sdk.py: accepted(), the script downloading nothing without the license, make_game failing before any download without the flag (and fetching with it), and a source check that the tick shows the license and the flag pair is added only when ticked and accepted.
tests/builder_dlss_update_test.cs: the FSR 4 update decision (saved tick with and without the license, mark/DLL/license combinations).
tests/builder_workers/Program.cs: Runner.Info's new parameter.
Ran locally on Windows: test_ffx_sdk, test_make_game_ngx, test_builder_dlss_update, test_builder_python, test_builder_status, the workers fixture, and dotnet build of the Builder, all passing.
Re-reviewed at
841aeb5. Everything from my parity comment is in, and it matches how DLSS works:--accept-amd-licensesits next to--fetch-fsr-runtime.fetch_fsr_runtime()fails with the exact message before anything is downloaded.ffx_sdk.pyrun by hand asks for a typed yes, using anaccepted()likengx_sdk.py's. The notice andLICENSE-AMD-FSR-SDK.mdare still there.FsrLicensehas the same shape asDlssLicense: whose files they are, what gets downloaded, the licence at the pinned commit, where the files go, and the question.FsrChangedis wired to the XAML and keeps the tick only on Yes.Runadds--fsr --fetch-fsr-runtime --accept-amd-license, andStorage.FfxSdkgoes throughRunnerasHOOKSHOT_FFX_SDK, the wayNgxSdkdoes (and is removed from the environment when FSR isn't ticked).--update: you get the saved accepted tick, or the.hookshot_fsrmark + AMD's DLL + the saved licence, which is the same rule as DLSS around L1228. Keeping acceptance separate from the tick (FsrLicensein the settings) so an FSR tick saved before this change doesn't count as accepted is the right call.accepted(), the script downloading nothing without the licence, the source check that the flag pair comes only with the tick plus acceptance, and the update cases inbuilder_dlss_update_test.cs(including "a DLL alone doesn't accept the licence").Program.csfollows the newInfoparameter.One nit inline: the readme has a form-feed byte where
work/ffx-sdkshould be.A small UX point, not blocking: a player whose FSR tick was saved before this change sees "AMD FSR 4 (downloads AMD's runtime under AMD's license)" ticked, but nothing gets fetched until they untick and tick again. You chose that on purpose, and it's fine. If you want, the build log could print one line in that case ("AMD's license not accepted yet: tick AMD FSR 4 again to have its runtime fetched").
CI on this head is still
action_required(a maintainer has to approve the runs), so the new tests haven't run in CI yet. You ran them locally on Windows, as you said. The #1831 overlap (RX_VK_FFX_DIR) still stands. Approving.Nit: there's a form-feed byte (0x0C) here:
work\x0cfx-sdk. It looks like\finwork\ffx-sdkgot turned into an escape somewhere along the way. It should readwork/ffx-sdk.github-sync referenced this pull request2026-11-12 06:22:54 +00:00