Bug 190325 – WebGPU: Rename old WebGPU prototype to WebMetal

• Component: New Bugs
• Status: RESOLVED FIXED

[Justin Fan]

WebGPU: Rename old WebGPU prototype to WebMetal

[Justin Fan]

<rdar://problem/44990443>

[Justin Fan]

Created attachment 351710 [details]
Patch

[EWS Watchlist]

This patch modifies the inspector protocol generator. Please ensure that you have rebaselined any generator test results (i.e., by running `Tools/Scripts/run-inspector-generator-tests --reset-results`)

[EWS Watchlist]

Attachment 351710 [details] did not pass style-queue:


ERROR: Source/WebCore/html/canvas/WebMetalDrawable.cpp:43:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
ERROR: Source/WebCore/html/canvas/WebMetalLibrary.cpp:42:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
ERROR: Source/WebCore/html/canvas/WebMetalCommandBuffer.cpp:49:  This { should be at the end of the previous line  [whitespace/braces] [4]
Total errors found: 3 in 217 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Justin Fan]

Created attachment 351712 [details]
Patch

[EWS Watchlist]

Attachment 351712 [details] did not pass style-queue:


ERROR: Source/WebCore/html/canvas/WebMetalDrawable.cpp:43:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
ERROR: Source/WebCore/html/canvas/WebMetalLibrary.cpp:42:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
ERROR: Source/WebCore/html/canvas/WebMetalCommandBuffer.cpp:49:  This { should be at the end of the previous line  [whitespace/braces] [4]
Total errors found: 3 in 219 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Devin Rousso]

Comment on attachment 351712 [details]
Patch

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

> Source/WebInspectorUI/UserInterface/Models/Canvas.js:-71
> -        case CanvasAgent.ContextType.WebGPU:

Ditto (308).

> Source/WebInspectorUI/UserInterface/Models/Canvas.js:-98
> -        case WI.Canvas.ContextType.WebGPU:

Ditto (308).

> Source/WebInspectorUI/UserInterface/Models/Canvas.js:-308
> -    WebGPU: "webgpu",

Given what I wrote below (InspectorBackendCommands.js:70), this will need to stay as is for iOS 11.3 and 12.0.  You can add `WebMetal` to it, but you need to keep `WebGPU`.

> Source/WebInspectorUI/UserInterface/Protocol/Legacy/11.3/InspectorBackendCommands.js:-70
> -InspectorBackend.registerEnum("Canvas.ContextType", {Canvas2D: "canvas-2d", BitmapRenderer: "bitmaprenderer", WebGL: "webgl", WebGL2: "webgl2", WebGPU: "webgpu"});

We don't want to modify this file.  It is used by newer frontends to know what commands are available on older backends (e.g. macOS 10.14 connecting to a iOS 11.3 device).  This file is effectively "frozen in time" based on what the iOS version supported at the time it was created.

> Source/WebInspectorUI/UserInterface/Protocol/Legacy/12.0/InspectorBackendCommands.js:-70
> -InspectorBackend.registerEnum("Canvas.ContextType", {Canvas2D: "canvas-2d", BitmapRenderer: "bitmaprenderer", WebGL: "webgl", WebGL2: "webgl2", WebGPU: "webgpu"});

Ditto.

[Justin Fan]

Got it, thanks for the review.

[Dean Jackson]

Comment on attachment 351712 [details]
Patch

We need to put something at Websites/webkit.org/demos/webgpu/index.html to point at the WebMetal demos.

[Joseph Pecoraro]

Comment on attachment 351712 [details]
Patch

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

>> Source/WebInspectorUI/UserInterface/Models/Canvas.js:-308
>> -    WebGPU: "webgpu",
> 
> Given what I wrote below (InspectorBackendCommands.js:70), this will need to stay as is for iOS 11.3 and 12.0.  You can add `WebMetal` to it, but you need to keep `WebGPU`.

To expand on Devin's point you will need to keep the WebGPU enum value too upport legacy backends. In all cases you can make it do the same thing as the new WebMetal enum you're introducing.

>> Source/WebInspectorUI/UserInterface/Protocol/Legacy/11.3/InspectorBackendCommands.js:-70
>> -InspectorBackend.registerEnum("Canvas.ContextType", {Canvas2D: "canvas-2d", BitmapRenderer: "bitmaprenderer", WebGL: "webgl", WebGL2: "webgl2", WebGPU: "webgpu"});
> 
> We don't want to modify this file.  It is used by newer frontends to know what commands are available on older backends (e.g. macOS 10.14 connecting to a iOS 11.3 device).  This file is effectively "frozen in time" based on what the iOS version supported at the time it was created.

Correct. Great catch Devin! These files are generated from Source/WebInspectorUI/Versions (as is written at the top of the file). These shouldn't be changing with this patch.

[Justin Fan]

Created attachment 351824 [details]
Patch

[EWS Watchlist]

Attachment 351824 [details] did not pass style-queue:


ERROR: Source/WebCore/html/canvas/WebMetalDrawable.cpp:43:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
ERROR: Source/WebCore/html/canvas/WebMetalLibrary.cpp:42:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
ERROR: Source/WebCore/html/canvas/WebMetalCommandBuffer.cpp:49:  This { should be at the end of the previous line  [whitespace/braces] [4]
Total errors found: 3 in 219 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[WebKit Commit Bot]

Comment on attachment 351824 [details]
Patch

Clearing flags on attachment: 351824

Committed r236954: <https://trac.webkit.org/changeset/236954>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Truitt Savell]

After the changes in https://trac.webkit.org/changeset/236954/webkit

The test inspector/canvas/create-context-webmetal.html which was modified is failing constantly. 

History:
https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=inspector%2Fcanvas%2Fcreate-context-webmetal.html

Diff:
https://build.webkit.org/results/Apple%20High%20Sierra%20Release%20WK2%20(Tests)/r236959%20(7175)/inspector/canvas/create-context-webmetal-pretty-diff.html