GPUSwapChainDescriptor has been simplified in the sketch IDL (https://github.com/gpuweb/gpuweb/blob/master/design/sketch.webidl#L630). For example, height and width of the swap chain are now gleaned from the canvas. In addition, GPUTextureUsageFlags has been implemented and should be taken into account when creating the swap chain.
<rdar://problem/47892466>
This also involves providing the GPUDevice to Web GPU during context/swapChain creation (canvas.getContext()) rather than by configuring it on the swap chain object.
Created attachment 364125 [details] Patch
Comment on attachment 364125 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=364125&action=review > Source/WebCore/Modules/webgpu/DOMWindowWebGPU.idl:31 > + [Replaceable, SameObject] readonly attribute WebGPU gpu; This isn't permanent, as the API now exposes "gpu" as just a namespace on window rather than as an object. I made this change while trying to fix Inspector.
Patch is still WIP as ChangeLogs have to be elaborated on, but tossing it at EWS to see what sticks.
Attachment 364125 [details] did not pass style-queue: ERROR: Source/WebCore/ChangeLog:8: You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible. [changelog/nonewtests] [5] Total errors found: 1 in 66 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 364125 [details] Patch Attachment 364125 [details] did not pass mac-wk2-ews (mac-wk2): Output: https://webkit-queues.webkit.org/results/11438496 New failing tests: accessibility/mac/selection-notification-focus-change.html
Created attachment 364129 [details] Archive of layout-test-results from ews107 for mac-highsierra-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews107 Port: mac-highsierra-wk2 Platform: Mac OS X 10.13.6
Created attachment 364235 [details] Patch
Comment on attachment 364235 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=364235&action=review > Source/WebCore/Modules/webgpu/WebGPUDevice.idl:36 > + GPUTextureUsageFlags usage = 16; // FIXME: How to set this to GPUTextureUsage.OUTPUT_ATTACHMENT in IDL? Might want to add a comment in GPUTextureUsage so we know if we modify those flags we have to modify this value too. Also would be good to file a new bug about this.
Comment on attachment 364235 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=364235&action=review > Source/WebCore/Modules/webgpu/GPUCanvasContext.cpp:71 > + if (m_swapChain && m_swapChain->swapChain()) > + m_swapChain->swapChain()->present(); I don't quite understand this. Maybe Dean can help here. > Source/WebCore/platform/graphics/gpu/cocoa/GPURenderPassEncoderMetal.mm:170 > + useAttachments(buffer, WTFMove(descriptor)); Cool. > Source/WebCore/platform/graphics/gpu/cocoa/GPUSwapChainMetal.mm:66 > +static RetainPtr<WebGPULayer> tryCreateSwapLayer(MTLDevice *device, MTLPixelFormat format, OptionSet<GPUTextureUsage::Flags> usage) "Create Swap"? "Swap Layer"? This could be named better. > Source/WebCore/platform/graphics/gpu/cocoa/GPUSwapChainMetal.mm:109 > + setLayerShape(swapLayer.get(), width, height); Shouldn't we be hooking this up to get called when the canvas backing store changes size?
(In reply to Myles C. Maxfield from comment #11) > Comment on attachment 364235 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=364235&action=review > > > Source/WebCore/Modules/webgpu/GPUCanvasContext.cpp:71 > > + if (m_swapChain && m_swapChain->swapChain()) > > + m_swapChain->swapChain()->present(); > > I don't quite understand this. Maybe Dean can help here. Yeah, I'm not entirely sure if this implementation is necessary either. > > Source/WebCore/platform/graphics/gpu/cocoa/GPUSwapChainMetal.mm:109 > > + setLayerShape(swapLayer.get(), width, height); > > Shouldn't we be hooking this up to get called when the canvas backing store > changes size? This should happen because GPUCanvasContext::reshape() also calls setLayerShape().
Created attachment 364314 [details] Patch for landing
Comment on attachment 364314 [details] Patch for landing Clearing flags on attachment: 364314 Committed r242759: <https://trac.webkit.org/changeset/242759>
All reviewed patches have been landed. Closing bug.
Comment on attachment 364314 [details] Patch for landing View in context: https://bugs.webkit.org/attachment.cgi?id=364314&action=review > Source/JavaScriptCore/inspector/scripts/codegen/generator.py:53 > + 'webgpu': 'WebGPU', # Canvas.ContextType.gpu NIT: `Canvas.ContextType.webgpu` to match Canvas.json. > Source/WebCore/inspector/agents/InspectorCanvasAgent.cpp:253 > + return toJS(&state, deprecatedGlobalObjectForPrototype(&state), downcast<GPUCanvasContext>(context)); You'd also want to update `WebCore::canvasRenderingContext` in PageConsoleClient.cpp with the same type of thing. > Source/WebInspectorUI/UserInterface/Models/Canvas.js:415 > + WebGPU: "gpu", We keep these values in sync with the ones in Canvas.json, so it should be "webgpu".
(In reply to Devin Rousso from comment #16) > Comment on attachment 364314 [details] > Patch for landing Dunno why this didn't save with my other comments, but here it is. Thanks for adding some inspector support for this! In the future, please reach out (or just CC) one of us on the bug so we can provide feedback (if needed). I noticed a few things while reading the inspector parts. I've commented on them inline.
Comment on attachment 364314 [details] Patch for landing View in context: https://bugs.webkit.org/attachment.cgi?id=364314&action=review >> Source/WebCore/inspector/agents/InspectorCanvasAgent.cpp:253 >> + return toJS(&state, deprecatedGlobalObjectForPrototype(&state), downcast<GPUCanvasContext>(context)); > > You'd also want to update `WebCore::canvasRenderingContext` in PageConsoleClient.cpp with the same type of thing. Actually, I take this back. We don't support recording Web GPU contexts right now, so you wouldn't want to add it there. My mistake!
(In reply to Devin Rousso from comment #17) > (In reply to Devin Rousso from comment #16) > > Comment on attachment 364314 [details] > > Patch for landing > Dunno why this didn't save with my other comments, but here it is. > > Thanks for adding some inspector support for this! In the future, please > reach out (or just CC) one of us on the bug so we can provide feedback (if > needed). I noticed a few things while reading the inspector parts. I've > commented on them inline. Thanks for looking it over! I apologize for not roping you guys in as I really only made the changes to Inspector because my renaming had caused it to keep crashing. In the future we can work together on improved Inspector support and I can clean up the naming (unfortunately it still isn't final).