Skip to content

Conversation

@mwyrzykowski
Copy link
Contributor

@mwyrzykowski mwyrzykowski commented Jul 22, 2025

ed52872

GPUBindGroupDescriptor::externalTextureMatches is implemented incorrectly
https://bugs.webkit.org/show_bug.cgi?id=296062
rdar://155971582

Reviewed by Tadeu Zagallo.

Auto-generated layouts can not be used interchangably, rather they
are fixed to a specific bind group layout and therefore bind group.

Prevent different auto-generated layouts from being intermixed, even
when their entries are otherwise identical.

This would previously always fail validation layer and is currently
preventing the drop down box on https://webgpu.github.io/webgpu-samples/?sample=videoUploading
from functioning.

* LayoutTests/fast/webgpu/regression/repro_296062-expected.txt: Added.
* LayoutTests/fast/webgpu/regression/repro_296062.html: Added.
Add regression test.

* Source/WebCore/Modules/WebGPU/GPUBindGroup.cpp:
(WebCore::GPUBindGroup::GPUBindGroup):
* Source/WebCore/Modules/WebGPU/GPUBindGroup.h:
(WebCore::GPUBindGroup::create):
(WebCore::GPUBindGroup::autogeneratedPipelineLayout const):
(WebCore::GPUBindGroup::GPUBindGroup): Deleted.
* Source/WebCore/Modules/WebGPU/GPUBindGroupLayout.h:
(WebCore::GPUBindGroupLayout::create):
(WebCore::GPUBindGroupLayout::autogeneratedPipelineLayout const):
(WebCore::GPUBindGroupLayout::GPUBindGroupLayout):
* Source/WebCore/Modules/WebGPU/GPUComputePipeline.cpp:
(WebCore::GPUComputePipeline::getBindGroupLayout):
* Source/WebCore/Modules/WebGPU/GPUComputePipeline.h:
(WebCore::GPUComputePipeline::create):
(WebCore::GPUComputePipeline::GPUComputePipeline):
* Source/WebCore/Modules/WebGPU/GPUDevice.cpp:
* Source/WebCore/Modules/WebGPU/GPUPipelineDescriptorBase.h:
(WebCore::nextUniqueAutogeneratedPipelineIdentifier):
(WebCore::GPUPipelineDescriptorBase::uniqueAutogeneratedId const):
* Source/WebCore/Modules/WebGPU/GPURenderPipeline.cpp:
(WebCore::GPURenderPipeline::getBindGroupLayout):
* Source/WebCore/Modules/WebGPU/GPURenderPipeline.h:
(WebCore::GPURenderPipeline::create):
(WebCore::GPURenderPipeline::GPURenderPipeline):
Ensure auto-generated layouts don't allow mismatches.

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

f34ad83

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 win
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 🧪 win-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 ✅ 🧪 mac-intel-wk2 🛠 playstation
✅ 🛠 tv ✅ 🛠 mac-safer-cpp
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@mwyrzykowski mwyrzykowski self-assigned this Jul 22, 2025
@mwyrzykowski mwyrzykowski added the WebGPU For bugs in WebGPU label Jul 22, 2025
@mwyrzykowski mwyrzykowski requested review from djg and tadeuzagallo July 22, 2025 17:45
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jul 22, 2025
@mwyrzykowski mwyrzykowski removed the merging-blocked Applied to prevent a change from being merged label Jul 22, 2025
@mwyrzykowski mwyrzykowski changed the title https://bugs.webkit.org/show_bug.cgi?id=296062 GPUBindGroupDescriptor::externalTextureMatches is implemented incorrectly Jul 22, 2025
@webkit-ews-buildbot
Copy link
Collaborator

Safer C++ Build #45187 (b7a170d)

❌ Found 1 failing file with 1 issue. Please address these issues before landing. See WebKit Guidelines for Safer C++ Programming.
(cc @rniwa)

WebGPU::BindGroup& backing() { return m_backing; }
const WebGPU::BindGroup& backing() const { return m_backing; }
bool updateExternalTextures(GPUExternalTexture&);
uint64_t autogeneratedPipelineLayout() const { return m_autogeneratedPipelineLayout; }
Copy link
Member

Choose a reason for hiding this comment

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

Just a nit, but I find the name a bit confusing, since I'd expect this to return an actual layout object. If I understood correctly, isn't this a unique identifier for the pipeline?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh yes, let me update that name

@mwyrzykowski mwyrzykowski added the safe-merge-queue Applied to automatically send a pull-request to merge-queue after passing EWS checks label Jul 22, 2025
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jul 22, 2025

Ref<GPUBindGroupLayout> GPUComputePipeline::getBindGroupLayout(uint32_t index)
{
// "A new GPUBindGroupLayout wrapper is returned each time"
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this comment a TODO/FIXME? Or documenting the spec?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Documenting the spec

@mwyrzykowski mwyrzykowski removed the merging-blocked Applied to prevent a change from being merged label Jul 23, 2025
@mwyrzykowski mwyrzykowski added merge-queue Applied to send a pull request to merge-queue and removed safe-merge-queue Applied to automatically send a pull-request to merge-queue after passing EWS checks labels Jul 23, 2025
…ctly

https://bugs.webkit.org/show_bug.cgi?id=296062
rdar://155971582

Reviewed by Tadeu Zagallo.

Auto-generated layouts can not be used interchangably, rather they
are fixed to a specific bind group layout and therefore bind group.

Prevent different auto-generated layouts from being intermixed, even
when their entries are otherwise identical.

This would previously always fail validation layer and is currently
preventing the drop down box on https://webgpu.github.io/webgpu-samples/?sample=videoUploading
from functioning.

* LayoutTests/fast/webgpu/regression/repro_296062-expected.txt: Added.
* LayoutTests/fast/webgpu/regression/repro_296062.html: Added.
Add regression test.

* Source/WebCore/Modules/WebGPU/GPUBindGroup.cpp:
(WebCore::GPUBindGroup::GPUBindGroup):
* Source/WebCore/Modules/WebGPU/GPUBindGroup.h:
(WebCore::GPUBindGroup::create):
(WebCore::GPUBindGroup::autogeneratedPipelineLayout const):
(WebCore::GPUBindGroup::GPUBindGroup): Deleted.
* Source/WebCore/Modules/WebGPU/GPUBindGroupLayout.h:
(WebCore::GPUBindGroupLayout::create):
(WebCore::GPUBindGroupLayout::autogeneratedPipelineLayout const):
(WebCore::GPUBindGroupLayout::GPUBindGroupLayout):
* Source/WebCore/Modules/WebGPU/GPUComputePipeline.cpp:
(WebCore::GPUComputePipeline::getBindGroupLayout):
* Source/WebCore/Modules/WebGPU/GPUComputePipeline.h:
(WebCore::GPUComputePipeline::create):
(WebCore::GPUComputePipeline::GPUComputePipeline):
* Source/WebCore/Modules/WebGPU/GPUDevice.cpp:
* Source/WebCore/Modules/WebGPU/GPUPipelineDescriptorBase.h:
(WebCore::nextUniqueAutogeneratedPipelineIdentifier):
(WebCore::GPUPipelineDescriptorBase::uniqueAutogeneratedId const):
* Source/WebCore/Modules/WebGPU/GPURenderPipeline.cpp:
(WebCore::GPURenderPipeline::getBindGroupLayout):
* Source/WebCore/Modules/WebGPU/GPURenderPipeline.h:
(WebCore::GPURenderPipeline::create):
(WebCore::GPURenderPipeline::GPURenderPipeline):
Ensure auto-generated layouts don't allow mismatches.

Canonical link: https://commits.webkit.org/297771@main
@webkit-commit-queue
Copy link
Collaborator

Committed 297771@main (ed52872): https://commits.webkit.org/297771@main

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

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

Labels

WebGPU For bugs in WebGPU

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants