Optimize Fragment Shader Type Match Validation

src/libANGLE/ErrorStrings.h

@@ -26,6 +26,8 @@ ERRMSG(CubemapIncomplete,
 ERRMSG(DefaultFramebufferInvalidAttachment,
        "Invalid attachment when the default framebuffer is bound.");
 ERRMSG(DefaultFramebufferTarget, "It is invalid to change default FBO's attachments");
+ERRMSG(DrawBufferTypeMismatch,
+       "Fragment shader output type does not match the bound framebuffer attachment type.");
 ERRMSG(EnumNotSupported, "Enum is not currently supported.");
 ERRMSG(EnumRequiresGLES31, "Enum requires GLES 3.1");
 ERRMSG(ES31Required, "OpenGL ES 3.1 Required");

src/libANGLE/Framebuffer.cpp

@@ -19,6 +19,7 @@
 #include "libANGLE/Renderbuffer.h"
 #include "libANGLE/Surface.h"
 #include "libANGLE/Texture.h"
+#include "libANGLE/angletypes.h"
 #include "libANGLE/formatutils.h"
 #include "libANGLE/renderer/ContextImpl.h"
 #include "libANGLE/renderer/FramebufferImpl.h"
@@ -260,6 +261,7 @@ FramebufferState::FramebufferState()
       mColorAttachments(1),
       mDrawBufferStates(1, GL_BACK),
       mReadBufferState(GL_BACK),
+      mDrawBufferTypeMask(),
       mDefaultWidth(0),
       mDefaultHeight(0),
       mDefaultSamples(0),
@@ -275,6 +277,7 @@ FramebufferState::FramebufferState(const Caps &caps)
       mColorAttachments(caps.maxColorAttachments),
       mDrawBufferStates(caps.maxDrawBuffers, GL_NONE),
       mReadBufferState(GL_COLOR_ATTACHMENT0_EXT),
+      mDrawBufferTypeMask(),
       mDefaultWidth(0),
       mDefaultHeight(0),
       mDefaultSamples(0),
@@ -621,6 +624,7 @@ Framebuffer::Framebuffer(const egl::Display *display, egl::Surface *surface)
                           FramebufferAttachment::kDefaultMultiviewLayout,
                           FramebufferAttachment::kDefaultViewportOffsets);
     }
+    mState.mDrawBufferTypeMask.setIndex(getDrawbufferWriteType(0), 0);
 }
 
 Framebuffer::Framebuffer(rx::GLImplFactory *factory)
@@ -632,6 +636,7 @@ Framebuffer::Framebuffer(rx::GLImplFactory *factory)
       mDirtyStencilAttachmentBinding(this, DIRTY_BIT_STENCIL_ATTACHMENT)
 {
     mDirtyColorAttachmentBindings.emplace_back(this, DIRTY_BIT_COLOR_ATTACHMENT_0);
+    mState.mDrawBufferTypeMask.setIndex(getDrawbufferWriteType(0), 0);
 }
 
 Framebuffer::~Framebuffer()
@@ -823,8 +828,12 @@ void Framebuffer::setDrawBuffers(size_t count, const GLenum *buffers)
     mDirtyBits.set(DIRTY_BIT_DRAW_BUFFERS);
 
     mState.mEnabledDrawBuffers.reset();
+    mState.mDrawBufferTypeMask.reset();
+
     for (size_t index = 0; index < count; ++index)
     {
+        mState.mDrawBufferTypeMask.setIndex(getDrawbufferWriteType(index), index);
+
         if (drawStates[index] != GL_NONE && mState.mColorAttachments[index].isAttached())
         {
             mState.mEnabledDrawBuffers.set(index);
@@ -857,6 +866,16 @@ GLenum Framebuffer::getDrawbufferWriteType(size_t drawBuffer) const
     }
 }
 
+DrawBufferTypeMask Framebuffer::getDrawBufferTypeMask() const
+{
+    return mState.mDrawBufferTypeMask;
+}
+
+DrawBufferMask Framebuffer::getDrawBufferMask() const
+{
+    return mState.mEnabledDrawBuffers;
+}
+
 bool Framebuffer::hasEnabledDrawBuffer() const
 {
     for (size_t drawbufferIdx = 0; drawbufferIdx < mState.mDrawBufferStates.size(); ++drawbufferIdx)
@@ -1689,6 +1708,7 @@ void Framebuffer::setAttachmentImpl(const Context *context,
             // formsRenderingFeedbackLoopWith
             bool enabled = (type != GL_NONE && getDrawBufferState(colorIndex) != GL_NONE);
             mState.mEnabledDrawBuffers.set(colorIndex, enabled);
+            mState.mDrawBufferTypeMask.setIndex(getDrawbufferWriteType(colorIndex), colorIndex);
         }
         break;
     }

src/libANGLE/Framebuffer.h

@@ -111,6 +111,7 @@ class FramebufferState final : angle::NonCopyable
     std::vector<GLenum> mDrawBufferStates;
     GLenum mReadBufferState;
     DrawBufferMask mEnabledDrawBuffers;
+    DrawBufferTypeMask mDrawBufferTypeMask;
 
     GLint mDefaultWidth;
     GLint mDefaultHeight;
@@ -196,6 +197,8 @@ class Framebuffer final : public LabeledObject, public OnAttachmentDirtyReceiver
     void setDrawBuffers(size_t count, const GLenum *buffers);
     const FramebufferAttachment *getDrawBuffer(size_t drawBuffer) const;
     GLenum getDrawbufferWriteType(size_t drawBuffer) const;
+    DrawBufferTypeMask getDrawBufferTypeMask() const;
+    DrawBufferMask getDrawBufferMask() const;
     bool hasEnabledDrawBuffer() const;
 
     GLenum getReadBufferState() const;

src/libANGLE/MemoryProgramCache.cpp

@@ -371,6 +371,11 @@ LinkResult MemoryProgramCache::Deserialize(const Context *context,
     {
         state->mOutputVariableTypes.push_back(stream.readInt<GLenum>());
     }
+
+    static_assert(IMPLEMENTATION_MAX_DRAW_BUFFER_TYPE_MASK == 8 * sizeof(uint16_t),
+                  "All bits of DrawBufferTypeMask can be contained in an uint16_t");
+    state->mDrawBufferTypeMask.from_ulong(stream.readInt<uint16_t>());
+
     static_assert(IMPLEMENTATION_MAX_DRAW_BUFFERS < 8 * sizeof(uint32_t),
                   "All bits of DrawBufferMask can be contained in an uint32_t");
     state->mActiveOutputVariables = stream.readInt<uint32_t>();
@@ -541,6 +546,10 @@ void MemoryProgramCache::Serialize(const Context *context,
         stream.writeInt(outputVariableType);
     }
 
+    static_assert(IMPLEMENTATION_MAX_DRAW_BUFFER_TYPE_MASK == 8 * sizeof(uint16_t),
+                  "All bits of DrawBufferTypeMask can be contained in an uint16_t");
+    stream.writeInt(static_cast<uint32_t>(state.mDrawBufferTypeMask.to_ulong()));
+
     static_assert(IMPLEMENTATION_MAX_DRAW_BUFFERS < 8 * sizeof(uint32_t),
                   "All bits of DrawBufferMask can be contained in an uint32_t");
     stream.writeInt(static_cast<uint32_t>(state.mActiveOutputVariables.to_ulong()));

src/libANGLE/Program.cpp

@@ -1008,6 +1008,7 @@ void Program::unlink()
     mState.mOutputVariables.clear();
     mState.mOutputLocations.clear();
     mState.mOutputVariableTypes.clear();
+    mState.mDrawBufferTypeMask.reset();
     mState.mActiveOutputVariables.reset();
     mState.mComputeShaderLocalSize.fill(1);
     mState.mSamplerBindings.clear();
@@ -2803,7 +2804,6 @@ ProgramMergedVaryings Program::getMergedVaryings(const Context *context) const
     return merged;
 }
 
-
 void Program::linkOutputVariables(const Context *context)
 {
     Shader *fragmentShader = mState.mAttachedFragmentShader;
@@ -2811,6 +2811,7 @@ void Program::linkOutputVariables(const Context *context)
 
     ASSERT(mState.mOutputVariableTypes.empty());
     ASSERT(mState.mActiveOutputVariables.none());
+    ASSERT(mState.mDrawBufferTypeMask.none());
 
     // Gather output variable types
     for (const auto &outputVariable : fragmentShader->getActiveOutputVariables(context))
@@ -2838,6 +2839,7 @@ void Program::linkOutputVariables(const Context *context)
             ASSERT(location < mState.mActiveOutputVariables.size());
             mState.mActiveOutputVariables.set(location);
             mState.mOutputVariableTypes[location] = VariableComponentType(outputVariable.type);
+            mState.mDrawBufferTypeMask.setIndex(mState.mOutputVariableTypes[location], location);
         }
     }
 

src/libANGLE/Program.h

@@ -369,6 +369,7 @@ class ProgramState final : angle::NonCopyable
 
     // Fragment output variable base types: FLOAT, INT, or UINT.  Ordered by location.
     std::vector<GLenum> mOutputVariableTypes;
+    DrawBufferTypeMask mDrawBufferTypeMask;
 
     bool mBinaryRetrieveableHint;
     bool mSeparable;
@@ -627,6 +628,8 @@ class Program final : angle::NonCopyable, public LabeledObject
     int getNumViews() const { return mState.getNumViews(); }
     bool usesMultiview() const { return mState.usesMultiview(); }
 
+    DrawBufferTypeMask getDrawBufferTypeMask() const { return mState.mDrawBufferTypeMask; }
+
   private:
     ~Program() override;
 

src/libANGLE/angletypes.cpp

@@ -256,4 +256,98 @@ bool operator!=(const Extents &lhs, const Extents &rhs)
 {
     return !(lhs == rhs);
 }
+
+DrawBufferTypeMask::DrawBufferTypeMask()
+{
+    mTypeMask.reset();
+}
+
+DrawBufferTypeMask::DrawBufferTypeMask(const DrawBufferTypeMask &other) = default;
+
+DrawBufferTypeMask::~DrawBufferTypeMask() = default;
+
+void DrawBufferTypeMask::reset()
+{
+    mTypeMask.reset();
+}
+
+bool DrawBufferTypeMask::none()
+{
+    if (mTypeMask.none())
+    {
+        return true;
+    }
+
+    return false;
+}
+
+void DrawBufferTypeMask::setIndex(GLenum type, size_t index)
+{
+    ASSERT(index <= IMPLEMENTATION_MAX_DRAW_BUFFERS);
+
+    mTypeMask &= ~(0x101 << index);
+
+    uint16_t m = 0;
+    switch (type)
+    {
+        case GL_INT:
+            m = 0x001;
+            break;
+        case GL_UNSIGNED_INT:
+            m = 0x100;
+            break;
+        case GL_FLOAT:
+            m = 0x101;
+            break;
+        case GL_NONE:
+            m = 0x000;
+            break;
+        default:
+            UNREACHABLE();
+    }
+
+    mTypeMask |= m << index;
+}
+
+unsigned long DrawBufferTypeMask::to_ulong() const
+{
+    return mTypeMask.to_ulong();
+}
+
+void DrawBufferTypeMask::from_ulong(unsigned long mask)
+{
+    mTypeMask = mask;
+}
+
+bool DrawBufferTypeMask::ProgramOutputsMatchFramebuffer(DrawBufferTypeMask outputTypes,
+                                                        DrawBufferTypeMask inputTypes,
+                                                        DrawBufferMask outputMask,
+                                                        DrawBufferMask inputMask)
+{
+    static_assert(IMPLEMENTATION_MAX_DRAW_BUFFER_TYPE_MASK == 16,
+                  "Draw buffer type masks should fit into 16 bits. 2 bits per draw buffer.");
+    static_assert(IMPLEMENTATION_MAX_DRAW_BUFFERS == 8,
+                  "Output/Input masks should fit into 8 bits. 1 bit per draw buffer");
+
+    // For performance reasons, draw buffer type validation is done using bit masks. We store two
+    // bits representing the type split, with the low bit in the lower 8 bits of the variable,
+    // and the high bit in the upper 8 bits of the variable. This is done so we can AND with the
+    // elswewhere used DrawBufferMask.
+    const unsigned long outputTypeBits = outputTypes.to_ulong();
+    const unsigned long inputTypeBits  = inputTypes.to_ulong();
+
+    unsigned long outputMaskBits = outputMask.to_ulong();
+    unsigned long inputMaskBits  = inputMask.to_ulong();
+
+    // OR the masks with themselves, shifted 8 bits. This is to match our split type bits.
+    outputMaskBits |= (outputMaskBits << 8);
+    inputMaskBits |= (inputMaskBits << 8);
+
+    // To validate:
+    // 1. Remove any indexes that are not enabled in the framebuffer (& inputMask)
+    // 2. Remove any indexes that exist in program, but not in framebuffer (& outputMask)
+    // 3. Use XOR to check for a match
+    return (outputTypeBits & inputMaskBits) == ((inputTypeBits & outputMaskBits) & inputMaskBits);
+}
+
 }  // namespace gl

src/libANGLE/angletypes.h

@@ -288,8 +288,27 @@ using AttributesMask = angle::BitSet<MAX_VERTEX_ATTRIBS>;
 // Used in Program
 using UniformBlockBindingMask = angle::BitSet<IMPLEMENTATION_MAX_COMBINED_SHADER_UNIFORM_BUFFERS>;
 
-// Used in Framebuffer
+// Used in Framebuffer / Program
 using DrawBufferMask = angle::BitSet<IMPLEMENTATION_MAX_DRAW_BUFFERS>;
+constexpr int IMPLEMENTATION_MAX_DRAW_BUFFER_TYPE_MASK = 16;
+struct DrawBufferTypeMask final
+{
+    DrawBufferTypeMask();
+    DrawBufferTypeMask(const DrawBufferTypeMask &other);
+    ~DrawBufferTypeMask();
+    void reset();
+    bool none();
+    void setIndex(GLenum type, size_t index);
+    unsigned long to_ulong() const;
+    void from_ulong(unsigned long mask);
+    static bool ProgramOutputsMatchFramebuffer(DrawBufferTypeMask outputTypes,
+                                               DrawBufferTypeMask inputTypes,
+                                               DrawBufferMask outputMask,
+                                               DrawBufferMask inputMask);
+
+  private:
+    angle::BitSet<IMPLEMENTATION_MAX_DRAW_BUFFER_TYPE_MASK> mTypeMask;
+};
 
 using ContextID = uintptr_t;
 

src/libANGLE/validationES.cpp

@@ -397,18 +397,12 @@ bool ValidateFragmentShaderColorBufferTypeMatch(ValidationContext *context)
     const Program *program         = context->getGLState().getProgram();
     const Framebuffer *framebuffer = context->getGLState().getDrawFramebuffer();
 
-    const auto &programOutputTypes = program->getOutputVariableTypes();
-    for (size_t drawBufferIdx = 0; drawBufferIdx < programOutputTypes.size(); drawBufferIdx++)
+    if (!DrawBufferTypeMask::ProgramOutputsMatchFramebuffer(
+            program->getDrawBufferTypeMask(), framebuffer->getDrawBufferTypeMask(),
+            program->getActiveOutputVariables(), framebuffer->getDrawBufferMask()))
     {
-        GLenum outputType = programOutputTypes[drawBufferIdx];
-        GLenum inputType  = framebuffer->getDrawbufferWriteType(drawBufferIdx);
-        if (outputType != GL_NONE && inputType != GL_NONE && inputType != outputType)
-        {
-            context->handleError(InvalidOperation() << "Fragment shader output type does not "
-                                                       "match the bound framebuffer attachment "
-                                                       "type.");
-            return false;
-        }
+        ANGLE_VALIDATION_ERR(context, InvalidOperation(), DrawBufferTypeMismatch);
+        return false;
     }
 
     return true;