fix(hdr): Native HDR scaled in 10 bits and shown as the game made it at 100% contrast #1942
Closed
j64esatxzdrhfg wants to merge 2 commits from
j64esatxzdrhfg/fix/hdr-native-format into main
pull from: j64esatxzdrhfg/fix/hdr-native-format
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!1942
Loading…
Reference in a new issue
No description provided.
Delete branch "j64esatxzdrhfg/fix/hdr-native-format"
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?
With HDR on Native and Sharpness, the Window scaling filter or ultrawide's menu sides on, the game's HDR10 picture goes through an RGBA8 image. Its PQ codes lose two bits, which shows as banding in the dark, and #1941's contrast row would grade that 8-bit picture.
Why, after the rebase: this PR first fixed Native never being taken: the texture cache shared one image between RGBA8 and RGB10A2 (
format_class), sotex->formatnever said RGB10A2. Main's0a28ff2(#2038, moosky73's fix) has since given RGB10A2 a class of its own, so Native is taken on main now. This PR keeps telling Native by the format the game presents, which no longer depends on the cache's classes, and keeps the picture in 10 bits.What changed (the last commit)
do_present: Native is told byvk_format(t->d.format)alone, the format the view is made in. The image's format is no longer asked: it would take an SDR picture for PQ in an image first made as RGB10A2. One log line names the presented format, its image's format and the HDR path when any of them changes.present_image: Native always goes through the HDR pass, which reads the picture through that view.framegen_scale(pq): with the scaling filter, Sharpness or ultrawide's sides, the game's picture is scaled into RGB10A2 images instead of RGBA8. Its crop is copied as bits, not blitted. TheRX_VK_SHOT_FXshot passespresent_native_pqtoo.shaders/fg_scale.comp:FG_SCALE_PQmakes the destinationrgb10_a2and the dither a 10-bit step.fg_scale_pq_comp_spv.hcomes fromtest_fg_scale_sharpen.py --write(glslang 16.6.0); the RGBA8 header's code is unchanged.hdr_encode_comp_spv.his regenerated with glslang 16.6.0.test_fg_scale_sharpen.pycovers both headers.upscale_independent_test.cppruns the 10-bit path: pipeline, images, crop copy, the switch back, and a failed pipeline.test_hdr_encode.pychecks the format test (the presented format alone), the pass-through at 100%, thatpresent_imagenever blits Native's image, and the shot'spq.test_present_aspect.pycovers Native without the pass: its scaled picture, else black.present_aspect_vulkan.cpp's stubs now followpresent_image, so the real-Vulkan check compiles again. It passes here; on main it does not compile.Testing: RTX 5080, 3840x2160 HDR10, 800 nits peak, 300 nits paper white. With this PR's earlier version, before main's class fix, Native showed the game's own HDR picture instead of a flat, grey one. That ran in a build of main (
8c92fe3) with this series and HDR contrast at 150% (screenshots), and earlier on v2.0.13 at 100%. Rebased on maind7a7306: it compiles with the build's clang.tests/run_all.pywith hdr_encode, fg_scale, dlss_actor, dlss_checks, upscale_independent, framegen_resources, present_, upscaler_rows and menu: 28 pass, includingpresent_aspect_vulkanon the RTX 5080.present_semaphoresfails the same way on main, because this PC has no g++.In game after the review changes (a build of main
d7a7306with this series): the present log saysthe game's PQ picture (Native)(format 64, its image's 64); at 100% contrast the game's own picture shows untouched, topping out at about 700 nits under the 800-nit peak row, and graded at 150% at 784 nits. Not triggered in game: the black fallback.Not tested: AMD/Intel, Linux.
Screenshots (Native, in a build of main
d7a7306with this series)HDR10 captures, tone-mapped to SDR for this page (paper white to white, highlights rolled off up to 800 nits).
🤖 Generated with Claude Code
Thanks, this is the piece #1941 was missing, and it answers both Native points from my review there: Native is now actually taken, and with Sharpness, the scaling filter or ultrawide's sides the game's PQ codes stay in 10 bits instead of going through RGBA8.
I went through the last commit (9aeb580) and it reads right:
shown_formatcomes fromvk_format(t->d.format), anddirect_viewis made bytexture_view(tex, t, ...), which also picks the view format fromt. So the scale pass and the HDR pass both read the RGB10A2 picture through an RGB10A2 view, even when the cached image is RGBA8 (format_classaliasing, MUTABLE_FORMAT images).vkCmdCopyImagewhenpqis set. That's a raw bit copy between two 4-byte formats, so RGB10A2 bits held in an RGBA8 image come through intact, where a blit would have converted them.present_imagewithcrop == nullptrfor the made frames, so the copy branch never runs for them. They're sampled as RGBA8 holding PQ values, which the pq scale and HDR passes handle fine.rgb10_a2storage writes aren't a new requirement, sincehdr_encode.compalready writes that format. Iffg_scale_pqcan't be built, the code logs once and falls back to the unscaled HDR pass, and the test covers that.pqresets the cached sizes, so the images get made again (with the device wait) in the other format. That only happens when the mode changes.test_fg_scale_sharpen.pyand the string checks intest_hdr_encode.py/test_present_aspect.pymatch the code.Open points:
|| tex->format == A2B10G10R10half of the Native test (inline). Is there a reason to keep it now thatshown_formatexists?hdr_encode.compcould skip the peak limit for mode 1 when contrast and exposure are both 1. That would keep it a pure re-encode. This is a judgment call, so I'd at least note it in the description.framegen_hdrreturns null under Native,present_imagestill falls back toblit_crop_to_rect(src, ...)on the image. That's exactly the format-converting blit the comment says garbles the colours. It only happens on a failure path, but it could takescaledor skip the blit.RX_VK_SHOT_FXcall toframegen_scale(vk_replay.cpp ~3078) still uses the defaultpq = false. Under Native, every shot flipsscale.pq, so the images are made twice with a device wait each time, and the shot goes through the RGBA8 path. It's debug-only, but passingpresent_native_pqthere is a one-word fix.framegen_scalestub intests/present_aspect_vulkan.cppstill has 8 parameters. That fixture already doesn't compile against main'spresent_image(nodev.hdr/framegen_hdr/present_native_pqin it), and run_all skips it without a Vulkan device. It's worth fixing while the signature is changing anyway.Stacking: this branch carries #1940 (
eea8875) and #1941 (69f7d3e) as its first two commits, so it can only land after them or together with them. My earlier suggestion to land #1941 and #1942 together still stands. #1943 to #1946 don't touch these hunks. #1944 adds lines near the end ofpresent_image, well away from this change. No CI has run on this branch (fork workflows are waiting for approval), so nothing is green or red yet. Whether the DLSS-for-Native part from #1940 goes in as is is still @NotRexed's call, so I'm leaving this as a comment and not an approval.With the format the game presents now known, why keep this half? It's the reverse of the bug this PR fixes. If the window's image was first made as RGB10A2 (the game started in HDR) and the game later presents an RGBA8 picture through it, this still says Native. The SDR picture then gets decoded as PQ, and with a crop the new
vkCmdCopyImagecopies RGBA8 bits into an RGB10A2 image, so you get garbage, not just a wrong tone.shown_formatalone (it falls back toVK_FORMAT_UNDEFINEDonly whentis null, andtexneedstanyway) looks like the accurate test. If there's a case wherevk_format(t->d.format)doesn't say RGB10A2 but the picture is PQ, a comment naming it would help.Thanks. This now builds on #1941 alone, rebased on main
d7a7306, which includes0a28ff2(#2038). That fix took care of the cache aliasing this PR first described, so the description now says what's left: the presented-format test and the 10-bit path. On your points:tex->formathalf is dropped (inline).hdr_encode.compnow writes the sampled PQ code as it is for mode 1, with no decode and encode in between. Native still goes through the pass, because the pass reads the right view. Off 100%, #1941's grade is fitted under the peak the game is told and doesn't limit light above it, so the two meet at 100%. The test checks that the pass-through comes before any decode.framegen_hdrreturns null under Native,present_imagetakesscaledif there is one, else clears to black and logs once. It never blitssrc.test_present_aspect.pycovers both, with and without the scaling filter.RX_VK_SHOT_FXcall passespresent_native_pq.load(...).present_aspect_vulkan.cpp's stubs now followpresent_image(framegen_scale'spq,framegen_hdr,framegen_scaled_view,dev.hdr,present_native_pq,logf). It compiles and passes here at this head on an RTX 5080. At #1941's head it still fails, as on main, because that head's present block readsopts.hdr_contrast.In game (a build of main
d7a7306with this series): the log saysthe game's PQ picture (Native), the game's picture shows untouched at 100% (about 700 nits at most under the 800-nit row) and graded at 150% stays under the row (784 nits). The black fallback wasn't triggered.No good reason; it's dropped. Native is now
shown_format == VK_FORMAT_A2B10G10R10_UNORM_PACK32alone, and the comment says why the image's format isn't asked.test_hdr_encode.pyfails iftex->formatcomes back into that store. With main's0a28ff2the two can't disagree for the window's textures any more, but the presented format is the one the view is made in, so it's the one to ask.This is on main through #2342's squash merge (
a0dfd28), which carried #1941, #1942 and #2243. I checked each file of this PR against origin/main:fg_scale_pqinclude (line 36). In vk_replay.cpp, theRX_VK_SHOT_FXread moved toenv_on(#2042) and theframegen_scale(..., present_native_pq)call is there at :3247. test_hdr_encode.py's docstring gained the DLSS paragraph.Nothing is left to merge, so I'd close it. #2243 is still open even though its commit also landed through #2342, so it's probably worth checking too.
This now conflicts with main in
gpu/vk/vk_menu.cpp,gpu/vk/vk_menu.h,gpu/vk/vk_replay.cpp,tests/test_hdr_encode.py. Could you merge main into the branch (or rebase) when you get a chance? I'll take another look once it's up to date.Closing as landed through #2342's squash merge (
a0dfd28): the 10-bit scaling, the presented-format test and the 100% pass-through are all on main. Thanks @Joey-Magnus for the file-by-file check and @hckrman101 for the review.