Skip to content

Commit

Permalink
[WebGPU] setDepthBias: is never called, depth bias and related functi…
Browse files Browse the repository at this point in the history
…ons are not implemented

https://bugs.webkit.org/show_bug.cgi?id=263906
<rdar://problem/117696050>

Reviewed by Dan Glastonbury.

Call setDepthBias for regular passes and ICBs and update the test expectations.

* LayoutTests/http/tests/webgpu/webgpu/api/operation/rendering/depth_bias-expected.txt:
* Source/WebCore/Modules/WebGPU/GPUDepthStencilState.h:
(WebCore::GPUDepthStencilState::convertToBacking const):
* Source/WebCore/Modules/WebGPU/Implementation/WebGPUDeviceImpl.cpp:
(WebCore::WebGPU::convertToBacking):
* Source/WebGPU/WebGPU/RenderBundleEncoder.h:
* Source/WebGPU/WebGPU/RenderBundleEncoder.mm:
(-[RenderBundleICBWithResources initWithICB:pipelineState:depthStencilState:cullMode:frontFace:depthClipMode:depthBias:depthBiasSlopeScale:depthBiasClamp:]):
(WebGPU::makeRenderBundleICBWithResources):
(WebGPU::RenderBundleEncoder::endCurrentICB):
(WebGPU::icbNeedsToBeSplit):
(WebGPU::RenderBundleEncoder::setPipeline):
(-[RenderBundleICBWithResources initWithICB:pipelineState:depthStencilState:cullMode:frontFace:depthClipMode:]): Deleted.
* Source/WebGPU/WebGPU/RenderPassEncoder.mm:
(WebGPU::RenderPassEncoder::executeBundles):
(WebGPU::RenderPassEncoder::setPipeline):
* Source/WebGPU/WebGPU/RenderPipeline.h:
(WebGPU::RenderPipeline::create):
(WebGPU::RenderPipeline::depthBias const):
(WebGPU::RenderPipeline::depthBiasSlopeScale const):
(WebGPU::RenderPipeline::depthBiasClamp const):
* Source/WebGPU/WebGPU/RenderPipeline.mm:
(WebGPU::Device::createRenderPipeline):
(WebGPU::RenderPipeline::RenderPipeline):

Canonical link: https://commits.webkit.org/269995@main
  • Loading branch information
mwyrzykowski committed Oct 31, 2023
1 parent 02a3627 commit 7111a43
Show file tree
Hide file tree
Showing 8 changed files with 89 additions and 32 deletions.
Original file line number Diff line number Diff line change
@@ -1 +1,15 @@
(Populate me when we're ready to investigate this test)

PASS :depth_bias:quadAngle=0;bias=8388608;biasSlopeScale=0;biasClamp=0
PASS :depth_bias:quadAngle=0;bias=8388608;biasSlopeScale=0;biasClamp=0.125
PASS :depth_bias:quadAngle=0;bias=-8388608;biasSlopeScale=0;biasClamp=0.125
PASS :depth_bias:quadAngle=0;bias=-8388608;biasSlopeScale=0;biasClamp=-0.125
PASS :depth_bias:quadAngle=1;bias=0;biasSlopeScale=0;biasClamp=0
PASS :depth_bias:quadAngle=1;bias=0;biasSlopeScale=1;biasClamp=0
PASS :depth_bias:quadAngle=1;bias=0;biasSlopeScale=-0.5;biasClamp=0
PASS :depth_bias_24bit_format:format="depth24plus";quadAngle=0;bias=8388608;biasSlopeScale=0;biasClamp=0
PASS :depth_bias_24bit_format:format="depth24plus";quadAngle=0;bias=8388608;biasSlopeScale=0;biasClamp=0.1
PASS :depth_bias_24bit_format:format="depth24plus";quadAngle=1;bias=8388608;biasSlopeScale=1;biasClamp=0
PASS :depth_bias_24bit_format:format="depth24plus-stencil8";quadAngle=0;bias=8388608;biasSlopeScale=0;biasClamp=0
PASS :depth_bias_24bit_format:format="depth24plus-stencil8";quadAngle=0;bias=8388608;biasSlopeScale=0;biasClamp=0.1
PASS :depth_bias_24bit_format:format="depth24plus-stencil8";quadAngle=1;bias=8388608;biasSlopeScale=1;biasClamp=0

20 changes: 10 additions & 10 deletions Source/WebCore/Modules/WebGPU/GPUDepthStencilState.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,16 +38,16 @@ struct GPUDepthStencilState {
WebGPU::DepthStencilState convertToBacking() const
{
return {
WebCore::convertToBacking(format),
depthWriteEnabled,
WebCore::convertToBacking(depthCompare),
stencilFront.convertToBacking(),
stencilBack.convertToBacking(),
stencilReadMask ? std::optional { *stencilReadMask } : std::nullopt,
stencilWriteMask ? std::optional { *stencilWriteMask } : std::nullopt,
depthBias,
depthBiasSlopeScale,
depthBiasClamp,
.format = WebCore::convertToBacking(format),
.depthWriteEnabled = depthWriteEnabled,
.depthCompare = WebCore::convertToBacking(depthCompare),
.stencilFront = stencilFront.convertToBacking(),
.stencilBack = stencilBack.convertToBacking(),
.stencilReadMask = stencilReadMask ? std::optional { *stencilReadMask } : std::nullopt,
.stencilWriteMask = stencilWriteMask ? std::optional { *stencilWriteMask } : std::nullopt,
.depthBias = depthBias,
.depthBiasSlopeScale = depthBiasSlopeScale,
.depthBiasClamp = depthBiasClamp,
};
}

Expand Down
22 changes: 12 additions & 10 deletions Source/WebCore/Modules/WebGPU/Implementation/WebGPUDeviceImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -394,25 +394,27 @@ static auto convertToBacking(const RenderPipelineDescriptor& descriptor, bool de
});

WGPUDepthStencilState depthStencilState {
nullptr,
descriptor.depthStencil ? convertToBackingContext.convertToBacking(descriptor.depthStencil->format) : WGPUTextureFormat_Undefined,
descriptor.depthStencil ? descriptor.depthStencil->depthWriteEnabled : false,
descriptor.depthStencil ? convertToBackingContext.convertToBacking(descriptor.depthStencil->depthCompare) : WGPUCompareFunction_Undefined, {
.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,
.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,
}, {
},
.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,
},
descriptor.depthStencil && descriptor.depthStencil->stencilReadMask ? *descriptor.depthStencil->stencilReadMask : 0,
descriptor.depthStencil && descriptor.depthStencil->stencilWriteMask ? *descriptor.depthStencil->stencilWriteMask : 0,
descriptor.depthStencil ? descriptor.depthStencil->depthBias : 0,
descriptor.depthStencil ? descriptor.depthStencil->depthBiasSlopeScale : 0,
descriptor.depthStencil ? descriptor.depthStencil->depthBiasClamp : 0,
.stencilReadMask = descriptor.depthStencil && descriptor.depthStencil->stencilReadMask ? *descriptor.depthStencil->stencilReadMask : 0,
.stencilWriteMask = descriptor.depthStencil && descriptor.depthStencil->stencilWriteMask ? *descriptor.depthStencil->stencilWriteMask : 0,
.depthBias = descriptor.depthStencil ? descriptor.depthStencil->depthBias : 0,
.depthBiasSlopeScale = descriptor.depthStencil ? descriptor.depthStencil->depthBiasSlopeScale : 0,
.depthBiasClamp = descriptor.depthStencil ? descriptor.depthStencil->depthBiasClamp : 0,
};

auto fragmentEntryPoint = descriptor.fragment ? descriptor.fragment->entryPoint.utf8() : CString("");
Expand Down
8 changes: 7 additions & 1 deletion Source/WebGPU/WebGPU/RenderBundleEncoder.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ struct WGPURenderBundleEncoderImpl {

@interface RenderBundleICBWithResources : NSObject

- (instancetype)initWithICB:(id<MTLIndirectCommandBuffer>)icb pipelineState:(id<MTLRenderPipelineState>)pipelineState depthStencilState:(id<MTLDepthStencilState>)depthStencilState cullMode:(MTLCullMode)cullMode frontFace:(MTLWinding)frontFace depthClipMode:(MTLDepthClipMode)depthClipMode NS_DESIGNATED_INITIALIZER;
- (instancetype)initWithICB:(id<MTLIndirectCommandBuffer>)icb pipelineState:(id<MTLRenderPipelineState>)pipelineState depthStencilState:(id<MTLDepthStencilState>)depthStencilState cullMode:(MTLCullMode)cullMode frontFace:(MTLWinding)frontFace depthClipMode:(MTLDepthClipMode)depthClipMode depthBias:(float)depthBias depthBiasSlopeScale:(float)depthBiasSlopeScale depthBiasClamp:(float)depthBiasClamp NS_DESIGNATED_INITIALIZER;
- (instancetype)init NS_UNAVAILABLE;

@property (readonly, nonatomic) id<MTLIndirectCommandBuffer> indirectCommandBuffer;
Expand All @@ -48,6 +48,9 @@ struct WGPURenderBundleEncoderImpl {
@property (readonly, nonatomic) MTLCullMode cullMode;
@property (readonly, nonatomic) MTLWinding frontFace;
@property (readonly, nonatomic) MTLDepthClipMode depthClipMode;
@property (readonly, nonatomic) float depthBias;
@property (readonly, nonatomic) float depthBiasSlopeScale;
@property (readonly, nonatomic) float depthBiasClamp;

- (Vector<WebGPU::BindableResources>*)resources;
@end
Expand Down Expand Up @@ -118,6 +121,9 @@ class RenderBundleEncoder : public WGPURenderBundleEncoderImpl, public RefCounte
MTLCullMode m_cullMode { MTLCullModeNone };
MTLWinding m_frontFace { MTLWindingClockwise };
MTLDepthClipMode m_depthClipMode { MTLDepthClipModeClip };
float m_depthBias { 0 };
float m_depthBiasSlopeScale { 0 };
float m_depthBiasClamp { 0 };

MTLPrimitiveType m_primitiveType { MTLPrimitiveTypeTriangle };
MTLIndexType m_indexType { MTLIndexTypeUInt16 };
Expand Down
28 changes: 24 additions & 4 deletions Source/WebGPU/WebGPU/RenderBundleEncoder.mm
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ @implementation RenderBundleICBWithResources {
Vector<WebGPU::BindableResources> _resources;
}

- (instancetype)initWithICB:(id<MTLIndirectCommandBuffer>)icb pipelineState:(id<MTLRenderPipelineState>)pipelineState depthStencilState:(id<MTLDepthStencilState>)depthStencilState cullMode:(MTLCullMode)cullMode frontFace:(MTLWinding)frontFace depthClipMode:(MTLDepthClipMode)depthClipMode
- (instancetype)initWithICB:(id<MTLIndirectCommandBuffer>)icb pipelineState:(id<MTLRenderPipelineState>)pipelineState depthStencilState:(id<MTLDepthStencilState>)depthStencilState cullMode:(MTLCullMode)cullMode frontFace:(MTLWinding)frontFace depthClipMode:(MTLDepthClipMode)depthClipMode depthBias:(float)depthBias depthBiasSlopeScale:(float)depthBiasSlopeScale depthBiasClamp:(float)depthBiasClamp
{
if (!(self = [super init]))
return nil;
Expand All @@ -50,6 +50,9 @@ - (instancetype)initWithICB:(id<MTLIndirectCommandBuffer>)icb pipelineState:(id<
_cullMode = cullMode;
_frontFace = frontFace;
_depthClipMode = depthClipMode;
_depthBias = depthBias;
_depthBiasSlopeScale = depthBiasSlopeScale;
_depthBiasClamp = depthBiasClamp;

return self;
}
Expand All @@ -63,7 +66,7 @@ - (instancetype)initWithICB:(id<MTLIndirectCommandBuffer>)icb pipelineState:(id<

namespace WebGPU {

static RenderBundleICBWithResources* makeRenderBundleICBWithResources(id<MTLIndirectCommandBuffer> icb, RenderBundle::ResourcesContainer* resources, id<MTLRenderPipelineState> renderPipelineState, id<MTLDepthStencilState> depthStencilState, MTLCullMode cullMode, MTLWinding frontFace, MTLDepthClipMode depthClipMode)
static RenderBundleICBWithResources* makeRenderBundleICBWithResources(id<MTLIndirectCommandBuffer> icb, RenderBundle::ResourcesContainer* resources, id<MTLRenderPipelineState> renderPipelineState, id<MTLDepthStencilState> depthStencilState, MTLCullMode cullMode, MTLWinding frontFace, MTLDepthClipMode depthClipMode, float depthBias, float depthBiasSlopeScale, float depthBiasClamp)
{
constexpr auto maxResourceUsageValue = MTLResourceUsageRead | MTLResourceUsageWrite;
constexpr auto maxStageValue = MTLRenderStageVertex | MTLRenderStageFragment;
Expand All @@ -75,7 +78,7 @@ - (instancetype)initWithICB:(id<MTLIndirectCommandBuffer>)icb pipelineState:(id<
stageResources[usageAndStage.renderStages - 1][usageAndStage.renderStages - 1].append(r);
}

RenderBundleICBWithResources* renderBundle = [[RenderBundleICBWithResources alloc] initWithICB:icb pipelineState:renderPipelineState depthStencilState:depthStencilState cullMode:cullMode frontFace:frontFace depthClipMode:depthClipMode];
RenderBundleICBWithResources* renderBundle = [[RenderBundleICBWithResources alloc] initWithICB:icb pipelineState:renderPipelineState depthStencilState:depthStencilState cullMode:cullMode frontFace:frontFace depthClipMode:depthClipMode depthBias:depthBias depthBiasSlopeScale:depthBiasSlopeScale depthBiasClamp:depthBiasClamp];

for (size_t stage = 0; stage < maxStageValue; ++stage) {
for (size_t i = 0; i < maxResourceUsageValue; ++i) {
Expand Down Expand Up @@ -349,7 +352,7 @@ - (instancetype)initWithICB:(id<MTLIndirectCommandBuffer>)icb pipelineState:(id<
else
m_recordedCommands.remove(0, lastIndexOfRecordedCommand);

[m_icbArray addObject:makeRenderBundleICBWithResources(m_indirectCommandBuffer, m_resources, m_currentPipelineState, m_depthStencilState, m_cullMode, m_frontFace, m_depthClipMode)];
[m_icbArray addObject:makeRenderBundleICBWithResources(m_indirectCommandBuffer, m_resources, m_currentPipelineState, m_depthStencilState, m_cullMode, m_frontFace, m_depthClipMode, m_depthBias, m_depthBiasSlopeScale, m_depthBiasClamp)];
m_icbDescriptor.maxVertexBufferBindCount = 0;
m_icbDescriptor.maxFragmentBufferBindCount = 0;
m_indirectCommandBuffer = nil;
Expand Down Expand Up @@ -508,6 +511,15 @@ static bool icbNeedsToBeSplit(const RenderPipeline& a, const RenderPipeline& b)
if (![a.depthStencilDescriptor() isEqual:b.depthStencilDescriptor()])
return true;

if (a.depthBias() != b.depthBias())
return true;

if (a.depthBiasSlopeScale() != b.depthBiasSlopeScale())
return true;

if (a.depthBiasClamp() != b.depthBiasClamp())
return true;

return false;
#endif
}
Expand All @@ -524,13 +536,19 @@ static bool icbNeedsToBeSplit(const RenderPipeline& a, const RenderPipeline& b)
auto previous_cullMode = m_cullMode;
auto previous_frontFace = m_frontFace;
auto previous_depthClipmpde = m_depthClipMode;
auto previous_depthBias = m_depthBias;
auto previous_depthBiasSlopeScale = m_depthBiasSlopeScale;
auto previous_depthBiasClamp = m_depthBiasClamp;

m_currentPipelineState = pipeline.renderPipelineState();
m_depthStencilState = pipeline.depthStencilState();
m_cullMode = pipeline.cullMode();
m_frontFace = pipeline.frontFace();
m_depthClipMode = pipeline.depthClipMode();
m_primitiveType = pipeline.primitiveType();
m_depthBias = pipeline.depthBias();
m_depthBiasSlopeScale = pipeline.depthBiasSlopeScale();
m_depthBiasClamp = pipeline.depthBiasClamp();

if (m_commandEncoder) {
id<MTLRenderPipelineState> renderPipeline = m_currentPipelineState;
Expand All @@ -546,6 +564,8 @@ static bool icbNeedsToBeSplit(const RenderPipeline& a, const RenderPipeline& b)
[m_commandEncoder setFrontFacingWinding:m_frontFace];
if (previous_depthClipmpde != m_depthClipMode)
[m_commandEncoder setDepthClipMode:m_depthClipMode];
if (previous_depthBias != m_depthBias || previous_depthBiasSlopeScale != m_depthBiasSlopeScale || previous_depthBiasClamp != m_depthBiasClamp)
[m_commandEncoder setDepthBias:m_depthBias slopeScale:m_depthBiasSlopeScale clamp:m_depthBiasClamp];
}
} else {
if (m_pipeline && icbNeedsToBeSplit(*m_pipeline, pipeline))
Expand Down
2 changes: 2 additions & 0 deletions Source/WebGPU/WebGPU/RenderPassEncoder.mm
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,7 @@
[m_renderCommandEncoder setCullMode:icb.cullMode];
[m_renderCommandEncoder setFrontFacingWinding:icb.frontFace];
[m_renderCommandEncoder setDepthClipMode:icb.depthClipMode];
[m_renderCommandEncoder setDepthBias:icb.depthBias slopeScale:icb.depthBiasSlopeScale clamp:icb.depthBiasClamp];

for (const auto& resource : *icb.resources) {
if (resource.renderStages & (MTLRenderStageVertex | MTLRenderStageFragment))
Expand Down Expand Up @@ -310,6 +311,7 @@
[m_renderCommandEncoder setCullMode:pipeline.cullMode()];
[m_renderCommandEncoder setFrontFacingWinding:pipeline.frontFace()];
[m_renderCommandEncoder setDepthClipMode:pipeline.depthClipMode()];
[m_renderCommandEncoder setDepthBias:pipeline.depthBias() slopeScale:pipeline.depthBiasSlopeScale() clamp:pipeline.depthBiasClamp()];
}

void RenderPassEncoder::setScissorRect(uint32_t x, uint32_t y, uint32_t width, uint32_t height)
Expand Down
12 changes: 9 additions & 3 deletions Source/WebGPU/WebGPU/RenderPipeline.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,9 @@ class PipelineLayout;
class RenderPipeline : public WGPURenderPipelineImpl, public RefCounted<RenderPipeline> {
WTF_MAKE_FAST_ALLOCATED;
public:
static Ref<RenderPipeline> create(id<MTLRenderPipelineState> renderPipelineState, MTLPrimitiveType primitiveType, std::optional<MTLIndexType> indexType, MTLWinding frontFace, MTLCullMode cullMode, MTLDepthClipMode depthClipMode, MTLDepthStencilDescriptor *depthStencilDescriptor, Ref<PipelineLayout>&& pipelineLayout, Device& device)
static Ref<RenderPipeline> create(id<MTLRenderPipelineState> renderPipelineState, MTLPrimitiveType primitiveType, std::optional<MTLIndexType> indexType, MTLWinding frontFace, MTLCullMode cullMode, MTLDepthClipMode depthClipMode, MTLDepthStencilDescriptor *depthStencilDescriptor, Ref<PipelineLayout>&& pipelineLayout, float depthBias, float depthBiasSlopeScale, float depthBiasClamp, Device& device)
{
return adoptRef(*new RenderPipeline(renderPipelineState, primitiveType, indexType, frontFace, cullMode, depthClipMode, depthStencilDescriptor, WTFMove(pipelineLayout), device));
return adoptRef(*new RenderPipeline(renderPipelineState, primitiveType, indexType, frontFace, cullMode, depthClipMode, depthStencilDescriptor, WTFMove(pipelineLayout), depthBias, depthBiasSlopeScale, depthBiasClamp, device));
}

static Ref<RenderPipeline> createInvalid(Device& device)
Expand All @@ -69,12 +69,15 @@ class RenderPipeline : public WGPURenderPipelineImpl, public RefCounted<RenderPi
MTLCullMode cullMode() const { return m_cullMode; }
MTLDepthClipMode depthClipMode() const { return m_clipMode; }
MTLDepthStencilDescriptor *depthStencilDescriptor() const { return m_depthStencilDescriptor; }
float depthBias() const { return m_depthBias; }
float depthBiasSlopeScale() const { return m_depthBiasSlopeScale; }
float depthBiasClamp() const { return m_depthBiasClamp; }

Device& device() const { return m_device; }
PipelineLayout& pipelineLayout() const;

private:
RenderPipeline(id<MTLRenderPipelineState>, MTLPrimitiveType, std::optional<MTLIndexType>, MTLWinding, MTLCullMode, MTLDepthClipMode, MTLDepthStencilDescriptor *, Ref<PipelineLayout>&&, Device&);
RenderPipeline(id<MTLRenderPipelineState>, MTLPrimitiveType, std::optional<MTLIndexType>, MTLWinding, MTLCullMode, MTLDepthClipMode, MTLDepthStencilDescriptor *, Ref<PipelineLayout>&&, float depthBias, float depthBiasSlopeScale, float depthBiasClamp, Device&);
RenderPipeline(Device&);

const id<MTLRenderPipelineState> m_renderPipelineState { nil };
Expand All @@ -85,6 +88,9 @@ class RenderPipeline : public WGPURenderPipelineImpl, public RefCounted<RenderPi
MTLWinding m_frontFace;
MTLCullMode m_cullMode;
MTLDepthClipMode m_clipMode;
float m_depthBias { 0 };
float m_depthBiasSlopeScale { 0 };
float m_depthBiasClamp { 0 };
MTLDepthStencilDescriptor *m_depthStencilDescriptor;
id<MTLDepthStencilState> m_depthStencilState;
Ref<PipelineLayout> m_pipelineLayout;
Expand Down
Loading

0 comments on commit 7111a43

Please sign in to comment.