Skip to content

CTS api,validation,encoding,cmds,render,* test is failing #29887

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

Jump to bottom
Merged
merged 1 commit into from
Jun 18, 2024
Merged

CTS api,validation,encoding,cmds,render,* test is failing #29887

merged 1 commit into from
Jun 18, 2024

Conversation

@mwyrzykowski
Copy link
Contributor

@mwyrzykowski mwyrzykowski commented Jun 17, 2024 •
edited by webkit-early-warning-system
Loading

e999bce

CTS api,validation,encoding,cmds,render,* test is failing
https://bugs.webkit.org/show_bug.cgi?id=275513
<radar://129874785>

Reviewed by Tadeu Zagallo.

279708@main introduced incorrect validation for invalid draw calls,
instead of reverting it which would reintroduce the buffer overflow,
correct the logic so invalid draws produce the expected validation error.

Also enable the impacted test to avoid regressing the CTS test moving
forwards.

* LayoutTests/fast/webgpu/regression/repro_275024-expected.txt:
* LayoutTests/fast/webgpu/regression/repro_275024b-expected.txt:
Update expectations, this case should not be a validation error per
the specification.

* LayoutTests/platform/mac-wk2/TestExpectations:
Add test to passing list.

* Source/WebGPU/WebGPU/RenderBundleEncoder.mm:
(WebGPU::RenderBundleEncoder::executePreDrawCommands):
(WebGPU::RenderBundleEncoder::computeMininumVertexInstanceCount const):
* Source/WebGPU/WebGPU/RenderPassEncoder.mm:
(WebGPU::RenderPassEncoder::executePreDrawCommands):
(WebGPU::RenderPassEncoder::computeMininumVertexInstanceCount const):
Correct logic.

Canonical link: https://commits.webkit.org/280129@main

8705d36

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ❌ 🛠 wpe ✅ 🛠 wincairo
✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ❌ 🧪 wpe-wk2 ✅ 🧪 wincairo-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ❌ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
✅ 🧪 api-ios loading-orange 🧪 mac-wk2 ❌ 🛠 gtk
⏳ 🛠 vision ✅ 🧪 mac-AS-debug-wk2   🧪 gtk-wk2
⏳ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ❌ 🧪 api-gtk
✅ 🛠 🧪 merge ⏳ 🧪 vision-wk2
✅ 🛠 tv
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@mwyrzykowski mwyrzykowski self-assigned this Jun 17, 2024
@mwyrzykowski mwyrzykowski added the WebGPU For bugs in WebGPU label Jun 17, 2024
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Jun 17, 2024 •
edited
Loading

EWS run on previous version of this PR (hash 5ce19dd)

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ✅ 🧪 wincairo-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
✅ 🧪 api-ios ❌ 🧪 mac-wk2 ✅ 🛠 gtk
⏳ 🛠 vision ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
⏳ 🛠 vision-sim ❌ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
⏳ 🧪 vision-wk2
✅ 🛠 tv
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@mwyrzykowski mwyrzykowski requested a review from djg June 17, 2024 05:52
@mwyrzykowski
Copy link
Contributor Author

mwyrzykowski commented Jun 17, 2024 •
edited
Loading

Test failure is due to #29882 - will rebase after it merges and it should pass

@mwyrzykowski mwyrzykowski force-pushed the eng/CTS-apivalidationencodingcmdsrender-test-is-failing branch from 5ce19dd to e3a42be Compare June 17, 2024 16:27
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Jun 17, 2024 •
edited
Loading

EWS run on previous version of this PR (hash e3a42be)

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug   🧪 wpe-wk2   🧪 wincairo-tests
✅ 🧪 webkitperl   🧪 ios-wk2   🧪 api-mac ✅ 🧪 api-wpe
  🧪 ios-wk2-wpt   🧪 mac-wk1 ✅ 🛠 wpe-cairo
⏳ 🧪 api-ios   🧪 mac-wk2 ✅ 🛠 gtk
⏳ 🛠 vision   🧪 mac-AS-debug-wk2   🧪 gtk-wk2
⏳ 🛠 vision-sim   🧪 mac-wk2-stress   🧪 api-gtk
⏳ 🧪 vision-wk2
✅ 🛠 tv
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@mwyrzykowski mwyrzykowski force-pushed the eng/CTS-apivalidationencodingcmdsrender-test-is-failing branch from e3a42be to 9649a24 Compare June 17, 2024 16:43
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Jun 17, 2024 •
edited
Loading

EWS run on previous version of this PR (hash 9649a24)

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ✅ 🧪 wincairo-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
✅ 🧪 api-ios ❌ 🧪 mac-wk2 ✅ 🛠 gtk
⏳ 🛠 vision ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
⏳ 🛠 vision-sim ❌ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
⏳ 🧪 vision-wk2
✅ 🛠 tv
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@mwyrzykowski mwyrzykowski force-pushed the eng/CTS-apivalidationencodingcmdsrender-test-is-failing branch from 9649a24 to 3ca1d02 Compare June 17, 2024 21:57
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Jun 17, 2024 •
edited
Loading

EWS run on previous version of this PR (hash 3ca1d02)

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ✅ 🧪 wincairo-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
✅ 🧪 api-ios ❌ 🧪 mac-wk2 ✅ 🛠 gtk
⏳ 🛠 vision ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
⏳ 🛠 vision-sim ❌ 🧪 mac-wk2-stress ❌ 🧪 api-gtk
⏳ 🧪 vision-wk2
✅ 🛠 tv
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jun 18, 2024
@mwyrzykowski mwyrzykowski removed the merging-blocked Applied to prevent a change from being merged label Jun 18, 2024
@mwyrzykowski mwyrzykowski force-pushed the eng/CTS-apivalidationencodingcmdsrender-test-is-failing branch from 3ca1d02 to abe9ed0 Compare June 18, 2024 06:44
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Jun 18, 2024 •
edited
Loading

EWS run on previous version of this PR (hash abe9ed0)

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ❌ 🛠 wpe ✅ 🛠 wincairo
✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ❌ 🧪 wpe-wk2   🧪 wincairo-tests
✅ 🧪 webkitperl   🧪 ios-wk2   🧪 api-mac ❌ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt   🧪 mac-wk1 ✅ 🛠 wpe-cairo
⏳ 🧪 api-ios   🧪 mac-wk2 ❌ 🛠 gtk
⏳ 🛠 vision ✅ 🧪 mac-AS-debug-wk2 ❌ 🧪 gtk-wk2
⏳ 🛠 vision-sim   🧪 mac-wk2-stress ❌ 🧪 api-gtk
  🛠 🧪 merge ⏳ 🧪 vision-wk2
✅ 🛠 tv
  🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@mwyrzykowski mwyrzykowski added merge-queue Applied to send a pull request to merge-queue merging-blocked Applied to prevent a change from being merged and removed merge-queue Applied to send a pull request to merge-queue merging-blocked Applied to prevent a change from being merged labels Jun 18, 2024
@mwyrzykowski mwyrzykowski force-pushed the eng/CTS-apivalidationencodingcmdsrender-test-is-failing branch from abe9ed0 to 8705d36 Compare June 18, 2024 07:09
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Jun 18, 2024 •
edited
Loading

EWS run on current version of this PR (hash 8705d36)

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ❌ 🛠 wpe ✅ 🛠 wincairo
✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ❌ 🧪 wpe-wk2 ✅ 🧪 wincairo-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ❌ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
✅ 🧪 api-ios loading-orange 🧪 mac-wk2 ❌ 🛠 gtk
⏳ 🛠 vision ✅ 🧪 mac-AS-debug-wk2   🧪 gtk-wk2
⏳ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ❌ 🧪 api-gtk
✅ 🛠 🧪 merge ⏳ 🧪 vision-wk2
✅ 🛠 tv
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@mwyrzykowski mwyrzykowski added the merge-queue Applied to send a pull request to merge-queue label Jun 18, 2024
@webkit-commit-queue webkit-commit-queue force-pushed the eng/CTS-apivalidationencodingcmdsrender-test-is-failing branch from 8705d36 to 1f51e7a Compare June 18, 2024 18:37
@mwyrzykowski
CTS api,validation,encoding,cmds,render,* test is failing
e999bce 
https://bugs.webkit.org/show_bug.cgi?id=275513
<radar://129874785>

Reviewed by Tadeu Zagallo.

279708@main introduced incorrect validation for invalid draw calls,
instead of reverting it which would reintroduce the buffer overflow,
correct the logic so invalid draws produce the expected validation error.

Also enable the impacted test to avoid regressing the CTS test moving
forwards.

* LayoutTests/fast/webgpu/regression/repro_275024-expected.txt:
* LayoutTests/fast/webgpu/regression/repro_275024b-expected.txt:
Update expectations, this case should not be a validation error per
the specification.

* LayoutTests/platform/mac-wk2/TestExpectations:
Add test to passing list.

* Source/WebGPU/WebGPU/RenderBundleEncoder.mm:
(WebGPU::RenderBundleEncoder::executePreDrawCommands):
(WebGPU::RenderBundleEncoder::computeMininumVertexInstanceCount const):
* Source/WebGPU/WebGPU/RenderPassEncoder.mm:
(WebGPU::RenderPassEncoder::executePreDrawCommands):
(WebGPU::RenderPassEncoder::computeMininumVertexInstanceCount const):
Correct logic.

Canonical link: https://commits.webkit.org/280129@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/CTS-apivalidationencodingcmdsrender-test-is-failing branch from 1f51e7a to e999bce Compare June 18, 2024 18:39
@webkit-commit-queue
Copy link
Collaborator

Committed 280129@main (e999bce): https://commits.webkit.org/280129@main

Reviewed commits have been landed. Closing PR #29887 and removing active labels.

@webkit-commit-queue webkit-commit-queue merged commit e999bce into WebKit:main Jun 18, 2024
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Jun 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tadeuzagallo tadeuzagallo tadeuzagallo approved these changes

@djg djg Awaiting requested review from djg

Assignees

@mwyrzykowski mwyrzykowski

Labels

WebGPU For bugs in WebGPU

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@mwyrzykowski @webkit-early-warning-system @webkit-commit-queue @tadeuzagallo @webkit-ews-buildbot