Remove WebGPUObject
Created attachment 345986 [details] Patch
Attachment 345986 [details] did not pass style-queue: ERROR: Source/WebCore/html/canvas/WebGPUDrawable.cpp:44: Code inside a namespace should not be indented. [whitespace/indent] [4] Total errors found: 1 in 56 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 345986 [details] Patch Attachment 345986 [details] did not pass win-ews (win): Output: https://webkit-queues.webkit.org/results/8683999 New failing tests: http/tests/security/video-poster-cross-origin-crash2.html
Created attachment 345994 [details] Archive of layout-test-results from ews200 for win-future The attached test failures were seen while running run-webkit-tests on the win-ews. Bot: ews200 Port: win-future Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit
Comment on attachment 345986 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=345986&action=review Great direction. I think we should reduce the use of WebGPURenderingContext even more. In fact, I think a better pattern might be to have the create functions take the GPUXXX&& arguments. The need for the rendering context argument drove a pattern where more of the logic was in the create functions, but I see no reason that callers, for example, couldn’t just create a GPUTexture and make a WebGPUTexture out of it. It would be nice if these WebGPU objects were closer to being pure wrappers. > Source/WebCore/html/canvas/WebGPUBuffer.h:40 > static RefPtr<WebGPUBuffer> create(WebGPURenderingContext&, const JSC::ArrayBufferView&); Consider changing the first argument from a WebGPURenderingContext& to a const GPUDevice&. > Source/WebCore/html/canvas/WebGPUCommandBuffer.h:59 > + WebGPUCommandBuffer(const GPUCommandQueue&); Should add explicit. > Source/WebCore/html/canvas/WebGPUCommandQueue.h:42 > static Ref<WebGPUCommandQueue> create(WebGPURenderingContext&); Consider changing the argument from a WebGPURenderingContext& to a const GPUDevice&. > Source/WebCore/html/canvas/WebGPUComputeCommandEncoder.h:50 > + WebGPUComputeCommandEncoder(const GPUCommandBuffer&); Should add explicit. > Source/WebCore/html/canvas/WebGPUComputePipelineState.h:41 > static Ref<WebGPUComputePipelineState> create(WebGPURenderingContext&, const GPUFunction&); Consider changing the first argument from a WebGPURenderingContext& to a const GPUDevice&. > Source/WebCore/html/canvas/WebGPUDepthStencilState.h:40 > static Ref<WebGPUDepthStencilState> create(WebGPURenderingContext&, const GPUDepthStencilDescriptor&); Consider changing the first argument from a WebGPURenderingContext& to a const GPUDevice&. > Source/WebCore/html/canvas/WebGPUDrawable.h:42 > static Ref<WebGPUDrawable> create(WebGPURenderingContext&); Consider changing the argument from a WebGPURenderingContext& to a const GPUDevice&. > Source/WebCore/html/canvas/WebGPUFunction.h:44 > + WebGPUFunction(GPUFunction&&); Should add explicit. > Source/WebCore/html/canvas/WebGPULibrary.h:42 > static Ref<WebGPULibrary> create(WebGPURenderingContext&, const String& sourceCode); Consider changing the first argument from a WebGPURenderingContext& to a const GPUDevice&. > Source/WebCore/html/canvas/WebGPURenderPassAttachmentDescriptor.h:56 > + explicit WebGPURenderPassAttachmentDescriptor(); Should remove explicit now that this has no arguments. > Source/WebCore/html/canvas/WebGPURenderPassColorAttachmentDescriptor.h:44 > + WebGPURenderPassColorAttachmentDescriptor(GPURenderPassColorAttachmentDescriptor&&); Should add explicit. > Source/WebCore/html/canvas/WebGPURenderPassDepthAttachmentDescriptor.h:44 > + WebGPURenderPassDepthAttachmentDescriptor(GPURenderPassDepthAttachmentDescriptor&&); Should add explicit. > Source/WebCore/html/canvas/WebGPURenderPipelineState.h:40 > static Ref<WebGPURenderPipelineState> create(WebGPURenderingContext&, const GPURenderPipelineDescriptor&); Consider changing the first argument from a WebGPURenderingContext& to a const GPUDevice&. > Source/WebCore/html/canvas/WebGPUTexture.h:41 > static Ref<WebGPUTexture> create(WebGPURenderingContext&, const GPUTextureDescriptor&); Consider changing the first argument from a WebGPURenderingContext& to a const GPUDevice&. Or consider eliminating this and promoting createFromDrawableTexture to be named create. The caller can create the GPUTexture.
Created attachment 345999 [details] Patch
Comment on attachment 345999 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=345999&action=review > Source/WebCore/html/canvas/WebGPURenderPassColorAttachmentDescriptor.cpp:43 > + : WebGPURenderPassAttachmentDescriptor() I don’t think explicit base class initialization is needed any more in this constructor. > Source/WebCore/html/canvas/WebGPURenderPassDepthAttachmentDescriptor.cpp:41 > + : WebGPURenderPassAttachmentDescriptor() Ditto.
Attachment 345999 [details] did not pass style-queue: ERROR: Source/WebCore/html/canvas/WebGPUDrawable.cpp:44: Code inside a namespace should not be indented. [whitespace/indent] [4] Total errors found: 1 in 56 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 346036 [details] Patch
Attachment 346036 [details] did not pass style-queue: ERROR: Source/WebCore/html/canvas/WebGPULibrary.cpp:42: Code inside a namespace should not be indented. [whitespace/indent] [4] ERROR: Source/WebCore/html/canvas/WebGPUDrawable.cpp:43: Code inside a namespace should not be indented. [whitespace/indent] [4] Total errors found: 2 in 57 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 346038 [details] Patch
Created attachment 346039 [details] Patch
Attachment 346039 [details] did not pass style-queue: ERROR: Source/WebCore/html/canvas/WebGPULibrary.cpp:42: Code inside a namespace should not be indented. [whitespace/indent] [4] ERROR: Source/WebCore/html/canvas/WebGPUDrawable.cpp:43: Code inside a namespace should not be indented. [whitespace/indent] [4] Total errors found: 2 in 59 files If any of these errors are false positives, please file a bug against check-webkit-style.
(In reply to Darin Adler from comment #5) > Comment on attachment 345986 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=345986&action=review > > Great direction. > > I think we should reduce the use of WebGPURenderingContext even more. In > fact, I think a better pattern might be to have the create functions take > the GPUXXX&& arguments. The need for the rendering context argument drove a > pattern where more of the logic was in the create functions, but I see no > reason that callers, for example, couldn’t just create a GPUTexture and make > a WebGPUTexture out of it. It would be nice if these WebGPU objects were > closer to being pure wrappers. Yeah, that does sound better. Updated the patch to do that. > > > Source/WebCore/html/canvas/WebGPUBuffer.h:40 > > static RefPtr<WebGPUBuffer> create(WebGPURenderingContext&, const JSC::ArrayBufferView&); > > Consider changing the first argument from a WebGPURenderingContext& to a > const GPUDevice&. Changed to take a GPUBuffer&& instead. > > > Source/WebCore/html/canvas/WebGPUCommandBuffer.h:59 > > + WebGPUCommandBuffer(const GPUCommandQueue&); > > Should add explicit. Done. > > > Source/WebCore/html/canvas/WebGPUCommandQueue.h:42 > > static Ref<WebGPUCommandQueue> create(WebGPURenderingContext&); > > Consider changing the argument from a WebGPURenderingContext& to a const > GPUDevice&. Changed to take a GPUCommandQueue&& instead. > > > Source/WebCore/html/canvas/WebGPUComputeCommandEncoder.h:50 > > + WebGPUComputeCommandEncoder(const GPUCommandBuffer&); > > Should add explicit. Done. > > > Source/WebCore/html/canvas/WebGPUComputePipelineState.h:41 > > static Ref<WebGPUComputePipelineState> create(WebGPURenderingContext&, const GPUFunction&); > > Consider changing the first argument from a WebGPURenderingContext& to a > const GPUDevice&. Changed to take a GPUComputePipelineState&& instead. > > > Source/WebCore/html/canvas/WebGPUDepthStencilState.h:40 > > static Ref<WebGPUDepthStencilState> create(WebGPURenderingContext&, const GPUDepthStencilDescriptor&); > > Consider changing the first argument from a WebGPURenderingContext& to a > const GPUDevice&. Changed to take a GPUDepthStencilState&& instead. > > > Source/WebCore/html/canvas/WebGPUDrawable.h:42 > > static Ref<WebGPUDrawable> create(WebGPURenderingContext&); > > Consider changing the argument from a WebGPURenderingContext& to a const > GPUDevice&. Changed to take a GPUDrawable.h&& instead. > > > Source/WebCore/html/canvas/WebGPUFunction.h:44 > > + WebGPUFunction(GPUFunction&&); > > Should add explicit. Done. > > > Source/WebCore/html/canvas/WebGPULibrary.h:42 > > static Ref<WebGPULibrary> create(WebGPURenderingContext&, const String& sourceCode); > > Consider changing the first argument from a WebGPURenderingContext& to a > const GPUDevice&. Changed to take a GPULibrary&& instead. > > > Source/WebCore/html/canvas/WebGPURenderPassAttachmentDescriptor.h:56 > > + explicit WebGPURenderPassAttachmentDescriptor(); > > Should remove explicit now that this has no arguments. Done. > > > Source/WebCore/html/canvas/WebGPURenderPassColorAttachmentDescriptor.h:44 > > + WebGPURenderPassColorAttachmentDescriptor(GPURenderPassColorAttachmentDescriptor&&); > > Should add explicit. Done. > > > Source/WebCore/html/canvas/WebGPURenderPassDepthAttachmentDescriptor.h:44 > > + WebGPURenderPassDepthAttachmentDescriptor(GPURenderPassDepthAttachmentDescriptor&&); > > Should add explicit. Done. > > > Source/WebCore/html/canvas/WebGPURenderPipelineState.h:40 > > static Ref<WebGPURenderPipelineState> create(WebGPURenderingContext&, const GPURenderPipelineDescriptor&); > > Consider changing the first argument from a WebGPURenderingContext& to a > const GPUDevice&. Changed to take a GPURenderPipelineState&& instead. > > > Source/WebCore/html/canvas/WebGPUTexture.h:41 > > static Ref<WebGPUTexture> create(WebGPURenderingContext&, const GPUTextureDescriptor&); > > Consider changing the first argument from a WebGPURenderingContext& to a > const GPUDevice&. > > Or consider eliminating this and promoting createFromDrawableTexture to be > named create. The caller can create the GPUTexture. I did that latter.
(In reply to Darin Adler from comment #7) > Comment on attachment 345999 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=345999&action=review > > > Source/WebCore/html/canvas/WebGPURenderPassColorAttachmentDescriptor.cpp:43 > > + : WebGPURenderPassAttachmentDescriptor() > > I don’t think explicit base class initialization is needed any more in this > constructor. > > > Source/WebCore/html/canvas/WebGPURenderPassDepthAttachmentDescriptor.cpp:41 > > + : WebGPURenderPassAttachmentDescriptor() > > Ditto. Removed the base class initialization in both cases.
Created attachment 346040 [details] Patch
Attachment 346040 [details] did not pass style-queue: ERROR: Source/WebCore/html/canvas/WebGPULibrary.cpp:42: Code inside a namespace should not be indented. [whitespace/indent] [4] ERROR: Source/WebCore/html/canvas/WebGPUDrawable.cpp:43: Code inside a namespace should not be indented. [whitespace/indent] [4] Total errors found: 2 in 59 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 346041 [details] Patch
Attachment 346041 [details] did not pass style-queue: ERROR: Source/WebCore/html/canvas/WebGPULibrary.cpp:42: Code inside a namespace should not be indented. [whitespace/indent] [4] ERROR: Source/WebCore/html/canvas/WebGPUDrawable.cpp:43: Code inside a namespace should not be indented. [whitespace/indent] [4] Total errors found: 2 in 59 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 346042 [details] Patch
Attachment 346042 [details] did not pass style-queue: ERROR: Source/WebCore/html/canvas/WebGPULibrary.cpp:42: Code inside a namespace should not be indented. [whitespace/indent] [4] ERROR: Source/WebCore/html/canvas/WebGPUDrawable.cpp:43: Code inside a namespace should not be indented. [whitespace/indent] [4] Total errors found: 2 in 59 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 346042 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=346042&action=review > Source/WebCore/html/canvas/WebGPURenderPipelineDescriptor.h:32 > +#include <wtf/Ref.h> RefPtr.h happens to include Ref.h, so we never have to include both. Not sure that’s an elegant thing, but I tend to go with it and leave out the Ref.h include when both are needed.
Comment on attachment 346042 [details] Patch Attachment 346042 [details] did not pass win-ews (win): Output: https://webkit-queues.webkit.org/results/8696590 New failing tests: http/tests/security/contentSecurityPolicy/userAgentShadowDOM/allow-video.html imported/blink/transitions/unprefixed-transform.html legacy-animation-engine/imported/blink/transitions/unprefixed-transform.html
Created attachment 346052 [details] Archive of layout-test-results from ews205 for win-future The attached test failures were seen while running run-webkit-tests on the win-ews. Bot: ews205 Port: win-future Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit
Created attachment 346073 [details] Patch
Attachment 346073 [details] did not pass style-queue: ERROR: Source/WebCore/html/canvas/WebGPULibrary.cpp:42: Code inside a namespace should not be indented. [whitespace/indent] [4] ERROR: Source/WebCore/html/canvas/WebGPUDrawable.cpp:43: Code inside a namespace should not be indented. [whitespace/indent] [4] Total errors found: 2 in 59 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 346554 [details] Patch
Attachment 346554 [details] did not pass style-queue: ERROR: Source/WebCore/html/canvas/WebGPULibrary.cpp:42: Code inside a namespace should not be indented. [whitespace/indent] [4] ERROR: Source/WebCore/html/canvas/WebGPUDrawable.cpp:43: Code inside a namespace should not be indented. [whitespace/indent] [4] Total errors found: 2 in 59 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 346554 [details] Patch Clearing flags on attachment: 346554 Committed r234564: <https://trac.webkit.org/changeset/234564>
All reviewed patches have been landed. Closing bug.
<rdar://problem/42920504>