Reland "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.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,
@@ -90,7 +92,8 @@ class VertexArrayGL : public VertexArrayImpl
     angle::Result streamAttributes(const gl::Context *context,
                                    const gl::AttributesMask &attribsToStream,
                                    GLsizei instanceCount,
-                                   const gl::IndexRange &indexRange) const;
+                                   const gl::IndexRange &indexRange,
+                                   bool applyExtraOffsetWorkaroundForInstancedAttributes) const;
     void syncDirtyAttrib(const gl::Context *context,
                          size_t attribIndex,
                          const gl::VertexArray::DirtyAttribBits &dirtyAttribBits);
@@ -116,6 +119,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 +140,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

src/tests/gl_tests/InstancingTest.cpp

@@ -122,10 +122,6 @@ class InstancingTest : public ANGLETest
         // Unknown problem.  FL9_3 is not officially supported anyway.
         ANGLE_SKIP_TEST_IF(IsD3D11_FL93() && geometry == Quad && draw == NonIndexed);
 
-        // http://anglebug.com/5271
-        ANGLE_SKIP_TEST_IF(IsOSX() && IsIntelUHD630Mobile() && IsDesktopOpenGL() &&
-                           draw == NonIndexed && offset != 0);
-
         // The window is divided into kMaxDrawn slices of size kDrawSize.
         // The slice drawn into is determined by the instance datum.
         // The instance data array selects all the slices in order.