Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.
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
7 changes: 7 additions & 0 deletions ci/licenses_golden/licenses_flutter
Original file line number Diff line number Diff line change
Expand Up @@ -521,6 +521,8 @@ FILE: ../../../flutter/shell/gpu/gpu_surface_metal.h
FILE: ../../../flutter/shell/gpu/gpu_surface_metal.mm
FILE: ../../../flutter/shell/gpu/gpu_surface_software.cc
FILE: ../../../flutter/shell/gpu/gpu_surface_software.h
FILE: ../../../flutter/shell/gpu/gpu_surface_software_delegate.cc
FILE: ../../../flutter/shell/gpu/gpu_surface_software_delegate.h
FILE: ../../../flutter/shell/gpu/gpu_surface_vulkan.cc
FILE: ../../../flutter/shell/gpu/gpu_surface_vulkan.h
FILE: ../../../flutter/shell/platform/android/AndroidManifest.xml
Expand Down Expand Up @@ -810,9 +812,13 @@ 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_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_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_safe_access.h
FILE: ../../../flutter/shell/platform/embedder/embedder_surface.cc
FILE: ../../../flutter/shell/platform/embedder/embedder_surface.h
Expand All @@ -824,6 +830,7 @@ FILE: ../../../flutter/shell/platform/embedder/embedder_task_runner.cc
FILE: ../../../flutter/shell/platform/embedder/embedder_task_runner.h
FILE: ../../../flutter/shell/platform/embedder/embedder_thread_host.cc
FILE: ../../../flutter/shell/platform/embedder/embedder_thread_host.h
FILE: ../../../flutter/shell/platform/embedder/fixtures/compositor.png
FILE: ../../../flutter/shell/platform/embedder/fixtures/main.dart
FILE: ../../../flutter/shell/platform/embedder/platform_view_embedder.cc
FILE: ../../../flutter/shell/platform/embedder/platform_view_embedder.h
Expand Down
13 changes: 10 additions & 3 deletions flow/embedded_views.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include "third_party/skia/include/core/SkRRect.h"
#include "third_party/skia/include/core/SkRect.h"
#include "third_party/skia/include/core/SkSize.h"
#include "third_party/skia/include/core/SkSurface.h"

namespace flutter {

Expand Down Expand Up @@ -194,11 +195,19 @@ class ExternalViewEmbedder {
public:
ExternalViewEmbedder() = default;

virtual ~ExternalViewEmbedder() = default;

// Usually, the root surface is not owned by the view embedder. However, if
// the view embedder wants to provide a surface to the rasterizer, it may
// return one here. This surface takes priority over the surface materialized
// from the on-screen render target.
virtual sk_sp<SkSurface> GetRootSurface() = 0;

// Call this in-lieu of |SubmitFrame| to clear pre-roll state and
// sets the stage for the next pre-roll.
virtual void CancelFrame() = 0;

virtual void BeginFrame(SkISize frame_size) = 0;
virtual void BeginFrame(SkISize frame_size, GrContext* context) = 0;

virtual void PrerollCompositeEmbeddedView(
int view_id,
Expand All @@ -220,8 +229,6 @@ class ExternalViewEmbedder {

virtual bool SubmitFrame(GrContext* context);

virtual ~ExternalViewEmbedder() = default;

FML_DISALLOW_COPY_AND_ASSIGN(ExternalViewEmbedder);

}; // ExternalViewEmbedder
Expand Down
40 changes: 40 additions & 0 deletions fml/closure.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,50 @@

#include <functional>

#include "flutter/fml/macros.h"

namespace fml {

using closure = std::function<void()>;

//------------------------------------------------------------------------------
/// @brief Wraps a closure that is invoked in the destructor unless
/// released by the caller.
///
/// This is especially useful in dealing with APIs that return a
/// resource by accepting ownership of a sub-resource and a closure
/// that releases that resource. When such APIs are chained, each
/// link in the chain must check that the next member in the chain
/// has accepted the resource. If not, it must invoke the closure
/// eagerly. Not doing this results in a resource leak in the
/// erroneous case. Using this wrapper, the closure can be released
/// once the next call in the chain has successfully accepted
/// ownership of the resource. If not, the closure gets invoked
/// automatically at the end of the scope. This covers the cases
/// where there are early returns as well.
///
class ScopedCleanupClosure {
public:
ScopedCleanupClosure(fml::closure closure) : closure_(closure) {}

~ScopedCleanupClosure() {
if (closure_) {
closure_();
}
}

fml::closure Release() {
fml::closure closure = closure_;
closure_ = nullptr;
return closure;
}

private:
fml::closure closure_;

FML_DISALLOW_COPY_AND_ASSIGN(ScopedCleanupClosure);
};

} // namespace fml

#endif // FLUTTER_FML_CLOSURE_H_
11 changes: 11 additions & 0 deletions fml/mapping.cc
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,17 @@ const uint8_t* DataMapping::GetMapping() const {

// NonOwnedMapping

NonOwnedMapping::NonOwnedMapping(const uint8_t* data,
size_t size,
ReleaseProc release_proc)
: data_(data), size_(size), release_proc_(release_proc) {}

NonOwnedMapping::~NonOwnedMapping() {
if (release_proc_) {
release_proc_(data_, size_);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't it make it an owned mapping once we have a release proc?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess so. The argument could also be made that this object does not own the mapping but gives the closure to collect it (so whoever passes in the closure to this owns the mapping).

}
}

size_t NonOwnedMapping::GetSize() const {
return size_;
}
Expand Down
9 changes: 7 additions & 2 deletions fml/mapping.h
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,12 @@ class DataMapping final : public Mapping {

class NonOwnedMapping final : public Mapping {
public:
NonOwnedMapping(const uint8_t* data, size_t size)
: data_(data), size_(size) {}
using ReleaseProc = std::function<void(const uint8_t* data, size_t size)>;
NonOwnedMapping(const uint8_t* data,
size_t size,
ReleaseProc release_proc = nullptr);

~NonOwnedMapping() override;

// |Mapping|
size_t GetSize() const override;
Expand All @@ -111,6 +115,7 @@ class NonOwnedMapping final : public Mapping {
private:
const uint8_t* const data_;
const size_t size_;
const ReleaseProc release_proc_;

FML_DISALLOW_COPY_AND_ASSIGN(NonOwnedMapping);
};
Expand Down
2 changes: 1 addition & 1 deletion lib/ui/painting/image_decoder_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ class TestIOManager final : public IOManager {
public:
TestIOManager(fml::RefPtr<fml::TaskRunner> task_runner,
bool has_gpu_context = true)
: gl_context_(has_gpu_context ? gl_surface_.CreateContext() : nullptr),
: gl_context_(has_gpu_context ? gl_surface_.CreateGrContext() : nullptr),
weak_gl_context_factory_(
has_gpu_context ? std::make_unique<fml::WeakPtrFactory<GrContext>>(
gl_context_.get())
Expand Down
31 changes: 25 additions & 6 deletions shell/common/rasterizer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -223,17 +223,34 @@ RasterStatus Rasterizer::DrawToSurface(flutter::LayerTree& layer_tree) {
// for instrumentation.
compositor_context_->ui_time().SetLapTime(layer_tree.build_time());

auto* canvas = frame->SkiaCanvas();

auto* external_view_embedder = surface_->GetExternalViewEmbedder();

sk_sp<SkSurface> embedder_root_surface;

if (external_view_embedder != nullptr) {
external_view_embedder->BeginFrame(layer_tree.frame_size());
external_view_embedder->BeginFrame(layer_tree.frame_size(),
surface_->GetContext());
embedder_root_surface = external_view_embedder->GetRootSurface();
}

// If the external view embedder has specified an optional root surface, the
// root surface transformation is set by the embedder instead of
// having to apply it here.
SkMatrix root_surface_transformation =
embedder_root_surface ? SkMatrix{} : surface_->GetRootTransformation();

auto root_surface_canvas = embedder_root_surface
? embedder_root_surface->getCanvas()
: frame->SkiaCanvas();

auto compositor_frame = compositor_context_->AcquireFrame(
surface_->GetContext(), canvas, external_view_embedder,
surface_->GetRootTransformation(), true, gpu_thread_merger_);
surface_->GetContext(), // skia GrContext
root_surface_canvas, // root surface canvas
external_view_embedder, // external view embedder
root_surface_transformation, // root surface transformation
true, // instrumentation enabled
gpu_thread_merger_ // thread merger
);

if (compositor_frame) {
RasterStatus raster_status = compositor_frame->Raster(layer_tree, false);
Expand All @@ -244,10 +261,12 @@ RasterStatus Rasterizer::DrawToSurface(flutter::LayerTree& layer_tree) {
if (external_view_embedder != nullptr) {
external_view_embedder->SubmitFrame(surface_->GetContext());
}

FireNextFrameCallbackIfPresent();

if (surface_->GetContext())
if (surface_->GetContext()) {
surface_->GetContext()->performDeferredCleanup(kSkiaCleanupExpiration);
}

return raster_status;
}
Expand Down
7 changes: 6 additions & 1 deletion shell/common/shell_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ ShellTestPlatformView::~ShellTestPlatformView() = default;

// |PlatformView|
std::unique_ptr<Surface> ShellTestPlatformView::CreateRenderingSurface() {
return std::make_unique<GPUSurfaceGL>(this);
return std::make_unique<GPUSurfaceGL>(this, true);
}

// |GPUSurfaceGLDelegate|
Expand Down Expand Up @@ -248,5 +248,10 @@ GPUSurfaceGLDelegate::GLProcResolver ShellTestPlatformView::GetGLProcResolver()
};
}

// |GPUSurfaceGLDelegate|
ExternalViewEmbedder* ShellTestPlatformView::GetExternalViewEmbedder() {
return nullptr;
}

} // namespace testing
} // namespace flutter
3 changes: 3 additions & 0 deletions shell/common/shell_test.h
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,9 @@ class ShellTestPlatformView : public PlatformView, public GPUSurfaceGLDelegate {
// |GPUSurfaceGLDelegate|
GLProcResolver GetGLProcResolver() const override;

// |GPUSurfaceGLDelegate|
ExternalViewEmbedder* GetExternalViewEmbedder() override;

FML_DISALLOW_COPY_AND_ASSIGN(ShellTestPlatformView);
};

Expand Down
2 changes: 2 additions & 0 deletions shell/gpu/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ source_set("gpu_surface_software") {
sources = [
"$gpu_dir/gpu_surface_software.cc",
"$gpu_dir/gpu_surface_software.h",
"$gpu_dir/gpu_surface_software_delegate.cc",
"$gpu_dir/gpu_surface_software_delegate.h",
]

deps = gpu_common_deps
Expand Down
27 changes: 22 additions & 5 deletions shell/gpu/gpu_surface_gl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,11 @@ static const int kGrCacheMaxCount = 8192;
// system channel.
static const size_t kGrCacheMaxByteSize = 24 * (1 << 20);

GPUSurfaceGL::GPUSurfaceGL(GPUSurfaceGLDelegate* delegate)
: delegate_(delegate), weak_factory_(this) {
GPUSurfaceGL::GPUSurfaceGL(GPUSurfaceGLDelegate* delegate,
bool render_to_surface)
: delegate_(delegate),
render_to_surface_(render_to_surface),
weak_factory_(this) {
if (!delegate_->GLContextMakeCurrent()) {
FML_LOG(ERROR)
<< "Could not make the context current to setup the gr context.";
Expand Down Expand Up @@ -68,13 +71,18 @@ GPUSurfaceGL::GPUSurfaceGL(GPUSurfaceGLDelegate* delegate)

delegate_->GLContextClearCurrent();

valid_ = true;
context_owner_ = true;

valid_ = true;
}

GPUSurfaceGL::GPUSurfaceGL(sk_sp<GrContext> gr_context,
GPUSurfaceGLDelegate* delegate)
: delegate_(delegate), context_(gr_context), weak_factory_(this) {
GPUSurfaceGLDelegate* delegate,
bool render_to_surface)
: delegate_(delegate),
context_(gr_context),
render_to_surface_(render_to_surface),
weak_factory_(this) {
if (!delegate_->GLContextMakeCurrent()) {
FML_LOG(ERROR)
<< "Could not make the context current to setup the gr context.";
Expand Down Expand Up @@ -236,6 +244,15 @@ std::unique_ptr<SurfaceFrame> GPUSurfaceGL::AcquireFrame(const SkISize& size) {
return nullptr;
}

// TODO(38466): Refactor GPU surface APIs take into account the fact that an
// external view embedder may want to render to the root surface.
if (!render_to_surface_) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW I feel like we've reached a point where we're applying concepts that doesn't align well with the higher level surface API and we should probably revisit it at some point. What do you think about leaving filing an issue saying we should refactor and leaving a TODO here describing the reason for this workaround and saying that we should clean it up in a coming refactoring?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, added a TODO and filed a bug flutter/flutter#38466

return std::make_unique<SurfaceFrame>(
nullptr, [](const SurfaceFrame& surface_frame, SkCanvas* canvas) {
return true;
});
}

const auto root_surface_transformation = GetRootTransformation();

sk_sp<SkSurface> surface =
Expand Down
13 changes: 10 additions & 3 deletions shell/gpu/gpu_surface_gl.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,12 @@ namespace flutter {

class GPUSurfaceGL : public Surface {
public:
GPUSurfaceGL(GPUSurfaceGLDelegate* delegate);
GPUSurfaceGL(GPUSurfaceGLDelegate* delegate, bool render_to_surface);

// Creates a new GL surface reusing an existing GrContext.
GPUSurfaceGL(sk_sp<GrContext> gr_context, GPUSurfaceGLDelegate* delegate);
GPUSurfaceGL(sk_sp<GrContext> gr_context,
GPUSurfaceGLDelegate* delegate,
bool render_to_surface);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The meaning of render_to_surface wasn't immediately clear to me (A GPU surface that does not render to surface?), maybe worth documenting it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I documented the ivar and filed a bug to get rid of this hack.


~GPUSurfaceGL() override;

Expand All @@ -49,9 +51,14 @@ class GPUSurfaceGL : public Surface {
sk_sp<GrContext> context_;
sk_sp<SkSurface> onscreen_surface_;
sk_sp<SkSurface> offscreen_surface_;
bool context_owner_;
// TODO(38466): Refactor GPU surface APIs take into account the fact that an
// external view embedder may want to render to the root surface. This is a
// hack to make avoid allocating resources for the root surface when an
// external view embedder is present.
const bool render_to_surface_;
bool valid_ = false;
fml::WeakPtrFactory<GPUSurfaceGL> weak_factory_;
bool context_owner_;

bool CreateOrUpdateSurfaces(const SkISize& size);

Expand Down
4 changes: 0 additions & 4 deletions shell/gpu/gpu_surface_gl_delegate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,6 @@ SkMatrix GPUSurfaceGLDelegate::GLContextSurfaceTransformation() const {
return matrix;
}

flutter::ExternalViewEmbedder* GPUSurfaceGLDelegate::GetExternalViewEmbedder() {
return nullptr;
}

GPUSurfaceGLDelegate::GLProcResolver GPUSurfaceGLDelegate::GetGLProcResolver()
const {
return nullptr;
Expand Down
2 changes: 1 addition & 1 deletion shell/gpu/gpu_surface_gl_delegate.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ class GPUSurfaceGLDelegate {

// Get a reference to the external views embedder. This happens on the same
// thread that the renderer is operating on.
virtual flutter::ExternalViewEmbedder* GetExternalViewEmbedder();
virtual ExternalViewEmbedder* GetExternalViewEmbedder() = 0;

sk_sp<const GrGLInterface> GetGLInterface() const;

Expand Down
5 changes: 0 additions & 5 deletions shell/gpu/gpu_surface_software.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,6 @@

namespace flutter {

flutter::ExternalViewEmbedder*
GPUSurfaceSoftwareDelegate::GetExternalViewEmbedder() {
return nullptr;
}

GPUSurfaceSoftware::GPUSurfaceSoftware(GPUSurfaceSoftwareDelegate* delegate)
: delegate_(delegate), weak_factory_(this) {}

Expand Down
Loading