Work around Intel driver bug with CopyTex{Sub}Image2D/DeleteTextures.

Dependencies seem to be incorrectly tracked in some Intel OpenGL
drivers (on macOS specifically), causing crashes in glDeleteTextures
if a GL command buffer is being constructed where those textures are
destinations of CopyTexImage2D/CopyTexSubImage2D. Work around this bug
by flushing before texture deletion if CopyTex{Sub}Image have been
called recently. The tracking is only done on a per-context rather
than a per-device basis, but seems sufficient to work around the
problem as identified.

Tested both with new ANGLE test on affected hardware, and in WebKit's
ANGLE backend for WebGL. Works around the crash reported in
https://bugs.webkit.org/show_bug.cgi?id=205707 .

Bug: angleproject:4267
Change-Id: I2266a5590759f6a3f19080def08710ef4b66d463
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/1987932
Commit-Queue: Kenneth Russell <kbr@chromium.org>
Reviewed-by: Geoff Lang <geofflang@chromium.org>
Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>

include/platform/FeaturesGL.h

@@ -394,6 +394,16 @@ struct FeaturesGL : FeatureSetBase
         "Quite some OpenGL ES drivers don't implement readPixels for RGBA/UNSIGNED_SHORT from "
         "EXT_texture_norm16 correctly",
         &members, "http://anglebug.com/4214"};
+
+    // Bugs exist in some Intel drivers where dependencies are incorrectly
+    // tracked for textures which are copy destinations (via CopyTexImage2D, for
+    // example). Flush before DeleteTexture if these entry points have been
+    // called recently.
+    Feature flushBeforeDeleteTextureIfCopiedTo = {
+        "flush_before_delete_texture_if_copied_to", FeatureCategory::OpenGLWorkarounds,
+        "Some drivers track CopyTex{Sub}Image texture dependencies incorrectly. Flush"
+        " before glDeleteTextures in this case",
+        &members, "http://anglebug.com/4267"};
 };
 
 inline FeaturesGL::FeaturesGL()  = default;

src/libANGLE/renderer/gl/ContextGL.cpp

@@ -901,4 +901,15 @@ void ContextGL::validateState() const
     stateManager->validateState();
 }
 
+void ContextGL::setNeedsFlushBeforeDeleteTextures()
+{
+    mRenderer->setNeedsFlushBeforeDeleteTextures();
+}
+
+void ContextGL::flushIfNecessaryBeforeDeleteTextures()
+{
+    mRenderer->flushIfNecessaryBeforeDeleteTextures();
+}
+
+
 }  // namespace rx

src/libANGLE/renderer/gl/ContextGL.h

@@ -265,6 +265,9 @@ class ContextGL : public ContextImpl
 
     void validateState() const;
 
+    void setNeedsFlushBeforeDeleteTextures();
+    void flushIfNecessaryBeforeDeleteTextures();
+
   private:
     angle::Result setDrawArraysState(const gl::Context *context,
                                      GLint first,

src/libANGLE/renderer/gl/RendererGL.cpp

@@ -284,6 +284,7 @@ RendererGL::~RendererGL()
 angle::Result RendererGL::flush()
 {
     mFunctions->flush();
+    mNeedsFlushBeforeDeleteTextures = false;
     return angle::Result::Continue;
 }
 
@@ -295,6 +296,7 @@ angle::Result RendererGL::finish()
     }
 
     mFunctions->finish();
+    mNeedsFlushBeforeDeleteTextures = false;
 
     if (mFeatures.finishDoesNotCauseQueriesToBeAvailable.enabled && mUseDebugOutput)
     {
@@ -682,6 +684,19 @@ void RendererGL::setMaxShaderCompilerThreads(GLuint count)
     }
 }
 
+void RendererGL::setNeedsFlushBeforeDeleteTextures()
+{
+    mNeedsFlushBeforeDeleteTextures = true;
+}
+
+void RendererGL::flushIfNecessaryBeforeDeleteTextures()
+{
+    if (mNeedsFlushBeforeDeleteTextures)
+    {
+        (void) flush();
+    }
+}
+
 ScopedWorkerContextGL::ScopedWorkerContextGL(RendererGL *renderer, std::string *infoLog)
     : mRenderer(renderer)
 {

src/libANGLE/renderer/gl/RendererGL.h

@@ -194,6 +194,9 @@ class RendererGL : angle::NonCopyable
 
     static unsigned int getMaxWorkerContexts();
 
+    void setNeedsFlushBeforeDeleteTextures();
+    void flushIfNecessaryBeforeDeleteTextures();
+
   protected:
     virtual WorkerContext *createWorkerContext(std::string *infoLog) = 0;
 
@@ -231,6 +234,9 @@ class RendererGL : angle::NonCopyable
     bool mNativeParallelCompileEnabled;
 
     angle::FeaturesGL mFeatures;
+
+    // Workaround for anglebug.com/4267
+    bool mNeedsFlushBeforeDeleteTextures;
 };
 
 }  // namespace rx

src/libANGLE/renderer/gl/TextureGL.cpp

@@ -151,6 +151,7 @@ TextureGL::~TextureGL()
 
 void TextureGL::onDestroy(const gl::Context *context)
 {
+    GetImplAs<ContextGL>(context)->flushIfNecessaryBeforeDeleteTextures();
     StateManagerGL *stateManager = GetStateManagerGL(context);
     stateManager->deleteTexture(mTextureID);
     mTextureID = 0;
@@ -734,6 +735,11 @@ angle::Result TextureGL::copyImage(const gl::Context *context,
         setLevelInfo(context, target, level, 1, levelInfo);
     }
 
+    if (features.flushBeforeDeleteTextureIfCopiedTo.enabled)
+    {
+        contextGL->setNeedsFlushBeforeDeleteTextures();
+    }
+
     return angle::Result::Continue;
 }
 
@@ -745,6 +751,7 @@ angle::Result TextureGL::copySubImage(const gl::Context *context,
 {
     const FunctionsGL *functions = GetFunctionsGL(context);
     StateManagerGL *stateManager = GetStateManagerGL(context);
+    const angle::FeaturesGL &features = GetFeaturesGL(context);
 
     gl::TextureTarget target                 = index.getTarget();
     size_t level                             = static_cast<size_t>(index.getLevelIndex());
@@ -792,6 +799,12 @@ angle::Result TextureGL::copySubImage(const gl::Context *context,
         }
     }
 
+    if (features.flushBeforeDeleteTextureIfCopiedTo.enabled)
+    {
+        ContextGL *contextGL = GetImplAs<ContextGL>(context);
+        contextGL->setNeedsFlushBeforeDeleteTextures();
+    }
+
     return angle::Result::Continue;
 }
 
@@ -852,13 +865,22 @@ angle::Result TextureGL::copySubTextureHelper(const gl::Context *context,
                                               bool unpackUnmultiplyAlpha,
                                               const gl::Texture *source)
 {
-    const FunctionsGL *functions = GetFunctionsGL(context);
+    const FunctionsGL *functions      = GetFunctionsGL(context);
-    BlitGL *blitter              = GetBlitGL(context);
+    const angle::FeaturesGL &features = GetFeaturesGL(context);
+    BlitGL *blitter                   = GetBlitGL(context);
 
     TextureGL *sourceGL = GetImplAs<TextureGL>(source);
     const gl::ImageDesc &sourceImageDesc =
         sourceGL->mState.getImageDesc(NonCubeTextureTypeToTarget(source->getType()), sourceLevel);
 
+    if (features.flushBeforeDeleteTextureIfCopiedTo.enabled)
+    {
+        // Conservatively indicate that this workaround is necessary. Not clear
+        // if it is on this code path, but added for symmetry.
+        ContextGL *contextGL = GetImplAs<ContextGL>(context);
+        contextGL->setNeedsFlushBeforeDeleteTextures();
+    }
+
     // Check is this is a simple copySubTexture that can be done with a copyTexSubImage
     ASSERT(sourceGL->getType() == gl::TextureType::_2D ||
            source->getType() == gl::TextureType::External ||

src/libANGLE/renderer/gl/renderergl_utils.cpp

@@ -1633,6 +1633,9 @@ void InitializeFeatures(const FunctionsGL *functions, angle::FeaturesGL *feature
                             functions->standard == STANDARD_GL_ES &&
                                 functions->isAtLeastGLES(gl::Version(3, 1)) &&
                                 functions->hasGLESExtension("GL_EXT_texture_norm16"));
+
+    // anglebug.com/4267
+    ANGLE_FEATURE_CONDITION(features, flushBeforeDeleteTextureIfCopiedTo, IsApple() && isIntel);
 }
 
 void InitializeFrontendFeatures(const FunctionsGL *functions, angle::FrontendFeatures *features)

src/tests/gl_tests/CopyTexImageTest.cpp

@@ -482,6 +482,53 @@ TEST_P(CopyTexImageTest, CopyTexSubImageToNonCubeCompleteDestination)
     }
 }
 
+// Deleting textures after copying to them. http://anglebug.com/4267
+TEST_P(CopyTexImageTest, DeleteAfterCopyingToTextures)
+{
+    // Asserts on Vulkan backend. http://anglebug.com/4274
+    ANGLE_SKIP_TEST_IF(IsVulkan());
+
+    GLTexture texture;
+    glBindTexture(GL_TEXTURE_2D, texture);
+    glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA, 2, 2, 0, GL_RGBA, GL_UNSIGNED_BYTE, nullptr);
+    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST);
+    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST);
+    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_S, GL_CLAMP_TO_EDGE);
+    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_T, GL_CLAMP_TO_EDGE);
+
+    GLTexture texture2;
+    glBindTexture(GL_TEXTURE_2D, texture2);
+    glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA, 2, 2, 0, GL_RGBA, GL_UNSIGNED_BYTE, nullptr);
+    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST);
+    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST);
+    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_S, GL_CLAMP_TO_EDGE);
+    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_T, GL_CLAMP_TO_EDGE);
+
+    GLFramebuffer framebuffer;
+    glBindFramebuffer(GL_FRAMEBUFFER, framebuffer);
+    glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, texture, 0);
+    ASSERT_GL_FRAMEBUFFER_COMPLETE(GL_FRAMEBUFFER);
+
+    // Perform CopyTexImage2D
+    glBindTexture(GL_TEXTURE_2D, texture);
+    glCopyTexImage2D(GL_TEXTURE_2D, 1, GL_RGBA, 0, 0, 2, 2, 0);
+    ASSERT_GL_NO_ERROR();
+    // Not necessary to do any CopyTexImage2D operations to texture2.
+
+    // Perform CopyTexSubImage2D
+    glBindTexture(GL_TEXTURE_2D, texture);
+    glCopyTexSubImage2D(GL_TEXTURE_2D, 1, 0, 0, 0, 0, 1, 1);
+    ASSERT_GL_NO_ERROR();
+    glBindTexture(GL_TEXTURE_2D, texture2);
+    glCopyTexSubImage2D(GL_TEXTURE_2D, 0, 0, 0, 0, 0, 1, 1);
+    ASSERT_GL_NO_ERROR();
+
+    // Clean up - provokes crash on buggy drivers.
+    texture.reset();
+    // Crashes on Intel GPUs on macOS.
+    texture2.reset();
+}
+
 // specialization of CopyTexImageTest is added so that some tests can be explicitly run with an ES3
 // context
 class CopyTexImageTestES3 : public CopyTexImageTest