Repository navigation
feat(videoout): present PQ display buffers through an HDR10 swapchain - #5697
Conversation
Accept the 10-bit BT.2100 PQ display formats 0x8100070422000000 (red in the low bits) and 0x8100070400000000 (blue in the low bits) and convert them to 8-bit sRGB when presented: PQ EOTF, BT.2020 to BT.709 primaries, a luminance tone map whose white level is the tone-map luminance sceSystemServiceGetHdrToneMapLuminance reports (reference white min(203 nits, white level)), and the sRGB encoding. The conversion runs on the CPU for linear buffers and DCC clear values, in the color transfer shader for tiled guest buffers, and in the presentation pass for resident images. The three share the PQ EOTF table computed once in double precision. AI-assisted: yes
Detect HDR output on the host: on Windows the monitor showing the window must run in HDR10 (DXGI IDXGIOutput6), and on every host the window's Vulkan surface must offer a 10-bit VK_COLOR_SPACE_HDR10_ST2084_EXT swapchain with VK_EXT_swapchain_colorspace and VK_EXT_hdr_metadata. The monitor's luminance comes from DXGI; hosts that report none get 1000 nits peak and full frame, 0 nits black. While HDR output is available, sceVideoOutGetOutputStatus reports dynamic range 2 and flag bit 0, sceSystemServiceGetHdrToneMapLuminance reports the monitor's luminance, and every open port's output mode event fires when that changes. PQ display buffers are presented unchanged through an HDR10 swapchain with HDR metadata; any other display buffer switches back to the SDR swapchain. Without HDR output the PQ to SDR conversion uses the peak the title was last told as its white level. AI-assisted: yes
|
Other open pull requests touch the same code. Merging one will break the other, or both implement the same thing. If this one builds on them, list them in Depends on.
|
adriadam10
left a comment
There was a problem hiding this comment.
Reviewed the second commit only (stacked on #5696, still open). Built and ran the scoped ctest on the PR merged onto main: agc_host_hdr, guest_video_out_hdr, agc_driver_presentation_pass, agc_resident_present, agc_display_buffer_dcc, linear_display_buffer and video_out_hdr_present pass (agc_driver_tessellation segfaults inside libvulkan_radeon.so here, same as on main).
HostHdrProbe::Poll(non-Windows branch) and the body ("a surface that offers HDR10 is taken to mean an HDR display"): this is wrong on a real Linux setup. On KDE Wayland with RADV,vulkaninfolistsA2B10G10R10_UNORM_PACK32andA2R10G10B10_UNORM_PACK32inCOLOR_SPACE_HDR10_ST2084_EXTwhilekscreen-doctor -oreportsHDR: disabledon the HDMI output andHDR: incapableon DP and eDP. With this PRsceVideoOutGetOutputStatuswould report HDR with a made-up 1000 nit peak, and PQ buffers would go to an HDR10 swapchain on an SDR monitor. Linux needs a signal that the output is actually in HDR (compositor/output state), or HDR output stays off there until one exists.- The HDR10 half of the PR (
SelectDynamicRange,recreateSwapchain,setHdrMetadata, the HDR10 scaler and the passthrough paths inVulkanDevice.cpp) has never run against a real HDR10 swapchain: the Tested section says NVIDIA's surface offered no HDR10 formats andvideo_out_hdr_presentpassed on its SDR half only. Please attach a run where it printed[videoout] presenting HDR10and the frames reached the display (Windows with a driver that offers HDR10, or Linux with an output really in HDR), including the switch back to SDR and the monitor-change path. - The PPSA01317 evidence is for a different patch ("This branch was not run in the title"). Please re-run the title on this branch once the HDR10 path has been exercised.
Rebase onto main after #5696 lands is ours to do; nothing needed from you there.
A Vulkan surface can offer HDR10 formats while the display is not in HDR (KDE Wayland with RADV lists them with the output's HDR disabled), so on hosts other than Windows the probe now reports an SDR monitor and HDR output stays off until an output-state signal exists. A probe test checks that a window without a Windows monitor report reads as SDR with an HDR10 surface. The HDR10 present test now also changes the monitor's luminance while presenting HDR10, and the presenter logs the HDR metadata it sets. AI-assisted: yes
|
Thanks for the review. All three points are addressed in 4e27d8d (a new commit; no force-push). 1. Non-Windows detection. You're right: the surface's formats don't say whether the output is in HDR. HDR output is now Windows-only.
2. The HDR10 half on real hardware. Windows HDR is now on here. DXGI reports the monitor in
Every flip completed through The test drives the monitor change through the same host-state entry point the DXGI probe uses. A physical change during a run (toggling Windows HDR, or moving the window to another monitor) has not been captured yet; I'll add it here when it has been done. The other HDR suites also pass with HDR on: 3. The title evidence. That evidence was for a different patch, and the body no longer suggests otherwise. This branch has not been run in PPSA01317. Stock Overlap noted in the body: #5800 conflicts textually on the two frame-dump conditions (whichever lands second ANDs AI-assisted: yes |
adriadam10
left a comment
There was a problem hiding this comment.
Re-checked 4e27d8d against the three points of the previous review. The non-Windows probe now stays SDR (covered by video_out_hdr_probe, which fails with the old behaviour), the HDR10 path has a real-hardware run (format 64 in VK_COLOR_SPACE_HDR10_ST2084_EXT, metadata, SDR/HDR10 switches, luminance change), and the PPSA01317 claim is scoped correctly. Built and ran the scoped ctest on the PR merged onto main: agc_host_hdr, guest_video_out_hdr, agc_driver_presentation_pass, agc_resident_present, agc_display_buffer_dcc, linear_display_buffer, video_out_hdr_present and video_out_hdr_probe pass. Merging once the Windows build is green.
What
Depends on #5696. This branch is built on that PR's branch, and its first commit is that PR. Review only the second commit,
feat(videoout): present PQ display buffers through an HDR10 swapchain.This PR reports HDR output to titles when the host can show HDR10, and presents their BT.2100 PQ display buffers unchanged through an HDR10 swapchain. When HDR output is unavailable, nothing changes: the status stays SDR, the swapchain stays B8G8R8A8 sRGB, and PQ buffers take #5696's SDR conversion.
Detection. HDR output is available when all of these hold:
VK_COLOR_SPACE_HDR10_ST2084_EXT, with the blit-destination and color-attachment features the presenter writes it with.VK_EXT_swapchain_colorspaceand the device hasVK_EXT_hdr_metadata. Both are enabled when present.IDXGIOutput6::GetDesc1reportsDXGI_COLOR_SPACE_RGB_FULL_G2084_NONE_P2020. Its peak, full-frame and minimum luminance come from the same call. The monitor is read again when the window moves to another monitor or the DXGI factory goes stale, which happens when HDR is toggled.docs/user/USAGE.mdand in the technical debt.The host state lives in libSceAgcDriver (
HostHdr.cpp). libSceVideoOut and libSceSystemService reach it only throughextern "C"_nid_postfixexports and header-inline code. objdump of the built.prxfiles shows every libSceVideoOut and libSceSystemService import from libSceAgcDriver in its export table.What the title sees.
sceVideoOutGetOutputStatus(and...WithVrr, which embeds it) reports dynamic range 2 and flag bit 0 while HDR output is available, and 1 and 0 otherwise. The values are named in SharpProspero (VideoOutOutputDynamicRange.Hdr = 2,VideoOutOutputStatusFlags.Hdr = 1 << 0).sceSystemServiceGetHdrToneMapLuminancereports the monitor's peak, full-frame peak and black level while HDR output is available, and 100/100/0 otherwise, as before. The peak it returns is recorded.sceSystemServiceGetHdrToneMapLuminancereports SDR. The availability found at start-up fires no event beyond the one every output-mode subscription already gets when it is made. This is documented in the technical debt.Presentation.
VulkanDevice::SelectDynamicRangeruns at each flip, afterResize, under the GPU mutex. A PQ display buffer while HDR output is available recreates the swapchain in the 10-bit HDR10 format, preferring the buffer's own channel order, and setsVkHdrMetadataEXT:The PQ texels then reach the swapchain unchanged on every path:
ColorTransfercopy goes through that same scaler.PresentationPassdraws to the HDR10 target with the 10-bit codes kept (RecordPresenttakes the destination color space).Any non-PQ display buffer switches back to the SDR swapchain. The surface formats and the host state are read again before every swapchain recreation. If HDR output went away or the HDR10 format is no longer offered (HDR turned off, or the window moved to an SDR monitor), the recreated swapchain is SDR, so an out-of-date HDR10 swapchain is never recreated in a color space the surface dropped. A blank flip keeps the current one. Frame dumps (
APS5_DUMP_FRAMES) of an HDR10 presentation hold its SDR conversion. Change to #5696's conversion (called out so it is reviewed here): without HDR output, the white level of the PQ to SDR conversion is now the peaksceSystemServiceGetHdrToneMapLuminancelast returned to the title, or the current one when it never asked. A title grades to the value it read and does not follow a later HDR change. With #5696 alone that peak is always 100 nits, so the change has no effect until this PR reports other values. That is why it is in this PR and not in #5696.No environment switches are added.
Overlap:
Output.cppand the presentation code inVulkanDevice.cpp, in other functions.PresentDisplayBuffer. It is a textual conflict: whichever lands second ANDsframeDumpOutput.Enabled_nid_no_patch()into this PR's conditions.USAGE.mdsection. Docs only; whichever lands second keeps both.VK_EXT_swapchain_colorspaceorVK_EXT_hdr_metadata.Tested
Windows 11 (10.0.26220), MinGW-w64 GCC 15.2.0, Release,
BUILD_TESTING=ON, NVIDIA GeForce RTX 5090 Laptop GPU (driver 616.92):agc_host_hdr(CPU):DynamicRangeSurfaceFormat): SDR without HDR output or for SDR buffers, the HDR10 format in the buffer's own order otherwise, and a throw when HDR output is reported but no HDR10 format is offered;PqClearColor).Each test function resets the host state first, so the order does not matter.
guest_video_out_hdr(guest ABI, opens VideoOut): drives the host state through the exports and checks:sceVideoOutGetOutputStatusand...WithVrr;sceSystemServiceGetHdrToneMapLuminancevalues and the recorded peak;agc_driver_presentation_pass(Vulkan):video_out_hdr_present(real window,sceVideoOutSubmitFlip):VK_COLOR_SPACE_HDR10_ST2084_EXT) with HDR metadata;The real monitor-change paths (toggling Windows HDR or moving the window during a run) have not been exercised; they go through the same DXGI re-read and the swapchain recreation shown here.
video_out_hdr_probe(offscreen SDL window): with the surface reporting HDR10, a window without a Windows monitor report reads as an SDR monitor and HDR output stays unavailable. On Windows the offscreen window has no Win32 handle, which takes the same SDR answer, so the non-Windows branch itself is exercised only on Linux CI.video_out_flip_testspresent,dcc,backtoback,lifetimeandaftergpu, run by hand in a real window: passed. ctest runs these modes only with SDL's offscreen driver, where they skip.ctest: 1136 tests, 0 failed. 8 skipped:video_out_flip_*with the offscreen driver,guest_video_out_wqhd_detection_no_vulkanandagc_driver_stencil_ref_export, as onmain, andvideo_out_hdr_presentin a run before the change above. This was before the review fixes and the rebase; the HDR tests and feat(agc): present BT.2100 PQ display buffers on an SDR output #5696's tests were rerun after both.DXGI_ERROR_NOT_CURRENTLY_AVAILABLEunder Remote Desktop) and a window without a Win32 handle as an SDR monitor, and never throws from the presenter loop. This path is untested.tools/check_conventions.pypasses.mainalone does not get the title far enough without unrelated local fixes. A different, equivalent HDR patch on the user's local tree did present the title's0x8100070422000000buffers through an HDR10 swapchain, and a Windows HDR toggle switched SDR and HDR10. That is not evidence for this branch.Checklist
main; no other open PR implements the same functionsNotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file.mdfiles or images added to the repository (attach them to this PR)