Skip to content

layers: NV per-viewport array count VUs - #12825

Open
Wint3rNight wants to merge 2 commits into
KhronosGroup:mainfrom
Wint3rNight:nv-viewport-count-vus
Open

layers: NV per-viewport array count VUs#12825
Wint3rNight wants to merge 2 commits into
KhronosGroup:mainfrom
Wint3rNight:nv-viewport-count-vus

Conversation

@Wint3rNight

Copy link
Copy Markdown

Four VUs require an NV per-viewport array to be at least as large as the
viewport count. Only the viewport swizzle one was implemented that way, so this
started as adding the missing draw time checks and turned up three bugs in the
pipeline creation ones along the way.

Fixing the pipeline creation VUs

VU Spec Was implemented as
01215 swizzle nv >= viewportCount nv < viewportCount — correct
01726 W-scaling nv >= viewportCount nv != viewportCount
02029 exclusive scissor nv == 0 || nv >= viewportCount nv != 0 && nv != viewportCount
02056 shading rate image nv >= viewportCount nv > viewportCount

All three rejected valid pipelines. 02056 is the worst of them: it was inverted,
so it also never caught the case the VU actually describes.

Three existing tests asserted the old behaviour and are updated. Since that
removed the only coverage of a real 02029 violation, a negative test is added
for it.

Adding the draw time VUs

With VK_DYNAMIC_STATE_VIEWPORT_WITH_COUNT the viewport count is only known at
draw time, so the pipeline's static NV arrays have to be checked against it. The
dynamic side of each of these was already validated, the static side was not.

  • VUID-vkCmdDraw-viewportCount-04137VkPipelineViewportWScalingStateCreateInfoNV
  • VUID-vkCmdDraw-viewportCount-04139VkPipelineViewportShadingRateImageStateCreateInfoNV
  • VUID-vkCmdDraw-viewportCount-07492VkPipelineViewportSwizzleStateCreateInfoNV
  • VUID-vkCmdDraw-VkPipelineVieportCreateInfo-04142VkPipelineViewportExclusiveScissorStateCreateInfoNV

Setting more viewports than the array holds means the implementation indexes
past the end of it, and nothing else reports that.

vk_validation_stats goes from 3520 to 3416 unimplemented, which is these four
across the 26 action commands, with nothing newly uncovered.

Question on 04141

I left VUID-vkCmdDraw-VkPipelineVieportCreateInfo-04141 unimplemented. It
compares against the same VkPipelineViewportSwizzleStateCreateInfoNV::viewportCount
as 07492 and only differs in how the condition is worded, so it looks like the
older form from before the swizzle state could be dynamic. Happy to emit it
alongside 07492, or add it to unimplementable_validation.h next to the other
redundant VUID variants, whichever you prefer.

Testing

  • 4 negative tests for the new VUs, 3 positive tests for the fixed ones, plus
    the new 02029 negative test
  • Reverting sl_pipeline.cpp fails all 3 positive tests, reverting
    cc_cmd_buffer_dynamic.cpp fails all 4 negative tests
  • 678 tests pass across NegativeDynamicState, PositiveDynamicState,
    NegativePipeline, PositivePipeline, NegativeCommand,
    NegativeFragmentShadingRate, NegativeShaderObject, PositiveShaderObject
    and NegativeViewportInheritance, with no failures
  • check_code_format.py passes, clang-format 22.1.8

Two things I could not check locally. The full suite does not finish here, it
crashes in PositiveDescriptorHeapUntyped.BufferAsFunctionParameter, and it
does that on an unmodified checkout too, so it is unrelated to this change but
it does mean I ran the suites above rather than everything.
NegativeFragmentShadingRate.ShadingRateImageNV also skips on my device, though
ShadingRateImageNVViewportCount, which is the one this changes, does run.

@Wint3rNight
Wint3rNight requested a review from a team as a code owner August 7, 2026 23:53
@ci-tester-lunarg

Copy link
Copy Markdown
Collaborator

Author Wint3rNight not on autobuild list. Waiting for curator authorization before starting CI build.

@CLAassistant

CLAassistant commented Aug 7, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@ci-tester-lunarg

Copy link
Copy Markdown
Collaborator

Author Wint3rNight not on autobuild list. Waiting for curator authorization before starting CI build.

Four VUs require an NV per-viewport array to be at least as large as
VkPipelineViewportStateCreateInfo::viewportCount. Only the viewport
swizzle one (01215) was implemented that way.

- 01726 (VkPipelineViewportWScalingStateCreateInfoNV) required equality
- 02029 (VkPipelineViewportExclusiveScissorStateCreateInfoNV) required
  equality
- 02056 (VkPipelineViewportShadingRateImageStateCreateInfoNV) was
  inverted and reported an error when the array was larger, so it also
  missed the case where the array is too small

All three rejected valid pipelines. Existing tests asserted the old
behaviour, so they are updated, and a negative test is added for the
02029 case that is a real violation.
When VK_DYNAMIC_STATE_VIEWPORT_WITH_COUNT is used, the viewport count is
only known at draw time, so the pipeline's static NV per-viewport arrays
have to be checked against it. The dynamic side of each of these was
already validated, the static side was not.

- 04137 VkPipelineViewportWScalingStateCreateInfoNV
- 04139 VkPipelineViewportShadingRateImageStateCreateInfoNV
- 07492 VkPipelineViewportSwizzleStateCreateInfoNV
- 04142 VkPipelineViewportExclusiveScissorStateCreateInfoNV

Setting more viewports than the array holds means the implementation
indexes past the end of it.

04141 is left alone, it looks like a duplicate of 07492 from before the
swizzle state could be dynamic.
@Wint3rNight
Wint3rNight force-pushed the nv-viewport-count-vus branch from c16e246 to 483eb82 Compare August 8, 2026 03:02
@ci-tester-lunarg

Copy link
Copy Markdown
Collaborator

Author Wint3rNight not on autobuild list. Waiting for curator authorization before starting CI build.

@ci-tester-lunarg

Copy link
Copy Markdown
Collaborator

CI Vulkan-ValidationLayers build queued with queue ID 71319.

@ci-tester-lunarg

Copy link
Copy Markdown
Collaborator

CI Vulkan-ValidationLayers build # 24074 running.

@ci-tester-lunarg

Copy link
Copy Markdown
Collaborator

CI Vulkan-ValidationLayers build # 24074 passed.

@spencer-lunarg

Copy link
Copy Markdown
Contributor

@Wint3rNight I was on PTO all of last week and catching up on things, will give this a review by end of the week. It "seems" good, but dynamic state stuff has many sharp edges and I need to read up on these vendor extensions

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.

4 participants