Skip to content

cube-vksc: Fix Windows D2D display acquisition and pipeline cache handling - #47

Open
dgkoch wants to merge 1 commit into
sc_mainfrom
fix/vksccube-winrt-display-acquisition
Open

cube-vksc: Fix Windows D2D display acquisition and pipeline cache handling#47
dgkoch wants to merge 1 commit into
sc_mainfrom
fix/vksccube-winrt-display-acquisition

Conversation

@dgkoch

@dgkoch dgkoch commented Aug 6, 2026

Copy link
Copy Markdown

Summary

  • Fix inverted WinRT display acquisition condition: wsi_platform != WSI_PLATFORM_DISPLAY was backwards — the display was never acquired when using display mode. Fix to == WSI_PLATFORM_DISPLAY. Also fix typo in error message ("get acqurie" → "acquire").

  • Add --native-resolution flag: Some implementations only support swapchain creation at the display's native panel resolution. When --native-resolution is specified, all available modes are enumerated and the one matching physicalResolution is selected; if none matches, falls back to mode[0] with a warning. Without the flag, mode[0] is used (default, spec-correct behaviour). If swapchain creation fails in display mode, an actionable error message directs the user to --native-resolution.

  • Fix cube.pc.json: Use named flag format for colorWriteMask as required by the pcutil JSON schema (VK_COLOR_COMPONENT_*_BIT instead of "0xf"). Use "NULL" for pViewports and pScissors when those fields are dynamic state, matching the format the pcutil serializer produces for null pointers.

  • Regenerate pipeline_cache.h using the VulkanSC SDK 1.0.22 emulation layer PCC.

Tested

On NVIDIA RTX 5070 with VulkanSC SDK 1.0.22:

  • --wsi display --native-resolution: spinning cube visible on 1920×1200 display, on both drivers that enumerate only native modes and drivers that enumerate non-native modes that fail swapchain creation. ✓
  • --wsi display without --native-resolution on an affected driver: clear error message directing the user to use --native-resolution. ✓
  • --wsi file: all pixels non-zero, max 230/255. ✓
  • Emulation ICD (VK_DRIVER_FILES=<vksconvk.json>): exit code 0, embedded pipeline cache works. ✓

🤖 Generated with Claude Code

@dgkoch
dgkoch marked this pull request as draft August 6, 2026 19:05
@dgkoch
dgkoch force-pushed the fix/vksccube-winrt-display-acquisition branch 2 times, most recently from 0b844c1 to 377d68e Compare August 6, 2026 22:17
@dgkoch
dgkoch marked this pull request as ready for review August 6, 2026 22:20
@dgkoch
dgkoch requested a review from aqnuep August 6, 2026 22:20
@aqnuep

aqnuep commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Fix inverted WinRT display acquisition condition: wsi_platform != WSI_PLATFORM_DISPLAY was backwards. The display was never acquired when using display mode. Fix to == WSI_PLATFORM_DISPLAY. Also fix typo in error message ("get acqurie" → "acquire").

No question, that's clearly a bug.

Select the best display mode: The original code called vkGetDisplayModePropertiesKHR with mode_count=1, picking whichever mode the driver returned first. On some drivers/displays this yields a low-resolution mode (e.g. 1280×960) that does not support swapchain creation - the driver requires the native panel resolution. Fix: enumerate all available modes (up to 256) and select the one matching the display's native resolution; fall back to highest area then highest refresh rate.

This seems arbitrary. The driver shouldn't report display modes that is not actually supported.

Fix colorWriteMask format: Change "0xf" to the named flag string format required by the pcutil JSON schema (VK_COLOR_COMPONENT_*_BIT).

This looks like a bug in the pcutil JSON schema, as we did intend to retain the ability of the old schema allowing integer values (encoded as integer or string). We'll have to look into that.

Add placeholder viewport and scissor entries: Some PCC implementations validate that pViewports/pScissors are non-empty even when those fields are listed in pDynamicState. Add placeholder values as a workaround. Note: the PCC should not require this per the Vulkan spec and pcutil schema — this is a PCC parser bug, that we'll address but will take some time to reach public drivers.

Such PCC implementations seem to be broken, as the spec clearly states that the viewport/scissor arrays can be NULL if they are dynamic.

Comment thread cube-vksc/cube.c Outdated
Comment thread cube-vksc/cube.pc.json
"dstAlphaBlendFactor": "VK_BLEND_FACTOR_ZERO",
"alphaBlendOp": "VK_BLEND_OP_ADD",
"colorWriteMask": "0xf"
"colorWriteMask": "VK_COLOR_COMPONENT_R_BIT | VK_COLOR_COMPONENT_G_BIT | VK_COLOR_COMPONENT_B_BIT | VK_COLOR_COMPONENT_A_BIT"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, I looked into the history of this. Apparently the 0xf format (while allowed by the legacy invalid JSON schema), was never accepted even by the legacy parser, but the legacy generator did produce it, so I think this part is a legitimate fix for remnants of the legacy tooling.

@dgkoch
dgkoch force-pushed the fix/vksccube-winrt-display-acquisition branch 2 times, most recently from 18d8b76 to 7484f28 Compare August 7, 2026 13:55
@dgkoch

dgkoch commented Aug 7, 2026

Copy link
Copy Markdown
Author

Add placeholder viewport and scissor entries: Some PCC implementations validate that pViewports/pScissors are non-empty even when those fields are listed in pDynamicState. Add placeholder values as a workaround. Note: the PCC should not require this per the Vulkan spec and pcutil schema — this is a PCC parser bug, that we'll address but will take some time to reach public drivers.

Such PCC implementations seem to be broken, as the spec clearly states that the viewport/scissor arrays can be NULL if they are dynamic.

I investigated this more after posting (trying to fix the alleged pcc bug), and it's actually the pcutils parser that rejects it. It turns out that the arrays can be NULL, but not [] as was used here, and this is also what the json serializer would produce, so have changed that in the latest update (and it works on both native NVIDIA Vulkan SC driver and the emulation driver). Incidentally if the vksc emulator would have used pcutils instead of a custom parser, it would have caught these issues too.

@aqnuep

aqnuep commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

I investigated this more after posting (trying to fix the alleged pcc bug), and it's actually the pcutils parser that rejects it. It turns out that the arrays can be NULL, but not [] as was used here, and this is also what the json serializer would produce, so have changed that in the latest update (and it works on both native NVIDIA Vulkan SC driver and the emulation driver). Incidentally if the vksc emulator would have used pcutils instead of a custom parser, it would have caught these issues too.

I see. Then yes, the JSON has to be fixed. Unfortunately, that JSON pre-dates the new tools and we never went there to re-validate / fix it.

Sure, the Vulkan SC Emulation PCC could use the new parser. It's one of the many good-to-have features we postponed due to other priorities. I guess we may have to revisit that.

Fix several issues preventing vksccube from running on Windows
Direct-to-Display (D2D) or on real VulkanSC hardware:

1. Fix inverted condition in WinRT display acquisition: the condition
   `wsi_platform != WSI_PLATFORM_DISPLAY` was backwards; acquire the
   display only when display mode is actually in use.  Also fix a typo
   in the error message ("get acqurie" -> "acquire").

2. Add --native-resolution flag: some implementations only support
   swapchain creation at the display's native panel resolution.  When
   --native-resolution is specified, enumerate all available modes and
   select the one matching the display's physicalResolution; fall back
   to mode[0] with a warning if no matching mode is found.  Without the
   flag, mode[0] is used (default, spec-correct behaviour).  If
   swapchain creation fails in display mode, an error message directs
   the user to --native-resolution.

3. Fix cube.pc.json: use named flag format for colorWriteMask as
   required by the pcutil schema (VK_COLOR_COMPONENT_*_BIT instead of
   the hex literal "0xf").  Use "NULL" for pViewports and pScissors
   when those fields are dynamic state, matching the format the pcutil
   serializer produces for null pointers.  Also regenerate
   pipeline_cache.h using the VulkanSC SDK 1.0.22 emulation layer PCC.

Tested on NVIDIA RTX 5070 with VulkanSC SDK 1.0.22:
- Direct-to-display with --native-resolution: spinning cube visible on
  1920x1200 display, on both drivers that enumerate only native modes
  and drivers that enumerate non-native modes that fail swapchain
  creation.
- Without --native-resolution on an affected driver: clear error
  message directing the user to use --native-resolution.
- File output mode (--wsi file): all pixels non-zero, max 230/255.
- File output mode with emulation ICD: exit code 0, embedded cache works.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
@dgkoch
dgkoch force-pushed the fix/vksccube-winrt-display-acquisition branch from 7484f28 to c1d9bfa Compare August 7, 2026 21:02
@dgkoch

dgkoch commented Aug 7, 2026

Copy link
Copy Markdown
Author

Select the best display mode: The original code called vkGetDisplayModePropertiesKHR with mode_count=1, picking whichever mode the driver returned first. On some drivers/displays this yields a low-resolution mode (e.g. 1280×960) that does not support swapchain creation - the driver requires the native panel resolution. Fix: enumerate all available modes (up to 256) and select the one matching the display's native resolution; fall back to highest area then highest refresh rate.

This seems arbitrary. The driver shouldn't report display modes that is not actually supported.

Fair. However, our direct display implementation has behaved that way for Vulkan (and now Vulkan SC) for at least several years, so while I'll look into improving it, all of our shipping drivers behave this way today. That said the heuristic was a bit janky, so I've just replaced it with an explicit --native-resolution option.

@dgkoch
dgkoch requested a review from aqnuep August 7, 2026 21:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants