Skip to content

Commit

Permalink
Lift restriction that embedders may not trample the render thread Ope…
Browse files Browse the repository at this point in the history
…nGL context in composition callbacks. (flutter#16653)

During the implementation of custom compositor integration, the embedder gets
callbacks on the render thread to prepare render targets (framebuffers,
textures, etc) for the engine to render into, callbacks to present these render
targets along with platform managed contents, and, callbacks to collect render
targets once they can no longer be recycled by the engine in subsequent frames.
During these callbacks, the engine mandates the OpenGL state on the render
thread be preserved. This restriction has been the source of hard to isolate
issues where the embedder trampled on the OpenGL bindings state in the callback
but failed to restore state before control went back to the engine. Due to the
nature of the OpenGL API, such errors are easy to make and overlook. This patch
lifts the restriction from the embedder. Embedders may now freely work with the
OpenGL state in custom compositor callbacks and the engine will make sure to
disregard OpenGL bindings when control flows back to it.

Disregarding current OpenGL state has a certain performance penalty and the
majority of this patch handles refactoring various engine embedder components
such that this happens only once per frame. The most trivial version of this
patch would reset context bindings on every transition of control flow from the
embedder to the engine. However, that naive approach would have necessitated
more than 50 binding resets in existing unit-test cases (depending on the number
of platform view interleaving levels and render target recycling hit rates). In
this implementation, bindings will be reset only once per frame and this does
not depend on the number of platform views in the scene.

The majority of this patch is a refactoring of engine subsystems used in
`ExternalViewEmbedder::SubmitFrame` which is thoroughly documented with each
opportunity for the embedder to invalidate OpenGL state tagged.

The refactoring also enables the implementation of the following optimizations
to engine behavior which should aid in reducing the memory needed for the
creation of render targets. These optimization include:
* The engine will only ask the embedder for render targets in which it expects
  to render into. This was a quirk in the way in which root and non-root render
  targets were handled. The engine could require the embedder to create a render
  target but then realize it didn’t have anything to render into it. In the
  presentation callback, it would skip that render target. But the embedder
  still had to allocate that extra render target. This will no longer be the
  case and should reduce memory use.
* The engine may now skip always realizing (via the embedder render target
  creation callback) and presenting the root render target. This was also a side
  effect of the same quirk. Previously, the engine would always ask the embedder
  to present the root render target even if it was empty. Since this is no
  longer the case, few render targets should be allocated which will reduce
  memory consumption.
* The engine will now ask the embedder to collect unused render targets before
  it asks it to create new ones. The previous behavior was to ask the embedder
  for new targets and then collect old ones. This would cause spikes in memory
  use when the size of the render targets would change. These memory use spikes
  should now be troughs.
* The previous render target cache also considered the platform view ID in cache
  viability considerations (instead of just the size of the render target). This
  was a bug which has been fixed. This should lead to better cache utilization
  in some situations.

These optimizations are now codified in unit-tests and the updated test
expectations are a result of these optimizations now being in place.

* Fixes flutter#50751
* Fixes flutter#46911
* Fixes flutter#43778
* Fixes b/146142979
  • Loading branch information
chinmaygarde committed Feb 18, 2020
1 parent 48d64c1 commit 110c1c9
Show file tree
Hide file tree
Showing 18 changed files with 835 additions and 263 deletions.
7 changes: 7 additions & 0 deletions ci/licenses_golden/licenses_flutter
Expand Up @@ -126,6 +126,8 @@ FILE: ../../../flutter/fml/file_unittest.cc
FILE: ../../../flutter/fml/gpu_thread_merger.cc
FILE: ../../../flutter/fml/gpu_thread_merger.h
FILE: ../../../flutter/fml/gpu_thread_merger_unittests.cc
FILE: ../../../flutter/fml/hash_combine.h
FILE: ../../../flutter/fml/hash_combine_unittests.cc
FILE: ../../../flutter/fml/icu_util.cc
FILE: ../../../flutter/fml/icu_util.h
FILE: ../../../flutter/fml/log_level.h
Expand Down Expand Up @@ -913,15 +915,20 @@ FILE: ../../../flutter/shell/platform/embedder/embedder_engine.cc
FILE: ../../../flutter/shell/platform/embedder/embedder_engine.h
FILE: ../../../flutter/shell/platform/embedder/embedder_external_texture_gl.cc
FILE: ../../../flutter/shell/platform/embedder/embedder_external_texture_gl.h
FILE: ../../../flutter/shell/platform/embedder/embedder_external_view.cc
FILE: ../../../flutter/shell/platform/embedder/embedder_external_view.h
FILE: ../../../flutter/shell/platform/embedder/embedder_external_view_embedder.cc
FILE: ../../../flutter/shell/platform/embedder/embedder_external_view_embedder.h
FILE: ../../../flutter/shell/platform/embedder/embedder_include.c
FILE: ../../../flutter/shell/platform/embedder/embedder_include2.c
FILE: ../../../flutter/shell/platform/embedder/embedder_layers.cc
FILE: ../../../flutter/shell/platform/embedder/embedder_layers.h
FILE: ../../../flutter/shell/platform/embedder/embedder_platform_message_response.cc
FILE: ../../../flutter/shell/platform/embedder/embedder_platform_message_response.h
FILE: ../../../flutter/shell/platform/embedder/embedder_render_target.cc
FILE: ../../../flutter/shell/platform/embedder/embedder_render_target.h
FILE: ../../../flutter/shell/platform/embedder/embedder_render_target_cache.cc
FILE: ../../../flutter/shell/platform/embedder/embedder_render_target_cache.h
FILE: ../../../flutter/shell/platform/embedder/embedder_safe_access.h
FILE: ../../../flutter/shell/platform/embedder/embedder_surface.cc
FILE: ../../../flutter/shell/platform/embedder/embedder_surface.h
Expand Down
2 changes: 2 additions & 0 deletions fml/BUILD.gn
Expand Up @@ -26,6 +26,7 @@ source_set("fml") {
"file.h",
"gpu_thread_merger.cc",
"gpu_thread_merger.h",
"hash_combine.h",
"icu_util.cc",
"icu_util.h",
"log_level.h",
Expand Down Expand Up @@ -232,6 +233,7 @@ executable("fml_unittests") {
"command_line_unittest.cc",
"file_unittest.cc",
"gpu_thread_merger_unittests.cc",
"hash_combine_unittests.cc",
"memory/ref_counted_unittest.cc",
"memory/weak_ptr_unittest.cc",
"message_loop_task_queues_merge_unmerge_unittests.cc",
Expand Down
38 changes: 38 additions & 0 deletions fml/hash_combine.h
@@ -0,0 +1,38 @@
// 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_FML_HASH_COMBINE_H_
#define FLUTTER_FML_HASH_COMBINE_H_

#include <functional>

namespace fml {

template <class Type>
constexpr void HashCombineSeed(std::size_t& seed, Type arg) {
seed ^= std::hash<Type>{}(arg) + 0x9e3779b9 + (seed << 6) + (seed >> 2);
}

template <class Type, class... Rest>
constexpr void HashCombineSeed(std::size_t& seed,
Type arg,
Rest... other_args) {
HashCombineSeed(seed, arg);
HashCombineSeed(seed, other_args...);
}

constexpr std::size_t HashCombine() {
return 0xdabbad00;
}

template <class... Type>
constexpr std::size_t HashCombine(Type... args) {
std::size_t seed = HashCombine();
HashCombineSeed(seed, args...);
return seed;
}

} // namespace fml

#endif // FLUTTER_FML_HASH_COMBINE_H_
23 changes: 23 additions & 0 deletions fml/hash_combine_unittests.cc
@@ -0,0 +1,23 @@
// 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 "flutter/fml/hash_combine.h"
#include "flutter/testing/testing.h"

namespace fml {
namespace testing {

TEST(HashCombineTest, CanHash) {
ASSERT_EQ(HashCombine(), HashCombine());
ASSERT_EQ(HashCombine("Hello"), HashCombine("Hello"));
ASSERT_NE(HashCombine("Hello"), HashCombine("World"));
ASSERT_EQ(HashCombine("Hello", "World"), HashCombine("Hello", "World"));
ASSERT_NE(HashCombine("World", "Hello"), HashCombine("Hello", "World"));
ASSERT_EQ(HashCombine(12u), HashCombine(12u));
ASSERT_NE(HashCombine(12u), HashCombine(12.0f));
ASSERT_EQ(HashCombine('a'), HashCombine('a'));
}

} // namespace testing
} // namespace fml
6 changes: 3 additions & 3 deletions shell/common/canvas_spy.h
Expand Up @@ -26,16 +26,16 @@ class CanvasSpy {
public:
CanvasSpy(SkCanvas* target_canvas);

//------------------------------------------------------------------------------
/// @brief Returns true if any non trasnparent content has been drawn
//----------------------------------------------------------------------------
/// @brief Returns true if any non transparent content has been drawn
/// into
/// the spying canvas. Note that this class does tries to detect
/// empty canvases but in some cases may return true even for
/// empty canvases (e.g when a transparent image is drawn into the
/// canvas).
bool DidDrawIntoCanvas();

//------------------------------------------------------------------------------
//----------------------------------------------------------------------------
/// @brief The returned canvas delegate all operations to the target
/// canvas
/// while spying on them.
Expand Down
5 changes: 5 additions & 0 deletions shell/platform/embedder/BUILD.gn
Expand Up @@ -29,15 +29,20 @@ template("embedder_source_set") {
"embedder_engine.h",
"embedder_external_texture_gl.cc",
"embedder_external_texture_gl.h",
"embedder_external_view.cc",
"embedder_external_view.h",
"embedder_external_view_embedder.cc",
"embedder_external_view_embedder.h",
"embedder_include.c",
"embedder_include2.c",
"embedder_layers.cc",
"embedder_layers.h",
"embedder_platform_message_response.cc",
"embedder_platform_message_response.h",
"embedder_render_target.cc",
"embedder_render_target.h",
"embedder_render_target_cache.cc",
"embedder_render_target_cache.h",
"embedder_safe_access.h",
"embedder_surface.cc",
"embedder_surface.h",
Expand Down
107 changes: 107 additions & 0 deletions shell/platform/embedder/embedder_external_view.cc
@@ -0,0 +1,107 @@
// 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 "flutter/shell/platform/embedder/embedder_external_view.h"
#include "flutter/fml/trace_event.h"
#include "flutter/shell/common/canvas_spy.h"

namespace flutter {

static SkISize TransformedSurfaceSize(const SkISize& size,
const SkMatrix& transformation) {
const auto source_rect = SkRect::MakeWH(size.width(), size.height());
const auto transformed_rect = transformation.mapRect(source_rect);
return SkISize::Make(transformed_rect.width(), transformed_rect.height());
}

EmbedderExternalView::EmbedderExternalView(
const SkISize& frame_size,
const SkMatrix& surface_transformation)
: EmbedderExternalView(frame_size, surface_transformation, {}, nullptr) {}

EmbedderExternalView::EmbedderExternalView(
const SkISize& frame_size,
const SkMatrix& surface_transformation,
ViewIdentifier view_identifier,
std::unique_ptr<EmbeddedViewParams> params)
: render_surface_size_(
TransformedSurfaceSize(frame_size, surface_transformation)),
surface_transformation_(surface_transformation),
view_identifier_(view_identifier),
embedded_view_params_(std::move(params)),
recorder_(std::make_unique<SkPictureRecorder>()),
canvas_spy_(std::make_unique<CanvasSpy>(
recorder_->beginRecording(frame_size.width(), frame_size.height()))) {
}

EmbedderExternalView::~EmbedderExternalView() = default;

EmbedderExternalView::RenderTargetDescriptor
EmbedderExternalView::CreateRenderTargetDescriptor() const {
return {render_surface_size_};
}

SkCanvas* EmbedderExternalView::GetCanvas() const {
return canvas_spy_->GetSpyingCanvas();
}

SkISize EmbedderExternalView::GetRenderSurfaceSize() const {
return render_surface_size_;
}

bool EmbedderExternalView::IsRootView() const {
return !HasPlatformView();
}

bool EmbedderExternalView::HasPlatformView() const {
return view_identifier_.platform_view_id.has_value();
}

bool EmbedderExternalView::HasEngineRenderedContents() const {
return canvas_spy_->DidDrawIntoCanvas();
}

EmbedderExternalView::ViewIdentifier EmbedderExternalView::GetViewIdentifier()
const {
return view_identifier_;
}

const EmbeddedViewParams* EmbedderExternalView::GetEmbeddedViewParams() const {
return embedded_view_params_.get();
}

bool EmbedderExternalView::Render(const EmbedderRenderTarget& render_target) {
TRACE_EVENT0("flutter", "EmbedderExternalView::Render");

FML_DCHECK(HasEngineRenderedContents())
<< "Unnecessarily asked to render into a render target when there was "
"nothing to render.";

auto picture = recorder_->finishRecordingAsPicture();
if (!picture) {
return false;
}

auto surface = render_target.GetRenderSurface();
if (!surface) {
return false;
}

FML_DCHECK(SkISize::Make(surface->width(), surface->height()) ==
render_surface_size_);

auto canvas = surface->getCanvas();
if (!canvas) {
return false;
}

canvas->setMatrix(surface_transformation_);
canvas->clear(SK_ColorTRANSPARENT);
canvas->drawPicture(picture);
canvas->flush();

return true;
}

} // namespace flutter
118 changes: 118 additions & 0 deletions shell/platform/embedder/embedder_external_view.h
@@ -0,0 +1,118 @@
// 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_PLATFORM_EMBEDDER_EMBEDDER_EXTERNAL_VIEW_H_
#define FLUTTER_SHELL_PLATFORM_EMBEDDER_EMBEDDER_EXTERNAL_VIEW_H_

#include <optional>
#include <unordered_map>
#include <unordered_set>

#include "flutter/flow/embedded_views.h"
#include "flutter/fml/hash_combine.h"
#include "flutter/fml/macros.h"
#include "flutter/shell/common/canvas_spy.h"
#include "flutter/shell/platform/embedder/embedder_render_target.h"
#include "third_party/skia/include/core/SkPictureRecorder.h"

namespace flutter {

class EmbedderExternalView {
public:
using PlatformViewID = int64_t;
struct ViewIdentifier {
std::optional<PlatformViewID> platform_view_id;

ViewIdentifier() {}

ViewIdentifier(PlatformViewID view_id) : platform_view_id(view_id) {}

struct Hash {
constexpr std::size_t operator()(const ViewIdentifier& desc) const {
if (!desc.platform_view_id.has_value()) {
return fml::HashCombine();
}

return fml::HashCombine(desc.platform_view_id.value());
}
};

struct Equal {
constexpr bool operator()(const ViewIdentifier& lhs,
const ViewIdentifier& rhs) const {
return lhs.platform_view_id == rhs.platform_view_id;
}
};
};

struct RenderTargetDescriptor {
SkISize surface_size = SkISize::MakeEmpty();

struct Hash {
constexpr std::size_t operator()(
const RenderTargetDescriptor& desc) const {
return fml::HashCombine(desc.surface_size.width(),
desc.surface_size.height());
}
};

struct Equal {
bool operator()(const RenderTargetDescriptor& lhs,
const RenderTargetDescriptor& rhs) const {
return lhs.surface_size == rhs.surface_size;
}
};
};

using ViewIdentifierSet = std::unordered_set<ViewIdentifier,
ViewIdentifier::Hash,
ViewIdentifier::Equal>;

using PendingViews = std::unordered_map<ViewIdentifier,
std::unique_ptr<EmbedderExternalView>,
ViewIdentifier::Hash,
ViewIdentifier::Equal>;

EmbedderExternalView(const SkISize& frame_size,
const SkMatrix& surface_transformation);

EmbedderExternalView(const SkISize& frame_size,
const SkMatrix& surface_transformation,
ViewIdentifier view_identifier,
std::unique_ptr<EmbeddedViewParams> params);

~EmbedderExternalView();

bool IsRootView() const;

bool HasPlatformView() const;

bool HasEngineRenderedContents() const;

ViewIdentifier GetViewIdentifier() const;

const EmbeddedViewParams* GetEmbeddedViewParams() const;

RenderTargetDescriptor CreateRenderTargetDescriptor() const;

SkCanvas* GetCanvas() const;

SkISize GetRenderSurfaceSize() const;

bool Render(const EmbedderRenderTarget& render_target);

private:
const SkISize render_surface_size_;
const SkMatrix surface_transformation_;
ViewIdentifier view_identifier_;
std::unique_ptr<EmbeddedViewParams> embedded_view_params_;
std::unique_ptr<SkPictureRecorder> recorder_;
std::unique_ptr<CanvasSpy> canvas_spy_;

FML_DISALLOW_COPY_AND_ASSIGN(EmbedderExternalView);
};

} // namespace flutter

#endif // FLUTTER_SHELL_PLATFORM_EMBEDDER_EMBEDDER_EXTERNAL_VIEW_H_

0 comments on commit 110c1c9

Please sign in to comment.