Summary: | [Web GPU] Enable drawing GPUTextures and GPUTextureViews in the render pipeline, and related GPUBindGroup updates | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Justin Fan <justin_fan> | ||||||
Component: | New Bugs | Assignee: | Justin Fan <justin_fan> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | commit-queue, dino, ews-watchlist, webkit-bug-importer | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | WebKit Nightly Build | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Justin Fan
2019-03-05 17:22:55 PST
Created attachment 363717 [details]
Patch
Comment on attachment 363717 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=363717&action=review > LayoutTests/webgpu/texture-triangle-strip.html:45 > + constexpr sampler s(coord::normalized, address::clamp_to_zero, filter::nearest); This is temporary until GPUSamplers are added in the next big patch. Attachment 363717 [details] did not pass style-queue:
ERROR: Source/WebCore/platform/graphics/gpu/GPUProgrammablePassEncoder.h:64: The parameter name "const" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/WebCore/platform/graphics/gpu/GPUProgrammablePassEncoder.h:65: The parameter name "const" adds no information, so it should be removed. [readability/parameter_name] [5]
Total errors found: 2 in 56 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 363717 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=363717&action=review > Source/WebCore/Modules/webgpu/WebGPUTexture.cpp:45 > +WebGPUTexture::~WebGPUTexture() I'm no longer calling destroy() in the destructor here as it would expose when garbage collection happens. Comment on attachment 363717 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=363717&action=review > Source/WebCore/ChangeLog:8 > + Reviewed by NOBODY (OOPS!). > + > + Test: webgpu/texture-triangle-strip.html It would be nice if you could provide a description of what this patch actually does. > Source/WebCore/Modules/webgpu/GPUBindGroupLayoutBinding.h:31 > +#include <wtf/OptionSet.h> I don't think you need this. > Source/WebCore/Modules/webgpu/GPUBindGroupLayoutBinding.h:46 > + GPUShaderStageFlags visibility; Shouldn't this be GPUShaderStageBit::Flags? > Source/WebCore/Modules/webgpu/WebGPURenderPassDescriptor.cpp:61 > + if (!colorAttachment.attachment > + || !colorAttachment.attachment->texture() > + || !colorAttachment.attachment->texture()->isOutputAttachment()) { This could all go on one line. > Source/WebCore/Modules/webgpu/WebGPUTexture.cpp:68 > + if (!m_texture) > + LOG(WebGPU, "GPUTexture::destroy(): Invalid operation!"); > + else > + destroyImpl(); Not sure this will compile in Release configuration. > Source/WebCore/platform/graphics/gpu/cocoa/GPUCommandBufferMetal.mm:80 > + endBlitEncoding(); I would prefer to put the conditional in here, and an ASSERT in the endBlitEncoding. if (m_blitEncoder) endBlitEncoding(); > Source/WebCore/platform/graphics/gpu/cocoa/GPUCommandBufferMetal.mm:91 > + if (!m_blitEncoder) > + return; And here would just be ASSERT(m_blitEncoder), unless you call it from other places. Comment on attachment 363717 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=363717&action=review >> Source/WebCore/Modules/webgpu/GPUBindGroupLayoutBinding.h:46 >> + GPUShaderStageFlags visibility; > > Shouldn't this be GPUShaderStageBit::Flags? The IDL calls for an unsigned int and bindings doesn't know how to convert, even though it's not a strongly-typed enum. >> Source/WebCore/platform/graphics/gpu/cocoa/GPUCommandBufferMetal.mm:91 >> + return; > > And here would just be ASSERT(m_blitEncoder), unless you call it from other places. Correct, this function is also called by GPUQueue::submit(), which doesn't need to know whether the command buffer actually has an active blit encoder or not. Created attachment 363800 [details]
Patch for landing
Comment on attachment 363800 [details] Patch for landing Clearing flags on attachment: 363800 Committed r242575: <https://trac.webkit.org/changeset/242575> All reviewed patches have been landed. Closing bug. |