Workaround for Mac Intel drawArraysInstanced with first > 0

include/platform/FeaturesGL.h

@@ -522,6 +522,13 @@ struct FeaturesGL : FeatureSetBase
     Feature initFragmentOutputVariables = {
         "init_fragment_output_variables", FeatureCategory::OpenGLWorkarounds,
         "No init gl_FragColor causes context lost", &members, "http://crbug.com/1171371"};
+
+    // On macOS with Intel GPUs, instanced array with divisor > 0 is buggy when first > 0 in
+    // drawArraysInstanced. Shift the attributes with extra offset to workaround.
+    Feature shiftInstancedArrayDataWithExtraOffset = {
+        "shift_instanced_array_data_with_offset", FeatureCategory::OpenGLWorkarounds,
+        "glDrawArraysInstanced is buggy on certain new Mac Intel GPUs", &members,
+        "http://crbug.com/1144207"};
 };
 
 inline FeaturesGL::FeaturesGL()  = default;

src/libANGLE/renderer/gl/ContextGL.cpp

@@ -210,7 +210,9 @@ ANGLE_INLINE angle::Result ContextGL::setDrawArraysState(const gl::Context *cont
                                                          GLsizei count,
                                                          GLsizei instanceCount)
 {
-    if (context->getStateCache().hasAnyActiveClientAttrib())
+    const angle::FeaturesGL &features = getFeaturesGL();
+    if (context->getStateCache().hasAnyActiveClientAttrib() ||
+        (features.shiftInstancedArrayDataWithExtraOffset.enabled && first > 0))
     {
         const gl::State &glState                = context->getState();
         const gl::ProgramExecutable *executable = getState().getProgramExecutable();
@@ -224,8 +226,16 @@ ANGLE_INLINE angle::Result ContextGL::setDrawArraysState(const gl::Context *cont
         vaoGL->validateState(context);
 #endif  // ANGLE_STATE_VALIDATION_ENABLED
     }
+    else if (features.shiftInstancedArrayDataWithExtraOffset.enabled && first == 0)
+    {
+        // There could be previous draw call that has modified the attributes
+        // Instead of forcefully streaming attributes, we just rebind the original ones
+        const gl::State &glState   = context->getState();
+        const gl::VertexArray *vao = glState.getVertexArray();
+        const VertexArrayGL *vaoGL = GetImplAs<VertexArrayGL>(vao);
+        vaoGL->recoverForcedStreamingAttributesForDrawArraysInstanced(context);
+    }
 
-    const angle::FeaturesGL &features = getFeaturesGL();
     if (features.setPrimitiveRestartFixedIndexForDrawArrays.enabled)
     {
         StateManagerGL *stateManager           = getStateManager();
@@ -248,6 +258,15 @@ ANGLE_INLINE angle::Result ContextGL::setDrawElementsState(const gl::Context *co
     const gl::VertexArray *vao              = glState.getVertexArray();
     const gl::StateCache &stateCache        = context->getStateCache();
 
+    const angle::FeaturesGL &features = getFeaturesGL();
+    if (features.shiftInstancedArrayDataWithExtraOffset.enabled)
+    {
+        // There might be instanced arrays that are forced streaming for drawArraysInstanced
+        // They cannot be ELEMENT_ARRAY_BUFFER
+        const VertexArrayGL *vaoGL = GetImplAs<VertexArrayGL>(vao);
+        vaoGL->recoverForcedStreamingAttributesForDrawArraysInstanced(context);
+    }
+
     if (stateCache.hasAnyActiveClientAttrib() || vao->getElementArrayBuffer() == nullptr)
     {
         const VertexArrayGL *vaoGL = GetImplAs<VertexArrayGL>(vao);
@@ -260,7 +279,6 @@ ANGLE_INLINE angle::Result ContextGL::setDrawElementsState(const gl::Context *co
         *outIndices = indices;
     }
 
-    const angle::FeaturesGL &features = getFeaturesGL();
     if (glState.isPrimitiveRestartEnabled() && features.emulatePrimitiveRestartFixedIndex.enabled)
     {
         StateManagerGL *stateManager = getStateManager();

src/libANGLE/renderer/gl/RendererGL.cpp

@@ -457,4 +457,9 @@ bool ScopedWorkerContextGL::operator()() const
     return mValid;
 }
 
+void RendererGL::handleGPUSwitch()
+{
+    nativegl_gl::ReInitializeFeaturesAtGPUSwitch(mFunctions.get(), &mFeatures);
+}
+
 }  // namespace rx

src/libANGLE/renderer/gl/RendererGL.h

@@ -135,6 +135,8 @@ class RendererGL : angle::NonCopyable
     void setNeedsFlushBeforeDeleteTextures();
     void flushIfNecessaryBeforeDeleteTextures();
 
+    void handleGPUSwitch();
+
   protected:
     virtual WorkerContext *createWorkerContext(std::string *infoLog) = 0;
 

src/libANGLE/renderer/gl/VertexArrayGL.cpp

@@ -103,6 +103,7 @@ VertexArrayGL::VertexArrayGL(const VertexArrayState &state, GLuint id)
     {
         mAppliedAttributes.emplace_back(i);
     }
+    mForcedStreamingAttributesFirstOffsets.fill(0);
 }
 
 VertexArrayGL::~VertexArrayGL() {}
@@ -170,6 +171,7 @@ angle::Result VertexArrayGL::syncDrawState(const gl::Context *context,
     // Determine if an index buffer needs to be streamed and the range of vertices that need to be
     // copied
     IndexRange indexRange;
+    const angle::FeaturesGL &features = GetFeaturesGL(context);
     if (type != gl::DrawElementsType::InvalidEnum)
     {
         ANGLE_TRY(syncIndexData(context, count, type, indices, primitiveRestartEnabled,
@@ -180,6 +182,41 @@ angle::Result VertexArrayGL::syncDrawState(const gl::Context *context,
         // Not an indexed call, set the range to [first, first + count - 1]
         indexRange.start = first;
         indexRange.end   = first + count - 1;
+
+        if (features.shiftInstancedArrayDataWithExtraOffset.enabled && first > 0)
+        {
+            gl::AttributesMask updatedStreamingAttribsMask = needsStreamingAttribs;
+            auto candidateAttributesMask =
+                mInstancedAttributesMask & mProgramActiveAttribLocationsMask;
+            for (auto attribIndex : candidateAttributesMask)
+            {
+
+                if (mForcedStreamingAttributesFirstOffsets[attribIndex] != first)
+                {
+                    updatedStreamingAttribsMask.set(attribIndex);
+                    mForcedStreamingAttributesForDrawArraysInstancedMask.set(attribIndex);
+                    mForcedStreamingAttributesFirstOffsets[attribIndex] = first;
+                }
+            }
+
+            // We need to recover attributes whose divisor used to be > 0 but is reset to 0 now if
+            // any
+            auto forcedStreamingAttributesNeedRecoverMask =
+                candidateAttributesMask ^ mForcedStreamingAttributesForDrawArraysInstancedMask;
+            if (forcedStreamingAttributesNeedRecoverMask.any())
+            {
+                recoverForcedStreamingAttributesForDrawArraysInstanced(
+                    context, &forcedStreamingAttributesNeedRecoverMask);
+                mForcedStreamingAttributesForDrawArraysInstancedMask = candidateAttributesMask;
+            }
+
+            if (updatedStreamingAttribsMask.any())
+            {
+                ANGLE_TRY(streamAttributes(context, updatedStreamingAttribsMask, instanceCount,
+                                           indexRange));
+            }
+            return angle::Result::Continue;
+        }
     }
 
     if (needsStreamingAttribs.any())
@@ -330,8 +367,14 @@ angle::Result VertexArrayGL::streamAttributes(const gl::Context *context,
 
     // If first is greater than zero, a slack space needs to be left at the beginning of the buffer
     // so that the same 'first' argument can be passed into the draw call.
-    const size_t bufferEmptySpace   = maxAttributeDataSize * indexRange.start;
-    const size_t requiredBufferSize = streamingDataSize + bufferEmptySpace;
+    const size_t bufferEmptySpace = maxAttributeDataSize * indexRange.start;
+    size_t requiredBufferSize     = streamingDataSize + bufferEmptySpace;
+
+    const angle::FeaturesGL &features = GetFeaturesGL(context);
+    if (features.shiftInstancedArrayDataWithExtraOffset.enabled)
+    {
+        requiredBufferSize += attribsToStream.count() * bufferEmptySpace;
+    }
 
     stateManager->bindBuffer(gl::BufferBinding::Array, mStreamingArrayBuffer);
     if (requiredBufferSize > mStreamingArrayBufferSize)
@@ -364,7 +407,9 @@ angle::Result VertexArrayGL::streamAttributes(const gl::Context *context,
             const auto &binding = bindings[attrib.bindingIndex];
 
             GLuint adjustedDivisor = GetAdjustedDivisor(mAppliedNumViews, binding.getDivisor());
-            const size_t streamedVertexCount = ComputeVertexBindingElementCount(
+            // streamedVertexCount is only going to be modified by
+            // shiftInstancedArrayDataWithExtraOffset workaround, otherwise it's const
+            size_t streamedVertexCount = ComputeVertexBindingElementCount(
                 adjustedDivisor, indexRange.vertexCount(), instanceCount);
 
             const size_t sourceStride = ComputeVertexAttributeStride(attrib, binding);
@@ -378,25 +423,80 @@ angle::Result VertexArrayGL::streamAttributes(const gl::Context *context,
             // https://www.opengl.org/registry/specs/ARB/vertex_attrib_binding.txt
             const uint8_t *inputPointer = static_cast<const uint8_t *>(attrib.pointer);
 
+            size_t outputExtraOffset      = 0;
+            size_t batchMemcpyInputOffset = sourceStride * firstIndex;
+            // store batchMemcpySize since streamedVertexCount could be changed by workaround
+            size_t batchMemcpySize                           = destStride * streamedVertexCount;
+            size_t startingVertexIdxForIteratingCopy         = 0;
+            bool needsUnmapAndRebindStreamingAttributeBuffer = false;
+
+            if (features.shiftInstancedArrayDataWithExtraOffset.enabled && adjustedDivisor > 0)
+            {
+                const size_t originalStreamedVertexCount = streamedVertexCount;
+                streamedVertexCount =
+                    (instanceCount + indexRange.start + adjustedDivisor - 1u) / adjustedDivisor;
+
+                const size_t copySize =
+                    sourceStride *
+                    originalStreamedVertexCount;  // the real data in the buffer we are streaming
+
+                const gl::Buffer *bindingBufferPointer = binding.getBuffer().get();
+                if (!bindingBufferPointer)
+                {
+                    if (!inputPointer)
+                    {
+                        continue;
+                    }
+                    inputPointer = static_cast<const uint8_t *>(attrib.pointer);
+                }
+                else
+                {
+                    needsUnmapAndRebindStreamingAttributeBuffer = true;
+                    const auto buffer = GetImplAs<BufferGL>(bindingBufferPointer);
+                    stateManager->bindBuffer(gl::BufferBinding::Array, buffer->getBufferID());
+                    // The workaround is only for latest Mac Intel so glMapBufferRange should be
+                    // supported
+                    ASSERT(CanMapBufferForRead(functions));
+                    uint8_t *inputBufferPointer = MapBufferRangeWithFallback(
+                        functions, GL_ARRAY_BUFFER, binding.getOffset(), copySize, GL_MAP_READ_BIT);
+                    ASSERT(inputBufferPointer);
+                    inputPointer = inputBufferPointer;
+                }
+
+                outputExtraOffset =
+                    destStride * indexRange.start;  // only for destStride == sourceStride
+                batchMemcpyInputOffset            = 0;
+                batchMemcpySize                   = copySize;
+                startingVertexIdxForIteratingCopy = indexRange.start;
+            }
+
             // Pack the data when copying it, user could have supplied a very large stride that
             // would cause the buffer to be much larger than needed.
             if (destStride == sourceStride)
             {
                 // Can copy in one go, the data is packed
-                memcpy(bufferPointer + curBufferOffset, inputPointer + (sourceStride * firstIndex),
-                       destStride * streamedVertexCount);
+                memcpy(bufferPointer + curBufferOffset + outputExtraOffset,
+                       inputPointer + batchMemcpyInputOffset, batchMemcpySize);
             }
             else
             {
-                // Copy each vertex individually
-                for (size_t vertexIdx = 0; vertexIdx < streamedVertexCount; vertexIdx++)
+                for (size_t vertexIdx = 0;
+                     vertexIdx < streamedVertexCount - startingVertexIdxForIteratingCopy;
+                     vertexIdx++)
                 {
-                    uint8_t *out      = bufferPointer + curBufferOffset + (destStride * vertexIdx);
+                    uint8_t *out = bufferPointer + curBufferOffset + outputExtraOffset +
+                                   (destStride * vertexIdx);
                     const uint8_t *in = inputPointer + sourceStride * (vertexIdx + firstIndex);
                     memcpy(out, in, destStride);
                 }
             }
 
+            if (needsUnmapAndRebindStreamingAttributeBuffer)
+            {
+                ANGLE_GL_TRY(context, functions->unmapBuffer(GL_ARRAY_BUFFER));
+                stateManager->bindBuffer(gl::BufferBinding::Array, mStreamingArrayBuffer);
+            }
+
             // Compute where the 0-index vertex would be.
             const size_t vertexStartOffset = curBufferOffset - (firstIndex * destStride);
 
@@ -425,6 +525,56 @@ angle::Result VertexArrayGL::streamAttributes(const gl::Context *context,
     return angle::Result::Continue;
 }
 
+void VertexArrayGL::recoverForcedStreamingAttributesForDrawArraysInstanced(
+    const gl::Context *context) const
+{
+    recoverForcedStreamingAttributesForDrawArraysInstanced(
+        context, &mForcedStreamingAttributesForDrawArraysInstancedMask);
+}
+
+void VertexArrayGL::recoverForcedStreamingAttributesForDrawArraysInstanced(
+    const gl::Context *context,
+    gl::AttributesMask *attributeMask) const
+{
+    if (attributeMask->none())
+    {
+        return;
+    }
+
+    StateManagerGL *stateManager = GetStateManagerGL(context);
+
+    stateManager->bindVertexArray(mVertexArrayID, getAppliedElementArrayBufferID());
+
+    const auto &attribs  = mState.getVertexAttributes();
+    const auto &bindings = mState.getVertexBindings();
+    for (auto idx : *attributeMask)
+    {
+        const auto &attrib = attribs[idx];
+        ASSERT(IsVertexAttribPointerSupported(idx, attrib));
+
+        const auto &binding = bindings[attrib.bindingIndex];
+        const auto buffer   = GetImplAs<BufferGL>(binding.getBuffer().get());
+        stateManager->bindBuffer(gl::BufferBinding::Array, buffer->getBufferID());
+
+        callVertexAttribPointer(context, static_cast<GLuint>(idx), attrib,
+                                static_cast<GLsizei>(binding.getStride()),
+                                static_cast<GLintptr>(binding.getOffset()));
+
+        // Restore the state to track their original buffers
+        mAppliedAttributes[idx].format = attrib.format;
+
+        mAppliedAttributes[idx].relativeOffset = 0;
+        mAppliedAttributes[idx].bindingIndex   = static_cast<GLuint>(attrib.bindingIndex);
+
+        mAppliedBindings[idx].setStride(binding.getStride());
+        mAppliedBindings[idx].setOffset(binding.getOffset());
+        mAppliedBindings[idx].setBuffer(context, binding.getBuffer().get());
+    }
+
+    attributeMask->reset();
+    mForcedStreamingAttributesFirstOffsets.fill(0);
+}
+
 GLuint VertexArrayGL::getVertexArrayID() const
 {
     return mVertexArrayID;
@@ -648,6 +798,16 @@ void VertexArrayGL::updateBindingDivisor(const gl::Context *context, size_t bind
     }
 
     mAppliedBindings[bindingIndex].setDivisor(adjustedDivisor);
+
+    if (adjustedDivisor > 0)
+    {
+        mInstancedAttributesMask.set(bindingIndex);
+    }
+    else if (mInstancedAttributesMask.test(bindingIndex))
+    {
+        // divisor is reset to 0
+        mInstancedAttributesMask.reset(bindingIndex);
+    }
 }
 
 void VertexArrayGL::syncDirtyAttrib(const gl::Context *context,

src/libANGLE/renderer/gl/VertexArrayGL.h

@@ -57,6 +57,8 @@ class VertexArrayGL : public VertexArrayImpl
 
     void validateState(const gl::Context *context) const;
 
+    void recoverForcedStreamingAttributesForDrawArraysInstanced(const gl::Context *context) const;
+
   private:
     angle::Result syncDrawState(const gl::Context *context,
                                 const gl::AttributesMask &activeAttributesMask,
@@ -116,6 +118,10 @@ class VertexArrayGL : public VertexArrayImpl
                                  GLsizei stride,
                                  GLintptr offset) const;
 
+    void recoverForcedStreamingAttributesForDrawArraysInstanced(
+        const gl::Context *context,
+        gl::AttributesMask *attributeMask) const;
+
     GLuint mVertexArrayID;
     int mAppliedNumViews;
 
@@ -133,6 +139,11 @@ class VertexArrayGL : public VertexArrayImpl
 
     mutable size_t mStreamingArrayBufferSize;
     mutable GLuint mStreamingArrayBuffer;
+
+    // Used for Mac Intel instanced draw workaround
+    mutable gl::AttributesMask mForcedStreamingAttributesForDrawArraysInstancedMask;
+    mutable gl::AttributesMask mInstancedAttributesMask;
+    mutable std::array<GLint, gl::MAX_VERTEX_ATTRIBS> mForcedStreamingAttributesFirstOffsets;
 };
 
 ANGLE_INLINE angle::Result VertexArrayGL::syncDrawElementsState(

src/libANGLE/renderer/gl/cgl/DisplayCGL.mm

@@ -658,6 +658,8 @@ egl::Error DisplayCGL::handleGPUSwitch()
             CGLSetCurrentContext(mContext);
             onStateChange(angle::SubjectMessage::SubjectChanged);
             mCurrentGPUID = gpuID;
+
+            mRenderer->handleGPUSwitch();
         }
     }
 

src/libANGLE/renderer/gl/renderergl_utils.cpp

@@ -1647,23 +1647,33 @@ void GenerateCaps(const FunctionsGL *functions,
     extensions->yuvTargetEXT = functions->hasGLESExtension("GL_EXT_YUV_target");
 }
 
-void InitializeFeatures(const FunctionsGL *functions, angle::FeaturesGL *features)
+bool GetSystemInfoVendorIDAndDeviceID(const FunctionsGL *functions,
+                                      angle::SystemInfo *outSystemInfo,
+                                      angle::VendorID *outVendor,
+                                      angle::DeviceID *outDevice)
 {
-    angle::VendorID vendor;
-    angle::DeviceID device;
-
-    angle::SystemInfo systemInfo;
-    bool isGetSystemInfoSuccess = angle::GetSystemInfo(&systemInfo);
-    if (isGetSystemInfoSuccess && !systemInfo.gpus.empty())
+    bool isGetSystemInfoSuccess = angle::GetSystemInfo(outSystemInfo);
+    if (isGetSystemInfoSuccess && !outSystemInfo->gpus.empty())
     {
-        vendor = systemInfo.gpus[systemInfo.activeGPUIndex].vendorId;
-        device = systemInfo.gpus[systemInfo.activeGPUIndex].deviceId;
+        *outVendor = outSystemInfo->gpus[outSystemInfo->activeGPUIndex].vendorId;
+        *outDevice = outSystemInfo->gpus[outSystemInfo->activeGPUIndex].deviceId;
     }
     else
     {
-        vendor = GetVendorID(functions);
-        device = GetDeviceID(functions);
+        *outVendor = GetVendorID(functions);
+        *outDevice = GetDeviceID(functions);
     }
+    return isGetSystemInfoSuccess;
+}
+
+void InitializeFeatures(const FunctionsGL *functions, angle::FeaturesGL *features)
+{
+    angle::VendorID vendor;
+    angle::DeviceID device;
+    angle::SystemInfo systemInfo;
+
+    bool isGetSystemInfoSuccess =
+        GetSystemInfoVendorIDAndDeviceID(functions, &systemInfo, &vendor, &device);
 
     bool isAMD      = IsAMD(vendor);
     bool isIntel    = IsIntel(vendor);
@@ -1931,6 +1941,12 @@ void InitializeFeatures(const FunctionsGL *functions, angle::FeaturesGL *feature
     // If output variable gl_FragColor is written by fragment shader, it may cause context lost with
     // Adreno 42x and 3xx.
     ANGLE_FEATURE_CONDITION(features, initFragmentOutputVariables, IsAdreno42xOr3xx(functions));
+
+    // http://crbug.com/1144207
+    // The Mac bot with Intel Iris GPU seems unaffected by this bug. Exclude the Haswell family for
+    // now.
+    ANGLE_FEATURE_CONDITION(features, shiftInstancedArrayDataWithExtraOffset,
+                            IsApple() && IsIntel(vendor) && !IsHaswell(device));
 }
 
 void InitializeFrontendFeatures(const FunctionsGL *functions, angle::FrontendFeatures *features)
@@ -1943,6 +1959,22 @@ void InitializeFrontendFeatures(const FunctionsGL *functions, angle::FrontendFea
     ANGLE_FEATURE_CONDITION(features, syncFramebufferBindingsOnTexImage, false);
 }
 
+void ReInitializeFeaturesAtGPUSwitch(const FunctionsGL *functions, angle::FeaturesGL *features)
+{
+    angle::VendorID vendor;
+    angle::DeviceID device;
+    angle::SystemInfo systemInfo;
+
+    GetSystemInfoVendorIDAndDeviceID(functions, &systemInfo, &vendor, &device);
+
+    // http://crbug.com/1144207
+    // The Mac bot with Intel Iris GPU seems unaffected by this bug. Exclude the Haswell family for
+    // now.
+    // We need to reinitialize this feature when switching between buggy and non-buggy GPUs.
+    ANGLE_FEATURE_CONDITION(features, shiftInstancedArrayDataWithExtraOffset,
+                            IsApple() && IsIntel(vendor) && !IsHaswell(device));
+}
+
 }  // namespace nativegl_gl
 
 namespace nativegl

src/libANGLE/renderer/gl/renderergl_utils.h

@@ -106,6 +106,7 @@ void GenerateCaps(const FunctionsGL *functions,
 
 void InitializeFeatures(const FunctionsGL *functions, angle::FeaturesGL *features);
 void InitializeFrontendFeatures(const FunctionsGL *functions, angle::FrontendFeatures *features);
+void ReInitializeFeaturesAtGPUSwitch(const FunctionsGL *functions, angle::FeaturesGL *features);
 }  // namespace nativegl_gl
 
 namespace nativegl