Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Loosen Viewport validation tests to match spec update #4106

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Loosen Viewport validation tests to match spec update #4106

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3a0c2ea
Loosen Viewport validation tests to match spec update
toji Dec 18, 2024
7644665
Address review feedback
toji Dec 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
120 changes: 73 additions & 47 deletions src/webgpu/api/validation/encoding/cmds/render/dynamic_state.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,71 +130,97 @@ class F extends ValidationTest {

export const g = makeTestGroup(F);
toji marked this conversation as resolved.
Show resolved Hide resolved

g.test('setViewport,x_y_width_height_nonnegative')
g.test('setViewport,width_height_nonnegative')
.desc(
`Test that the parameters of setViewport to define the box must be non-negative.
`Test that the width and height parameters of setViewport must be non-negative.

TODO Test -0 (it should be valid) but can't be tested because the harness complains about duplicate parameters.
TODO Test the first value smaller than -0`
)
.paramsSubcasesOnly([
// Control case: everything to 0 is ok, covers the empty viewport case.
{ x: 0, y: 0, w: 0, h: 0 },
{ w: 0, h: 0 },

// Test -1
{ x: -1, y: 0, w: 0, h: 0 },
{ x: 0, y: -1, w: 0, h: 0 },
{ x: 0, y: 0, w: -1, h: 0 },
{ x: 0, y: 0, w: 0, h: -1 },
{ w: -1, h: 0 },
{ w: 0, h: -1 },
toji marked this conversation as resolved.
Show resolved Hide resolved
])
.fn(t => {
const { x, y, w, h } = t.params;
const success = x >= 0 && y >= 0 && w >= 0 && h >= 0;
t.testViewportCall(success, { x, y, w, h, minDepth: 0, maxDepth: 1 });
const { w, h } = t.params;
const success = w >= 0 && h >= 0;
t.testViewportCall(success, { x: 0, y: 0, w, h, minDepth: 0, maxDepth: 1 });
});

g.test('setViewport,xy_rect_contained_in_attachment')
g.test('setViewport,exceeds_attachment_size')
.desc(`Test that the viewport can exceed the attachment size`)
.paramsSubcasesOnly([
{ attachmentWidth: 3, attachmentHeight: 3 },
{ attachmentWidth: 1024, attachmentHeight: 1024 },
])
.fn(t => {
const { attachmentWidth, attachmentHeight } = t.params;
t.testViewportCall(
true,
{ x: 0, y: 0, w: attachmentWidth + 1, h: attachmentHeight + 1, minDepth: 0, maxDepth: 1 },
{ width: attachmentWidth, height: attachmentHeight, depthOrArrayLayers: 1 }
);
});

g.test('setViewport,xy_rect_contained_in_bounds')
.desc(
'Test that the rectangle defined by x, y, width, height must be contained in the attachments'
`Test that the rectangle defined by x, y, width, height must be contained in the maximum viewport bounds
and that the viewport size cannot exceed the maximum.`
)
.paramsSubcasesOnly(u =>
u
.combineWithParams([
{ attachmentWidth: 3, attachmentHeight: 5 },
{ attachmentWidth: 5, attachmentHeight: 3 },
{ attachmentWidth: 1024, attachmentHeight: 1 },
{ attachmentWidth: 1, attachmentHeight: 1024 },
])
.combineWithParams([
// Control case: a full viewport is valid.
{ dx: 0, dy: 0, dw: 0, dh: 0 },

// Other valid cases with a partial viewport.
{ dx: 1, dy: 0, dw: -1, dh: 0 },
{ dx: 0, dy: 1, dw: 0, dh: -1 },
{ dx: 0, dy: 0, dw: -1, dh: 0 },
{ dx: 0, dy: 0, dw: 0, dh: -1 },

// Test with a small value that causes the viewport to go outside the attachment.
{ dx: 1, dy: 0, dw: 0, dh: 0 },
{ dx: 0, dy: 1, dw: 0, dh: 0 },
{ dx: 0, dy: 0, dw: 1, dh: 0 },
{ dx: 0, dy: 0, dw: 0, dh: 1 },
])
u.combineWithParams([
toji marked this conversation as resolved.
Show resolved Hide resolved
// Control case: max viewport is valid.
{ mx: 0, my: 0, dx: 0, dy: 0, dw: 0, dh: 0 },

// Other valid cases
{ mx: -1, my: 0, dx: 0, dy: 0, dw: 0, dh: 0 },
{ mx: -2, my: 0, dx: 0, dy: 0, dw: 0, dh: 0 },
{ mx: 1, my: 0, dx: -1, dy: 0, dw: 0, dh: 0 },
{ mx: 0, my: -1, dx: 0, dy: 0, dw: 0, dh: 0 },
{ mx: 0, my: -2, dx: 0, dy: 0, dw: 0, dh: 0 },
{ mx: 0, my: 1, dx: 0, dy: -1, dw: 0, dh: 0 },
{ mx: 0, my: 0, dx: -1, dy: 0, dw: 0, dh: 0 },
{ mx: 0, my: 0, dx: 1, dy: 0, dw: 0, dh: 0 },
{ mx: 0, my: 0, dx: 0, dy: -1, dw: 0, dh: 0 },
{ mx: 0, my: 0, dx: 0, dy: 1, dw: 0, dh: 0 },
{ mx: 1, my: 0, dx: 0, dy: 0, dw: -1, dh: 0 },
{ mx: 0, my: 1, dx: 0, dy: 0, dw: 0, dh: -1 },

// Cases that go outside the allowed bounds
{ mx: -2, my: 0, dx: -1, dy: 0, dw: 0, dh: 0 },
{ mx: 0, my: -2, dx: 0, dy: -1, dw: 0, dh: 0 },
{ mx: 1, my: 0, dx: 0, dy: 0, dw: 0, dh: 0 },
{ mx: 0, my: 1, dx: 0, dy: 0, dw: 0, dh: 0 },
{ mx: 1, my: 0, dx: 1, dy: 0, dw: -1, dh: 0 },
{ mx: 0, my: 1, dx: 0, dy: 1, dw: 0, dh: -1 },
toji marked this conversation as resolved.
Show resolved Hide resolved

// Cases that exceed the max viewport size
{ mx: 0, my: 0, dx: 0, dy: 0, dw: 1, dh: 0 },
{ mx: 0, my: 0, dx: 0, dy: 0, dw: 0, dh: 1 },
])
)
.fn(t => {
const { attachmentWidth, attachmentHeight, dx, dy, dw, dh } = t.params;
const x = dx;
const y = dy;
const w = attachmentWidth + dw;
const h = attachmentWidth + dh;

const success = x + w <= attachmentWidth && y + h <= attachmentHeight;
t.testViewportCall(
success,
{ x, y, w, h, minDepth: 0, maxDepth: 1 },
{ width: attachmentWidth, height: attachmentHeight, depthOrArrayLayers: 1 }
);
const { mx, my, dx, dy } = t.params;
const maxViewportSize = t.device.limits.maxTextureDimension2D;
const maxViewportBounds = maxViewportSize * 2;

const x = maxViewportSize * mx + dx;
const y = maxViewportSize * my + dy;
const w = maxViewportSize;
const h = maxViewportSize;

const inBounds =
x >= -maxViewportBounds &&
y >= -maxViewportBounds &&
x + w <= maxViewportBounds - 1 &&
toji marked this conversation as resolved.
Show resolved Hide resolved
y + h <= maxViewportBounds - 1;
const validSize = w <= maxViewportSize && h <= maxViewportSize;
const success = inBounds && validSize;
t.testViewportCall(success, { x, y, w, h, minDepth: 0, maxDepth: 1 });
});

g.test('setViewport,depth_rangeAndOrder')
Expand Down
4 changes: 2 additions & 2 deletions src/webgpu/listing_meta.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -499,8 +499,8 @@
"webgpu:api,validation,encoding,cmds,render,dynamic_state:setScissorRect,xy_rect_contained_in_attachment:*": { "subcaseMS": 1.325 },
"webgpu:api,validation,encoding,cmds,render,dynamic_state:setStencilReference:*": { "subcaseMS": 3.450 },
"webgpu:api,validation,encoding,cmds,render,dynamic_state:setViewport,depth_rangeAndOrder:*": { "subcaseMS": 1.667 },
"webgpu:api,validation,encoding,cmds,render,dynamic_state:setViewport,x_y_width_height_nonnegative:*": { "subcaseMS": 0.400 },
"webgpu:api,validation,encoding,cmds,render,dynamic_state:setViewport,xy_rect_contained_in_attachment:*": { "subcaseMS": 0.200 },
"webgpu:api,validation,encoding,cmds,render,dynamic_state:setViewport,width_height_nonnegative:*": { "subcaseMS": 0.400 },
"webgpu:api,validation,encoding,cmds,render,dynamic_state:setViewport,xy_rect_contained_in_bounds:*": { "subcaseMS": 0.200 },
"webgpu:api,validation,encoding,cmds,render,indirect_draw:indirect_buffer,device_mismatch:*": { "subcaseMS": 2.000 },
"webgpu:api,validation,encoding,cmds,render,indirect_draw:indirect_buffer_state:*": { "subcaseMS": 2.708 },
"webgpu:api,validation,encoding,cmds,render,indirect_draw:indirect_buffer_usage:*": { "subcaseMS": 2.733 },
Expand Down
Loading