Skip to content

Commit

Permalink
[WebGPU] validation,render_pipeline,depth_stencil_state:* is failing
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=266668
<radar://119898641>

Reviewed by Tadeu Zagallo.

Add validation for the RenderPipeline. CTS test validation/render_pipeline/depth_stencil_state is
now passing after this change.

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

* LayoutTests/http/tests/webgpu/webgpu/api/validation/render_pipeline/depth_stencil_state.spec.js:
Update to latest version of this test.

* Source/WebCore/Modules/WebGPU/GPUDepthStencilState.h:
(WebCore::GPUDepthStencilState::convertToBacking const):

* Source/WebCore/Modules/WebGPU/GPUDepthStencilState.idl:
depthCompare and depthWriteEnabled are now optional.

* Source/WebCore/Modules/WebGPU/GPUInternalError.h:
(WebCore::GPUInternalError::stack const):
* Source/WebCore/Modules/WebGPU/GPUInternalError.idl:
* Source/WebCore/Modules/WebGPU/GPUPipelineError.h:
* Source/WebCore/Modules/WebGPU/GPUPipelineError.idl:
* Source/WebCore/Modules/WebGPU/GPUValidationError.h:
(WebCore::GPUValidationError::stack const):
* Source/WebCore/Modules/WebGPU/GPUValidationError.idl:
Workaround lack of stack property until gpuweb/cts#3222
is addressed as it causes too many false failures in the live version
of the CTS.

* Source/WebCore/Modules/WebGPU/Implementation/WebGPUDeviceImpl.cpp:
(WebCore::WebGPU::convertToBacking):
* Source/WebCore/Modules/WebGPU/InternalAPI/WebGPUDepthStencilState.h:
* Source/WebGPU/WGSL/Parser.cpp:
(WGSL::Parser<Lexer>::parseAttribute):
* Source/WebGPU/WGSL/WGSLShaderModule.h:
(WGSL::ShaderModule::usesFragDepth const):
(WGSL::ShaderModule::setUsesFragDepth):
* Source/WebGPU/WebGPU/RenderPipeline.mm:
(WebGPU::returnInvalidRenderPipeline):
(WebGPU::name):
(WebGPU::errorValidatingDepthStencilState):
(WebGPU::Device::createRenderPipeline):
* Source/WebGPU/WebGPU/Texture.h:
* Source/WebGPU/WebGPU/Texture.mm:
(WebGPU::Texture::isRenderableFormat):
(WebGPU::Texture::supportsMultisampling):
(WebGPU::Texture::supportsBlending):
(WebGPU::Device::errorValidatingTextureCreation):
(WebGPU::isRenderableFormat): Deleted.
(WebGPU::supportsMultisampling): Deleted.
* Source/WebGPU/WebGPU/WebGPU.h:
* Source/WebKit/Shared/WebGPU/WebGPUDepthStencilState.h:
* Source/WebKit/Shared/WebGPU/WebGPUDepthStencilState.serialization.in:
Implement validation in render pipeline creation.

Canonical link: https://commits.webkit.org/272562@main
  • Loading branch information
mwyrzykowski committed Jan 2, 2024
1 parent cab109e commit b797eac
Show file tree
Hide file tree
Showing 20 changed files with 1,731 additions and 275 deletions.

Large diffs are not rendered by default.

Large diffs are not rendered by default.

9 changes: 6 additions & 3 deletions Source/WebCore/Modules/WebGPU/GPUDepthStencilState.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,13 @@ namespace WebCore {
struct GPUDepthStencilState {
WebGPU::DepthStencilState convertToBacking() const
{
std::optional<WebGPU::CompareFunction> optionalDepthCompare;
if (depthCompare)
optionalDepthCompare = WebCore::convertToBacking(*depthCompare);
return {
.format = WebCore::convertToBacking(format),
.depthWriteEnabled = depthWriteEnabled,
.depthCompare = WebCore::convertToBacking(depthCompare),
.depthCompare = optionalDepthCompare,
.stencilFront = stencilFront.convertToBacking(),
.stencilBack = stencilBack.convertToBacking(),
.stencilReadMask = stencilReadMask ? std::optional { *stencilReadMask } : std::nullopt,
Expand All @@ -53,8 +56,8 @@ struct GPUDepthStencilState {

GPUTextureFormat format { GPUTextureFormat::R8unorm };

bool depthWriteEnabled { false };
GPUCompareFunction depthCompare { GPUCompareFunction::Always };
std::optional<bool> depthWriteEnabled;
std::optional<GPUCompareFunction> depthCompare;

GPUStencilFaceState stencilFront;
GPUStencilFaceState stencilBack;
Expand Down
4 changes: 2 additions & 2 deletions Source/WebCore/Modules/WebGPU/GPUDepthStencilState.idl
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@ typedef [EnforceRange] long GPUDepthBias;
dictionary GPUDepthStencilState {
required GPUTextureFormat format;

required boolean depthWriteEnabled;
required GPUCompareFunction depthCompare;
boolean depthWriteEnabled;
GPUCompareFunction depthCompare;

GPUStencilFaceState stencilFront;
GPUStencilFaceState stencilBack;
Expand Down
1 change: 1 addition & 0 deletions Source/WebCore/Modules/WebGPU/GPUInternalError.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ class GPUInternalError : public RefCounted<GPUInternalError> {

WebGPU::InternalError* backing() { return m_backing.get(); }
const WebGPU::InternalError* backing() const { return m_backing.get(); }
String stack() const { return "_"_s; }

private:
GPUInternalError(String&& message)
Expand Down
1 change: 1 addition & 0 deletions Source/WebCore/Modules/WebGPU/GPUInternalError.idl
Original file line number Diff line number Diff line change
Expand Up @@ -31,4 +31,5 @@
interface GPUInternalError {
constructor(DOMString message);
readonly attribute DOMString message;
readonly attribute DOMString stack;
};
1 change: 1 addition & 0 deletions Source/WebCore/Modules/WebGPU/GPUPipelineError.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ class GPUPipelineError final : public DOMException {
}

GPUPipelineErrorReason reason() const { return m_reason.reason; }
String stack() const { return "_"_s; }

private:
GPUPipelineError(String&& message, GPUPipelineErrorInit);
Expand Down
1 change: 1 addition & 0 deletions Source/WebCore/Modules/WebGPU/GPUPipelineError.idl
Original file line number Diff line number Diff line change
Expand Up @@ -34,4 +34,5 @@
interface GPUPipelineError : DOMException {
constructor(DOMString message, GPUPipelineErrorInit options);
readonly attribute GPUPipelineErrorReason reason;
readonly attribute DOMString stack;
};
1 change: 1 addition & 0 deletions Source/WebCore/Modules/WebGPU/GPUValidationError.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ class GPUValidationError : public RefCounted<GPUValidationError> {

WebGPU::ValidationError* backing() { return m_backing.get(); }
const WebGPU::ValidationError* backing() const { return m_backing.get(); }
String stack() const { return "_"_s; }

private:
GPUValidationError(String&& message)
Expand Down
1 change: 1 addition & 0 deletions Source/WebCore/Modules/WebGPU/GPUValidationError.idl
Original file line number Diff line number Diff line change
Expand Up @@ -31,4 +31,5 @@
interface GPUValidationError {
constructor(DOMString message);
readonly attribute DOMString message;
readonly attribute DOMString stack;
};
Original file line number Diff line number Diff line change
Expand Up @@ -402,18 +402,18 @@ static auto convertToBacking(const RenderPipelineDescriptor& descriptor, bool de
.nextInChain = nullptr,
.format = descriptor.depthStencil ? convertToBackingContext.convertToBacking(descriptor.depthStencil->format) : WGPUTextureFormat_Undefined,
.depthWriteEnabled = descriptor.depthStencil ? descriptor.depthStencil->depthWriteEnabled : false,
.depthCompare = descriptor.depthStencil ? convertToBackingContext.convertToBacking(descriptor.depthStencil->depthCompare) : WGPUCompareFunction_Undefined,
.depthCompare = (descriptor.depthStencil && descriptor.depthStencil->depthCompare) ? convertToBackingContext.convertToBacking(*descriptor.depthStencil->depthCompare) : WGPUCompareFunction_Undefined,
.stencilFront = {
descriptor.depthStencil ? convertToBackingContext.convertToBacking(descriptor.depthStencil->stencilFront.compare) : WGPUCompareFunction_Undefined,
descriptor.depthStencil ? convertToBackingContext.convertToBacking(descriptor.depthStencil->stencilFront.failOp) : WGPUStencilOperation_Keep,
descriptor.depthStencil ? convertToBackingContext.convertToBacking(descriptor.depthStencil->stencilFront.depthFailOp) : WGPUStencilOperation_Keep,
descriptor.depthStencil ? convertToBackingContext.convertToBacking(descriptor.depthStencil->stencilFront.passOp) : WGPUStencilOperation_Keep,
.compare = descriptor.depthStencil ? convertToBackingContext.convertToBacking(descriptor.depthStencil->stencilFront.compare) : WGPUCompareFunction_Undefined,
.failOp = descriptor.depthStencil ? convertToBackingContext.convertToBacking(descriptor.depthStencil->stencilFront.failOp) : WGPUStencilOperation_Keep,
.depthFailOp = descriptor.depthStencil ? convertToBackingContext.convertToBacking(descriptor.depthStencil->stencilFront.depthFailOp) : WGPUStencilOperation_Keep,
.passOp = descriptor.depthStencil ? convertToBackingContext.convertToBacking(descriptor.depthStencil->stencilFront.passOp) : WGPUStencilOperation_Keep,
},
.stencilBack = {
descriptor.depthStencil ? convertToBackingContext.convertToBacking(descriptor.depthStencil->stencilBack.compare) : WGPUCompareFunction_Undefined,
descriptor.depthStencil ? convertToBackingContext.convertToBacking(descriptor.depthStencil->stencilBack.failOp) : WGPUStencilOperation_Keep,
descriptor.depthStencil ? convertToBackingContext.convertToBacking(descriptor.depthStencil->stencilBack.depthFailOp) : WGPUStencilOperation_Keep,
descriptor.depthStencil ? convertToBackingContext.convertToBacking(descriptor.depthStencil->stencilBack.passOp) : WGPUStencilOperation_Keep,
.compare = descriptor.depthStencil ? convertToBackingContext.convertToBacking(descriptor.depthStencil->stencilBack.compare) : WGPUCompareFunction_Undefined,
.failOp = descriptor.depthStencil ? convertToBackingContext.convertToBacking(descriptor.depthStencil->stencilBack.failOp) : WGPUStencilOperation_Keep,
.depthFailOp = descriptor.depthStencil ? convertToBackingContext.convertToBacking(descriptor.depthStencil->stencilBack.depthFailOp) : WGPUStencilOperation_Keep,
.passOp = descriptor.depthStencil ? convertToBackingContext.convertToBacking(descriptor.depthStencil->stencilBack.passOp) : WGPUStencilOperation_Keep,
},
.stencilReadMask = descriptor.depthStencil && descriptor.depthStencil->stencilReadMask ? *descriptor.depthStencil->stencilReadMask : 0,
.stencilWriteMask = descriptor.depthStencil && descriptor.depthStencil->stencilWriteMask ? *descriptor.depthStencil->stencilWriteMask : 0,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ namespace WebCore::WebGPU {
struct DepthStencilState {
TextureFormat format { TextureFormat::R8unorm };

bool depthWriteEnabled { false };
CompareFunction depthCompare { CompareFunction::Always };
std::optional<bool> depthWriteEnabled;
std::optional<CompareFunction> depthCompare;

StencilFaceState stencilFront;
StencilFaceState stencilBack;
Expand Down
2 changes: 2 additions & 0 deletions Source/WebGPU/WGSL/Parser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -640,6 +640,8 @@ Result<AST::Attribute::Ref> Parser<Lexer>::parseAttribute()
auto* builtin = parseBuiltin(name);
if (!builtin)
FAIL("Unknown builtin value. Expected 'vertex_index', 'instance_index', 'position', 'front_facing', 'frag_depth', 'sample_index', 'sample_mask', 'local_invocation_id', 'local_invocation_index', 'global_invocation_id', 'workgroup_id' or 'num_workgroups'"_s);
if (*builtin == Builtin::FragDepth)
m_shaderModule.setUsesFragDepth();
CONSUME_TYPE(ParenRight);
RETURN_ARENA_NODE(BuiltinAttribute, *builtin);
}
Expand Down
3 changes: 3 additions & 0 deletions Source/WebGPU/WGSL/WGSLShaderModule.h
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,8 @@ class ShaderModule {

bool usesAtomicCompareExchange() const { return m_usesAtomicCompareExchange; }
void setUsesAtomicCompareExchange() { m_usesAtomicCompareExchange = true; }
bool usesFragDepth() const { return m_usesFragDepth; }
void setUsesFragDepth() { m_usesFragDepth = true; }

template<typename T>
std::enable_if_t<std::is_base_of_v<AST::Node, T>, void> replace(T* current, T&& replacement)
Expand Down Expand Up @@ -249,6 +251,7 @@ class ShaderModule {
bool m_usesFrexp { false };
bool m_usesModf { false };
bool m_usesAtomicCompareExchange { false };
bool m_usesFragDepth { false };
OptionSet<Extension> m_enabledExtensions;
OptionSet<LanguageFeature> m_requiredFeatures;
Configuration m_configuration;
Expand Down
Loading

0 comments on commit b797eac

Please sign in to comment.