Bug 192516 – [WebGPU] Implement WebGPUBuffer, and some nullibility consistency in WebGPU

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Justin Fan]

[WebGPU] Implement WebGPUBuffer

[Justin Fan]

Created attachment 356840 [details]
Patch

[Justin Fan]

Created attachment 356982 [details]
Patch

[EWS Watchlist]

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


ERROR: Source/WebCore/platform/graphics/gpu/GPUBufferUsage.h:39:  MAP_READ is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/platform/graphics/gpu/GPUBufferUsage.h:40:  MAP_WRITE is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/platform/graphics/gpu/GPUBufferUsage.h:41:  TRANSFER_SRC is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/platform/graphics/gpu/GPUBufferUsage.h:42:  TRANSFER_DST is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 4 in 42 files


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

[Justin Fan]

Created attachment 356983 [details]
Patch

[EWS Watchlist]

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


ERROR: Source/WebCore/platform/graphics/gpu/GPUBufferUsage.h:39:  MAP_READ is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/platform/graphics/gpu/GPUBufferUsage.h:40:  MAP_WRITE is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/platform/graphics/gpu/GPUBufferUsage.h:41:  TRANSFER_SRC is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/platform/graphics/gpu/GPUBufferUsage.h:42:  TRANSFER_DST is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 4 in 42 files


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

[EWS Watchlist]

Comment on attachment 356983 [details]
Patch

Attachment 356983 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/10341940

New failing tests:
webgpu/buffers.html

[EWS Watchlist]

Created attachment 356996 [details]
Archive of layout-test-results from ews107 for mac-sierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-sierra-wk2  Platform: Mac OS X 10.12.6

[Justin Fan]

Created attachment 356998 [details]
Patch

[EWS Watchlist]

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


ERROR: Source/WebCore/platform/graphics/gpu/GPUBufferUsage.h:39:  MAP_READ is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/platform/graphics/gpu/GPUBufferUsage.h:40:  MAP_WRITE is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/platform/graphics/gpu/GPUBufferUsage.h:41:  TRANSFER_SRC is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/platform/graphics/gpu/GPUBufferUsage.h:42:  TRANSFER_DST is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 4 in 42 files


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

[Dean Jackson]

Comment on attachment 356998 [details]
Patch

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

> Source/WebCore/Modules/webgpu/WebGPURenderPassEncoder.cpp:38
> +RefPtr<WebGPURenderPassEncoder> WebGPURenderPassEncoder::create(Ref<WebGPUCommandBuffer>&& creator, Ref<GPURenderPassEncoder>&& encoder)
>  {
> -    return adoptRef(*new WebGPURenderPassEncoder(WTFMove(creator), WTFMove(encoder)));
> +    return adoptRef(new WebGPURenderPassEncoder(WTFMove(creator), WTFMove(encoder)));

s/creator/commandBuffer/

> Source/WebCore/platform/graphics/gpu/GPUBuffer.h:40
> +namespace JSC {
> +
> +class ArrayBuffer;
> +    
> +}

No need for blank lines.

> Source/WebCore/platform/graphics/gpu/GPUBufferUsage.h:36
> +class GPUBufferUsage : public RefCounted<GPUBufferUsage> {

This doesn't need to be RefCounted. There will never be an instance of this class.

> Source/WebCore/platform/graphics/gpu/GPUBufferUsage.h:46
> +    static const unsigned long NONE = 0;
> +    static const unsigned long MAP_READ = 1;
> +    static const unsigned long MAP_WRITE = 2;
> +    static const unsigned long TRANSFER_SRC = 4;
> +    static const unsigned long TRANSFER_DST = 8;
> +    static const unsigned long INDEX = 16;
> +    static const unsigned long VERTEX = 32;
> +    static const unsigned long UNIFORM = 64;
> +    static const unsigned long STORAGE = 128;

I think we should follow WebKit style here and use CamelCase. As long as the values are what the IDL exposes, we should be ok.

e.g.

enum class GPUBufferUsage : uint32_t {
  None = 0,
  MapRead = 1,
  MapWrite = 2,
  ...
}

> Source/WebCore/platform/graphics/gpu/cocoa/GPUBufferMetal.mm:48
> +    const char* const functionName = "GPUBuffer::create()";
> +#if LOG_DISABLED
> +    UNUSED_PARAM(functionName);
> +#endif

I suggest just putting this into each LOG statement rather than having a local variable.

> LayoutTests/webgpu/buffers.html:10
> +'use strict';

Not needed for this test.

> LayoutTests/webgpu/buffers.html:14
> +    assert_true(buffer instanceof WebGPUBuffer, "creatBuffer returned a WebGPUBuffer");

Typo: create.

> LayoutTests/webgpu/buffers.html:23
> +    let floatArray = new Float32Array(arrayBuffer);
> +    assert_equals(floatArray.length, 4);
> +
> +    floatArray[0] = -1;
> +    floatArray[1] = 1;
> +    floatArray[2] = 0;
> +    floatArray[3] = 1;

It turns out that this isn't how you'd use the API, but it's still exercising the things you've built, so we should keep it.

[Justin Fan]

Created attachment 357025 [details]
Patch

[Justin Fan]

Created attachment 357026 [details]
Patch for landing

[Justin Fan]

<rdar://problem/46289663>

[WebKit Commit Bot]

Comment on attachment 357026 [details]
Patch for landing

Rejecting attachment 357026 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-03', 'build', '--no-clean', '--no-update', '--build-style=release', '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Last 5000 characters of output:
O -DENABLE_VIDEO_PRESENTATION_MODE -DENABLE_VIDEO_TRACK -DENABLE_VIDEO_USES_ELEMENT_FULLSCREEN -DENABLE_WEB_AUDIO -DENABLE_WEB_AUTHN -DENABLE_WEB_RTC -DENABLE_WEBGL -DENABLE_WEBGL2 -DENABLE_WEBGPU -DENABLE_WEBMETAL -DENABLE_WIRELESS_PLAYBACK_TARGET -DENABLE_XSLT -DBUILDING_WEBKIT -DU_DISABLE_RENAMING=1 -DU_SHOW_CPLUSPLUS_API=0 -DGL_SILENCE_DEPRECATION=1 -DGLES_SILENCE_DEPRECATION=1 -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.12.sdk -fasm-blocks -fstrict-aliasing -Wdeprecated-declarations -Winvalid-offsetof -mmacosx-version-min=10.12 -g -fvisibility=hidden -fvisibility-inlines-hidden -fno-threadsafe-statics -Wno-sign-conversion -Winfinite-recursion -Wmove -iquote /Volumes/Data/EWS/WebKit/WebKitBuild/WebCore.build/Release/WebCore.build/WebCore-generated-files.hmap -I/Volumes/Data/EWS/WebKit/WebKitBuild/WebCore.build/Release/WebCore.build/WebCore-own-target-headers.hmap -I/Volumes/Data/EWS/WebKit/WebKitBuild/WebCore.build/Release/WebCore.build/WebCore-all-target-headers.hmap -iquote /Volumes/Data/EWS/WebKit/WebKitBuild/WebCore.build/Release/WebCore.build/WebCore-project-headers.hmap -I/Volumes/Data/EWS/WebKit/WebKitBuild/Release/include -IPAL -IForwardingHeaders -I/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.12.sdk/usr/include/libxslt -I/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.12.sdk/usr/include/libxml2 -I/Volumes/Data/EWS/WebKit/WebKitBuild/Release/DerivedSources/WebCore -I/Volumes/Data/EWS/WebKit/WebKitBuild/Release/usr/local/include -I/Volumes/Data/EWS/WebKit/WebKitBuild/Release/usr/local/include/WebKitAdditions -I/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.12.sdk/usr/local/include/WebKitAdditions -I/Volumes/Data/EWS/WebKit/WebKitBuild/Release/usr/local/include/webrtc -I/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.12.sdk/usr/local/include/webrtc -I/Volumes/Data/EWS/WebKit/WebKitBuild/Release/usr/local/include/webrtc/sdk/objc/Framework/Headers -I/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.12.sdk/usr/local/include/webrtc/sdk/objc/Framework/Headers -I/Volumes/Data/EWS/WebKit/Source/WebCore -I/Volumes/Data/EWS/WebKit/WebKitBuild/WebCore.build/Release/WebCore.build/DerivedSources/x86_64 -I/Volumes/Data/EWS/WebKit/WebKitBuild/WebCore.build/Release/WebCore.build/DerivedSources -Wall -Wextra -Wcast-qual -Wchar-subscripts -Wextra-tokens -Wformat=2 -Winit-self -Wmissing-format-attribute -Wmissing-noreturn -Wpacked -Wpointer-arith -Wredundant-decls -Wundef -Wwrite-strings -Wexit-time-destructors -Wglobal-constructors -Wtautological-compare -Wimplicit-fallthrough -Wno-unknown-warning-option -F/Volumes/Data/EWS/WebKit/WebKitBuild/Release -iframework /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.12.sdk/System/Library/PrivateFrameworks -iframework /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.12.sdk/System/Library/Frameworks -isystem /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.12.sdk/System/Library/Frameworks/System.framework/PrivateHeaders -iframework /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.12.sdk/System/Library/Frameworks/Carbon.framework/Frameworks -iframework /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.12.sdk/System/Library/Frameworks/ApplicationServices.framework/Frameworks -iframework /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.12.sdk/System/Library/Frameworks/CoreServices.framework/Frameworks -iframework /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.12.sdk/System/Library/Frameworks/Quartz.framework/Frameworks -include /Volumes/Data/EWS/WebKit/WebKitBuild/PrecompiledHeaders/WebCorePrefix-ezexlykycjvolzdbzvgtiulbztir/WebCorePrefix.h -MMD -MT dependencies -MF /Volumes/Data/EWS/WebKit/WebKitBuild/WebCore.build/Release/WebCore.build/Objects-normal/x86_64/UnifiedSource112.d --serialize-diagnostics /Volumes/Data/EWS/WebKit/WebKitBuild/WebCore.build/Release/WebCore.build/Objects-normal/x86_64/UnifiedSource112.dia -c /Volumes/Data/EWS/WebKit/WebKitBuild/Release/DerivedSources/WebCore/unified-sources/UnifiedSource112.cpp -o /Volumes/Data/EWS/WebKit/WebKitBuild/WebCore.build/Release/WebCore.build/Objects-normal/x86_64/UnifiedSource112.o

** BUILD FAILED **


The following build commands failed:
	CompileC /Volumes/Data/EWS/WebKit/WebKitBuild/WebCore.build/Release/WebCore.build/Objects-normal/x86_64/UnifiedSource105.o /Volumes/Data/EWS/WebKit/WebKitBuild/Release/DerivedSources/WebCore/unified-sources/UnifiedSource105.cpp normal x86_64 c++ com.apple.compilers.llvm.clang.1_0.compiler
(1 failure)

Full output: https://webkit-queues.webkit.org/results/10346646

[Justin Fan]

Created attachment 357086 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 357086 [details]
Patch for landing

Rejecting attachment 357086 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-02', 'apply-attachment', '--no-update', '--non-interactive', 357086, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Logging in as [e-mail]...
Fetching: https://bugs.webkit.org/attachment.cgi?id=357086&action=edit
Fetching: https://bugs.webkit.org/show_bug.cgi?id=192516&ctype=xml&excludefield=attachmentdata
Processing 1 patch from 1 bug.
Processing patch 357086 from bug 192516.
Fetching: https://bugs.webkit.org/attachment.cgi?id=357086
Failed to run "[u'/Volumes/Data/EWS/WebKit/Tools/Scripts/svn-apply', '--force']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

Parsed 41 diffs from patch file(s).
patching file Source/WebCore/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file Source/WebCore/CMakeLists.txt
patching file Source/WebCore/DerivedSources.make
patching file Source/WebCore/Modules/webgpu/WebGPUBuffer.cpp
patching file Source/WebCore/Modules/webgpu/WebGPUBuffer.h
patching file Source/WebCore/Modules/webgpu/WebGPUBuffer.idl
patching file Source/WebCore/Modules/webgpu/WebGPUBufferDescriptor.h
patching file Source/WebCore/Modules/webgpu/WebGPUBufferDescriptor.idl
patching file Source/WebCore/Modules/webgpu/WebGPUBufferUsage.h
patching file Source/WebCore/Modules/webgpu/WebGPUBufferUsage.idl
patching file Source/WebCore/Modules/webgpu/WebGPUCommandBuffer.cpp
patching file Source/WebCore/Modules/webgpu/WebGPUCommandBuffer.h
patching file Source/WebCore/Modules/webgpu/WebGPUDevice.cpp
patching file Source/WebCore/Modules/webgpu/WebGPUDevice.h
patching file Source/WebCore/Modules/webgpu/WebGPUDevice.idl
patching file Source/WebCore/Modules/webgpu/WebGPUQueue.cpp
patching file Source/WebCore/Modules/webgpu/WebGPURenderPassEncoder.cpp
patching file Source/WebCore/Modules/webgpu/WebGPURenderPassEncoder.h
patching file Source/WebCore/Modules/webgpu/WebGPUShaderModule.cpp
patching file Source/WebCore/Modules/webgpu/WebGPUShaderModule.h
patching file Source/WebCore/Modules/webgpu/WebGPUSwapChain.idl
patching file Source/WebCore/Modules/webgpu/WebGPUTexture.cpp
patching file Source/WebCore/Modules/webgpu/WebGPUTextureView.cpp
patching file Source/WebCore/Modules/webgpu/WebGPUTextureView.h
patching file Source/WebCore/Sources.txt
Hunk #2 succeeded at 3222 (offset 2 lines).
patching file Source/WebCore/SourcesCocoa.txt
patching file Source/WebCore/WebCore.xcodeproj/project.pbxproj
Hunk #1 succeeded at 4227 (offset -1 lines).
Hunk #5 succeeded at 25510 (offset 1 line).
Hunk #6 succeeded at 25575 (offset 1 line).
Hunk #7 FAILED at 30011.
Hunk #8 FAILED at 30021.
Hunk #9 FAILED at 30623.
3 out of 10 hunks FAILED -- saving rejects to file Source/WebCore/WebCore.xcodeproj/project.pbxproj.rej
patching file Source/WebCore/bindings/js/WebCoreBuiltinNames.h
patching file Source/WebCore/platform/graphics/gpu/GPUBuffer.h
patching file Source/WebCore/platform/graphics/gpu/GPUBufferDescriptor.h
patching file Source/WebCore/platform/graphics/gpu/GPUDevice.cpp
patching file Source/WebCore/platform/graphics/gpu/GPUDevice.h
patching file Source/WebCore/platform/graphics/gpu/cocoa/GPUBufferMetal.mm
patching file Source/WebCore/platform/graphics/gpu/cocoa/GPUQueueMetal.mm
patching file Source/WebCore/platform/graphics/gpu/cocoa/GPURenderPipelineMetal.mm
patching file Source/WebCore/platform/graphics/gpu/cocoa/GPUShaderModuleMetal.mm
patching file Source/WebCore/platform/graphics/gpu/cocoa/GPUSwapChainMetal.mm
patching file Source/WebCore/platform/graphics/gpu/cocoa/GPUTextureMetal.mm
patching file LayoutTests/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file LayoutTests/webgpu/buffers-expected.txt
patching file LayoutTests/webgpu/buffers.html

Failed to run "[u'/Volumes/Data/EWS/WebKit/Tools/Scripts/svn-apply', '--force']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

Full output: https://webkit-queues.webkit.org/results/10358933

[Justin Fan]

This patch hates me.

[Justin Fan]

Created attachment 357087 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 357087 [details]
Patch for landing

Clearing flags on attachment: 357087

Committed r239094: <https://trac.webkit.org/changeset/239094>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.