[Impeller] libImpeller: Reset the GL state when transitioning control back to the embedder. (flutter/engine#56597)

Impeller is resilient to OpenGL state being trampled upon when accessing the GL context. But the embedder may not necessarily be. Ideally, we'd be using saving the state and restoring it. But that might be too involved. For now, this sets the GL state to a sane "clean" state.

We could, in theory, do this after each render pass but that unnecessarily increases API traffic. For now, I have added it at the transition of the embedder boundary.

engine/src/flutter/impeller/renderer/backend/gles/context_gles.cc

@@ -9,6 +9,7 @@
 #include "impeller/base/validation.h"
 #include "impeller/renderer/backend/gles/command_buffer_gles.h"
 #include "impeller/renderer/backend/gles/gpu_tracer_gles.h"
+#include "impeller/renderer/backend/gles/render_pass_gles.h"
 #include "impeller/renderer/command_queue.h"
 
 namespace impeller {
@@ -145,4 +146,15 @@ std::shared_ptr<CommandQueue> ContextGLES::GetCommandQueue() const {
   return command_queue_;
 }
 
+// |Context|
+void ContextGLES::ResetThreadLocalState() const {
+  if (!IsValid()) {
+    return;
+  }
+  [[maybe_unused]] auto result =
+      reactor_->AddOperation([](const ReactorGLES& reactor) {
+        RenderPassGLES::ResetGLState(reactor.GetProcTable());
+      });
+}
+
 }  // namespace impeller

engine/src/flutter/impeller/renderer/backend/gles/context_gles.h

@@ -93,6 +93,9 @@ class ContextGLES final : public Context,
   // |Context|
   void Shutdown() override;
 
+  // |Context|
+  void ResetThreadLocalState() const override;
+
   ContextGLES(const ContextGLES&) = delete;
 
   ContextGLES& operator=(const ContextGLES&) = delete;

engine/src/flutter/impeller/renderer/backend/gles/render_pass_gles.cc

@@ -172,6 +172,19 @@ static bool BindVertexBuffer(const ProcTableGLES& gl,
   return true;
 }
 
+void RenderPassGLES::ResetGLState(const ProcTableGLES& gl) {
+  gl.Disable(GL_SCISSOR_TEST);
+  gl.Disable(GL_DEPTH_TEST);
+  gl.Disable(GL_STENCIL_TEST);
+  gl.Disable(GL_CULL_FACE);
+  gl.Disable(GL_BLEND);
+  gl.Disable(GL_DITHER);
+  gl.ColorMask(GL_TRUE, GL_TRUE, GL_TRUE, GL_TRUE);
+  gl.DepthMask(GL_TRUE);
+  gl.StencilMaskSeparate(GL_FRONT, 0xFFFFFFFF);
+  gl.StencilMaskSeparate(GL_BACK, 0xFFFFFFFF);
+}
+
 [[nodiscard]] bool EncodeCommandsInReactor(
     const RenderPassData& pass_data,
     const std::shared_ptr<Allocator>& transients_allocator,
@@ -267,16 +280,7 @@ static bool BindVertexBuffer(const ProcTableGLES& gl,
     clear_bits |= GL_STENCIL_BUFFER_BIT;
   }
 
-  gl.Disable(GL_SCISSOR_TEST);
-  gl.Disable(GL_DEPTH_TEST);
-  gl.Disable(GL_STENCIL_TEST);
-  gl.Disable(GL_CULL_FACE);
-  gl.Disable(GL_BLEND);
-  gl.Disable(GL_DITHER);
-  gl.ColorMask(GL_TRUE, GL_TRUE, GL_TRUE, GL_TRUE);
-  gl.DepthMask(GL_TRUE);
-  gl.StencilMaskSeparate(GL_FRONT, 0xFFFFFFFF);
-  gl.StencilMaskSeparate(GL_BACK, 0xFFFFFFFF);
+  RenderPassGLES::ResetGLState(gl);
 
   gl.Clear(clear_bits);
 

engine/src/flutter/impeller/renderer/backend/gles/render_pass_gles.h

@@ -19,6 +19,8 @@ class RenderPassGLES final
   // |RenderPass|
   ~RenderPassGLES() override;
 
+  static void ResetGLState(const ProcTableGLES& gl);
+
  private:
   friend class CommandBufferGLES;
 

engine/src/flutter/impeller/renderer/context.cc

@@ -25,4 +25,12 @@ bool Context::FlushCommandBuffers() {
   return true;
 }
 
+std::shared_ptr<const IdleWaiter> Context::GetIdleWaiter() const {
+  return nullptr;
+}
+
+void Context::ResetThreadLocalState() const {
+  // Nothing to do.
+}
+
 }  // namespace impeller

engine/src/flutter/impeller/renderer/context.h

@@ -222,9 +222,17 @@ class Context {
   /// rendering a 2D workload.
   [[nodiscard]] virtual bool FlushCommandBuffers();
 
-  virtual std::shared_ptr<const IdleWaiter> GetIdleWaiter() const {
-    return nullptr;
-  }
+  virtual std::shared_ptr<const IdleWaiter> GetIdleWaiter() const;
+
+  //----------------------------------------------------------------------------
+  /// Resets any thread local state that may interfere with embedders.
+  ///
+  /// Today, only the OpenGL backend can trample on thread local state that the
+  /// embedder can access. This call puts the GL state in a sane "clean" state.
+  ///
+  /// Impeller itself is resilient to a dirty thread local state table.
+  ///
+  virtual void ResetThreadLocalState() const;
 
  protected:
   Context();

engine/src/flutter/impeller/toolkit/interop/impeller_unittests.cc

@@ -192,6 +192,44 @@ TEST_P(InteropPlaygroundTest, CanCreateOpenGLImage) {
       }));
 }
 
+TEST_P(InteropPlaygroundTest, ClearsOpenGLStancilStateAfterTransition) {
+  auto context = GetInteropContext();
+  auto impeller_context = context->GetContext();
+  if (impeller_context->GetBackendType() !=
+      impeller::Context::BackendType::kOpenGLES) {
+    GTEST_SKIP() << "This test works with OpenGL handles is only suitable for "
+                    "that backend.";
+    return;
+  }
+  const auto& gl_context = ContextGLES::Cast(*impeller_context);
+  const auto& gl = gl_context.GetReactor()->GetProcTable();
+  auto builder =
+      Adopt<DisplayListBuilder>(ImpellerDisplayListBuilderNew(nullptr));
+  auto paint = Adopt<Paint>(ImpellerPaintNew());
+  ImpellerColor color = {0.0, 0.0, 1.0, 1.0};
+  ImpellerPaintSetColor(paint.GetC(), &color);
+  ImpellerRect rect = {10, 20, 100, 200};
+  ImpellerDisplayListBuilderDrawRect(builder.GetC(), &rect, paint.GetC());
+  color = {1.0, 0.0, 0.0, 1.0};
+  ImpellerPaintSetColor(paint.GetC(), &color);
+  ImpellerDisplayListBuilderTranslate(builder.GetC(), 110, 210);
+  ImpellerDisplayListBuilderClipRect(builder.GetC(), &rect,
+                                     kImpellerClipOperationDifference);
+  ImpellerDisplayListBuilderDrawRect(builder.GetC(), &rect, paint.GetC());
+  auto dl = Adopt<DisplayList>(
+      ImpellerDisplayListBuilderCreateDisplayListNew(builder.GetC()));
+  ASSERT_TRUE(dl);
+  ASSERT_TRUE(
+      OpenPlaygroundHere([&](const auto& context, const auto& surface) -> bool {
+        ImpellerSurfaceDrawDisplayList(surface.GetC(), dl.GetC());
+        // OpenGL state is reset even though the operations above enable a
+        // stencil check.
+        GLboolean stencil_enabled = true;
+        gl.GetBooleanv(GL_STENCIL_TEST, &stencil_enabled);
+        return stencil_enabled == GL_FALSE;
+      }));
+}
+
 TEST_P(InteropPlaygroundTest, CanCreateParagraphs) {
   // Create a typography context.
   auto type_context = Adopt<TypographyContext>(ImpellerTypographyContextNew());

engine/src/flutter/impeller/toolkit/interop/surface.cc

@@ -62,8 +62,10 @@ bool Surface::DrawDisplayList(const DisplayList& dl) const {
   auto skia_cull_rect =
       SkIRect::MakeWH(cull_rect.GetWidth(), cull_rect.GetHeight());
 
-  return RenderToOnscreen(content_context, render_target, display_list,
-                          skia_cull_rect, /*reset_host_buffer=*/true);
+  auto result = RenderToOnscreen(content_context, render_target, display_list,
+                                 skia_cull_rect, /*reset_host_buffer=*/true);
+  context_->GetContext()->ResetThreadLocalState();
+  return result;
 }
 
 }  // namespace impeller::interop