Bug 191990

Summary: [WebGPU] Begin implementation of WebGPURenderPassEncoder and barebones WebGPURenderPassDescriptor
Product: WebKit Reporter: Justin Fan <justin_fan>
Component: WebGPUAssignee: Justin Fan <justin_fan>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, dino, justin_fan, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch for landing
none
Patch for landing none

Justin Fan
Reported 2018-11-26 16:32:03 PST
[WebGPU] Begin implementation of WebGPURenderPassEncoder and barebones WebGPURenderPassDescriptor
Attachments
Patch (76.69 KB, patch)
2018-11-27 12:33 PST, Justin Fan 		no flags
DetailsFormatted DiffDiff
Patch (76.90 KB, patch)
2018-11-27 16:43 PST, Justin Fan 		no flags
DetailsFormatted DiffDiff
Patch for landing (73.31 KB, patch)
2018-11-28 11:25 PST, Justin Fan 		no flags
DetailsFormatted DiffDiff
Patch for landing (73.31 KB, patch)
2018-11-28 11:36 PST, Justin Fan 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Justin Fan
Comment 1 2018-11-27 12:33:05 PST
Created attachment 355763 [details] Patch
Justin Fan
Comment 2 2018-11-27 16:43:39 PST
Created attachment 355819 [details] Patch
Radar WebKit Bug Importer
Comment 3 2018-11-27 17:15:55 PST
<rdar://problem/46295050>
Dean Jackson
Comment 4 2018-11-28 10:34:09 PST
Comment on attachment 355819 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=355819&action=review > Source/WebCore/Modules/webgpu/WebGPUCommandBuffer.cpp:64 > + if (!encoder) > + return nullptr; We'll have to check in the specification, but I think we're required to always return an object. It would just be a no-op to use it. This can come in a later patch. > Source/WebCore/Modules/webgpu/WebGPUProgrammablePassEncoder.h:42 > +class WebGPUProgrammablePassEncoder : public RefCounted<WebGPUProgrammablePassEncoder> { > +public: > + virtual ~WebGPUProgrammablePassEncoder() = default; > +protected: > + virtual GPUProgrammablePassEncoder& passEncoder() const = 0; > + > +}; Put some blank lines in between the end of public and the "private:" keyword. Then get rid of the final empty line. > Source/WebCore/Modules/webgpu/WebGPURenderPassDescriptor.h:35 > + // FIXME: Temporary shortcut implementation for demo. demo -> prototyping > Source/WebCore/Modules/webgpu/WebGPURenderPassDescriptor.idl:31 > + // FIXME: Temporary shortcut for demo purposes. Same. > Source/WebCore/WebCore.xcodeproj/xcshareddata/xcschemes/WebCoreOnly.xcscheme:4 > +<?xml version="1.0" encoding="UTF-8"?> > +<Scheme > + LastUpgradeVersion = "1100" > + version = "1.3"> You should not commit this file. It's something you added. > Source/WebCore/platform/graphics/gpu/GPUCommandBuffer.h:47 > + PlatformCommandBuffer *platformCommandBuffer() const { return m_platformCommandBuffer.get(); } Even though this is an ObjC value for us, I think the * should be attached to the PlatformCommandBuffer return type. > Source/WebCore/platform/graphics/gpu/GPUProgrammablePassEncoder.h:42 > + virtual PlatformProgrammablePassEncoder *platformPassEncoder() const = 0; Same here.
Dean Jackson
Comment 5 2018-11-28 10:42:31 PST
Comment on attachment 355819 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=355819&action=review r+ but don't commit the new scheme. > Source/WebCore/platform/graphics/gpu/cocoa/GPURenderPassEncoderMetal.mm:73 > + // The command encoder must not be expecting more commands when it is destroyed. I think we should be a bit more descriptive in this comment. e.g. // The MTLRenderCommandEncoder object must have finished encoding before it can be released, whether or not any commands were encoded. Then also add // FIXME: Remember if we've ended encoding and only call this if we haven't. > LayoutTests/webgpu/render-passes.html:2 > +<!DOCTYPE html> > +<html> I was trying to start writing tests that used the WPT approach with the idea of contributing it back to the WebGPU group. This test is fine for now, but I think we should keep this goal in mind.
Justin Fan
Comment 6 2018-11-28 11:25:43 PST
Created attachment 355896 [details] Patch for landing
WebKit Commit Bot
Comment 7 2018-11-28 11:27:12 PST
Comment on attachment 355896 [details] Patch for landing Rejecting attachment 355896 [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', 'validate-changelog', '--check-oops', '--non-interactive', 355896, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit ChangeLog entry in LayoutTests/ChangeLog contains OOPS!. Full output: https://webkit-queues.webkit.org/results/10185179
Justin Fan
Comment 8 2018-11-28 11:36:36 PST
Created attachment 355898 [details] Patch for landing
WebKit Commit Bot
Comment 9 2018-11-28 12:24:39 PST
The commit-queue encountered the following flaky tests while processing attachment 355898 [details]: webgl/1.0.2/conformance/more/functions/drawElementsBadArgs.html bug 192095 (author: roger_fong@apple.com) The commit-queue is continuing to process your patch.
WebKit Commit Bot
Comment 10 2018-11-28 12:24:44 PST
The commit-queue encountered the following flaky tests while processing attachment 355898 [details]: webgl/2.0.0/conformance/glsl/functions/glsl-function-sin.html bug 192096 (author: justin_fan@apple.com) The commit-queue is continuing to process your patch.
WebKit Commit Bot
Comment 11 2018-11-28 12:52:08 PST
The commit-queue encountered the following flaky tests while processing attachment 355898 [details]: inspector/unit-tests/globals-unhandled-rejection-in-test-suite.html bug 192098 (author: bburg@apple.com) The commit-queue is continuing to process your patch.
WebKit Commit Bot
Comment 12 2018-11-28 12:53:24 PST
Comment on attachment 355898 [details] Patch for landing Clearing flags on attachment: 355898 Committed r238629: <https://trac.webkit.org/changeset/238629>
WebKit Commit Bot
Comment 13 2018-11-28 12:53:26 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.