Commit f6c937f8 by Shahbaz Youssefi Committed by Commit Bot

Vulkan: fix masked stencil clear

Previously, masked stencil clear was done by clearing every stencil bit to the ClearValue & Mask. The correct behavior as implemented in this change is to clear only the bits that are set in Mask. This can only be done through a draw call, with ClearValue as the stencil reference, and Mask as the stencil write mask. Note: this change relies on the depthClamp Vulkan feature which is not available on ARM. Bug: angleproject:3241 Change-Id: I0a181c32f881ee813f144e7bdd6f42c8ea6f1966 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/1548442 Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: 's avatarJamie Madill <jmadill@chromium.org> Reviewed-by: 's avatarTobin Ehlis <tobine@google.com>
parent 194a9674
...@@ -1349,7 +1349,31 @@ angle::Result Framebuffer::clear(const Context *context, GLbitfield mask) ...@@ -1349,7 +1349,31 @@ angle::Result Framebuffer::clear(const Context *context, GLbitfield mask)
return angle::Result::Continue; return angle::Result::Continue;
} }
ANGLE_TRY(mImpl->clear(context, mask)); // Remove clear bits that are ineffective. An effective clear changes at least one fragment. If
// color/depth/stencil masks make the clear ineffective we skip it altogether.
// If all color channels are masked, don't attempt to clear color.
if (context->getState().getBlendState().allChannelsMasked())
{
mask &= ~GL_COLOR_BUFFER_BIT;
}
// If depth write is disabled, don't attempt to clear depth.
if (!context->getState().getDepthStencilState().depthMask)
{
mask &= ~GL_DEPTH_BUFFER_BIT;
}
// If all stencil bits are masked, don't attempt to clear stencil.
if (context->getState().getDepthStencilState().stencilWritemask == 0)
{
mask &= ~GL_STENCIL_BUFFER_BIT;
}
if (mask != 0)
{
ANGLE_TRY(mImpl->clear(context, mask));
}
return angle::Result::Continue; return angle::Result::Continue;
} }
...@@ -1364,6 +1388,23 @@ angle::Result Framebuffer::clearBufferfv(const Context *context, ...@@ -1364,6 +1388,23 @@ angle::Result Framebuffer::clearBufferfv(const Context *context,
return angle::Result::Continue; return angle::Result::Continue;
} }
if (buffer == GL_DEPTH)
{
// If depth write is disabled, don't attempt to clear depth.
if (!context->getState().getDepthStencilState().depthMask)
{
return angle::Result::Continue;
}
}
else
{
// If all color channels are masked, don't attempt to clear color.
if (context->getState().getBlendState().allChannelsMasked())
{
return angle::Result::Continue;
}
}
ANGLE_TRY(mImpl->clearBufferfv(context, buffer, drawbuffer, values)); ANGLE_TRY(mImpl->clearBufferfv(context, buffer, drawbuffer, values));
return angle::Result::Continue; return angle::Result::Continue;
...@@ -1379,6 +1420,12 @@ angle::Result Framebuffer::clearBufferuiv(const Context *context, ...@@ -1379,6 +1420,12 @@ angle::Result Framebuffer::clearBufferuiv(const Context *context,
return angle::Result::Continue; return angle::Result::Continue;
} }
// If all color channels are masked, don't attempt to clear color.
if (context->getState().getBlendState().allChannelsMasked())
{
return angle::Result::Continue;
}
ANGLE_TRY(mImpl->clearBufferuiv(context, buffer, drawbuffer, values)); ANGLE_TRY(mImpl->clearBufferuiv(context, buffer, drawbuffer, values));
return angle::Result::Continue; return angle::Result::Continue;
...@@ -1394,6 +1441,23 @@ angle::Result Framebuffer::clearBufferiv(const Context *context, ...@@ -1394,6 +1441,23 @@ angle::Result Framebuffer::clearBufferiv(const Context *context,
return angle::Result::Continue; return angle::Result::Continue;
} }
if (buffer == GL_STENCIL)
{
// If all stencil bits are masked, don't attempt to clear stencil.
if (context->getState().getDepthStencilState().stencilWritemask == 0)
{
return angle::Result::Continue;
}
}
else
{
// If all color channels are masked, don't attempt to clear color.
if (context->getState().getBlendState().allChannelsMasked())
{
return angle::Result::Continue;
}
}
ANGLE_TRY(mImpl->clearBufferiv(context, buffer, drawbuffer, values)); ANGLE_TRY(mImpl->clearBufferiv(context, buffer, drawbuffer, values));
return angle::Result::Continue; return angle::Result::Continue;
...@@ -1410,7 +1474,22 @@ angle::Result Framebuffer::clearBufferfi(const Context *context, ...@@ -1410,7 +1474,22 @@ angle::Result Framebuffer::clearBufferfi(const Context *context,
return angle::Result::Continue; return angle::Result::Continue;
} }
ANGLE_TRY(mImpl->clearBufferfi(context, buffer, drawbuffer, depth, stencil)); bool clearDepth = context->getState().getDepthStencilState().depthMask;
bool clearStencil = context->getState().getDepthStencilState().stencilWritemask != 0;
if (clearDepth && clearStencil)
{
ASSERT(buffer == GL_DEPTH_STENCIL);
ANGLE_TRY(mImpl->clearBufferfi(context, GL_DEPTH_STENCIL, drawbuffer, depth, stencil));
}
else if (clearDepth && !clearStencil)
{
ANGLE_TRY(mImpl->clearBufferfv(context, GL_DEPTH, drawbuffer, &depth));
}
else if (!clearDepth && clearStencil)
{
ANGLE_TRY(mImpl->clearBufferiv(context, GL_STENCIL, drawbuffer, &stencil));
}
return angle::Result::Continue; return angle::Result::Continue;
} }
......
...@@ -59,6 +59,11 @@ BlendState::BlendState(const BlendState &other) ...@@ -59,6 +59,11 @@ BlendState::BlendState(const BlendState &other)
memcpy(this, &other, sizeof(BlendState)); memcpy(this, &other, sizeof(BlendState));
} }
bool BlendState::allChannelsMasked() const
{
return !colorMaskRed && !colorMaskGreen && !colorMaskBlue && !colorMaskAlpha;
}
bool operator==(const BlendState &a, const BlendState &b) bool operator==(const BlendState &a, const BlendState &b)
{ {
return memcmp(&a, &b, sizeof(BlendState)) == 0; return memcmp(&a, &b, sizeof(BlendState)) == 0;
......
...@@ -146,6 +146,8 @@ struct BlendState final ...@@ -146,6 +146,8 @@ struct BlendState final
BlendState(); BlendState();
BlendState(const BlendState &other); BlendState(const BlendState &other);
bool allChannelsMasked() const;
bool blend; bool blend;
GLenum sourceBlendRGB; GLenum sourceBlendRGB;
GLenum destBlendRGB; GLenum destBlendRGB;
......
...@@ -179,8 +179,12 @@ class FramebufferVk : public FramebufferImpl ...@@ -179,8 +179,12 @@ class FramebufferVk : public FramebufferImpl
const VkClearDepthStencilValue &clearDepthStencilValue); const VkClearDepthStencilValue &clearDepthStencilValue);
angle::Result clearWithDraw(ContextVk *contextVk, angle::Result clearWithDraw(ContextVk *contextVk,
gl::DrawBufferMask clearColorBuffers, gl::DrawBufferMask clearColorBuffers,
bool clearDepth,
bool clearStencil,
VkColorComponentFlags colorMaskFlags,
uint8_t stencilMask,
const VkClearColorValue &clearColorValue, const VkClearColorValue &clearColorValue,
VkColorComponentFlags colorMaskFlags); const VkClearDepthStencilValue &clearDepthStencilValue);
void updateActiveColorMasks(size_t colorIndex, bool r, bool g, bool b, bool a); void updateActiveColorMasks(size_t colorIndex, bool r, bool g, bool b, bool a);
void updateRenderPassDesc(); void updateRenderPassDesc();
......
...@@ -979,6 +979,7 @@ angle::Result RendererVk::initializeDevice(DisplayVk *displayVk, uint32_t queueF ...@@ -979,6 +979,7 @@ angle::Result RendererVk::initializeDevice(DisplayVk *displayVk, uint32_t queueF
enabledFeatures.features.independentBlend = mPhysicalDeviceFeatures.independentBlend; enabledFeatures.features.independentBlend = mPhysicalDeviceFeatures.independentBlend;
enabledFeatures.features.robustBufferAccess = mPhysicalDeviceFeatures.robustBufferAccess; enabledFeatures.features.robustBufferAccess = mPhysicalDeviceFeatures.robustBufferAccess;
enabledFeatures.features.samplerAnisotropy = mPhysicalDeviceFeatures.samplerAnisotropy; enabledFeatures.features.samplerAnisotropy = mPhysicalDeviceFeatures.samplerAnisotropy;
enabledFeatures.features.depthClamp = mPhysicalDeviceFeatures.depthClamp;
if (!vk::CommandBuffer::ExecutesInline()) if (!vk::CommandBuffer::ExecutesInline())
{ {
enabledFeatures.features.inheritedQueries = mPhysicalDeviceFeatures.inheritedQueries; enabledFeatures.features.inheritedQueries = mPhysicalDeviceFeatures.inheritedQueries;
......
...@@ -200,6 +200,7 @@ void UtilsVk::destroy(VkDevice device) ...@@ -200,6 +200,7 @@ void UtilsVk::destroy(VkDevice device)
{ {
program.destroy(device); program.destroy(device);
} }
mImageClearProgramVSOnly.destroy(device);
for (vk::ShaderProgramHelper &program : mImageClearProgram) for (vk::ShaderProgramHelper &program : mImageClearProgram)
{ {
program.destroy(device); program.destroy(device);
...@@ -365,7 +366,10 @@ angle::Result UtilsVk::setupProgram(vk::Context *context, ...@@ -365,7 +366,10 @@ angle::Result UtilsVk::setupProgram(vk::Context *context,
else else
{ {
program->setShader(gl::ShaderType::Vertex, vsShader); program->setShader(gl::ShaderType::Vertex, vsShader);
program->setShader(gl::ShaderType::Fragment, fsCsShader); if (fsCsShader)
{
program->setShader(gl::ShaderType::Fragment, fsCsShader);
}
// This value is not used but is passed to getGraphicsPipeline to avoid a nullptr check. // This value is not used but is passed to getGraphicsPipeline to avoid a nullptr check.
const vk::GraphicsPipelineDesc *descPtr; const vk::GraphicsPipelineDesc *descPtr;
...@@ -637,9 +641,9 @@ angle::Result UtilsVk::startRenderPass(ContextVk *contextVk, ...@@ -637,9 +641,9 @@ angle::Result UtilsVk::startRenderPass(ContextVk *contextVk,
return angle::Result::Continue; return angle::Result::Continue;
} }
angle::Result UtilsVk::clearImage(ContextVk *contextVk, angle::Result UtilsVk::clearFramebuffer(ContextVk *contextVk,
FramebufferVk *framebuffer, FramebufferVk *framebuffer,
const ClearImageParameters &params) const ClearFramebufferParameters &params)
{ {
RendererVk *renderer = contextVk->getRenderer(); RendererVk *renderer = contextVk->getRenderer();
...@@ -652,24 +656,54 @@ angle::Result UtilsVk::clearImage(ContextVk *contextVk, ...@@ -652,24 +656,54 @@ angle::Result UtilsVk::clearImage(ContextVk *contextVk,
} }
ImageClearShaderParams shaderParams; ImageClearShaderParams shaderParams;
shaderParams.clearValue = params.clearValue; shaderParams.clearValue = params.colorClearValue;
uint32_t flags = GetImageClearFlags(*params.format, params.attachmentIndex);
vk::GraphicsPipelineDesc pipelineDesc; vk::GraphicsPipelineDesc pipelineDesc;
pipelineDesc.initDefaults(); pipelineDesc.initDefaults();
pipelineDesc.setColorWriteMask(0, gl::DrawBufferMask()); pipelineDesc.setColorWriteMask(0, gl::DrawBufferMask());
pipelineDesc.setSingleColorWriteMask(params.attachmentIndex, params.colorMaskFlags); pipelineDesc.setSingleColorWriteMask(params.colorAttachmentIndex, params.colorMaskFlags);
pipelineDesc.setRenderPassDesc(*params.renderPassDesc); pipelineDesc.setRenderPassDesc(*params.renderPassDesc);
// Note: depth test is disabled by default so this should be unnecessary, but works around an // Note: depth test is disabled by default so this should be unnecessary, but works around an
// Intel bug on windows. http://anglebug.com/3348 // Intel bug on windows. http://anglebug.com/3348
pipelineDesc.setDepthWriteEnabled(false); pipelineDesc.setDepthWriteEnabled(false);
// Clear depth by enabling depth clamping and setting the viewport depth range to the clear
// value.
if (params.clearDepth)
{
pipelineDesc.setDepthTestEnabled(true);
pipelineDesc.setDepthWriteEnabled(true);
pipelineDesc.setDepthFunc(VK_COMPARE_OP_ALWAYS);
pipelineDesc.setDepthClampEnabled(true);
}
// Clear stencil by enabling stencil write with the right mask.
if (params.clearStencil)
{
const uint8_t compareMask = 0xFF;
const uint8_t clearStencilValue =
static_cast<uint8_t>(params.depthStencilClearValue.stencil);
pipelineDesc.setStencilTestEnabled(true);
pipelineDesc.setStencilFrontFuncs(clearStencilValue, VK_COMPARE_OP_ALWAYS, compareMask);
pipelineDesc.setStencilBackFuncs(clearStencilValue, VK_COMPARE_OP_ALWAYS, compareMask);
pipelineDesc.setStencilFrontOps(VK_STENCIL_OP_REPLACE, VK_STENCIL_OP_REPLACE,
VK_STENCIL_OP_REPLACE);
pipelineDesc.setStencilBackOps(VK_STENCIL_OP_REPLACE, VK_STENCIL_OP_REPLACE,
VK_STENCIL_OP_REPLACE);
pipelineDesc.setStencilFrontWriteMask(params.stencilMask);
pipelineDesc.setStencilBackWriteMask(params.stencilMask);
}
const gl::Rectangle &renderArea = framebuffer->getFramebuffer()->getRenderPassRenderArea(); const gl::Rectangle &renderArea = framebuffer->getFramebuffer()->getRenderPassRenderArea();
bool invertViewport = contextVk->isViewportFlipEnabledForDrawFBO(); bool invertViewport = contextVk->isViewportFlipEnabledForDrawFBO();
VkViewport viewport; VkViewport viewport;
gl_vk::GetViewport(renderArea, 0.0f, 1.0f, invertViewport, params.renderAreaHeight, &viewport); // Set depth range to clear value. If clearing depth, the vertex shader depth output is clamped
// to this value, thus clearing the depth buffer to the desired clear value.
const float clearDepthValue = params.depthStencilClearValue.depth;
gl_vk::GetViewport(renderArea, clearDepthValue, clearDepthValue, invertViewport,
params.renderAreaHeight, &viewport);
pipelineDesc.setViewport(viewport); pipelineDesc.setViewport(viewport);
VkRect2D scissor; VkRect2D scissor;
...@@ -680,11 +714,18 @@ angle::Result UtilsVk::clearImage(ContextVk *contextVk, ...@@ -680,11 +714,18 @@ angle::Result UtilsVk::clearImage(ContextVk *contextVk,
vk::ShaderLibrary &shaderLibrary = renderer->getShaderLibrary(); vk::ShaderLibrary &shaderLibrary = renderer->getShaderLibrary();
vk::RefCounted<vk::ShaderAndSerial> *vertexShader = nullptr; vk::RefCounted<vk::ShaderAndSerial> *vertexShader = nullptr;
vk::RefCounted<vk::ShaderAndSerial> *fragmentShader = nullptr; vk::RefCounted<vk::ShaderAndSerial> *fragmentShader = nullptr;
vk::ShaderProgramHelper *imageClearProgram = &mImageClearProgramVSOnly;
ANGLE_TRY(shaderLibrary.getFullScreenQuad_vert(contextVk, 0, &vertexShader)); ANGLE_TRY(shaderLibrary.getFullScreenQuad_vert(contextVk, 0, &vertexShader));
ANGLE_TRY(shaderLibrary.getImageClear_frag(contextVk, flags, &fragmentShader)); if (params.clearColor)
{
uint32_t flags = GetImageClearFlags(*params.colorFormat, params.colorAttachmentIndex);
ANGLE_TRY(shaderLibrary.getImageClear_frag(contextVk, flags, &fragmentShader));
imageClearProgram = &mImageClearProgram[flags];
}
ANGLE_TRY(setupProgram(contextVk, Function::ImageClear, fragmentShader, vertexShader, ANGLE_TRY(setupProgram(contextVk, Function::ImageClear, fragmentShader, vertexShader,
&mImageClearProgram[flags], &pipelineDesc, VK_NULL_HANDLE, &shaderParams, imageClearProgram, &pipelineDesc, VK_NULL_HANDLE, &shaderParams,
sizeof(shaderParams), commandBuffer)); sizeof(shaderParams), commandBuffer));
commandBuffer->draw(6, 0); commandBuffer->draw(6, 0);
return angle::Result::Continue; return angle::Result::Continue;
......
...@@ -59,14 +59,22 @@ class UtilsVk : angle::NonCopyable ...@@ -59,14 +59,22 @@ class UtilsVk : angle::NonCopyable
size_t destOffset; size_t destOffset;
}; };
struct ClearImageParameters struct ClearFramebufferParameters
{ {
VkClearColorValue clearValue;
VkColorComponentFlags colorMaskFlags;
GLint renderAreaHeight;
const vk::RenderPassDesc *renderPassDesc; const vk::RenderPassDesc *renderPassDesc;
const angle::Format *format; GLint renderAreaHeight;
uint32_t attachmentIndex;
bool clearColor;
bool clearDepth;
bool clearStencil;
uint8_t stencilMask;
VkColorComponentFlags colorMaskFlags;
uint32_t colorAttachmentIndex;
const angle::Format *colorFormat;
VkClearColorValue colorClearValue;
VkClearDepthStencilValue depthStencilClearValue;
}; };
struct CopyImageParameters struct CopyImageParameters
...@@ -95,12 +103,9 @@ class UtilsVk : angle::NonCopyable ...@@ -95,12 +103,9 @@ class UtilsVk : angle::NonCopyable
vk::BufferHelper *src, vk::BufferHelper *src,
const ConvertVertexParameters &params); const ConvertVertexParameters &params);
// Note: this function takes a FramebufferVk instead of ImageHelper, as that's the only user, angle::Result clearFramebuffer(ContextVk *contextVk,
// which avoids recreating a framebuffer. An overload taking ImageHelper can be added when FramebufferVk *framebuffer,
// necessary. const ClearFramebufferParameters &params);
angle::Result clearImage(ContextVk *contextVk,
FramebufferVk *framebuffer,
const ClearImageParameters &params);
angle::Result copyImage(ContextVk *contextVk, angle::Result copyImage(ContextVk *contextVk,
vk::ImageHelper *dest, vk::ImageHelper *dest,
...@@ -232,6 +237,7 @@ class UtilsVk : angle::NonCopyable ...@@ -232,6 +237,7 @@ class UtilsVk : angle::NonCopyable
vk::ShaderProgramHelper vk::ShaderProgramHelper
mConvertVertexPrograms[vk::InternalShader::ConvertVertex_comp::kFlagsMask | mConvertVertexPrograms[vk::InternalShader::ConvertVertex_comp::kFlagsMask |
vk::InternalShader::ConvertVertex_comp::kConversionMask]; vk::InternalShader::ConvertVertex_comp::kConversionMask];
vk::ShaderProgramHelper mImageClearProgramVSOnly;
vk::ShaderProgramHelper vk::ShaderProgramHelper
mImageClearProgram[vk::InternalShader::ImageClear_frag::kAttachmentIndexMask | mImageClearProgram[vk::InternalShader::ImageClear_frag::kAttachmentIndexMask |
vk::InternalShader::ImageClear_frag::kFormatMask]; vk::InternalShader::ImageClear_frag::kFormatMask];
......
...@@ -373,7 +373,17 @@ class GraphicsPipelineDesc final ...@@ -373,7 +373,17 @@ class GraphicsPipelineDesc final
const gl::DrawBufferMask &alphaMask); const gl::DrawBufferMask &alphaMask);
// Depth/stencil states. // Depth/stencil states.
void setDepthTestEnabled(bool enabled);
void setDepthWriteEnabled(bool enabled); void setDepthWriteEnabled(bool enabled);
void setDepthFunc(VkCompareOp op);
void setDepthClampEnabled(bool enabled);
void setStencilTestEnabled(bool enabled);
void setStencilFrontFuncs(uint8_t reference, VkCompareOp compareOp, uint8_t compareMask);
void setStencilBackFuncs(uint8_t reference, VkCompareOp compareOp, uint8_t compareMask);
void setStencilFrontOps(VkStencilOp failOp, VkStencilOp passOp, VkStencilOp depthFailOp);
void setStencilBackOps(VkStencilOp failOp, VkStencilOp passOp, VkStencilOp depthFailOp);
void setStencilFrontWriteMask(uint8_t mask);
void setStencilBackWriteMask(uint8_t mask);
void updateDepthTestEnabled(GraphicsPipelineTransitionBits *transition, void updateDepthTestEnabled(GraphicsPipelineTransitionBits *transition,
const gl::DepthStencilState &depthStencilState, const gl::DepthStencilState &depthStencilState,
const gl::Framebuffer *drawFramebuffer); const gl::Framebuffer *drawFramebuffer);
......
...@@ -266,12 +266,6 @@ ...@@ -266,12 +266,6 @@
// General Vulkan failures // General Vulkan failures
3300 VULKAN : dEQP-GLES2.functional.shaders.texture_functions.vertex.texturecubelod = FAIL 3300 VULKAN : dEQP-GLES2.functional.shaders.texture_functions.vertex.texturecubelod = FAIL
// Depth/stencil clear
3241 VULKAN : dEQP-GLES2.functional.depth_stencil_clear.stencil_masked = FAIL
3241 VULKAN : dEQP-GLES2.functional.depth_stencil_clear.stencil_scissored_masked = FAIL
3241 VULKAN : dEQP-GLES2.functional.depth_stencil_clear.depth_stencil_masked = FAIL
3241 VULKAN : dEQP-GLES2.functional.depth_stencil_clear.depth_stencil_scissored_masked = FAIL
// Only seen failing on Android // Only seen failing on Android
3241 VULKAN ANDROID : dEQP-GLES2.functional.depth_stencil_clear.depth_scissored_masked = FAIL 3241 VULKAN ANDROID : dEQP-GLES2.functional.depth_stencil_clear.depth_scissored_masked = FAIL
...@@ -378,4 +372,4 @@ ...@@ -378,4 +372,4 @@
2976 VULKAN NVIDIA : dEQP-GLES2.functional.shaders.invariance.* = FAIL 2976 VULKAN NVIDIA : dEQP-GLES2.functional.shaders.invariance.* = FAIL
// Tests were being hidden by flakiness (anglebug.com/3271) // Tests were being hidden by flakiness (anglebug.com/3271)
3328 VULKAN WIN NVIDIA : dEQP-GLES2.functional.shaders.swizzles.vector_swizzles.mediump_vec4_qqqt_vertex = SKIP 3328 VULKAN WIN NVIDIA : dEQP-GLES2.functional.shaders.swizzles.vector_swizzles.mediump_vec4_qqqt_vertex = SKIP
\ No newline at end of file
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment