diff --git a/src/libANGLE/ErrorStrings.h b/src/libANGLE/ErrorStrings.h index 3e681ce4066..a2322c01d3e 100644 --- a/src/libANGLE/ErrorStrings.h +++ b/src/libANGLE/ErrorStrings.h @@ -84,6 +84,7 @@ MSG kDestinationLevelNotDefined = "The destination level of the destination text MSG kDestinationTextureTooSmall = "Destination texture too small."; MSG kDimensionsMustBePow2 = "Texture dimensions must be power-of-two."; MSG kDispatchIndirectBufferNotBound = "Dispatch indirect buffer must be bound."; +MSG kDrawBufferMaskMismatch = "Active draw buffers with missing fragment shader outputs."; 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."; diff --git a/src/libANGLE/renderer/gl/ContextGL.cpp b/src/libANGLE/renderer/gl/ContextGL.cpp index bb3b4aa2252..3e0dbcb7706 100644 --- a/src/libANGLE/renderer/gl/ContextGL.cpp +++ b/src/libANGLE/renderer/gl/ContextGL.cpp @@ -200,13 +200,6 @@ ANGLE_INLINE angle::Result ContextGL::setDrawArraysState(const gl::Context *cont count, instanceCount)); } - if (context->getExtensions().webglCompatibility) - { - const gl::State &glState = context->getState(); - FramebufferGL *framebufferGL = GetImplAs(glState.getDrawFramebuffer()); - framebufferGL->maskOutInactiveOutputDrawBuffers(context); - } - return angle::Result::Continue; } @@ -236,24 +229,6 @@ ANGLE_INLINE angle::Result ContextGL::setDrawElementsState(const gl::Context *co *outIndices = indices; } - if (context->getExtensions().webglCompatibility) - { - FramebufferGL *framebufferGL = GetImplAs(glState.getDrawFramebuffer()); - framebufferGL->maskOutInactiveOutputDrawBuffers(context); - } - - return angle::Result::Continue; -} - -ANGLE_INLINE angle::Result ContextGL::setDrawIndirectState(const gl::Context *context) -{ - if (context->getExtensions().webglCompatibility) - { - const gl::State &glState = context->getState(); - FramebufferGL *framebufferGL = GetImplAs(glState.getDrawFramebuffer()); - framebufferGL->maskOutInactiveOutputDrawBuffers(context); - } - return angle::Result::Continue; } @@ -375,7 +350,6 @@ angle::Result ContextGL::drawArraysIndirect(const gl::Context *context, gl::PrimitiveMode mode, const void *indirect) { - ANGLE_TRY(setDrawIndirectState(context)); getFunctions()->drawArraysIndirect(ToGLenum(mode), indirect); return angle::Result::Continue; } @@ -385,7 +359,6 @@ angle::Result ContextGL::drawElementsIndirect(const gl::Context *context, gl::DrawElementsType type, const void *indirect) { - ANGLE_TRY(setDrawIndirectState(context)); getFunctions()->drawElementsIndirect(ToGLenum(mode), ToGLenum(type), indirect); return angle::Result::Continue; } diff --git a/src/libANGLE/renderer/gl/ContextGL.h b/src/libANGLE/renderer/gl/ContextGL.h index eebc5e26965..0b08b3c1f3c 100644 --- a/src/libANGLE/renderer/gl/ContextGL.h +++ b/src/libANGLE/renderer/gl/ContextGL.h @@ -231,8 +231,6 @@ class ContextGL : public ContextImpl GLsizei instanceCount, const void **outIndices); - angle::Result setDrawIndirectState(const gl::Context *context); - protected: std::shared_ptr mRenderer; }; diff --git a/src/libANGLE/renderer/gl/FramebufferGL.cpp b/src/libANGLE/renderer/gl/FramebufferGL.cpp index e1db56a857e..0185f21a6e2 100644 --- a/src/libANGLE/renderer/gl/FramebufferGL.cpp +++ b/src/libANGLE/renderer/gl/FramebufferGL.cpp @@ -726,30 +726,6 @@ bool FramebufferGL::isDefault() const return mIsDefault; } -void FramebufferGL::maskOutInactiveOutputDrawBuffersImpl(const gl::Context *context, - DrawBufferMask targetAppliedDrawBuffers) -{ - - ASSERT(mAppliedEnabledDrawBuffers != targetAppliedDrawBuffers); - mAppliedEnabledDrawBuffers = targetAppliedDrawBuffers; - - const auto &stateDrawBuffers = mState.getDrawBufferStates(); - GLsizei drawBufferCount = static_cast(stateDrawBuffers.size()); - ASSERT(drawBufferCount <= IMPLEMENTATION_MAX_DRAW_BUFFERS); - - GLenum drawBuffers[IMPLEMENTATION_MAX_DRAW_BUFFERS]; - for (GLenum i = 0; static_cast(i) < drawBufferCount; ++i) - { - drawBuffers[i] = targetAppliedDrawBuffers[i] ? stateDrawBuffers[i] : GL_NONE; - } - - const FunctionsGL *functions = GetFunctionsGL(context); - StateManagerGL *stateManager = GetStateManagerGL(context); - - ASSERT(stateManager->getFramebufferID(angle::FramebufferBindingDraw) == mFramebufferID); - functions->drawBuffers(drawBufferCount, drawBuffers); -} - void FramebufferGL::syncClearState(const gl::Context *context, GLbitfield mask) { const FunctionsGL *functions = GetFunctionsGL(context); diff --git a/src/libANGLE/renderer/gl/FramebufferGL.h b/src/libANGLE/renderer/gl/FramebufferGL.h index 6b1017de0bd..1ae988557f5 100644 --- a/src/libANGLE/renderer/gl/FramebufferGL.h +++ b/src/libANGLE/renderer/gl/FramebufferGL.h @@ -86,20 +86,6 @@ class FramebufferGL : public FramebufferImpl GLuint getFramebufferID() const; bool isDefault() const; - ANGLE_INLINE void maskOutInactiveOutputDrawBuffers(const gl::Context *context) - { - ASSERT(context->getExtensions().webglCompatibility); - - const gl::DrawBufferMask &maxSet = - context->getState().getProgram()->getActiveOutputVariables(); - - gl::DrawBufferMask targetAppliedDrawBuffers = mState.getEnabledDrawBuffers() & maxSet; - if (mAppliedEnabledDrawBuffers != targetAppliedDrawBuffers) - { - maskOutInactiveOutputDrawBuffersImpl(context, targetAppliedDrawBuffers); - } - } - private: void syncClearState(const gl::Context *context, GLbitfield mask); void syncClearBufferState(const gl::Context *context, GLenum buffer, GLint drawBuffer); diff --git a/src/libANGLE/validationES.cpp b/src/libANGLE/validationES.cpp index 68749a915d8..54c54a8b581 100644 --- a/src/libANGLE/validationES.cpp +++ b/src/libANGLE/validationES.cpp @@ -362,6 +362,17 @@ bool ValidateTextureMaxAnisotropyValue(Context *context, GLfloat paramValue) return true; } +bool ValidateFragmentShaderColorBufferMaskMatch(Context *context) +{ + const Program *program = context->getState().getLinkedProgram(context); + const Framebuffer *framebuffer = context->getState().getDrawFramebuffer(); + + auto drawBufferMask = framebuffer->getDrawBufferMask().to_ulong(); + auto fragmentOutputMask = program->getActiveOutputVariables().to_ulong(); + + return drawBufferMask == (drawBufferMask & fragmentOutputMask); +} + bool ValidateFragmentShaderColorBufferTypeMatch(Context *context) { const Program *program = context->getState().getLinkedProgram(context); @@ -2767,6 +2778,12 @@ const char *ValidateDrawStates(Context *context) return kVertexShaderTypeMismatch; } + // Detect that if there's active color buffer without fragment shader output + if (!ValidateFragmentShaderColorBufferMaskMatch(context)) + { + return kDrawBufferMaskMismatch; + } + // Detect that the color buffer types match the fragment shader output types if (!ValidateFragmentShaderColorBufferTypeMatch(context)) { diff --git a/src/tests/gl_tests/DrawBuffersTest.cpp b/src/tests/gl_tests/DrawBuffersTest.cpp index d4b2bb3c30a..7553bbf4fa5 100644 --- a/src/tests/gl_tests/DrawBuffersTest.cpp +++ b/src/tests/gl_tests/DrawBuffersTest.cpp @@ -357,53 +357,6 @@ TEST_P(DrawBuffersTest, DefaultFramebufferDrawBufferQuery) EXPECT_EQ(GL_NONE, drawbuffer); } -// Tests masking out some of the draw buffers by not writing to them in the program. -TEST_P(DrawBuffersWebGL2Test, SomeProgramOutputsDisabled) -{ - ANGLE_SKIP_TEST_IF(!setupTest()); - - // TODO(ynovikov): Investigate the failure (https://anglebug.com/1533) - ANGLE_SKIP_TEST_IF(IsWindows() && IsAMD() && IsDesktopOpenGL()); - - bool flags[8] = {false}; - GLenum bufs[4] = {GL_NONE}; - - constexpr GLuint kMaxBuffers = 4; - constexpr GLuint kHalfMaxBuffers = 2; - - // Enable all draw buffers. - for (GLuint texIndex = 0; texIndex < kMaxBuffers; texIndex++) - { - glBindTexture(GL_TEXTURE_2D, mTextures[texIndex]); - glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0 + texIndex, GL_TEXTURE_2D, - mTextures[texIndex], 0); - bufs[texIndex] = GL_COLOR_ATTACHMENT0 + texIndex; - - // Mask out the first two buffers. - flags[texIndex] = texIndex >= kHalfMaxBuffers; - } - - GLuint program; - setupMRTProgram(flags, &program); - - setDrawBuffers(kMaxBuffers, bufs); - drawQuad(program, positionAttrib(), 0.5, 1.0f, true); - - for (GLuint texIndex = 0; texIndex < kHalfMaxBuffers; texIndex++) - { - verifyAttachment2DUnwritten(texIndex, mTextures[texIndex], GL_TEXTURE_2D, 0); - } - - for (GLuint texIndex = kHalfMaxBuffers; texIndex < kMaxBuffers; texIndex++) - { - verifyAttachment2D(texIndex, mTextures[texIndex], GL_TEXTURE_2D, 0); - } - - EXPECT_GL_NO_ERROR(); - - glDeleteProgram(program); -} - // Same as above but adds a state change from a program with different masks after a clear. TEST_P(DrawBuffersWebGL2Test, TwoProgramsWithDifferentOutputsAndClear) { @@ -463,7 +416,7 @@ TEST_P(DrawBuffersWebGL2Test, TwoProgramsWithDifferentOutputsAndClear) verifyAttachment2DColor(3, mTextures[3], GL_TEXTURE_2D, 0, GLColor::green); // Draw with MRT program. - setDrawBuffers(kMaxBuffers, allBufs); + setDrawBuffers(kMaxBuffers, someBufs); drawQuad(program, positionAttrib(), 0.5, 1.0f, true); ASSERT_GL_NO_ERROR(); @@ -473,6 +426,11 @@ TEST_P(DrawBuffersWebGL2Test, TwoProgramsWithDifferentOutputsAndClear) verifyAttachment2D(2, mTextures[2], GL_TEXTURE_2D, 0); verifyAttachment2D(3, mTextures[3], GL_TEXTURE_2D, 0); + // Active draw buffers with no fragment output is not allowed. + setDrawBuffers(kMaxBuffers, allBufs); + drawQuad(program, positionAttrib(), 0.5, 1.0f, true); + ASSERT_GL_ERROR(GL_INVALID_OPERATION); + // Clear again. All attachments should be cleared. glClear(GL_COLOR_BUFFER_BIT); verifyAttachment2DColor(0, mTextures[0], GL_TEXTURE_2D, 0, GLColor::green); diff --git a/src/tests/gl_tests/MultiviewDrawTest.cpp b/src/tests/gl_tests/MultiviewDrawTest.cpp index 3a71ea95c1e..27026a1ad1c 100644 --- a/src/tests/gl_tests/MultiviewDrawTest.cpp +++ b/src/tests/gl_tests/MultiviewDrawTest.cpp @@ -462,8 +462,9 @@ TEST_P(MultiviewDrawValidationTest, IndirectDraw) "#version 300 es\n" "#extension GL_OVR_multiview2 : require\n" "precision mediump float;\n" + "out vec4 color;\n" "void main()\n" - "{}\n"; + "{color = vec4(1);}\n"; GLVertexArray vao; GLBuffer vertexBuffer; @@ -544,8 +545,9 @@ TEST_P(MultiviewDrawValidationTest, NumViewsMismatch) "#version 300 es\n" "#extension GL_OVR_multiview2 : require\n" "precision mediump float;\n" + "out vec4 color;\n" "void main()\n" - "{}\n"; + "{color = vec4(1);}\n"; ANGLE_GL_PROGRAM(program, kVS, kFS); glUseProgram(program); @@ -684,6 +686,9 @@ void main() GLTexture tex2DArray; initOnePixelColorTexture2DMultiLayered(tex2DArray); + GLenum bufs[] = {GL_NONE}; + glDrawBuffers(1, bufs); + // Check that drawArrays generates an error when there is an active transform feedback object // and the number of views in the draw framebuffer is greater than 1. { @@ -777,6 +782,9 @@ TEST_P(MultiviewDrawValidationTest, ActiveTimeElapsedQuery) GLTexture tex2DArr; initOnePixelColorTexture2DMultiLayered(tex2DArr); + GLenum bufs[] = {GL_NONE}; + glDrawBuffers(1, bufs); + // Check first case. { glUseProgram(dualViewProgram); diff --git a/src/tests/gl_tests/WebGLCompatibilityTest.cpp b/src/tests/gl_tests/WebGLCompatibilityTest.cpp index adcf182e18f..5f31c5c73ef 100644 --- a/src/tests/gl_tests/WebGLCompatibilityTest.cpp +++ b/src/tests/gl_tests/WebGLCompatibilityTest.cpp @@ -4065,7 +4065,7 @@ TEST_P(WebGLCompatibilityTest, FramebufferAttachmentQuery) EXPECT_GL_ERROR(GL_INVALID_ENUM); } -// Tests the WebGL removal of undefined behavior when attachments aren't written to. +// Tests WebGL reports INVALID_OPERATION for mismatch of drawbuffers and fragment output TEST_P(WebGLCompatibilityTest, DrawBuffers) { // Make sure we can use at least 4 attachments for the tests. @@ -4152,12 +4152,12 @@ TEST_P(WebGLCompatibilityTest, DrawBuffers) GLenum halfDrawBuffers[] = { GL_NONE, + GL_COLOR_ATTACHMENT1, GL_NONE, - GL_COLOR_ATTACHMENT2, GL_COLOR_ATTACHMENT3, }; - // Test that when using gl_FragColor, only the first attachment is written to. + // Test that when using gl_FragColor with no-array const char *fragESSL1 = R"(precision highp float; void main() @@ -4167,28 +4167,10 @@ void main() ANGLE_GL_PROGRAM(programESSL1, essl1_shaders::vs::Simple(), fragESSL1); { - ClearEverythingToRed(renderbuffers); - glBindFramebuffer(GL_FRAMEBUFFER, drawFBO); DrawBuffers(useEXT, 4, allDrawBuffers); drawQuad(programESSL1, essl1_shaders::PositionAttrib(), 0.5, 1.0, true); - ASSERT_GL_NO_ERROR(); - - CheckColors(renderbuffers, 0b0001, GLColor::green); - CheckColors(renderbuffers, 0b1110, GLColor::red); - } - - // Test that when using gl_FragColor, but the first draw buffer is 0, then no attachment is - // written to. - { - ClearEverythingToRed(renderbuffers); - - glBindFramebuffer(GL_FRAMEBUFFER, drawFBO); - DrawBuffers(useEXT, 4, halfDrawBuffers); - drawQuad(programESSL1, essl1_shaders::PositionAttrib(), 0.5, 1.0, true); - ASSERT_GL_NO_ERROR(); - - CheckColors(renderbuffers, 0b1111, GLColor::red); + EXPECT_GL_ERROR(GL_INVALID_OPERATION); } // Test what happens when rendering to a subset of the outputs. There is a behavior difference @@ -4199,11 +4181,8 @@ void main() const char *positionAttrib; const char *writeOddOutputsVert; const char *writeOddOutputsFrag; - GLColor unwrittenColor; if (useEXT) { - // In the extension, when an attachment isn't written to, it should get 0's - unwrittenColor = GLColor(0, 0, 0, 0); positionAttrib = essl1_shaders::PositionAttrib(); writeOddOutputsVert = essl1_shaders::vs::Simple(); writeOddOutputsFrag = @@ -4217,9 +4196,6 @@ void main() } else { - // In ES3 if an attachment isn't declared, it shouldn't get written and should be red - // because of the preceding clears. - unwrittenColor = GLColor::red; positionAttrib = essl3_shaders::PositionAttrib(); writeOddOutputsVert = essl3_shaders::vs::Simple(); writeOddOutputsFrag = @@ -4235,21 +4211,30 @@ void main() } ANGLE_GL_PROGRAM(writeOddOutputsProgram, writeOddOutputsVert, writeOddOutputsFrag); - // Test that attachments not written to get the "unwritten" color + // Test that attachments not written to get the "unwritten" color (useEXT) + // Or INVALID_OPERATION is generated if there's active draw buffer receive no output { ClearEverythingToRed(renderbuffers); glBindFramebuffer(GL_FRAMEBUFFER, drawFBO); DrawBuffers(useEXT, 4, allDrawBuffers); drawQuad(writeOddOutputsProgram, positionAttrib, 0.5, 1.0, true); - ASSERT_GL_NO_ERROR(); - CheckColors(renderbuffers, 0b1010, GLColor::green); - CheckColors(renderbuffers, 0b0101, unwrittenColor); + if (useEXT) + { + ASSERT_GL_NO_ERROR(); + CheckColors(renderbuffers, 0b1010, GLColor::green); + // In the extension, when an attachment isn't written to, it should get 0's + CheckColors(renderbuffers, 0b0101, GLColor(0, 0, 0, 0)); + } + else + { + EXPECT_GL_ERROR(GL_INVALID_OPERATION); + } } - // Test that attachments not written to get the "unwritten" color but that even when the - // extension is used, disabled attachments are not written at all and stay red. + // Test that attachments written to get the correct color from shader output but that even when + // the extension is used, disabled attachments are not written at all and stay red. { ClearEverythingToRed(renderbuffers); @@ -4258,9 +4243,8 @@ void main() drawQuad(writeOddOutputsProgram, positionAttrib, 0.5, 1.0, true); ASSERT_GL_NO_ERROR(); - CheckColors(renderbuffers, 0b1000, GLColor::green); - CheckColors(renderbuffers, 0b0100, unwrittenColor); - CheckColors(renderbuffers, 0b0011, GLColor::red); + CheckColors(renderbuffers, 0b1010, GLColor::green); + CheckColors(renderbuffers, 0b0101, GLColor::red); } }