Skip to content

Commit

Permalink
Always use IOSGLContextSwitch to access EAGLContexts to prevent plu…
Browse files Browse the repository at this point in the history
…gins from polluting Flutter's EAGLContext (#13314)
  • Loading branch information
Chris Yang committed Nov 8, 2019
1 parent 5f5713e commit bec5542
Show file tree
Hide file tree
Showing 39 changed files with 713 additions and 115 deletions.
4 changes: 4 additions & 0 deletions ci/licenses_golden/licenses_flutter
Original file line number Diff line number Diff line change
Expand Up @@ -495,6 +495,8 @@ FILE: ../../../flutter/shell/common/canvas_spy_unittests.cc
FILE: ../../../flutter/shell/common/engine.cc
FILE: ../../../flutter/shell/common/engine.h
FILE: ../../../flutter/shell/common/fixtures/shell_test.dart
FILE: ../../../flutter/shell/common/gl_context_switch_manager.cc
FILE: ../../../flutter/shell/common/gl_context_switch_manager.h
FILE: ../../../flutter/shell/common/input_events_unittests.cc
FILE: ../../../flutter/shell/common/isolate_configuration.cc
FILE: ../../../flutter/shell/common/isolate_configuration.h
Expand Down Expand Up @@ -799,6 +801,8 @@ FILE: ../../../flutter/shell/platform/darwin/ios/ios_external_texture_gl.h
FILE: ../../../flutter/shell/platform/darwin/ios/ios_external_texture_gl.mm
FILE: ../../../flutter/shell/platform/darwin/ios/ios_gl_context.h
FILE: ../../../flutter/shell/platform/darwin/ios/ios_gl_context.mm
FILE: ../../../flutter/shell/platform/darwin/ios/ios_gl_context_switch_manager.h
FILE: ../../../flutter/shell/platform/darwin/ios/ios_gl_context_switch_manager.mm
FILE: ../../../flutter/shell/platform/darwin/ios/ios_gl_render_target.h
FILE: ../../../flutter/shell/platform/darwin/ios/ios_gl_render_target.mm
FILE: ../../../flutter/shell/platform/darwin/ios/ios_surface.h
Expand Down
2 changes: 2 additions & 0 deletions shell/common/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,8 @@ source_set("common") {
"canvas_spy.h",
"engine.cc",
"engine.h",
"gl_context_switch_manager.cc",
"gl_context_switch_manager.h",
"isolate_configuration.cc",
"isolate_configuration.h",
"persistent_cache.cc",
Expand Down
28 changes: 28 additions & 0 deletions shell/common/gl_context_switch_manager.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
// Copyright 2013 The Flutter Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "gl_context_switch_manager.h"

namespace flutter {

GLContextSwitchManager::GLContextSwitchManager() = default;

GLContextSwitchManager::~GLContextSwitchManager() = default;

GLContextSwitchManager::GLContextSwitch::GLContextSwitch() = default;

GLContextSwitchManager::GLContextSwitch::~GLContextSwitch(){};

GLContextSwitchManager::GLContextSwitchPureResult::GLContextSwitchPureResult(
bool switch_result)
: switch_result_(switch_result){};

GLContextSwitchManager::GLContextSwitchPureResult::
~GLContextSwitchPureResult() = default;

bool GLContextSwitchManager::GLContextSwitchPureResult::GetSwitchResult() {
return switch_result_;
}

} // namespace flutter
116 changes: 116 additions & 0 deletions shell/common/gl_context_switch_manager.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
// Copyright 2013 The Flutter Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#ifndef FLUTTER_SHELL_COMMON_GL_CONTEXT_SWITCH_MANAGER_H_
#define FLUTTER_SHELL_COMMON_GL_CONTEXT_SWITCH_MANAGER_H_

#include <memory>
#include "flutter/fml/macros.h"

namespace flutter {

//------------------------------------------------------------------------------
/// Manages `GLContextSwitch`.
///
/// Should be subclassed for platforms that uses GL and requires context
/// switching. Always use `MakeCurrent` and `ResourceMakeCurrent` in the
/// `GLContextSwitchManager` to set gl contexts.
///
class GLContextSwitchManager {
public:
//------------------------------------------------------------------------------
/// Switches the gl context to the flutter's contexts.
///
/// Should be subclassed for each platform embedder that uses GL.
/// In construction, it should set the current context to a flutter's context
/// In destruction, it should rest the current context.
///
class GLContextSwitch {
public:
GLContextSwitch();

virtual ~GLContextSwitch();

virtual bool GetSwitchResult() = 0;

FML_DISALLOW_COPY_AND_ASSIGN(GLContextSwitch);
};

GLContextSwitchManager();
~GLContextSwitchManager();

//----------------------------------------------------------------------------
/// @brief Creates a shell instance using the provided settings. The
/// callbacks to create the various shell subcomponents will be
/// called on the appropriate threads before this method returns.
/// If this is the first instance of a shell in the process, this
/// call also bootstraps the Dart VM.
///
/// @param[in] task_runners The task runners
/// @param[in] settings The settings
/// @param[in] on_create_platform_view The callback that must return a
/// platform view. This will be called on
/// the platform task runner before this
/// method returns.
/// @param[in] on_create_rasterizer That callback that must provide a
/// valid rasterizer. This will be called
/// on the render task runner before this
/// method returns.
///
/// @return A full initialized shell if the settings and callbacks are
/// valid. The root isolate has been created but not yet launched.
/// It may be launched by obtaining the engine weak pointer and
/// posting a task onto the UI task runner with a valid run
/// configuration to run the isolate. The embedder must always
/// check the validity of the shell (using the IsSetup call)
/// immediately after getting a pointer to it.
///

//----------------------------------------------------------------------------
/// @brief Make the flutter's context as current context.
///
/// @return A `GLContextSwitch` with `GetSwitchResult` returning true if
/// the setting process is succesful.
virtual std::unique_ptr<GLContextSwitch> MakeCurrent() = 0;

//----------------------------------------------------------------------------
/// @brief Make the flutter's resources context as current context.
///
/// @return A `GLContextSwitch` with `GetSwitchResult` returning true if
/// the setting process is succesful.
virtual std::unique_ptr<GLContextSwitch> ResourceMakeCurrent() = 0;

//------------------------------------------------------------------------------
/// A representation of a `GLContextSwitch` that doesn't require actual
/// context switching.
///
class GLContextSwitchPureResult final : public GLContextSwitch {
public:
// Constructor that creates an `GLContextSwitchPureResult`.
// The `GetSwitchResult` will return the same value as `switch_result`.

//----------------------------------------------------------------------------
/// @brief Constructs a `GLContextSwitchPureResult`.
///
/// @param[in] switch_result the switch result that will be returned
/// in `GetSwitchResult()`
///
GLContextSwitchPureResult(bool switch_result);

~GLContextSwitchPureResult();

bool GetSwitchResult() override;

private:
bool switch_result_;

FML_DISALLOW_COPY_AND_ASSIGN(GLContextSwitchPureResult);
};

FML_DISALLOW_COPY_AND_ASSIGN(GLContextSwitchManager);
};

} // namespace flutter

#endif
11 changes: 9 additions & 2 deletions shell/common/shell_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -355,8 +355,10 @@ PointerDataDispatcherMaker ShellTestPlatformView::GetDispatcherMaker() {
}

// |GPUSurfaceGLDelegate|
bool ShellTestPlatformView::GLContextMakeCurrent() {
return gl_surface_.MakeCurrent();
std::unique_ptr<GLContextSwitchManager::GLContextSwitch>
ShellTestPlatformView::GLContextMakeCurrent() {
return std::make_unique<GLContextSwitchManager::GLContextSwitchPureResult>(
gl_surface_.MakeCurrent());
}

// |GPUSurfaceGLDelegate|
Expand Down Expand Up @@ -387,5 +389,10 @@ ExternalViewEmbedder* ShellTestPlatformView::GetExternalViewEmbedder() {
return nullptr;
}

std::shared_ptr<GLContextSwitchManager>
ShellTestPlatformView::GetGLContextSwitchManager() {
return nullptr;
}

} // namespace testing
} // namespace flutter
5 changes: 4 additions & 1 deletion shell/common/shell_test.h
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,8 @@ class ShellTestPlatformView : public PlatformView, public GPUSurfaceGLDelegate {
PointerDataDispatcherMaker GetDispatcherMaker() override;

// |GPUSurfaceGLDelegate|
bool GLContextMakeCurrent() override;
std::unique_ptr<GLContextSwitchManager::GLContextSwitch>
GLContextMakeCurrent() override;

// |GPUSurfaceGLDelegate|
bool GLContextClearCurrent() override;
Expand All @@ -161,6 +162,8 @@ class ShellTestPlatformView : public PlatformView, public GPUSurfaceGLDelegate {
// |GPUSurfaceGLDelegate|
ExternalViewEmbedder* GetExternalViewEmbedder() override;

std::shared_ptr<GLContextSwitchManager> GetGLContextSwitchManager() override;

FML_DISALLOW_COPY_AND_ASSIGN(ShellTestPlatformView);
};

Expand Down
6 changes: 4 additions & 2 deletions shell/common/surface.cc
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,10 @@ flutter::ExternalViewEmbedder* Surface::GetExternalViewEmbedder() {
return nullptr;
}

bool Surface::MakeRenderContextCurrent() {
return true;
std::unique_ptr<GLContextSwitchManager::GLContextSwitch>
Surface::MakeRenderContextCurrent() {
return std::make_unique<GLContextSwitchManager::GLContextSwitchPureResult>(
true);
}

} // namespace flutter
4 changes: 3 additions & 1 deletion shell/common/surface.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include "flutter/flow/compositor_context.h"
#include "flutter/flow/embedded_views.h"
#include "flutter/fml/macros.h"
#include "flutter/shell/common/gl_context_switch_manager.h"
#include "third_party/skia/include/core/SkCanvas.h"

namespace flutter {
Expand Down Expand Up @@ -58,7 +59,8 @@ class Surface {

virtual flutter::ExternalViewEmbedder* GetExternalViewEmbedder();

virtual bool MakeRenderContextCurrent();
virtual std::unique_ptr<GLContextSwitchManager::GLContextSwitch>
MakeRenderContextCurrent();

private:
FML_DISALLOW_COPY_AND_ASSIGN(Surface);
Expand Down
33 changes: 21 additions & 12 deletions shell/gpu/gpu_surface_gl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include "flutter/fml/logging.h"
#include "flutter/fml/size.h"
#include "flutter/fml/trace_event.h"
#include "flutter/shell/common/gl_context_switch_manager.h"
#include "flutter/shell/common/persistent_cache.h"
#include "third_party/skia/include/core/SkColorFilter.h"
#include "third_party/skia/include/core/SkSurface.h"
Expand Down Expand Up @@ -39,7 +40,10 @@ GPUSurfaceGL::GPUSurfaceGL(GPUSurfaceGLDelegate* delegate,
: delegate_(delegate),
render_to_surface_(render_to_surface),
weak_factory_(this) {
if (!delegate_->GLContextMakeCurrent()) {
std::unique_ptr<GLContextSwitchManager::GLContextSwitch> context_switch =
delegate_->GLContextMakeCurrent();

if (!context_switch->GetSwitchResult()) {
FML_LOG(ERROR)
<< "Could not make the context current to setup the gr context.";
return;
Expand Down Expand Up @@ -87,8 +91,6 @@ GPUSurfaceGL::GPUSurfaceGL(GPUSurfaceGLDelegate* delegate,
}
FML_LOG(INFO) << "Found " << caches.size() << " SkSL shaders; precompiled "
<< compiled_count;

delegate_->GLContextClearCurrent();
}

GPUSurfaceGL::GPUSurfaceGL(sk_sp<GrContext> gr_context,
Expand All @@ -98,7 +100,9 @@ GPUSurfaceGL::GPUSurfaceGL(sk_sp<GrContext> gr_context,
context_(gr_context),
render_to_surface_(render_to_surface),
weak_factory_(this) {
if (!delegate_->GLContextMakeCurrent()) {
std::unique_ptr<GLContextSwitchManager::GLContextSwitch> context_switch =
delegate_->GLContextMakeCurrent();
if (!context_switch->GetSwitchResult()) {
FML_LOG(ERROR)
<< "Could not make the context current to setup the gr context.";
return;
Expand All @@ -114,8 +118,9 @@ GPUSurfaceGL::~GPUSurfaceGL() {
if (!valid_) {
return;
}

if (!delegate_->GLContextMakeCurrent()) {
std::unique_ptr<GLContextSwitchManager::GLContextSwitch> context_switch =
delegate_->GLContextMakeCurrent();
if (!context_switch->GetSwitchResult()) {
FML_LOG(ERROR) << "Could not make the context current to destroy the "
"GrContext resources.";
return;
Expand All @@ -126,8 +131,6 @@ GPUSurfaceGL::~GPUSurfaceGL() {
context_->releaseResourcesAndAbandonContext();
}
context_ = nullptr;

delegate_->GLContextClearCurrent();
}

// |Surface|
Expand Down Expand Up @@ -253,7 +256,9 @@ std::unique_ptr<SurfaceFrame> GPUSurfaceGL::AcquireFrame(const SkISize& size) {
return nullptr;
}

if (!delegate_->GLContextMakeCurrent()) {
std::unique_ptr<GLContextSwitchManager::GLContextSwitch> context_switch =
delegate_->GLContextMakeCurrent();
if (!context_switch->GetSwitchResult()) {
FML_LOG(ERROR)
<< "Could not make the context current to acquire the frame.";
return nullptr;
Expand Down Expand Up @@ -285,14 +290,18 @@ std::unique_ptr<SurfaceFrame> GPUSurfaceGL::AcquireFrame(const SkISize& size) {
return weak ? weak->PresentSurface(canvas) : false;
};

return std::make_unique<SurfaceFrame>(surface, submit_callback);
std::unique_ptr<SurfaceFrame> result =
std::make_unique<SurfaceFrame>(surface, submit_callback);
return result;
}

bool GPUSurfaceGL::PresentSurface(SkCanvas* canvas) {
if (delegate_ == nullptr || canvas == nullptr || context_ == nullptr) {
return false;
}

std::unique_ptr<GLContextSwitchManager::GLContextSwitch> context_switch =
delegate_->GLContextMakeCurrent();
if (offscreen_surface_ != nullptr) {
TRACE_EVENT0("flutter", "CopyTextureOnscreen");
SkPaint paint;
Expand Down Expand Up @@ -329,7 +338,6 @@ bool GPUSurfaceGL::PresentSurface(SkCanvas* canvas) {

onscreen_surface_ = std::move(new_onscreen_surface);
}

return true;
}

Expand Down Expand Up @@ -360,7 +368,8 @@ flutter::ExternalViewEmbedder* GPUSurfaceGL::GetExternalViewEmbedder() {
}

// |Surface|
bool GPUSurfaceGL::MakeRenderContextCurrent() {
std::unique_ptr<GLContextSwitchManager::GLContextSwitch>
GPUSurfaceGL::MakeRenderContextCurrent() {
return delegate_->GLContextMakeCurrent();
}

Expand Down
3 changes: 2 additions & 1 deletion shell/gpu/gpu_surface_gl.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,8 @@ class GPUSurfaceGL : public Surface {
flutter::ExternalViewEmbedder* GetExternalViewEmbedder() override;

// |Surface|
bool MakeRenderContextCurrent() override;
std::unique_ptr<GLContextSwitchManager::GLContextSwitch>
MakeRenderContextCurrent() override;

private:
GPUSurfaceGLDelegate* delegate_;
Expand Down
7 changes: 6 additions & 1 deletion shell/gpu/gpu_surface_gl_delegate.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

#include "flutter/flow/embedded_views.h"
#include "flutter/fml/macros.h"
#include "flutter/shell/common/gl_context_switch_manager.h"
#include "flutter/shell/gpu/gpu_surface_delegate.h"
#include "third_party/skia/include/core/SkMatrix.h"
#include "third_party/skia/include/gpu/gl/GrGLInterface.h"
Expand All @@ -16,7 +17,8 @@ namespace flutter {
class GPUSurfaceGLDelegate : public GPUSurfaceDelegate {
public:
// Called to make the main GL context current on the current thread.
virtual bool GLContextMakeCurrent() = 0;
virtual std::unique_ptr<GLContextSwitchManager::GLContextSwitch>
GLContextMakeCurrent() = 0;

// Called to clear the current GL context on the thread. This may be called on
// either the GPU or IO threads.
Expand Down Expand Up @@ -59,6 +61,9 @@ class GPUSurfaceGLDelegate : public GPUSurfaceDelegate {
// instrumentation to specific GL calls can specify custom GL functions
// here.
virtual GLProcResolver GetGLProcResolver() const;

virtual std::shared_ptr<GLContextSwitchManager>
GetGLContextSwitchManager() = 0;
};

} // namespace flutter
Expand Down
Loading

0 comments on commit bec5542

Please sign in to comment.