Validate active draw buffers for dual-source blending

Fail if more than MAX_DUAL_SOURCE_DRAW_BUFFERS_EXT
draw buffers are enabled when dual-source blending
is used.

Drive-by:
  Do not invalidate draw state on changing blend
  equations if KHR_blend_equation_advanced is
  not enabled.

Bug: angleproject:1085
Bug: angleproject:7177
Change-Id: Ieff80ce777c53b1d8183e1d0a52b7d2224347448
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/4823164
Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
Commit-Queue: Alexey Knyazev <lexa.knyazev@gmail.com>
This commit is contained in:
Alexey Knyazev
2023-08-21 00:00:00 +00:00
committed by Angle LUCI CQ
parent 6698fb69b0
commit 8b0af482db
8 changed files with 213 additions and 31 deletions

View File

@@ -51,6 +51,8 @@ TEST(BlendStateExt, Init)
ASSERT_EQ(blendStateExt.getUsesAdvancedBlendEquationMask().to_ulong(), 0u);
ASSERT_EQ(blendStateExt.getUsesExtendedBlendFactorMask().to_ulong(), 0u);
ASSERT_EQ(blendStateExt.getSrcColorBits(), sourceColorAlpha[c]);
ASSERT_EQ(blendStateExt.getSrcAlphaBits(), sourceColorAlpha[c]);
ASSERT_EQ(blendStateExt.getDstColorBits(), 0u);

View File

@@ -170,8 +170,26 @@ class PrivateStateCache final : angle::NonCopyable
void onCapChange() { mIsCachedBasicDrawStatesErrorValid = false; }
void onColorMaskChange() { mIsCachedBasicDrawStatesErrorValid = false; }
void onDefaultVertexAttributeChange() { mIsCachedBasicDrawStatesErrorValid = false; }
void onBlendFuncIndexedChange() { mIsCachedBasicDrawStatesErrorValid = false; }
void onBlendEquationChange() { mIsCachedBasicDrawStatesErrorValid = false; }
// Blending updates invalidate draw
// state in the following cases:
//
// * Blend equations have been changed and the context
// supports KHR_blend_equation_advanced. The number
// of enabled draw buffers may need to be checked
// to not be greater than 1.
//
// * Blend funcs have been changed with indexed
// commands. The D3D11 backend cannot support
// constant color and alpha blend funcs together
// so a check is needed across all draw buffers.
//
// * Blend funcs have been changed and the context
// supports EXT_blend_func_extended. The number
// of enabled draw buffers may need to be checked
// against MAX_DUAL_SOURCE_DRAW_BUFFERS_EXT limit.
void onBlendEquationOrFuncChange() { mIsCachedBasicDrawStatesErrorValid = false; }
void onStencilStateChange() { mIsCachedBasicDrawStatesErrorValid = false; }
bool isCachedBasicDrawStatesErrorValid() const { return mIsCachedBasicDrawStatesErrorValid; }
@@ -237,8 +255,7 @@ class StateCache final : angle::NonCopyable
// 2. onStencilStateChange.
// 3. onDefaultVertexAttributeChange.
// 4. onColorMaskChange.
// 5. onBlendFuncIndexedChange.
// 6. onBlendEquationChange.
// 5. onBlendEquationOrFuncChange.
intptr_t getBasicDrawStatesErrorString(const Context *context,
const PrivateStateCache *privateStateCache) const
{

View File

@@ -102,6 +102,7 @@ MSG kES31OrDrawBuffersIndexedExtensionNotAvailable = "EXT/OES_draw_buffers_index
MSG kDrawBufferTypeMismatch = "Fragment shader output type does not match the bound framebuffer attachment type.";
MSG kDrawFramebufferIncomplete = "Draw framebuffer is incomplete";
MSG kDrawIndirectBufferNotBound = "Draw indirect buffer must be bound.";
MSG kDualSourceBlendingDrawBuffersLimit = "Dual-source blending functions limit the number of supported draw buffers.";
MSG kEGLImageCannotCreate2DMultisampled = "Cannot create a 2D texture from a multisampled EGL image.";
MSG kEGLImageRenderbufferFormatNotSupported = "EGL image internal format is not supported as a renderbuffer.";
MSG kEGLImageTextureFormatNotSupported = "EGL image internal format is not supported as a texture.";

View File

@@ -42,6 +42,12 @@ bool IsAdvancedBlendEquation(gl::BlendEquationType blendEquation)
return blendEquation >= gl::BlendEquationType::Multiply &&
blendEquation <= gl::BlendEquationType::HslLuminosity;
}
bool IsExtendedBlendFactor(gl::BlendFactorType blendFactor)
{
return blendFactor >= gl::BlendFactorType::Src1Alpha &&
blendFactor <= gl::BlendFactorType::OneMinusSrc1Alpha;
}
} // anonymous namespace
RasterizerState::RasterizerState()
@@ -558,6 +564,12 @@ BlendStateExt::FactorStorage::Type BlendStateExt::expandFactorValue(const GLenum
return FactorStorage::GetReplicatedValue(FromGLenum<BlendFactorType>(func), mParameterMask);
}
BlendStateExt::FactorStorage::Type BlendStateExt::expandFactorValue(
const gl::BlendFactorType func) const
{
return FactorStorage::GetReplicatedValue(func, mParameterMask);
}
BlendStateExt::FactorStorage::Type BlendStateExt::expandSrcColorIndexed(const size_t index) const
{
ASSERT(index < mDrawBufferCount);
@@ -591,10 +603,25 @@ void BlendStateExt::setFactors(const GLenum srcColor,
const GLenum srcAlpha,
const GLenum dstAlpha)
{
mSrcColor = expandFactorValue(srcColor);
mDstColor = expandFactorValue(dstColor);
mSrcAlpha = expandFactorValue(srcAlpha);
mDstAlpha = expandFactorValue(dstAlpha);
const gl::BlendFactorType srcColorFactor = FromGLenum<BlendFactorType>(srcColor);
const gl::BlendFactorType dstColorFactor = FromGLenum<BlendFactorType>(dstColor);
const gl::BlendFactorType srcAlphaFactor = FromGLenum<BlendFactorType>(srcAlpha);
const gl::BlendFactorType dstAlphaFactor = FromGLenum<BlendFactorType>(dstAlpha);
mSrcColor = expandFactorValue(srcColorFactor);
mDstColor = expandFactorValue(dstColorFactor);
mSrcAlpha = expandFactorValue(srcAlphaFactor);
mDstAlpha = expandFactorValue(dstAlphaFactor);
if (IsExtendedBlendFactor(srcColorFactor) || IsExtendedBlendFactor(dstColorFactor) ||
IsExtendedBlendFactor(srcAlphaFactor) || IsExtendedBlendFactor(dstAlphaFactor))
{
mUsesExtendedBlendFactorMask = mAllEnabledMask;
}
else
{
mUsesExtendedBlendFactorMask.reset();
}
}
void BlendStateExt::setFactorsIndexed(const size_t index,
@@ -604,10 +631,21 @@ void BlendStateExt::setFactorsIndexed(const size_t index,
const GLenum dstAlpha)
{
ASSERT(index < mDrawBufferCount);
FactorStorage::SetValueIndexed(index, FromGLenum<BlendFactorType>(srcColor), &mSrcColor);
FactorStorage::SetValueIndexed(index, FromGLenum<BlendFactorType>(dstColor), &mDstColor);
FactorStorage::SetValueIndexed(index, FromGLenum<BlendFactorType>(srcAlpha), &mSrcAlpha);
FactorStorage::SetValueIndexed(index, FromGLenum<BlendFactorType>(dstAlpha), &mDstAlpha);
const gl::BlendFactorType srcColorFactor = FromGLenum<BlendFactorType>(srcColor);
const gl::BlendFactorType dstColorFactor = FromGLenum<BlendFactorType>(dstColor);
const gl::BlendFactorType srcAlphaFactor = FromGLenum<BlendFactorType>(srcAlpha);
const gl::BlendFactorType dstAlphaFactor = FromGLenum<BlendFactorType>(dstAlpha);
FactorStorage::SetValueIndexed(index, srcColorFactor, &mSrcColor);
FactorStorage::SetValueIndexed(index, dstColorFactor, &mDstColor);
FactorStorage::SetValueIndexed(index, srcAlphaFactor, &mSrcAlpha);
FactorStorage::SetValueIndexed(index, dstAlphaFactor, &mDstAlpha);
const bool isExtended =
IsExtendedBlendFactor(srcColorFactor) || IsExtendedBlendFactor(dstColorFactor) ||
IsExtendedBlendFactor(srcAlphaFactor) || IsExtendedBlendFactor(dstAlphaFactor);
mUsesExtendedBlendFactorMask.set(index, isExtended);
}
void BlendStateExt::setFactorsIndexed(const size_t index,
@@ -616,14 +654,25 @@ void BlendStateExt::setFactorsIndexed(const size_t index,
{
ASSERT(index < mDrawBufferCount);
ASSERT(sourceIndex < source.mDrawBufferCount);
FactorStorage::SetValueIndexed(
index, FactorStorage::GetValueIndexed(sourceIndex, source.mSrcColor), &mSrcColor);
FactorStorage::SetValueIndexed(
index, FactorStorage::GetValueIndexed(sourceIndex, source.mDstColor), &mDstColor);
FactorStorage::SetValueIndexed(
index, FactorStorage::GetValueIndexed(sourceIndex, source.mSrcAlpha), &mSrcAlpha);
FactorStorage::SetValueIndexed(
index, FactorStorage::GetValueIndexed(sourceIndex, source.mDstAlpha), &mDstAlpha);
const gl::BlendFactorType srcColorFactor =
FactorStorage::GetValueIndexed(sourceIndex, source.mSrcColor);
const gl::BlendFactorType dstColorFactor =
FactorStorage::GetValueIndexed(sourceIndex, source.mDstColor);
const gl::BlendFactorType srcAlphaFactor =
FactorStorage::GetValueIndexed(sourceIndex, source.mSrcAlpha);
const gl::BlendFactorType dstAlphaFactor =
FactorStorage::GetValueIndexed(sourceIndex, source.mDstAlpha);
FactorStorage::SetValueIndexed(index, srcColorFactor, &mSrcColor);
FactorStorage::SetValueIndexed(index, dstColorFactor, &mDstColor);
FactorStorage::SetValueIndexed(index, srcAlphaFactor, &mSrcAlpha);
FactorStorage::SetValueIndexed(index, dstAlphaFactor, &mDstAlpha);
const bool isExtended =
IsExtendedBlendFactor(srcColorFactor) || IsExtendedBlendFactor(dstColorFactor) ||
IsExtendedBlendFactor(srcAlphaFactor) || IsExtendedBlendFactor(dstAlphaFactor);
mUsesExtendedBlendFactorMask.set(index, isExtended);
}
GLenum BlendStateExt::getSrcColorIndexed(size_t index) const

View File

@@ -729,6 +729,7 @@ class BlendStateExt final
///////// Blend Factors /////////
FactorStorage::Type expandFactorValue(const GLenum func) const;
FactorStorage::Type expandFactorValue(const gl::BlendFactorType func) const;
FactorStorage::Type expandSrcColorIndexed(const size_t index) const;
FactorStorage::Type expandDstColorIndexed(const size_t index) const;
FactorStorage::Type expandSrcAlphaIndexed(const size_t index) const;
@@ -775,6 +776,11 @@ class BlendStateExt final
return mUsesAdvancedBlendEquationMask;
}
constexpr DrawBufferMask getUsesExtendedBlendFactorMask() const
{
return mUsesExtendedBlendFactorMask;
}
constexpr uint8_t getDrawBufferCount() const { return mDrawBufferCount; }
constexpr void setSrcColorBits(const FactorStorage::Type srcColor) { mSrcColor = srcColor; }
@@ -819,17 +825,20 @@ class BlendStateExt final
// Cache of whether the blend equation for each index is from KHR_blend_equation_advanced.
DrawBufferMask mUsesAdvancedBlendEquationMask;
// Cache of whether the blend factor for each index is from EXT_blend_func_extended.
DrawBufferMask mUsesExtendedBlendFactorMask;
uint8_t mDrawBufferCount;
ANGLE_MAYBE_UNUSED_PRIVATE_FIELD uint32_t kUnused = 0;
ANGLE_MAYBE_UNUSED_PRIVATE_FIELD uint8_t kUnused[3] = {};
};
static_assert(sizeof(BlendStateExt) == sizeof(uint64_t) +
(sizeof(BlendStateExt::FactorStorage::Type) * 4 +
sizeof(BlendStateExt::EquationStorage::Type) * 2 +
sizeof(BlendStateExt::ColorMaskStorage::Type) * 2 +
sizeof(DrawBufferMask) * 3 + sizeof(uint8_t)) +
sizeof(uint32_t),
sizeof(DrawBufferMask) * 4 + sizeof(uint8_t)) +
sizeof(uint8_t) * 3,
"The BlendStateExt class must not contain gaps.");
// Used in StateCache

View File

@@ -495,7 +495,10 @@ void ContextPrivateBlendEquation(PrivateState *privateState,
GLenum mode)
{
privateState->setBlendEquation(mode, mode);
privateStateCache->onBlendEquationChange();
if (privateState->getExtensions().blendEquationAdvancedKHR)
{
privateStateCache->onBlendEquationOrFuncChange();
}
}
void ContextPrivateBlendEquationi(PrivateState *privateState,
@@ -504,7 +507,10 @@ void ContextPrivateBlendEquationi(PrivateState *privateState,
GLenum mode)
{
privateState->setBlendEquationIndexed(mode, mode, buf);
privateStateCache->onBlendEquationChange();
if (privateState->getExtensions().blendEquationAdvancedKHR)
{
privateStateCache->onBlendEquationOrFuncChange();
}
}
void ContextPrivateBlendEquationSeparate(PrivateState *privateState,
@@ -513,7 +519,10 @@ void ContextPrivateBlendEquationSeparate(PrivateState *privateState,
GLenum modeAlpha)
{
privateState->setBlendEquation(modeRGB, modeAlpha);
privateStateCache->onBlendEquationChange();
if (privateState->getExtensions().blendEquationAdvancedKHR)
{
privateStateCache->onBlendEquationOrFuncChange();
}
}
void ContextPrivateBlendEquationSeparatei(PrivateState *privateState,
@@ -523,7 +532,10 @@ void ContextPrivateBlendEquationSeparatei(PrivateState *privateState,
GLenum modeAlpha)
{
privateState->setBlendEquationIndexed(modeRGB, modeAlpha, buf);
privateStateCache->onBlendEquationChange();
if (privateState->getExtensions().blendEquationAdvancedKHR)
{
privateStateCache->onBlendEquationOrFuncChange();
}
}
void ContextPrivateBlendFunc(PrivateState *privateState,
@@ -532,6 +544,10 @@ void ContextPrivateBlendFunc(PrivateState *privateState,
GLenum dfactor)
{
privateState->setBlendFactors(sfactor, dfactor, sfactor, dfactor);
if (privateState->getExtensions().blendFuncExtendedEXT)
{
privateStateCache->onBlendEquationOrFuncChange();
}
}
void ContextPrivateBlendFunci(PrivateState *privateState,
@@ -541,9 +557,10 @@ void ContextPrivateBlendFunci(PrivateState *privateState,
GLenum dst)
{
privateState->setBlendFactorsIndexed(src, dst, src, dst, buf);
if (privateState->noSimultaneousConstantColorAndAlphaBlendFunc())
if (privateState->noSimultaneousConstantColorAndAlphaBlendFunc() ||
privateState->getExtensions().blendFuncExtendedEXT)
{
privateStateCache->onBlendFuncIndexedChange();
privateStateCache->onBlendEquationOrFuncChange();
}
}
@@ -555,6 +572,10 @@ void ContextPrivateBlendFuncSeparate(PrivateState *privateState,
GLenum dstAlpha)
{
privateState->setBlendFactors(srcRGB, dstRGB, srcAlpha, dstAlpha);
if (privateState->getExtensions().blendFuncExtendedEXT)
{
privateStateCache->onBlendEquationOrFuncChange();
}
}
void ContextPrivateBlendFuncSeparatei(PrivateState *privateState,
@@ -566,9 +587,10 @@ void ContextPrivateBlendFuncSeparatei(PrivateState *privateState,
GLenum dstAlpha)
{
privateState->setBlendFactorsIndexed(srcRGB, dstRGB, srcAlpha, dstAlpha, buf);
if (privateState->noSimultaneousConstantColorAndAlphaBlendFunc())
if (privateState->noSimultaneousConstantColorAndAlphaBlendFunc() ||
privateState->getExtensions().blendFuncExtendedEXT)
{
privateStateCache->onBlendFuncIndexedChange();
privateStateCache->onBlendEquationOrFuncChange();
}
}

View File

@@ -4237,6 +4237,19 @@ const char *ValidateDrawStates(const Context *context, GLenum *outErrorCode)
}
}
// Dual-source blending functions limit the number of supported draw buffers.
if (blendStateExt.getUsesExtendedBlendFactorMask().any())
{
// Imply the strictest spec interpretation to pass on all OpenGL drivers:
// dual-source blending is considered active if the blend state contains
// any SRC1 factor no matter what.
const DrawBufferMask bufferMask = framebuffer->getDrawBufferMask();
if (bufferMask.any() && bufferMask.last() >= context->getCaps().maxDualSourceDrawBuffers)
{
return kDualSourceBlendingDrawBuffersLimit;
}
}
if (context->getStateCache().hasAnyEnabledClientAttrib())
{
if (extensions.webglCompatibilityANGLE || !state.areClientArraysEnabled())

View File

@@ -380,6 +380,75 @@ TEST_P(EXTBlendFuncExtendedTest, TestMaxDualSourceDrawBuffers)
ASSERT_GL_NO_ERROR();
}
// Test that SRC1 factors limit the number of allowed draw buffers.
TEST_P(EXTBlendFuncExtendedTest, MaxDualSourceDrawBuffersError)
{
ANGLE_SKIP_TEST_IF(!IsGLExtensionEnabled("GL_EXT_blend_func_extended"));
ANGLE_SKIP_TEST_IF(!IsGLExtensionEnabled("GL_EXT_draw_buffers"));
ANGLE_SKIP_TEST_IF(!IsGLExtensionEnabled("GL_OES_rgb8_rgba8"));
GLint maxDualSourceDrawBuffers = 0;
glGetIntegerv(GL_MAX_DUAL_SOURCE_DRAW_BUFFERS_EXT, &maxDualSourceDrawBuffers);
ANGLE_SKIP_TEST_IF(maxDualSourceDrawBuffers != 1);
ANGLE_GL_PROGRAM(redProgram, essl1_shaders::vs::Simple(), essl1_shaders::fs::Red());
GLFramebuffer fbo;
glBindFramebuffer(GL_FRAMEBUFFER, fbo);
GLRenderbuffer rb0;
glBindRenderbuffer(GL_RENDERBUFFER, rb0);
glRenderbufferStorage(GL_RENDERBUFFER, GL_RGBA8_OES, 1, 1);
glFramebufferRenderbuffer(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0_EXT, GL_RENDERBUFFER, rb0);
GLRenderbuffer rb1;
glBindRenderbuffer(GL_RENDERBUFFER, rb1);
glRenderbufferStorage(GL_RENDERBUFFER, GL_RGBA8_OES, 1, 1);
glFramebufferRenderbuffer(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT1_EXT, GL_RENDERBUFFER, rb1);
ASSERT_GL_FRAMEBUFFER_COMPLETE(GL_FRAMEBUFFER);
const GLenum bufs[] = {GL_COLOR_ATTACHMENT0_EXT, GL_COLOR_ATTACHMENT1_EXT};
glDrawBuffersEXT(2, bufs);
ASSERT_GL_NO_ERROR();
for (const GLenum func : {GL_SRC1_COLOR_EXT, GL_ONE_MINUS_SRC1_COLOR_EXT, GL_SRC1_ALPHA_EXT,
GL_ONE_MINUS_SRC1_ALPHA_EXT})
{
for (size_t slot = 0; slot < 4; slot++)
{
switch (slot)
{
case 0:
glBlendFuncSeparate(func, GL_ONE, GL_ONE, GL_ONE);
break;
case 1:
glBlendFuncSeparate(GL_ONE, func, GL_ONE, GL_ONE);
break;
case 2:
glBlendFuncSeparate(GL_ONE, GL_ONE, func, GL_ONE);
break;
case 3:
glBlendFuncSeparate(GL_ONE, GL_ONE, GL_ONE, func);
break;
}
// Limit must be applied even with blending disabled
glDisable(GL_BLEND);
drawQuad(redProgram, essl1_shaders::PositionAttrib(), 0.0);
EXPECT_GL_ERROR(GL_INVALID_OPERATION);
glEnable(GL_BLEND);
drawQuad(redProgram, essl1_shaders::PositionAttrib(), 0.0);
EXPECT_GL_ERROR(GL_INVALID_OPERATION);
// Limit is not applied when non-SRC1 funcs are used
glBlendFunc(GL_ONE, GL_ONE);
drawQuad(redProgram, essl1_shaders::PositionAttrib(), 0.0);
EXPECT_GL_NO_ERROR();
}
}
}
// Test a shader with EXT_blend_func_extended and gl_SecondaryFragColorEXT.
// Outputs to primary color buffer using primary and secondary colors.
TEST_P(EXTBlendFuncExtendedDrawTest, FragColor)