Skip to content

Commit

Permalink
Rework GLContextSwitch, get rid of RendererContextManager (flutter#18601
Browse files Browse the repository at this point in the history
)
  • Loading branch information
Chris Yang authored Jun 4, 2020
1 parent 4ce831e commit 1482d9b
Show file tree
Hide file tree
Showing 44 changed files with 418 additions and 452 deletions.
12 changes: 7 additions & 5 deletions ci/licenses_golden/licenses_flutter
Original file line number Diff line number Diff line change
Expand Up @@ -575,6 +575,11 @@ FILE: ../../../flutter/shell/common/engine.cc
FILE: ../../../flutter/shell/common/engine.h
FILE: ../../../flutter/shell/common/fixtures/shell_test.dart
FILE: ../../../flutter/shell/common/fixtures/shelltest_screenshot.png
FILE: ../../../flutter/shell/common/gl_context_switch.cc
FILE: ../../../flutter/shell/common/gl_context_switch.h
FILE: ../../../flutter/shell/common/gl_context_switch_test.cc
FILE: ../../../flutter/shell/common/gl_context_switch_test.h
FILE: ../../../flutter/shell/common/gl_context_switch_unittests.cc
FILE: ../../../flutter/shell/common/input_events_unittests.cc
FILE: ../../../flutter/shell/common/isolate_configuration.cc
FILE: ../../../flutter/shell/common/isolate_configuration.h
Expand All @@ -590,11 +595,6 @@ FILE: ../../../flutter/shell/common/pointer_data_dispatcher.cc
FILE: ../../../flutter/shell/common/pointer_data_dispatcher.h
FILE: ../../../flutter/shell/common/rasterizer.cc
FILE: ../../../flutter/shell/common/rasterizer.h
FILE: ../../../flutter/shell/common/renderer_context_manager.cc
FILE: ../../../flutter/shell/common/renderer_context_manager.h
FILE: ../../../flutter/shell/common/renderer_context_manager_unittests.cc
FILE: ../../../flutter/shell/common/renderer_context_test.cc
FILE: ../../../flutter/shell/common/renderer_context_test.h
FILE: ../../../flutter/shell/common/run_configuration.cc
FILE: ../../../flutter/shell/common/run_configuration.h
FILE: ../../../flutter/shell/common/shell.cc
Expand Down Expand Up @@ -942,6 +942,8 @@ FILE: ../../../flutter/shell/platform/darwin/ios/ios_surface_metal.h
FILE: ../../../flutter/shell/platform/darwin/ios/ios_surface_metal.mm
FILE: ../../../flutter/shell/platform/darwin/ios/ios_surface_software.h
FILE: ../../../flutter/shell/platform/darwin/ios/ios_surface_software.mm
FILE: ../../../flutter/shell/platform/darwin/ios/ios_switchable_gl_context.h
FILE: ../../../flutter/shell/platform/darwin/ios/ios_switchable_gl_context.mm
FILE: ../../../flutter/shell/platform/darwin/ios/platform_view_ios.h
FILE: ../../../flutter/shell/platform/darwin/ios/platform_view_ios.mm
FILE: ../../../flutter/shell/platform/darwin/ios/rendering_api_selection.h
Expand Down
2 changes: 1 addition & 1 deletion flow/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ source_set("flow") {
deps = [
"//flutter/common",
"//flutter/fml",
"//flutter/shell/common:surface_frame",
"//flutter/shell/common:common_standalone",
"//third_party/skia",
]

Expand Down
15 changes: 8 additions & 7 deletions shell/common/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,6 @@ source_set("common") {
"pointer_data_dispatcher.h",
"rasterizer.cc",
"rasterizer.h",
"renderer_context_manager.cc",
"renderer_context_manager.h",
"run_configuration.cc",
"run_configuration.h",
"shell.cc",
Expand All @@ -120,7 +118,7 @@ source_set("common") {
]

deps = [
":surface_frame",
":common_standalone",
"//flutter/assets",
"//flutter/common",
"//flutter/flow",
Expand All @@ -142,8 +140,10 @@ source_set("common") {
public_configs = [ "//flutter:config" ]
}

source_set("surface_frame") {
source_set("common_standalone") {
sources = [
"gl_context_switch.cc",
"gl_context_switch.h",
"surface_frame.cc",
"surface_frame.h",
]
Expand Down Expand Up @@ -203,8 +203,9 @@ if (enable_unittests) {
testonly = true

sources = [
"renderer_context_test.cc",
"renderer_context_test.h",
"gl_context_switch_test.cc",
"gl_context_switch_test.h",
"gl_context_switch_unittests.cc",
"shell_test.cc",
"shell_test.h",
"shell_test_external_view_embedder.cc",
Expand All @@ -216,6 +217,7 @@ if (enable_unittests) {
]

public_deps = [
":common_standalone",
"//flutter/flow",
"//flutter/fml/dart",
"//flutter/lib/ui",
Expand Down Expand Up @@ -267,7 +269,6 @@ if (enable_unittests) {
"input_events_unittests.cc",
"persistent_cache_unittests.cc",
"pipeline_unittests.cc",
"renderer_context_manager_unittests.cc",
"shell_unittests.cc",
]

Expand Down
34 changes: 34 additions & 0 deletions shell/common/gl_context_switch.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
// 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.h"

namespace flutter {

SwitchableGLContext::SwitchableGLContext() = default;

SwitchableGLContext::~SwitchableGLContext() = default;

GLContextResult::GLContextResult() = default;
GLContextResult::~GLContextResult() = default;
GLContextResult::GLContextResult(bool static_result) : result_(static_result){};
bool GLContextResult::GetResult() {
return result_;
};

GLContextDefaultResult::~GLContextDefaultResult() = default;
GLContextDefaultResult::GLContextDefaultResult(bool static_result)
: GLContextResult(static_result){};

GLContextSwitch::GLContextSwitch(std::unique_ptr<SwitchableGLContext> context)
: context_(std::move(context)) {
FML_CHECK(context_ != nullptr);
result_ = context_->SetCurrent();
};

GLContextSwitch::~GLContextSwitch() {
context_->RemoveCurrent();
};

} // namespace flutter
117 changes: 117 additions & 0 deletions shell/common/gl_context_switch.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
// 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_H_
#define FLUTTER_SHELL_COMMON_GL_CONTEXT_SWITCH_H_

#include <functional>
#include <memory>
#include <vector>

#include "flutter/fml/logging.h"
#include "flutter/fml/macros.h"

namespace flutter {

//------------------------------------------------------------------------------
/// An abstract class represents a gl context that can be switched by
/// GLContextSwitch
///
/// The subclass should wrap a "Context" object inside this class. For example,
/// in iOS while using GL rendering surface, the subclass should wrap an
/// |EAGLContext|.
class SwitchableGLContext {
public:
//------------------------------------------------------------------------------
/// Implement this to set the context wrapped by this |SwitchableGLContext|
/// object to the current context.
virtual bool SetCurrent() = 0;

//------------------------------------------------------------------------------
/// Implement this to remove the context wrapped by this |SwitchableGLContext|
/// object from current context;
virtual bool RemoveCurrent() = 0;

SwitchableGLContext();

virtual ~SwitchableGLContext();

FML_DISALLOW_COPY_AND_ASSIGN(SwitchableGLContext);
};

//------------------------------------------------------------------------------
/// Represents the result of setting a gl context.
/// This class exists because context results are used in places applies to all
/// the platforms. On certain platforms(for example lower end iOS devices that
/// uses gl), a |GLContextSwitch| is required to protect flutter's gl contect
/// from being polluted by other programs(embedded platform views). A
/// |GLContextSwitch| is a subclass of |GLContextResult|, which can be returned
/// on platforms that requires context switching. A |GLContextDefaultResult| is
/// also a subclass of |GLContextResult|, which can be returned on platforms
/// that doesn't require context switching.
class GLContextResult {
public:
GLContextResult();
virtual ~GLContextResult();

//----------------------------------------------------------------------------
// Returns true if the gl context is set successfully.
bool GetResult();

protected:
GLContextResult(bool static_result);
bool result_;

FML_DISALLOW_COPY_AND_ASSIGN(GLContextResult);
};

//------------------------------------------------------------------------------
/// The default implementation of |GLContextResult|.
///
/// Use this class on platforms that doesn't require gl context switching.
/// * See also |GLContextSwitch| if the platform requires gl context switching.
class GLContextDefaultResult : public GLContextResult {
public:
//----------------------------------------------------------------------------
/// Constructs a |GLContextDefaultResult| with a static result.
///
/// Used this on platforms that doesn't require gl context switching. (For
/// example, metal on iOS)
///
/// @param static_result a static value that will be returned from
/// |GetResult|
GLContextDefaultResult(bool static_result);

~GLContextDefaultResult() override;

FML_DISALLOW_COPY_AND_ASSIGN(GLContextDefaultResult);
};

//------------------------------------------------------------------------------
/// Switches the gl context to the a context that is passed in the
/// constructor.
///
/// In destruction, it should restore the current context to what was
/// before the construction of this switch.
class GLContextSwitch final : public GLContextResult {
public:
//----------------------------------------------------------------------------
/// Constructs a |GLContextSwitch|.
///
/// @param context The context that is going to be set as the current
/// context. The |GLContextSwitch| should not outlive the owner of the gl
/// context wrapped inside the `context`.
GLContextSwitch(std::unique_ptr<SwitchableGLContext> context);

~GLContextSwitch() override;

private:
std::unique_ptr<SwitchableGLContext> context_;

FML_DISALLOW_COPY_AND_ASSIGN(GLContextSwitch);
};

} // namespace flutter

#endif
43 changes: 43 additions & 0 deletions shell/common/gl_context_switch_test.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
// 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_test.h"

#include "flutter/fml/thread_local.h"

namespace flutter {
namespace testing {

FML_THREAD_LOCAL fml::ThreadLocalUniquePtr<int> current_context;

GLContextSwitchTest::GLContextSwitchTest() = default;

TestSwitchableGLContext::TestSwitchableGLContext(int context)
: context_(context){};

TestSwitchableGLContext::~TestSwitchableGLContext() = default;

bool TestSwitchableGLContext::SetCurrent() {
SetCurrentContext(context_);
return true;
};

bool TestSwitchableGLContext::RemoveCurrent() {
SetCurrentContext(-1);
return true;
};

int TestSwitchableGLContext::GetContext() {
return context_;
};

int TestSwitchableGLContext::GetCurrentContext() {
return *(current_context.get());
};

void TestSwitchableGLContext::SetCurrentContext(int context) {
current_context.reset(new int(context));
};
} // namespace testing
} // namespace flutter
Original file line number Diff line number Diff line change
Expand Up @@ -5,45 +5,43 @@
#ifndef FLUTTER_SHELL_RENDERER_CONTEXT_TEST_H_
#define FLUTTER_SHELL_RENDERER_CONTEXT_TEST_H_

#include "flutter/fml/thread_local.h"
#include "gl_context_switch.h"
#include "gtest/gtest.h"
#include "renderer_context_manager.h"

namespace flutter {
namespace testing {

class RendererContextTest : public ::testing::Test {
class GLContextSwitchTest : public ::testing::Test {
public:
RendererContextTest();
GLContextSwitchTest();
};

//------------------------------------------------------------------------------
/// The renderer context used for testing
class TestRendererContext : public RendererContext {
class TestSwitchableGLContext : public SwitchableGLContext {
public:
TestRendererContext(int context);
TestSwitchableGLContext(int context);

~TestRendererContext() override;
~TestSwitchableGLContext() override;

bool SetCurrent() override;

void RemoveCurrent() override;
bool RemoveCurrent() override;

int GetContext();

static int GetCurrentContext();

//------------------------------------------------------------------------------
/// Set the current context without going through the
/// |RendererContextManager|.
/// Set the current context
///
/// This is to mimic how other programs outside flutter sets the context.
static void SetCurrentContext(int context);

private:
int context_;

FML_DISALLOW_COPY_AND_ASSIGN(TestRendererContext);
FML_DISALLOW_COPY_AND_ASSIGN(TestSwitchableGLContext);
};
} // namespace testing
} // namespace flutter
Expand Down
27 changes: 27 additions & 0 deletions shell/common/gl_context_switch_unittests.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
// 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.
#define FML_USED_ON_EMBEDDER

#include <functional>
#include <future>
#include <memory>

#include "flutter/shell/common/gl_context_switch.h"
#include "gl_context_switch_test.h"
#include "gtest/gtest.h"

namespace flutter {
namespace testing {

TEST_F(GLContextSwitchTest, SwitchKeepsContextCurrentWhileInScope) {
{
auto test_gl_context = std::make_unique<TestSwitchableGLContext>(0);
auto context_switch = GLContextSwitch(std::move(test_gl_context));
ASSERT_EQ(TestSwitchableGLContext::GetCurrentContext(), 0);
}
ASSERT_EQ(TestSwitchableGLContext::GetCurrentContext(), -1);
}

} // namespace testing
} // namespace flutter
3 changes: 2 additions & 1 deletion shell/common/rasterizer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,8 @@ sk_sp<SkImage> Rasterizer::DoMakeRasterSnapshot(
result = DrawSnapshot(surface, draw_callback);
})
.SetIfFalse([&] {
if (!surface_->MakeRenderContextCurrent()) {
auto context_switch = surface_->MakeRenderContextCurrent();
if (!context_switch->GetResult()) {
return;
}

Expand Down
Loading

0 comments on commit 1482d9b

Please sign in to comment.