Bug 202031 – Web Inspector: Canvas: modifications to shader modules can be shared between vertex/fragment shaders

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Devin Rousso]

# STEPS TO REPRODUCE:
1. inspect <https://webkit.org/demos/webgpu/compute-boids.html>
2. select Render Pipeline 1 (the same shader module is used for the vertex and fragment shaders)
3. edit the vertex shader
4. refresh both shaders
 => the fragment shaders update to reflect the changes made to the vertex shader

[Devin Rousso]

Created attachment 379219 [details]
Patch

[Devin Rousso]

Comment on attachment 379219 [details]
Patch

After speaking with Saam, he suggested that if the vertex and fragment shaders both shared the same shader module, we shouldn't use a split content view and instead update both the vertex and fragment shaders at the same time.  I think this is a great idea, as it handles the case where there are utility functions that are shared between the vertex and fragment shaders, so that any edits are shared.

[Devin Rousso]

*** This bug has been marked as a duplicate of bug 202341 ***

[Devin Rousso]

Oops, wrong bug :P

[Devin Rousso]

Created attachment 380408 [details]
Patch

Needs tests, but the UI is all there

[Devin Rousso]

Created attachment 380409 [details]
Patch

Rebase (still needs tests)

[EWS Watchlist]

This patch modifies the inspector protocol. Please ensure that any frontend changes appropriately use feature checks for new protocol features.

[Devin Rousso]

Created attachment 380452 [details]
Patch

[Devin Rousso]

Comment on attachment 380452 [details]
Patch

Ah, I forgot to adjust the test expectations so that 'inspector/canvas/updateShader-webgpu-sharedVertexFragment.html' doesn't get run on non-WebGPU-supporting platforms :(

[Dean Jackson]

Comment on attachment 380452 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=380452&action=review

> Source/WebCore/inspector/InspectorShaderProgram.cpp:283
> +    auto payload = Inspector::Protocol::Canvas::ShaderProgram::create()
> +        .setProgramId(m_identifier)
> +        .setProgramType(programType.value())
> +        .setCanvasId(m_canvas.identifier())
> +        .release();

Did you swap to Java here? :)

[Devin Rousso]

Created attachment 380467 [details]
Patch

[Devin Rousso]

Comment on attachment 380452 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=380452&action=review

>> Source/WebCore/inspector/InspectorShaderProgram.cpp:283
>> +        .release();
> 
> Did you swap to Java here? :)

lol.  This is typical Web Inspector style :P
The code that generates these objects is pretty awesome too!  Makes some really clever usage of C++ templates to ensure that the required properties get added in the right order =D

[Devin Rousso]

Comment on attachment 380467 [details]
Patch

I'm cq+ this now, since bugzilla EWS won't run any of the WebGPU tests anyways :|

[WebKit Commit Bot]

Comment on attachment 380467 [details]
Patch

Clearing flags on attachment: 380467

Committed r250874: <https://trac.webkit.org/changeset/250874>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/56097158>