[Impeller] libImpeller: Reset the GL state when transitioning control back to the embedder. (#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.
diff --git a/impeller/renderer/backend/gles/context_gles.cc b/impeller/renderer/backend/gles/context_gles.cc
index 7629eff..c4070a5 100644
--- a/impeller/renderer/backend/gles/context_gles.cc
+++ b/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 @@
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
diff --git a/impeller/renderer/backend/gles/context_gles.h b/impeller/renderer/backend/gles/context_gles.h
index 2a0f09a..d677471 100644
--- a/impeller/renderer/backend/gles/context_gles.h
+++ b/impeller/renderer/backend/gles/context_gles.h
@@ -93,6 +93,9 @@
// |Context|
void Shutdown() override;
+ // |Context|
+ void ResetThreadLocalState() const override;
+
ContextGLES(const ContextGLES&) = delete;
ContextGLES& operator=(const ContextGLES&) = delete;
diff --git a/impeller/renderer/backend/gles/render_pass_gles.cc b/impeller/renderer/backend/gles/render_pass_gles.cc
index f8c1a25..5ca2f5d 100644
--- a/impeller/renderer/backend/gles/render_pass_gles.cc
+++ b/impeller/renderer/backend/gles/render_pass_gles.cc
@@ -172,6 +172,19 @@
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 @@
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);
diff --git a/impeller/renderer/backend/gles/render_pass_gles.h b/impeller/renderer/backend/gles/render_pass_gles.h
index e753a75..c0bd9e6 100644
--- a/impeller/renderer/backend/gles/render_pass_gles.h
+++ b/impeller/renderer/backend/gles/render_pass_gles.h
@@ -19,6 +19,8 @@
// |RenderPass|
~RenderPassGLES() override;
+ static void ResetGLState(const ProcTableGLES& gl);
+
private:
friend class CommandBufferGLES;
diff --git a/impeller/renderer/context.cc b/impeller/renderer/context.cc
index e238741..e563ac7 100644
--- a/impeller/renderer/context.cc
+++ b/impeller/renderer/context.cc
@@ -25,4 +25,12 @@
return true;
}
+std::shared_ptr<const IdleWaiter> Context::GetIdleWaiter() const {
+ return nullptr;
+}
+
+void Context::ResetThreadLocalState() const {
+ // Nothing to do.
+}
+
} // namespace impeller
diff --git a/impeller/renderer/context.h b/impeller/renderer/context.h
index 657083c..2735ac2 100644
--- a/impeller/renderer/context.h
+++ b/impeller/renderer/context.h
@@ -222,9 +222,17 @@
/// 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();
diff --git a/impeller/toolkit/interop/impeller_unittests.cc b/impeller/toolkit/interop/impeller_unittests.cc
index e45389e..e2c90dd 100644
--- a/impeller/toolkit/interop/impeller_unittests.cc
+++ b/impeller/toolkit/interop/impeller_unittests.cc
@@ -192,6 +192,44 @@
}));
}
+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());
diff --git a/impeller/toolkit/interop/surface.cc b/impeller/toolkit/interop/surface.cc
index 11e6def..b5e7f72 100644
--- a/impeller/toolkit/interop/surface.cc
+++ b/impeller/toolkit/interop/surface.cc
@@ -62,8 +62,10 @@
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
