Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Impeller] libImpeller: Reset the GL state when transitioning control back to the embedder. #56597

Merged
merged 2 commits into from
Nov 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions impeller/renderer/backend/gles/context_gles.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
3 changes: 3 additions & 0 deletions impeller/renderer/backend/gles/context_gles.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
24 changes: 14 additions & 10 deletions impeller/renderer/backend/gles/render_pass_gles.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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);

Expand Down
2 changes: 2 additions & 0 deletions impeller/renderer/backend/gles/render_pass_gles.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ class RenderPassGLES final
// |RenderPass|
~RenderPassGLES() override;

static void ResetGLState(const ProcTableGLES& gl);

private:
friend class CommandBufferGLES;

Expand Down
8 changes: 8 additions & 0 deletions impeller/renderer/context.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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
14 changes: 11 additions & 3 deletions impeller/renderer/context.h
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
38 changes: 38 additions & 0 deletions impeller/toolkit/interop/impeller_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
6 changes: 4 additions & 2 deletions impeller/toolkit/interop/surface.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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