Skip to content

Commit

Permalink
[WebGPU] CTS validation/render_pipeline/primitive_state.html is failing
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=266737
<radar://119958339>

Reviewed by Tadeu Zagallo.

Add passing expectations for primitive_state and correct some missing
validation logic.

* LayoutTests/http/tests/webgpu/webgpu/api/validation/render_pipeline/primitive_state-expected.txt:
Add passing expectations.

* Source/WebCore/Modules/WebGPU/Implementation/WebGPUDeviceImpl.cpp:
(WebCore::WebGPU::convertToBacking):
(WebCore::WebGPU::DeviceImpl::createRenderPipeline):
(WebCore::WebGPU::DeviceImpl::createRenderPipelineAsync):
* Source/WebGPU/WebGPU/RenderPipeline.mm:
(WebGPU::Device::createRenderPipeline):

Canonical link: https://commits.webkit.org/272646@main
  • Loading branch information
mwyrzykowski committed Jan 4, 2024
1 parent a507875 commit 2c3ec21
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 9 deletions.
Original file line number Diff line number Diff line change
@@ -1 +1,42 @@
(Populate me when we're ready to investigate this test)

PASS :strip_index_format:isAsync=false;topology="_undef_";stripIndexFormat="_undef_"
PASS :strip_index_format:isAsync=false;topology="_undef_";stripIndexFormat="uint16"
PASS :strip_index_format:isAsync=false;topology="_undef_";stripIndexFormat="uint32"
PASS :strip_index_format:isAsync=false;topology="point-list";stripIndexFormat="_undef_"
PASS :strip_index_format:isAsync=false;topology="point-list";stripIndexFormat="uint16"
PASS :strip_index_format:isAsync=false;topology="point-list";stripIndexFormat="uint32"
PASS :strip_index_format:isAsync=false;topology="line-list";stripIndexFormat="_undef_"
PASS :strip_index_format:isAsync=false;topology="line-list";stripIndexFormat="uint16"
PASS :strip_index_format:isAsync=false;topology="line-list";stripIndexFormat="uint32"
PASS :strip_index_format:isAsync=false;topology="line-strip";stripIndexFormat="_undef_"
PASS :strip_index_format:isAsync=false;topology="line-strip";stripIndexFormat="uint16"
PASS :strip_index_format:isAsync=false;topology="line-strip";stripIndexFormat="uint32"
PASS :strip_index_format:isAsync=false;topology="triangle-list";stripIndexFormat="_undef_"
PASS :strip_index_format:isAsync=false;topology="triangle-list";stripIndexFormat="uint16"
PASS :strip_index_format:isAsync=false;topology="triangle-list";stripIndexFormat="uint32"
PASS :strip_index_format:isAsync=false;topology="triangle-strip";stripIndexFormat="_undef_"
PASS :strip_index_format:isAsync=false;topology="triangle-strip";stripIndexFormat="uint16"
PASS :strip_index_format:isAsync=false;topology="triangle-strip";stripIndexFormat="uint32"
PASS :strip_index_format:isAsync=true;topology="_undef_";stripIndexFormat="_undef_"
PASS :strip_index_format:isAsync=true;topology="_undef_";stripIndexFormat="uint16"
PASS :strip_index_format:isAsync=true;topology="_undef_";stripIndexFormat="uint32"
PASS :strip_index_format:isAsync=true;topology="point-list";stripIndexFormat="_undef_"
PASS :strip_index_format:isAsync=true;topology="point-list";stripIndexFormat="uint16"
PASS :strip_index_format:isAsync=true;topology="point-list";stripIndexFormat="uint32"
PASS :strip_index_format:isAsync=true;topology="line-list";stripIndexFormat="_undef_"
PASS :strip_index_format:isAsync=true;topology="line-list";stripIndexFormat="uint16"
PASS :strip_index_format:isAsync=true;topology="line-list";stripIndexFormat="uint32"
PASS :strip_index_format:isAsync=true;topology="line-strip";stripIndexFormat="_undef_"
PASS :strip_index_format:isAsync=true;topology="line-strip";stripIndexFormat="uint16"
PASS :strip_index_format:isAsync=true;topology="line-strip";stripIndexFormat="uint32"
PASS :strip_index_format:isAsync=true;topology="triangle-list";stripIndexFormat="_undef_"
PASS :strip_index_format:isAsync=true;topology="triangle-list";stripIndexFormat="uint16"
PASS :strip_index_format:isAsync=true;topology="triangle-list";stripIndexFormat="uint32"
PASS :strip_index_format:isAsync=true;topology="triangle-strip";stripIndexFormat="_undef_"
PASS :strip_index_format:isAsync=true;topology="triangle-strip";stripIndexFormat="uint16"
PASS :strip_index_format:isAsync=true;topology="triangle-strip";stripIndexFormat="uint32"
PASS :unclipped_depth:isAsync=false;unclippedDepth=false
PASS :unclipped_depth:isAsync=false;unclippedDepth=true
PASS :unclipped_depth:isAsync=true;unclippedDepth=false
PASS :unclipped_depth:isAsync=true;unclippedDepth=true

Original file line number Diff line number Diff line change
Expand Up @@ -357,7 +357,7 @@ Ref<ComputePipeline> DeviceImpl::createComputePipeline(const ComputePipelineDesc
}

template <typename T>
static auto convertToBacking(const RenderPipelineDescriptor& descriptor, bool depthClipControlIsEnabled, ConvertToBackingContext& convertToBackingContext, T&& callback)
static auto convertToBacking(const RenderPipelineDescriptor& descriptor, ConvertToBackingContext& convertToBackingContext, T&& callback)
{
auto label = descriptor.label.utf8();

Expand Down Expand Up @@ -516,7 +516,7 @@ static auto convertToBacking(const RenderPipelineDescriptor& descriptor, bool de
static_cast<uint32_t>(backingBuffers.size()),
backingBuffers.data(),
}, {
descriptor.primitive && depthClipControlIsEnabled ? &depthClipControl.chain : nullptr,
descriptor.primitive && descriptor.primitive->unclippedDepth ? &depthClipControl.chain : nullptr,
descriptor.primitive ? convertToBackingContext.convertToBacking(descriptor.primitive->topology) : WGPUPrimitiveTopology_TriangleList,
descriptor.primitive && descriptor.primitive->stripIndexFormat ? convertToBackingContext.convertToBacking(*descriptor.primitive->stripIndexFormat) : WGPUIndexFormat_Undefined,
descriptor.primitive ? convertToBackingContext.convertToBacking(descriptor.primitive->frontFace) : WGPUFrontFace_CCW,
Expand All @@ -536,8 +536,7 @@ static auto convertToBacking(const RenderPipelineDescriptor& descriptor, bool de

Ref<RenderPipeline> DeviceImpl::createRenderPipeline(const RenderPipelineDescriptor& descriptor)
{
bool depthClipControlIsEnabled = wgpuDeviceHasFeature(m_backing.get(), WGPUFeatureName_DepthClipControl);
return convertToBacking(descriptor, depthClipControlIsEnabled, m_convertToBackingContext, [backing = m_backing.copyRef(), &convertToBackingContext = m_convertToBackingContext.get()](const WGPURenderPipelineDescriptor& backingDescriptor) {
return convertToBacking(descriptor, m_convertToBackingContext, [backing = m_backing.copyRef(), &convertToBackingContext = m_convertToBackingContext.get()](const WGPURenderPipelineDescriptor& backingDescriptor) {
return RenderPipelineImpl::create(adoptWebGPU(wgpuDeviceCreateRenderPipeline(backing.get(), &backingDescriptor)), convertToBackingContext);
});
}
Expand Down Expand Up @@ -571,8 +570,7 @@ static void createRenderPipelineAsyncCallback(WGPUCreatePipelineAsyncStatus stat

void DeviceImpl::createRenderPipelineAsync(const RenderPipelineDescriptor& descriptor, CompletionHandler<void(RefPtr<RenderPipeline>&&)>&& callback)
{
bool depthClipControlIsEnabled = wgpuDeviceHasFeature(m_backing.get(), WGPUFeatureName_DepthClipControl);
convertToBacking(descriptor, depthClipControlIsEnabled, m_convertToBackingContext, [backing = m_backing.copyRef(), convertToBackingContext = m_convertToBackingContext.copyRef(), callback = WTFMove(callback)](const WGPURenderPipelineDescriptor& backingDescriptor) mutable {
convertToBacking(descriptor, m_convertToBackingContext, [backing = m_backing.copyRef(), convertToBackingContext = m_convertToBackingContext.copyRef(), callback = WTFMove(callback)](const WGPURenderPipelineDescriptor& backingDescriptor) mutable {
auto blockPtr = makeBlockPtr([convertToBackingContext = convertToBackingContext.copyRef(), callback = WTFMove(callback)](WGPUCreatePipelineAsyncStatus status, WGPURenderPipeline pipeline, const char*) mutable {
if (status == WGPUCreatePipelineAsyncStatus_Success)
callback(RenderPipelineImpl::create(adoptWebGPU(pipeline), convertToBackingContext));
Expand Down
10 changes: 8 additions & 2 deletions Source/WebGPU/WebGPU/RenderPipeline.mm
Original file line number Diff line number Diff line change
Expand Up @@ -1060,7 +1060,7 @@ static bool textureFormatAllowedForRetunType(WGPUTextureFormat format, MTLDataTy
MTLDepthClipMode mtlDepthClipMode = MTLDepthClipModeClip;
if (descriptor.primitive.nextInChain) {
if (!hasFeature(WGPUFeatureName_DepthClipControl) || descriptor.primitive.nextInChain->sType != WGPUSType_PrimitiveDepthClipControl || descriptor.primitive.nextInChain->next)
return RenderPipeline::createInvalid(*this);
return returnInvalidRenderPipeline(*this, isAsync, "unclippedDepth used without enabling depth-clip-contro feature"_s);

auto* depthClipControl = reinterpret_cast<const WGPUPrimitiveDepthClipControl*>(descriptor.primitive.nextInChain);

Expand All @@ -1073,7 +1073,13 @@ static bool textureFormatAllowedForRetunType(WGPUTextureFormat format, MTLDataTy
// These properties are to be used by the render command encoder, not the render pipeline.
// Therefore, the render pipeline stores these, and when the render command encoder is assigned
// a pipeline, the render command encoder can get these information out of the render pipeline.
auto mtlPrimitiveType = primitiveType(descriptor.primitive.topology);
auto primitiveTopology = descriptor.primitive.topology;
if (primitiveTopology != WGPUPrimitiveTopology_LineStrip && primitiveTopology != WGPUPrimitiveTopology_TriangleStrip) {
if (descriptor.primitive.stripIndexFormat != WGPUIndexFormat_Undefined)
return returnInvalidRenderPipeline(*this, isAsync, "If primitive.topology is not line-strip or triangle-strip, primitive.stripIndexFormat must be undefined."_s);
}

auto mtlPrimitiveType = primitiveType(primitiveTopology);
auto mtlIndexType = indexType(descriptor.primitive.stripIndexFormat);
auto mtlFrontFace = frontFace(descriptor.primitive.frontFace);
auto mtlCullMode = cullMode(descriptor.primitive.cullMode);
Expand Down

0 comments on commit 2c3ec21

Please sign in to comment.