From 4535e89e030a2c7a19cfd3c58954d190ca2f58fe Mon Sep 17 00:00:00 2001 From: Hannes Winkler Date: Wed, 12 Jul 2023 15:37:05 +0200 Subject: [PATCH 01/30] Add FlutterOpenGLSurface backing store Allows using an EGL surface as a flutter backing store. Way more convenient for GBM than hacking gbm bo's into GL FBOs. --- shell/platform/embedder/embedder.cc | 80 ++++++++++++++++++- shell/platform/embedder/embedder.h | 21 +++++ .../embedder/embedder_external_view.cc | 6 ++ .../embedder/embedder_render_target.h | 12 +++ .../embedder/embedder_render_target_skia.cc | 6 +- .../embedder/embedder_render_target_skia.h | 16 +++- .../embedder/tests/embedder_assertions.h | 22 +++++ 7 files changed, 158 insertions(+), 5 deletions(-) diff --git a/shell/platform/embedder/embedder.cc b/shell/platform/embedder/embedder.cc index 6631c2582c9a1..0c63e8d3019f0 100644 --- a/shell/platform/embedder/embedder.cc +++ b/shell/platform/embedder/embedder.cc @@ -278,6 +278,13 @@ static const SkIRect FlutterRectToSkIRect(FlutterRect flutter_rect) { static_cast(flutter_rect.bottom)}; return rect; } + +// We need GL_BGRA8_EXT for creating SkSurfaces from FlutterOpenGLSurfaces +// below. +#ifndef GL_BGRA8_EXT +#define GL_BGRA8_EXT 0x93A1 +#endif + #endif static inline flutter::Shell::CreateCallback @@ -826,6 +833,47 @@ static sk_sp MakeSkSurfaceFromBackingStore( #endif } +static sk_sp MakeSkSurfaceFromBackingStore( + GrDirectContext* context, + const FlutterBackingStoreConfig& config, + const FlutterOpenGLSurface* surface) { +#ifdef SHELL_ENABLE_GL + GrGLFramebufferInfo framebuffer_info = {}; + framebuffer_info.fFormat = GL_BGRA8_EXT; + framebuffer_info.fFBOID = 0; + + GrBackendRenderTarget backend_render_target( + config.size.width, // width + config.size.height, // height + 1, // sample count + 0, // stencil bits + framebuffer_info // framebuffer info + ); + + SkSurfaceProps surface_properties(0, kUnknown_SkPixelGeometry); + + auto sk_surface = SkSurfaces::WrapBackendRenderTarget( + context, // context + backend_render_target, // backend render target + kBottomLeft_GrSurfaceOrigin, // surface origin + kN32_SkColorType, // color type + SkColorSpace::MakeSRGB(), // color space + &surface_properties, // surface properties + static_cast( + surface->destruction_callback), // release proc + surface->user_data // release context + ); + + if (!sk_surface) { + FML_LOG(ERROR) << "Could not wrap embedder supplied frame-buffer."; + return nullptr; + } + return sk_surface; +#else + return nullptr; +#endif +} + static sk_sp MakeSkSurfaceFromBackingStore( GrDirectContext* context, const FlutterBackingStoreConfig& config, @@ -1155,12 +1203,22 @@ static sk_sp MakeSkSurfaceFromBackingStore( static std::unique_ptr MakeRenderTargetFromSkSurface(FlutterBackingStore backing_store, sk_sp skia_surface, - fml::closure on_release) { + fml::closure on_release, + fml::closure on_make_current) { if (!skia_surface) { return nullptr; } return std::make_unique( - backing_store, std::move(skia_surface), std::move(on_release)); + backing_store, std::move(skia_surface), std::move(on_release), + std::move(on_make_current)); +} + +static std::unique_ptr +MakeRenderTargetFromSkSurface(FlutterBackingStore backing_store, + sk_sp skia_surface, + fml::closure on_release) { + return MakeRenderTargetFromSkSurface(backing_store, skia_surface, on_release, + nullptr); } static std::unique_ptr @@ -1231,6 +1289,24 @@ CreateEmbedderRenderTarget( collect_callback.Release()); break; } + + case kFlutterOpenGLTargetTypeSurface: { + auto skia_surface = MakeSkSurfaceFromBackingStore( + context, config, &backing_store.open_gl.surface); + + auto make_current_callback = + backing_store.open_gl.surface.make_current_callback; + auto make_current_context = backing_store.open_gl.surface.user_data; + + auto on_make_current = [=] { + make_current_callback(make_current_context); + }; + + render_target = MakeRenderTargetFromSkSurface( + backing_store, std::move(skia_surface), + collect_callback.Release(), on_make_current); + break; + } } } break; diff --git a/shell/platform/embedder/embedder.h b/shell/platform/embedder/embedder.h index 969af06f7d36b..287b3f059c87d 100644 --- a/shell/platform/embedder/embedder.h +++ b/shell/platform/embedder/embedder.h @@ -304,6 +304,9 @@ typedef enum { /// Specifies an OpenGL frame-buffer target type. Framebuffers are specified /// using the FlutterOpenGLFramebuffer struct. kFlutterOpenGLTargetTypeFramebuffer, + /// Specifies an OpenGL on-screen surface target type. Surfaces are specified + /// using the FlutterOpenGLSurface struct. + kFlutterOpenGLTargetTypeSurface, } FlutterOpenGLTargetType; /// A pixel format to be used for software rendering. @@ -408,6 +411,21 @@ typedef struct { VoidCallback destruction_callback; } FlutterOpenGLFramebuffer; +typedef struct { + size_t struct_size; + + /// User data to be returned on the invocation of the destruction callback. + void* user_data; + + /// Callback invoked (on an engine managed thread) that asks the embedder to + /// make the window surface current. + VoidCallback make_current_callback; + + /// Callback invoked (on an engine managed thread) that asks the embedder to + /// collect the framebuffer. + VoidCallback destruction_callback; +} FlutterOpenGLSurface; + typedef bool (*BoolCallback)(void* /* user data */); typedef FlutterTransformation (*TransformationCallback)(void* /* user data */); typedef uint32_t (*UIntCallback)(void* /* user data */); @@ -1619,6 +1637,9 @@ typedef struct { /// A framebuffer for Flutter to render into. The embedder must ensure that /// the framebuffer is complete. FlutterOpenGLFramebuffer framebuffer; + /// A surface for Flutter to render into. Basically a wrapper around + /// a closure that'll be called when the surface should be made current. + FlutterOpenGLSurface surface; }; } FlutterOpenGLBackingStore; diff --git a/shell/platform/embedder/embedder_external_view.cc b/shell/platform/embedder/embedder_external_view.cc index c7806ff11b95e..2cbebd5f0cad4 100644 --- a/shell/platform/embedder/embedder_external_view.cc +++ b/shell/platform/embedder/embedder_external_view.cc @@ -143,6 +143,12 @@ bool EmbedderExternalView::Render(const EmbedderRenderTarget& render_target, return false; } + bool invalidate_gl_state = render_target.MaybeMakeCurrent(); + // TODO(ardera): Implement + // if (invalidate_gl_state) { + // skia_surface->recordingContext()->asDirectContext()->resetContext(kAll_GrBackendState); + // } + FML_DCHECK(render_target.GetRenderTargetSize() == render_surface_size_); auto canvas = skia_surface->getCanvas(); diff --git a/shell/platform/embedder/embedder_render_target.h b/shell/platform/embedder/embedder_render_target.h index cfbb67c07f5f3..074ac057e364a 100644 --- a/shell/platform/embedder/embedder_render_target.h +++ b/shell/platform/embedder/embedder_render_target.h @@ -77,6 +77,18 @@ class EmbedderRenderTarget { /// const FlutterBackingStore* GetBackingStore() const; + //---------------------------------------------------------------------------- + /// @brief Sometimes render targets are actually (for example) + /// EGL surfaces instead of framebuffers or textures. + /// In that case, we can't fully wrap them as SkSurfaces, instead, + /// the embedder will provide a callback that should be called + /// when the target surface should be made current. + /// + /// @return True if any GL (not EGL) state has changed and skia should + /// not assume any GL state values are the same as before + /// MaybeMakeCurrent was called. + virtual bool MaybeMakeCurrent() const { return false; } + protected: //---------------------------------------------------------------------------- /// @brief Creates a render target whose backing store is managed by the diff --git a/shell/platform/embedder/embedder_render_target_skia.cc b/shell/platform/embedder/embedder_render_target_skia.cc index e8d60ca60890f..9f013ad80ec09 100644 --- a/shell/platform/embedder/embedder_render_target_skia.cc +++ b/shell/platform/embedder/embedder_render_target_skia.cc @@ -11,9 +11,11 @@ namespace flutter { EmbedderRenderTargetSkia::EmbedderRenderTargetSkia( FlutterBackingStore backing_store, sk_sp render_surface, - fml::closure on_release) + fml::closure on_release, + fml::closure on_make_current) : EmbedderRenderTarget(backing_store, std::move(on_release)), - render_surface_(std::move(render_surface)) { + render_surface_(std::move(render_surface)), + on_make_current_(std::move(on_make_current)) { FML_DCHECK(render_surface_); } diff --git a/shell/platform/embedder/embedder_render_target_skia.h b/shell/platform/embedder/embedder_render_target_skia.h index 2ccacfee33a2e..29fb3b2371434 100644 --- a/shell/platform/embedder/embedder_render_target_skia.h +++ b/shell/platform/embedder/embedder_render_target_skia.h @@ -13,7 +13,8 @@ class EmbedderRenderTargetSkia final : public EmbedderRenderTarget { public: EmbedderRenderTargetSkia(FlutterBackingStore backing_store, sk_sp render_surface, - fml::closure on_release); + fml::closure on_release, + fml::closure on_make_current); // |EmbedderRenderTarget| ~EmbedderRenderTargetSkia() override; @@ -30,9 +31,22 @@ class EmbedderRenderTargetSkia final : public EmbedderRenderTarget { // |EmbedderRenderTarget| SkISize GetRenderTargetSize() const override; + bool MaybeMakeCurrent() const override { + if (on_make_current_ != nullptr) { + on_make_current_(); + + /// TODO: Implement return value + return false; + } + + return false; + } + private: sk_sp render_surface_; + fml::closure on_make_current_; + FML_DISALLOW_COPY_AND_ASSIGN(EmbedderRenderTargetSkia); }; diff --git a/shell/platform/embedder/tests/embedder_assertions.h b/shell/platform/embedder/tests/embedder_assertions.h index 09b0d48bd0830..8145602ec3eee 100644 --- a/shell/platform/embedder/tests/embedder_assertions.h +++ b/shell/platform/embedder/tests/embedder_assertions.h @@ -67,6 +67,13 @@ inline bool operator==(const FlutterOpenGLFramebuffer& a, a.destruction_callback == b.destruction_callback; } +inline bool operator==(const FlutterOpenGLSurface& a, + const FlutterOpenGLSurface& b) { + return a.make_current_callback == b.make_current_callback && + a.user_data == b.user_data && + a.destruction_callback == b.destruction_callback; +} + inline bool operator==(const FlutterMetalTexture& a, const FlutterMetalTexture& b) { return a.texture_id == b.texture_id && a.texture == b.texture; @@ -98,6 +105,8 @@ inline bool operator==(const FlutterOpenGLBackingStore& a, return a.texture == b.texture; case kFlutterOpenGLTargetTypeFramebuffer: return a.framebuffer == b.framebuffer; + case kFlutterOpenGLTargetTypeSurface: + return a.surface == b.surface; } return false; @@ -296,6 +305,14 @@ inline std::ostream& operator<<(std::ostream& out, << reinterpret_cast(item.destruction_callback); } +inline std::ostream& operator<<(std::ostream& out, + const FlutterOpenGLSurface& item) { + return out << "(FlutterOpenGLSurface) Make Current Callback: " + << reinterpret_cast(item.make_current_callback) + << " User Data: " << item.user_data << " Destruction Callback: " + << reinterpret_cast(item.destruction_callback); +} + inline std::ostream& operator<<(std::ostream& out, const FlutterMetalTexture& item) { return out << "(FlutterMetalTexture) Texture ID: " << std::hex @@ -368,6 +385,8 @@ inline std::string FlutterOpenGLTargetTypeToString( return "kFlutterOpenGLTargetTypeTexture"; case kFlutterOpenGLTargetTypeFramebuffer: return "kFlutterOpenGLTargetTypeFramebuffer"; + case kFlutterOpenGLTargetTypeSurface: + return "kFlutterOpenGLTargetTypeSurface"; } return "Unknown"; } @@ -406,6 +425,9 @@ inline std::ostream& operator<<(std::ostream& out, case kFlutterOpenGLTargetTypeFramebuffer: out << item.framebuffer; break; + case kFlutterOpenGLTargetTypeSurface: + out << item.surface; + break; } return out; } From 1d1b0a01e7e5dcafcd55e7c59c7e67274fa7eb7e Mon Sep 17 00:00:00 2001 From: Hannes Winkler Date: Sun, 16 Jul 2023 18:47:44 +0200 Subject: [PATCH 02/30] fix scoping & style --- shell/platform/embedder/embedder.cc | 29 ++++++++++--------- .../embedder/embedder_render_target_skia.h | 1 + 2 files changed, 16 insertions(+), 14 deletions(-) diff --git a/shell/platform/embedder/embedder.cc b/shell/platform/embedder/embedder.cc index 0c63e8d3019f0..ffd9d2c49e880 100644 --- a/shell/platform/embedder/embedder.cc +++ b/shell/platform/embedder/embedder.cc @@ -1289,28 +1289,29 @@ CreateEmbedderRenderTarget( collect_callback.Release()); break; } + } - case kFlutterOpenGLTargetTypeSurface: { - auto skia_surface = MakeSkSurfaceFromBackingStore( - context, config, &backing_store.open_gl.surface); + case kFlutterOpenGLTargetTypeSurface: { + auto skia_surface = MakeSkSurfaceFromBackingStore( + context, config, &backing_store.open_gl.surface); - auto make_current_callback = - backing_store.open_gl.surface.make_current_callback; - auto make_current_context = backing_store.open_gl.surface.user_data; + auto make_current_callback = + backing_store.open_gl.surface.make_current_callback; + auto make_current_context = backing_store.open_gl.surface.user_data; - auto on_make_current = [=] { - make_current_callback(make_current_context); - }; + auto on_make_current = [=] { + make_current_callback(make_current_context); + }; - render_target = MakeRenderTargetFromSkSurface( - backing_store, std::move(skia_surface), - collect_callback.Release(), on_make_current); - break; - } + render_target = MakeRenderTargetFromSkSurface( + backing_store, std::move(skia_surface), + collect_callback.Release(), on_make_current); + break; } } break; } + case kFlutterBackingStoreTypeSoftware: { auto skia_surface = MakeSkSurfaceFromBackingStore( context, config, &backing_store.software); diff --git a/shell/platform/embedder/embedder_render_target_skia.h b/shell/platform/embedder/embedder_render_target_skia.h index 29fb3b2371434..dae01aef51f74 100644 --- a/shell/platform/embedder/embedder_render_target_skia.h +++ b/shell/platform/embedder/embedder_render_target_skia.h @@ -31,6 +31,7 @@ class EmbedderRenderTargetSkia final : public EmbedderRenderTarget { // |EmbedderRenderTarget| SkISize GetRenderTargetSize() const override; + // |EmbedderRenderTarget| bool MaybeMakeCurrent() const override { if (on_make_current_ != nullptr) { on_make_current_(); From c30360365b323ec3b9cb13fa00ca497cc536abcd Mon Sep 17 00:00:00 2001 From: Hannes Winkler Date: Tue, 25 Jul 2023 17:09:50 +0200 Subject: [PATCH 03/30] implement EGL surface backing stores Use `std::pair ()` as function signature for internal make and clear current callbacks, `bool (*)(void*, bool*)` as signature for the embedder-given callbacks. Actually implement invalidating graphics API state cache. Log errors when make / clear current operation fails. Document FlutterOpenGLSurface in embedder API. --- shell/platform/embedder/embedder.cc | 55 +++++++++++++------ shell/platform/embedder/embedder.h | 41 +++++++++++++- .../embedder/embedder_external_view.cc | 47 ++++++++++++++-- .../embedder/embedder_render_target.h | 32 +++++++++-- .../embedder/embedder_render_target_skia.cc | 22 +++++++- .../embedder/embedder_render_target_skia.h | 18 +++--- 6 files changed, 171 insertions(+), 44 deletions(-) diff --git a/shell/platform/embedder/embedder.cc b/shell/platform/embedder/embedder.cc index ffd9d2c49e880..b57947b7930a5 100644 --- a/shell/platform/embedder/embedder.cc +++ b/shell/platform/embedder/embedder.cc @@ -1201,16 +1201,19 @@ static sk_sp MakeSkSurfaceFromBackingStore( } static std::unique_ptr -MakeRenderTargetFromSkSurface(FlutterBackingStore backing_store, - sk_sp skia_surface, - fml::closure on_release, - fml::closure on_make_current) { +MakeRenderTargetFromSkSurface( + FlutterBackingStore backing_store, + sk_sp skia_surface, + fml::closure on_release, + flutter::EmbedderRenderTarget::MakeOrClearCurrentCallback on_make_current, + flutter::EmbedderRenderTarget::MakeOrClearCurrentCallback + on_clear_current) { if (!skia_surface) { return nullptr; } return std::make_unique( backing_store, std::move(skia_surface), std::move(on_release), - std::move(on_make_current)); + std::move(on_make_current), std::move(on_clear_current)); } static std::unique_ptr @@ -1218,7 +1221,7 @@ MakeRenderTargetFromSkSurface(FlutterBackingStore backing_store, sk_sp skia_surface, fml::closure on_release) { return MakeRenderTargetFromSkSurface(backing_store, skia_surface, on_release, - nullptr); + nullptr, nullptr); } static std::unique_ptr @@ -1292,21 +1295,37 @@ CreateEmbedderRenderTarget( } case kFlutterOpenGLTargetTypeSurface: { - auto skia_surface = MakeSkSurfaceFromBackingStore( - context, config, &backing_store.open_gl.surface); - - auto make_current_callback = - backing_store.open_gl.surface.make_current_callback; - auto make_current_context = backing_store.open_gl.surface.user_data; + auto on_make_current = + [callback = backing_store.open_gl.surface.make_current_callback, + context = backing_store.open_gl.surface.user_data]() + -> std::pair { + bool invalidate_api_state = false; + bool ok = callback(context, &invalidate_api_state); + return {ok, invalidate_api_state}; + }; - auto on_make_current = [=] { - make_current_callback(make_current_context); + auto on_clear_current = + [callback = backing_store.open_gl.surface.clear_current_callback, + context = backing_store.open_gl.surface.user_data]() + -> std::pair { + bool invalidate_api_state = false; + bool ok = callback(context, &invalidate_api_state); + return {ok, invalidate_api_state}; }; - render_target = MakeRenderTargetFromSkSurface( - backing_store, std::move(skia_surface), - collect_callback.Release(), on_make_current); - break; + if (enable_impeller) { + /// TODO: Implement + FML_LOG(ERROR) << "Unimplemented"; + break; + } else { + auto skia_surface = MakeSkSurfaceFromBackingStore( + context, config, &backing_store.open_gl.surface); + + render_target = MakeRenderTargetFromSkSurface( + backing_store, std::move(skia_surface), + collect_callback.Release(), on_make_current, on_clear_current); + break; + } } } break; diff --git a/shell/platform/embedder/embedder.h b/shell/platform/embedder/embedder.h index 287b3f059c87d..580d7e11e9353 100644 --- a/shell/platform/embedder/embedder.h +++ b/shell/platform/embedder/embedder.h @@ -411,18 +411,53 @@ typedef struct { VoidCallback destruction_callback; } FlutterOpenGLFramebuffer; +typedef bool (*FlutterOpenGLSurfaceCallback)(void* /* user data */, + bool* /* opengl state changed */); + typedef struct { + /// The size of this struct. Must be sizeof(FlutterOpenGLSurface). size_t struct_size; /// User data to be returned on the invocation of the destruction callback. void* user_data; /// Callback invoked (on an engine managed thread) that asks the embedder to - /// make the window surface current. - VoidCallback make_current_callback; + /// make the surface current. + /// + /// Should return true if the operation succeeded, false if the surface could + /// not be made current and rendering should be cancelled. + /// + /// The second parameter 'opengl state changed' should be set to true if + /// any OpenGL API state has changed and flutter should not assume any native + /// API state is the same as before this callback was called. + /// + /// In that case, flutter will invalidate the internal OpenGL API state cache, + /// which is a somewhat expensive operation. + /// + /// @attention required. (non-null) + FlutterOpenGLSurfaceCallback make_current_callback; + + /// Callback invoked (on an engine managed thread) when the current surface + /// can be cleared. + /// + /// Should return true if the operation succeeded, false if an error ocurred. + /// That error will be logged but otherwise not handled by the engine. + /// + /// The second parameter 'opengl state changed' is the same as with the + /// @ref make_current_callback. + /// + /// The embedder might clear the surface here after it was previously made + /// current. That's not required however, it's also possible to clear it in + /// the destruction callback. There's no way to signal opengl state + /// changes in the destruction callback though. + /// + /// @attention required. (non-null) + FlutterOpenGLSurfaceCallback clear_current_callback; /// Callback invoked (on an engine managed thread) that asks the embedder to - /// collect the framebuffer. + /// collect the surface. + /// + /// @attention required. (non-null) VoidCallback destruction_callback; } FlutterOpenGLSurface; diff --git a/shell/platform/embedder/embedder_external_view.cc b/shell/platform/embedder/embedder_external_view.cc index 2cbebd5f0cad4..7eab1a59b9ab7 100644 --- a/shell/platform/embedder/embedder_external_view.cc +++ b/shell/platform/embedder/embedder_external_view.cc @@ -7,6 +7,8 @@ #include "flutter/display_list/dl_builder.h" #include "flutter/fml/trace_event.h" #include "flutter/shell/common/dl_op_spy.h" +#include "third_party/skia/include/gpu/GrDirectContext.h" +#include "third_party/skia/include/gpu/GrRecordingContext.h" #ifdef IMPELLER_SUPPORTS_RENDERING #include "impeller/display_list/dl_dispatcher.h" // nogncheck @@ -88,8 +90,25 @@ const EmbeddedViewParams* EmbedderExternalView::GetEmbeddedViewParams() const { return embedded_view_params_.get(); } +static void InvalidateApiState(SkSurface& skia_surface) { + auto recording_context = skia_surface.recordingContext(); + + // Should never happen. + FML_DCHECK(recording_context) << "Recording context was null."; + + auto direct_context = recording_context->asDirectContext(); + if (direct_context == nullptr) { + // Can happen when using software rendering. + // Print an error but otherwise continue in that case. + FML_LOG(ERROR) << "Embedder asked to invalidate cached graphics API state " + "but flutter is not using a graphics API."; + } else { + direct_context->resetContext(kAll_GrBackendState); + } +} + bool EmbedderExternalView::Render(const EmbedderRenderTarget& render_target, - bool clear_surface) { + bool clear_surface) { TRACE_EVENT0("flutter", "EmbedderExternalView::Render"); TryEndRecording(); FML_DCHECK(HasEngineRenderedContents()) @@ -143,11 +162,27 @@ bool EmbedderExternalView::Render(const EmbedderRenderTarget& render_target, return false; } - bool invalidate_gl_state = render_target.MaybeMakeCurrent(); - // TODO(ardera): Implement - // if (invalidate_gl_state) { - // skia_surface->recordingContext()->asDirectContext()->resetContext(kAll_GrBackendState); - // } + auto [ok, invalidate_api_state] = render_target.MaybeMakeCurrent(); + + if (invalidate_api_state) { + InvalidateApiState(*skia_surface); + } + if (!ok) { + FML_LOG(ERROR) << "Could not make the surface current."; + return false; + } + + // clear the current render target (most likely EGLSurface) at the + // end of this scope. + fml::ScopedCleanupClosure clear_surface([&]() { + auto [ok, invalidate_api_state] = render_target.MaybeClearCurrent(); + if (invalidate_api_state) { + InvalidateApiState(*skia_surface); + } + if (!ok) { + FML_LOG(ERROR) << "Could not clear the current surface."; + } + }); FML_DCHECK(render_target.GetRenderTargetSize() == render_surface_size_); diff --git a/shell/platform/embedder/embedder_render_target.h b/shell/platform/embedder/embedder_render_target.h index 074ac057e364a..87120a52d6697 100644 --- a/shell/platform/embedder/embedder_render_target.h +++ b/shell/platform/embedder/embedder_render_target.h @@ -27,6 +27,8 @@ namespace flutter { /// class EmbedderRenderTarget { public: + using MakeOrClearCurrentCallback = std::function()>; + //---------------------------------------------------------------------------- /// @brief Destroys this instance of the render target and invokes the /// callback for the embedder to release its resource associated @@ -78,16 +80,38 @@ class EmbedderRenderTarget { const FlutterBackingStore* GetBackingStore() const; //---------------------------------------------------------------------------- - /// @brief Sometimes render targets are actually (for example) + /// @brief Make the render target current. + /// + /// Sometimes render targets are actually (for example) /// EGL surfaces instead of framebuffers or textures. /// In that case, we can't fully wrap them as SkSurfaces, instead, /// the embedder will provide a callback that should be called /// when the target surface should be made current. /// - /// @return True if any GL (not EGL) state has changed and skia should - /// not assume any GL state values are the same as before + /// @return A pair of booleans. The first bool is true if the operation + /// succeeded (even if it was a no-op), false if the target could + /// not be make current. + /// The second boolean is true if any native graphics API + /// (e.g. GL, but not EGL) state has changed and skia/impeller + /// should not assume any GL state values are the same as before /// MaybeMakeCurrent was called. - virtual bool MaybeMakeCurrent() const { return false; } + virtual std::pair MaybeMakeCurrent() const { + return {true, false}; + } + + //---------------------------------------------------------------------------- + /// @brief Clear the current render target. @see MaybeMakeCurrent + /// + /// @return A pair of booleans. The first bool is true if the operation + /// succeeded (even if it was a no-op), false if the target could + /// not be cleared. + /// The second boolean is true if any native graphics API + /// (e.g. GL, but not EGL) state has changed and skia/impeller + /// should not assume any GL state values are the same as before + /// MaybeClearCurrent was called. + virtual std::pair MaybeClearCurrent() const { + return {true, false}; + } protected: //---------------------------------------------------------------------------- diff --git a/shell/platform/embedder/embedder_render_target_skia.cc b/shell/platform/embedder/embedder_render_target_skia.cc index 9f013ad80ec09..2d5167db4958a 100644 --- a/shell/platform/embedder/embedder_render_target_skia.cc +++ b/shell/platform/embedder/embedder_render_target_skia.cc @@ -12,10 +12,12 @@ EmbedderRenderTargetSkia::EmbedderRenderTargetSkia( FlutterBackingStore backing_store, sk_sp render_surface, fml::closure on_release, - fml::closure on_make_current) + MakeOrClearCurrentCallback on_make_current, + MakeOrClearCurrentCallback on_clear_current) : EmbedderRenderTarget(backing_store, std::move(on_release)), render_surface_(std::move(render_surface)), - on_make_current_(std::move(on_make_current)) { + on_make_current_(std::move(on_make_current)), + on_clear_current_(std::move(on_clear_current)) { FML_DCHECK(render_surface_); } @@ -39,4 +41,20 @@ SkISize EmbedderRenderTargetSkia::GetRenderTargetSize() const { return SkISize::Make(render_surface_->width(), render_surface_->height()); } +std::pair EmbedderRenderTargetSkia::MaybeMakeCurrent() const { + if (on_make_current_ != nullptr) { + return on_make_current_(); + } + + return {true, false}; +} + +std::pair EmbedderRenderTargetSkia::MaybeClearCurrent() const { + if (on_clear_current_ != nullptr) { + return on_clear_current_(); + } + + return {true, false}; +} + } // namespace flutter diff --git a/shell/platform/embedder/embedder_render_target_skia.h b/shell/platform/embedder/embedder_render_target_skia.h index dae01aef51f74..48e2c8604b980 100644 --- a/shell/platform/embedder/embedder_render_target_skia.h +++ b/shell/platform/embedder/embedder_render_target_skia.h @@ -14,7 +14,8 @@ class EmbedderRenderTargetSkia final : public EmbedderRenderTarget { EmbedderRenderTargetSkia(FlutterBackingStore backing_store, sk_sp render_surface, fml::closure on_release, - fml::closure on_make_current); + MakeOrClearCurrentCallback on_make_current, + MakeOrClearCurrentCallback on_clear_current); // |EmbedderRenderTarget| ~EmbedderRenderTargetSkia() override; @@ -32,21 +33,16 @@ class EmbedderRenderTargetSkia final : public EmbedderRenderTarget { SkISize GetRenderTargetSize() const override; // |EmbedderRenderTarget| - bool MaybeMakeCurrent() const override { - if (on_make_current_ != nullptr) { - on_make_current_(); + std::pair MaybeMakeCurrent() const override; - /// TODO: Implement return value - return false; - } - - return false; - } + // |EmbedderRenderTarget| + std::pair MaybeClearCurrent() const override; private: sk_sp render_surface_; - fml::closure on_make_current_; + MakeOrClearCurrentCallback on_make_current_; + MakeOrClearCurrentCallback on_clear_current_; FML_DISALLOW_COPY_AND_ASSIGN(EmbedderRenderTargetSkia); }; From 28992bd1c3a437481ad16bcf5de2fcdd6c182ad9 Mon Sep 17 00:00:00 2001 From: Hannes Winkler Date: Sat, 19 Aug 2023 12:17:45 +0200 Subject: [PATCH 04/30] fix embedder gl surface docs --- shell/platform/embedder/embedder.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/shell/platform/embedder/embedder.h b/shell/platform/embedder/embedder.h index 580d7e11e9353..dfdf3fe1cb614 100644 --- a/shell/platform/embedder/embedder.h +++ b/shell/platform/embedder/embedder.h @@ -418,7 +418,8 @@ typedef struct { /// The size of this struct. Must be sizeof(FlutterOpenGLSurface). size_t struct_size; - /// User data to be returned on the invocation of the destruction callback. + /// User data to be passed to the make_current, clear_current and + /// destruction callback. void* user_data; /// Callback invoked (on an engine managed thread) that asks the embedder to From 2858e262a33041e8569dc8326ce23285b8a8eff4 Mon Sep 17 00:00:00 2001 From: Hannes Winkler Date: Sat, 19 Aug 2023 12:17:55 +0200 Subject: [PATCH 05/30] Add tests for OpenGL Surface backing stores All the EGL context & config details where previously hidden as implementation details in TestGLSurface. It was necessary to expose that and make it explicit, so you can create multiple surfaces that share a context. That explicit EGL context & config is contained in class TestEGLContext. All opengl tests now have a single TestEGLContext (see EmbedderTestContext->egl_context_) that's used by default when creating the GL Backing Store producer and the root GL surface that's used for all the OpenGL Renderer callbacks. The GL Backing Store producer will use that EGL context to create any GL surface backing stores, when it is configured to do so. Furthermore, for creating such backing stores, TestGLSurfaces are kinda heavyweight, so a new TestGLOnscreenOnlySurface was introduced, which is a lighter variant of that. The snapshotting mechanism in embedder_test_compositor_gl.cc had to be modified a bit, since snapshotting GL surfaces is a bit more involved. (Surface needs to be made current) --- shell/platform/embedder/embedder.cc | 2 +- .../embedder/embedder_external_view.cc | 4 +- .../embedder/tests/embedder_config_builder.cc | 12 +- .../embedder/tests/embedder_gl_unittests.cc | 324 ++++++++++++++++++ .../embedder_test_backingstore_producer.cc | 108 ++++-- .../embedder_test_backingstore_producer.h | 19 + .../tests/embedder_test_compositor_gl.cc | 27 +- .../embedder_test_compositor_software.cc | 3 +- .../embedder/tests/embedder_test_context.h | 4 + .../tests/embedder_test_context_gl.cc | 6 +- .../embedder/tests/embedder_test_context_gl.h | 2 + .../embedder/tests/embedder_unittests.cc | 5 +- testing/test_gl_surface.cc | 191 ++++++----- testing/test_gl_surface.h | 63 +++- 14 files changed, 631 insertions(+), 139 deletions(-) diff --git a/shell/platform/embedder/embedder.cc b/shell/platform/embedder/embedder.cc index b57947b7930a5..04f1c78cbc5eb 100644 --- a/shell/platform/embedder/embedder.cc +++ b/shell/platform/embedder/embedder.cc @@ -842,7 +842,7 @@ static sk_sp MakeSkSurfaceFromBackingStore( framebuffer_info.fFormat = GL_BGRA8_EXT; framebuffer_info.fFBOID = 0; - GrBackendRenderTarget backend_render_target( + auto backend_render_target = GrBackendRenderTargets::MakeGL( config.size.width, // width config.size.height, // height 1, // sample count diff --git a/shell/platform/embedder/embedder_external_view.cc b/shell/platform/embedder/embedder_external_view.cc index 7eab1a59b9ab7..6cf7a7fa5dd6b 100644 --- a/shell/platform/embedder/embedder_external_view.cc +++ b/shell/platform/embedder/embedder_external_view.cc @@ -108,7 +108,7 @@ static void InvalidateApiState(SkSurface& skia_surface) { } bool EmbedderExternalView::Render(const EmbedderRenderTarget& render_target, - bool clear_surface) { + bool clear_surface) { TRACE_EVENT0("flutter", "EmbedderExternalView::Render"); TryEndRecording(); FML_DCHECK(HasEngineRenderedContents()) @@ -174,7 +174,7 @@ bool EmbedderExternalView::Render(const EmbedderRenderTarget& render_target, // clear the current render target (most likely EGLSurface) at the // end of this scope. - fml::ScopedCleanupClosure clear_surface([&]() { + fml::ScopedCleanupClosure clear_current_surface([&]() { auto [ok, invalidate_api_state] = render_target.MaybeClearCurrent(); if (invalidate_api_state) { InvalidateApiState(*skia_surface); diff --git a/shell/platform/embedder/tests/embedder_config_builder.cc b/shell/platform/embedder/tests/embedder_config_builder.cc index fa89c66e65b5d..3d39cca7ed3f3 100644 --- a/shell/platform/embedder/tests/embedder_config_builder.cc +++ b/shell/platform/embedder/tests/embedder_config_builder.cc @@ -407,11 +407,17 @@ void EmbedderConfigBuilder::SetRenderTargetType( EmbedderTestBackingStoreProducer::RenderTargetType type, FlutterSoftwarePixelFormat software_pixfmt) { auto& compositor = context_.GetCompositor(); + + auto producer = std::make_unique( + compositor.GetGrContext(), type, software_pixfmt); + +#ifdef SHELL_ENABLE_GL + producer->SetEGLContext(context_.egl_context_); +#endif + // TODO(wrightgeorge): figure out a better way of plumbing through the // GrDirectContext - compositor.SetBackingStoreProducer( - std::make_unique( - compositor.GetGrContext(), type, software_pixfmt)); + compositor.SetBackingStoreProducer(std::move(producer)); } UniqueEngine EmbedderConfigBuilder::LaunchEngine() const { diff --git a/shell/platform/embedder/tests/embedder_gl_unittests.cc b/shell/platform/embedder/tests/embedder_gl_unittests.cc index 0d01bc19187c7..9f3d82da09654 100644 --- a/shell/platform/embedder/tests/embedder_gl_unittests.cc +++ b/shell/platform/embedder/tests/embedder_gl_unittests.cc @@ -4806,6 +4806,330 @@ TEST_F(EmbedderTest, CanRenderWithImpellerOpenGL) { ASSERT_FALSE(present_called); } +TEST_F(EmbedderTest, CompositorMustBeAbleToRenderToOpenGLSurface) { + auto& context = GetEmbedderContext(EmbedderTestContextType::kOpenGLContext); + + EmbedderConfigBuilder builder(context); + builder.SetOpenGLRendererConfig(SkISize::Make(800, 600)); + builder.SetCompositor(); + builder.SetDartEntrypoint("can_composite_platform_views"); + + builder.SetRenderTargetType( + EmbedderTestBackingStoreProducer::RenderTargetType::kOpenGLSurface); + + fml::CountDownLatch latch(3); + context.GetCompositor().SetNextPresentCallback( + [&](const FlutterLayer** layers, size_t layers_count) { + ASSERT_EQ(layers_count, 3u); + + { + FlutterBackingStore backing_store = *layers[0]->backing_store; + backing_store.struct_size = sizeof(backing_store); + backing_store.type = kFlutterBackingStoreTypeOpenGL; + backing_store.did_update = true; + backing_store.open_gl.type = kFlutterOpenGLTargetTypeSurface; + + FlutterRect paint_region_rects[] = { + FlutterRectMakeLTRB(0, 0, 800, 600), + }; + FlutterRegion paint_region = { + .struct_size = sizeof(FlutterRegion), + .rects_count = 1, + .rects = paint_region_rects, + }; + FlutterBackingStorePresentInfo present_info = { + .struct_size = sizeof(FlutterBackingStorePresentInfo), + .paint_region = &paint_region, + }; + + FlutterLayer layer = {}; + layer.struct_size = sizeof(layer); + layer.type = kFlutterLayerContentTypeBackingStore; + layer.backing_store = &backing_store; + layer.size = FlutterSizeMake(800.0, 600.0); + layer.offset = FlutterPointMake(0, 0); + layer.backing_store_present_info = &present_info; + ASSERT_EQ(*layers[0], layer); + } + + { + FlutterPlatformView platform_view = *layers[1]->platform_view; + platform_view.struct_size = sizeof(platform_view); + platform_view.identifier = 42; + + FlutterLayer layer = {}; + layer.struct_size = sizeof(layer); + layer.type = kFlutterLayerContentTypePlatformView; + layer.platform_view = &platform_view; + layer.size = FlutterSizeMake(123.0, 456.0); + layer.offset = FlutterPointMake(1.0, 2.0); + + ASSERT_EQ(*layers[1], layer); + } + + { + FlutterBackingStore backing_store = *layers[2]->backing_store; + backing_store.struct_size = sizeof(backing_store); + backing_store.type = kFlutterBackingStoreTypeOpenGL; + backing_store.did_update = true; + backing_store.open_gl.type = kFlutterOpenGLTargetTypeSurface; + + FlutterRect paint_region_rects[] = { + FlutterRectMakeLTRB(2, 3, 800, 600), + }; + FlutterRegion paint_region = { + .struct_size = sizeof(FlutterRegion), + .rects_count = 1, + .rects = paint_region_rects, + }; + FlutterBackingStorePresentInfo present_info = { + .struct_size = sizeof(FlutterBackingStorePresentInfo), + .paint_region = &paint_region, + }; + + FlutterLayer layer = {}; + layer.struct_size = sizeof(layer); + layer.type = kFlutterLayerContentTypeBackingStore; + layer.backing_store = &backing_store; + layer.size = FlutterSizeMake(800.0, 600.0); + layer.offset = FlutterPointMake(0.0, 0.0); + layer.backing_store_present_info = &present_info; + + ASSERT_EQ(*layers[2], layer); + } + + latch.CountDown(); + }); + + context.AddNativeCallback( + "SignalNativeTest", + CREATE_NATIVE_ENTRY( + [&latch](Dart_NativeArguments args) { latch.CountDown(); })); + + auto engine = builder.LaunchEngine(); + + // Send a window metrics events so frames may be scheduled. + FlutterWindowMetricsEvent event = {}; + event.struct_size = sizeof(event); + event.width = 800; + event.height = 600; + event.pixel_ratio = 1.0; + ASSERT_EQ(FlutterEngineSendWindowMetricsEvent(engine.get(), &event), + kSuccess); + ASSERT_TRUE(engine.is_valid()); + + latch.Wait(); +} + +TEST_F(EmbedderTest, CompositorMustBeAbleToRenderKnownSceneToOpenGLSurfaces) { + auto& context = GetEmbedderContext(EmbedderTestContextType::kOpenGLContext); + + EmbedderConfigBuilder builder(context); + builder.SetOpenGLRendererConfig(SkISize::Make(800, 600)); + builder.SetCompositor(); + builder.SetDartEntrypoint("can_composite_platform_views_with_known_scene"); + + builder.SetRenderTargetType( + EmbedderTestBackingStoreProducer::RenderTargetType::kOpenGLSurface); + + fml::CountDownLatch latch(5); + + auto scene_image = context.GetNextSceneImage(); + + context.GetCompositor().SetNextPresentCallback( + [&](const FlutterLayer** layers, size_t layers_count) { + ASSERT_EQ(layers_count, 5u); + + // Layer Root + { + FlutterBackingStore backing_store = *layers[0]->backing_store; + backing_store.type = kFlutterBackingStoreTypeOpenGL; + backing_store.did_update = true; + backing_store.open_gl.type = kFlutterOpenGLTargetTypeSurface; + + FlutterRect paint_region_rects[] = { + FlutterRectMakeLTRB(0, 0, 800, 600), + }; + FlutterRegion paint_region = { + .struct_size = sizeof(FlutterRegion), + .rects_count = 1, + .rects = paint_region_rects, + }; + FlutterBackingStorePresentInfo present_info = { + .struct_size = sizeof(FlutterBackingStorePresentInfo), + .paint_region = &paint_region, + }; + + FlutterLayer layer = {}; + layer.struct_size = sizeof(layer); + layer.type = kFlutterLayerContentTypeBackingStore; + layer.backing_store = &backing_store; + layer.size = FlutterSizeMake(800.0, 600.0); + layer.offset = FlutterPointMake(0.0, 0.0); + layer.backing_store_present_info = &present_info; + + ASSERT_EQ(*layers[0], layer); + } + + // Layer 1 + { + FlutterPlatformView platform_view = *layers[1]->platform_view; + platform_view.struct_size = sizeof(platform_view); + platform_view.identifier = 1; + + FlutterLayer layer = {}; + layer.struct_size = sizeof(layer); + layer.type = kFlutterLayerContentTypePlatformView; + layer.platform_view = &platform_view; + layer.size = FlutterSizeMake(50.0, 150.0); + layer.offset = FlutterPointMake(20.0, 20.0); + + ASSERT_EQ(*layers[1], layer); + } + + // Layer 2 + { + FlutterBackingStore backing_store = *layers[2]->backing_store; + backing_store.type = kFlutterBackingStoreTypeOpenGL; + backing_store.did_update = true; + backing_store.open_gl.type = kFlutterOpenGLTargetTypeSurface; + + FlutterRect paint_region_rects[] = { + FlutterRectMakeLTRB(30, 30, 80, 180), + }; + FlutterRegion paint_region = { + .struct_size = sizeof(FlutterRegion), + .rects_count = 1, + .rects = paint_region_rects, + }; + FlutterBackingStorePresentInfo present_info = { + .struct_size = sizeof(FlutterBackingStorePresentInfo), + .paint_region = &paint_region, + }; + + FlutterLayer layer = {}; + layer.struct_size = sizeof(layer); + layer.type = kFlutterLayerContentTypeBackingStore; + layer.backing_store = &backing_store; + layer.size = FlutterSizeMake(800.0, 600.0); + layer.offset = FlutterPointMake(0.0, 0.0); + layer.backing_store_present_info = &present_info; + + ASSERT_EQ(*layers[2], layer); + } + + // Layer 3 + { + FlutterPlatformView platform_view = *layers[3]->platform_view; + platform_view.struct_size = sizeof(platform_view); + platform_view.identifier = 2; + + FlutterLayer layer = {}; + layer.struct_size = sizeof(layer); + layer.type = kFlutterLayerContentTypePlatformView; + layer.platform_view = &platform_view; + layer.size = FlutterSizeMake(50.0, 150.0); + layer.offset = FlutterPointMake(40.0, 40.0); + + ASSERT_EQ(*layers[3], layer); + } + + // Layer 4 + { + FlutterBackingStore backing_store = *layers[4]->backing_store; + backing_store.type = kFlutterBackingStoreTypeOpenGL; + backing_store.did_update = true; + backing_store.open_gl.type = kFlutterOpenGLTargetTypeSurface; + + FlutterRect paint_region_rects[] = { + FlutterRectMakeLTRB(50, 50, 100, 200), + }; + FlutterRegion paint_region = { + .struct_size = sizeof(FlutterRegion), + .rects_count = 1, + .rects = paint_region_rects, + }; + FlutterBackingStorePresentInfo present_info = { + .struct_size = sizeof(FlutterBackingStorePresentInfo), + .paint_region = &paint_region, + }; + + FlutterLayer layer = {}; + layer.struct_size = sizeof(layer); + layer.type = kFlutterLayerContentTypeBackingStore; + layer.backing_store = &backing_store; + layer.size = FlutterSizeMake(800.0, 600.0); + layer.offset = FlutterPointMake(0.0, 0.0); + layer.backing_store_present_info = &present_info; + + ASSERT_EQ(*layers[4], layer); + } + + latch.CountDown(); + }); + + context.GetCompositor().SetPlatformViewRendererCallback( + [&](const FlutterLayer& layer, + GrDirectContext* context) -> sk_sp { + auto surface = CreateRenderSurface(layer, context); + auto canvas = surface->getCanvas(); + FML_CHECK(canvas != nullptr); + + switch (layer.platform_view->identifier) { + case 1: { + SkPaint paint; + // See dart test for total order. + paint.setColor(SK_ColorGREEN); + paint.setAlpha(127); + const auto& rect = + SkRect::MakeWH(layer.size.width, layer.size.height); + canvas->drawRect(rect, paint); + latch.CountDown(); + } break; + case 2: { + SkPaint paint; + // See dart test for total order. + paint.setColor(SK_ColorMAGENTA); + paint.setAlpha(127); + const auto& rect = + SkRect::MakeWH(layer.size.width, layer.size.height); + canvas->drawRect(rect, paint); + latch.CountDown(); + } break; + default: + // Asked to render an unknown platform view. + FML_CHECK(false) + << "Test was asked to composite an unknown platform view."; + } + + return surface->makeImageSnapshot(); + }); + + context.AddNativeCallback( + "SignalNativeTest", + CREATE_NATIVE_ENTRY( + [&latch](Dart_NativeArguments args) { latch.CountDown(); })); + + auto engine = builder.LaunchEngine(); + + // Send a window metrics events so frames may be scheduled. + FlutterWindowMetricsEvent event = {}; + event.struct_size = sizeof(event); + event.width = 800; + event.height = 600; + event.pixel_ratio = 1.0; + ASSERT_EQ(FlutterEngineSendWindowMetricsEvent(engine.get(), &event), + kSuccess); + ASSERT_TRUE(engine.is_valid()); + + latch.Wait(); + + ASSERT_TRUE(ImageMatchesFixture("compositor.png", scene_image)); + + // There should no present calls on the root surface. + ASSERT_EQ(context.GetSurfacePresentCount(), 0u); +} + INSTANTIATE_TEST_SUITE_P( EmbedderTestGlVk, EmbedderTestMultiBackend, diff --git a/shell/platform/embedder/tests/embedder_test_backingstore_producer.cc b/shell/platform/embedder/tests/embedder_test_backingstore_producer.cc index ac6959cacb66b..074a00081033e 100644 --- a/shell/platform/embedder/tests/embedder_test_backingstore_producer.cc +++ b/shell/platform/embedder/tests/embedder_test_backingstore_producer.cc @@ -29,7 +29,11 @@ #ifdef SHELL_ENABLE_METAL #include "third_party/skia/include/gpu/ganesh/mtl/GrMtlBackendSurface.h" #include "third_party/skia/include/gpu/ganesh/mtl/GrMtlTypes.h" -#endif +#endif // SHELL_ENABLE_METAL + +#ifdef SHELL_ENABLE_GL +#include "flutter/testing/test_gl_surface.h" +#endif // SHELL_ENABLE_GL // TODO(zanderso): https://github.com/flutter/flutter/issues/127701 // NOLINTBEGIN(bugprone-unchecked-optional-access) @@ -44,6 +48,10 @@ EmbedderTestBackingStoreProducer::EmbedderTestBackingStoreProducer( : context_(std::move(context)), type_(type), software_pixfmt_(software_pixfmt) +#ifdef SHELL_ENABLE_GL + , + test_egl_context_(nullptr) +#endif #ifdef SHELL_ENABLE_METAL , test_metal_context_(std::make_unique()) @@ -66,6 +74,16 @@ EmbedderTestBackingStoreProducer::EmbedderTestBackingStoreProducer( EmbedderTestBackingStoreProducer::~EmbedderTestBackingStoreProducer() = default; +#ifdef SHELL_ENABLE_GL +void EmbedderTestBackingStoreProducer::SetEGLContext( + std::shared_ptr context) { + // Ideally this would be set in the constructor, however we can't do that + // without introducing different constructors depending on different graphics + // APIs, which is a bit ugly. + test_egl_context_ = context; +} +#endif + bool EmbedderTestBackingStoreProducer::Create( const FlutterBackingStoreConfig* config, FlutterBackingStore* renderer_out) { @@ -79,6 +97,8 @@ bool EmbedderTestBackingStoreProducer::Create( return CreateTexture(config, renderer_out); case RenderTargetType::kOpenGLFramebuffer: return CreateFramebuffer(config, renderer_out); + case RenderTargetType::kOpenGLSurface: + return CreateSurface(config, renderer_out); #endif #ifdef SHELL_ENABLE_METAL case RenderTargetType::kMetalTexture: @@ -130,16 +150,18 @@ bool EmbedderTestBackingStoreProducer::CreateFramebuffer( return false; } + auto userdata = new GLUserData{nullptr, surface}; + backing_store_out->type = kFlutterBackingStoreTypeOpenGL; - backing_store_out->user_data = surface.get(); + backing_store_out->user_data = userdata; backing_store_out->open_gl.type = kFlutterOpenGLTargetTypeFramebuffer; backing_store_out->open_gl.framebuffer.target = framebuffer_info.fFormat; backing_store_out->open_gl.framebuffer.name = framebuffer_info.fFBOID; // The balancing unref is in the destruction callback. surface->ref(); - backing_store_out->open_gl.framebuffer.user_data = surface.get(); + backing_store_out->open_gl.framebuffer.user_data = userdata; backing_store_out->open_gl.framebuffer.destruction_callback = - [](void* user_data) { reinterpret_cast(user_data)->unref(); }; + [](void* user_data) { delete reinterpret_cast(user_data); }; return true; #else @@ -183,17 +205,64 @@ bool EmbedderTestBackingStoreProducer::CreateTexture( return false; } + auto userdata = new GLUserData{nullptr, surface}; + backing_store_out->type = kFlutterBackingStoreTypeOpenGL; - backing_store_out->user_data = surface.get(); + backing_store_out->user_data = userdata; backing_store_out->open_gl.type = kFlutterOpenGLTargetTypeTexture; backing_store_out->open_gl.texture.target = texture_info.fTarget; backing_store_out->open_gl.texture.name = texture_info.fID; backing_store_out->open_gl.texture.format = texture_info.fFormat; - // The balancing unref is in the destruction callback. - surface->ref(); - backing_store_out->open_gl.texture.user_data = surface.get(); + backing_store_out->open_gl.texture.user_data = userdata; backing_store_out->open_gl.texture.destruction_callback = - [](void* user_data) { reinterpret_cast(user_data)->unref(); }; + [](void* user_data) { delete reinterpret_cast(user_data); }; + + return true; +#else + return false; +#endif +} + +bool EmbedderTestBackingStoreProducer::CreateSurface( + const FlutterBackingStoreConfig* config, + FlutterBackingStore* backing_store_out) { +#ifdef SHELL_ENABLE_GL + FML_CHECK(test_egl_context_); + auto surface = std::make_unique( + test_egl_context_, + SkSize::Make(config->size.width, config->size.height).toRound()); + + auto make_current = [](void* userdata, bool* invalidate_state) -> bool { + *invalidate_state = false; + return reinterpret_cast(userdata)->gl_surface->MakeCurrent(); + }; + + auto clear_current = [](void* userdata, bool* invalidate_state) -> bool { + *invalidate_state = false; + // return + // reinterpret_cast(userdata)->gl_surface->ClearCurrent(); + return true; + }; + + auto destruction_callback = [](void* userdata) { + delete reinterpret_cast(userdata); + }; + + auto sk_surface = surface->GetOnscreenSurface(); + + auto userdata = new GLUserData{ + std::move(surface), + sk_surface, + }; + + backing_store_out->type = kFlutterBackingStoreTypeOpenGL; + backing_store_out->user_data = userdata; + backing_store_out->open_gl.type = kFlutterOpenGLTargetTypeSurface; + backing_store_out->open_gl.surface.user_data = userdata; + backing_store_out->open_gl.surface.make_current_callback = make_current; + backing_store_out->open_gl.surface.clear_current_callback = clear_current; + backing_store_out->open_gl.surface.destruction_callback = + destruction_callback; return true; #else @@ -219,16 +288,16 @@ bool EmbedderTestBackingStoreProducer::CreateSoftware( return false; } + auto userdata = new GLUserData{nullptr, surface}; + backing_store_out->type = kFlutterBackingStoreTypeSoftware; - backing_store_out->user_data = surface.get(); + backing_store_out->user_data = userdata; backing_store_out->software.allocation = pixmap.addr(); backing_store_out->software.row_bytes = pixmap.rowBytes(); backing_store_out->software.height = pixmap.height(); - // The balancing unref is in the destruction callback. - surface->ref(); - backing_store_out->software.user_data = surface.get(); + backing_store_out->software.user_data = userdata; backing_store_out->software.destruction_callback = [](void* user_data) { - reinterpret_cast(user_data)->unref(); + delete reinterpret_cast(user_data); }; return true; @@ -256,19 +325,18 @@ bool EmbedderTestBackingStoreProducer::CreateSoftware2( return false; } + auto userdata = new GLUserData{nullptr, surface}; + backing_store_out->type = kFlutterBackingStoreTypeSoftware2; - backing_store_out->user_data = surface.get(); + backing_store_out->user_data = userdata; backing_store_out->software2.struct_size = sizeof(FlutterSoftwareBackingStore2); - backing_store_out->software2.user_data = surface.get(); backing_store_out->software2.allocation = pixmap.writable_addr(); backing_store_out->software2.row_bytes = pixmap.rowBytes(); backing_store_out->software2.height = pixmap.height(); - // The balancing unref is in the destruction callback. - surface->ref(); - backing_store_out->software2.user_data = surface.get(); + backing_store_out->software2.user_data = userdata; backing_store_out->software2.destruction_callback = [](void* user_data) { - reinterpret_cast(user_data)->unref(); + delete reinterpret_cast(user_data); }; backing_store_out->software2.pixel_format = software_pixfmt_; diff --git a/shell/platform/embedder/tests/embedder_test_backingstore_producer.h b/shell/platform/embedder/tests/embedder_test_backingstore_producer.h index 51b15f791bc95..18e39f4eec325 100644 --- a/shell/platform/embedder/tests/embedder_test_backingstore_producer.h +++ b/shell/platform/embedder/tests/embedder_test_backingstore_producer.h @@ -21,6 +21,10 @@ #include "flutter/testing/test_vulkan_context.h" // nogncheck #endif +#ifdef SHELL_ENABLE_GL +#include "flutter/testing/test_gl_surface.h" +#endif + namespace flutter { namespace testing { @@ -31,11 +35,17 @@ class EmbedderTestBackingStoreProducer { FlutterVulkanImage* image; }; + struct GLUserData { + std::unique_ptr gl_surface; + sk_sp surface; + }; + enum class RenderTargetType { kSoftwareBuffer, kSoftwareBuffer2, kOpenGLFramebuffer, kOpenGLTexture, + kOpenGLSurface, kMetalTexture, kVulkanImage, }; @@ -46,6 +56,8 @@ class EmbedderTestBackingStoreProducer { kFlutterSoftwarePixelFormatNative32); ~EmbedderTestBackingStoreProducer(); + void SetEGLContext(std::shared_ptr context); + bool Create(const FlutterBackingStoreConfig* config, FlutterBackingStore* renderer_out); @@ -56,6 +68,9 @@ class EmbedderTestBackingStoreProducer { bool CreateTexture(const FlutterBackingStoreConfig* config, FlutterBackingStore* renderer_out); + bool CreateSurface(const FlutterBackingStoreConfig* config, + FlutterBackingStore* renderer_out); + bool CreateSoftware(const FlutterBackingStoreConfig* config, FlutterBackingStore* backing_store_out); @@ -72,6 +87,10 @@ class EmbedderTestBackingStoreProducer { RenderTargetType type_; FlutterSoftwarePixelFormat software_pixfmt_; +#ifdef SHELL_ENABLE_GL + std::shared_ptr test_egl_context_; +#endif + #ifdef SHELL_ENABLE_METAL std::unique_ptr test_metal_context_; #endif diff --git a/shell/platform/embedder/tests/embedder_test_compositor_gl.cc b/shell/platform/embedder/tests/embedder_test_compositor_gl.cc index 50b4da39593a9..9c05c21e857ee 100644 --- a/shell/platform/embedder/tests/embedder_test_compositor_gl.cc +++ b/shell/platform/embedder/tests/embedder_test_compositor_gl.cc @@ -58,12 +58,29 @@ bool EmbedderTestCompositorGL::UpdateOffscrenComposition( SkIPoint canvas_offset = SkIPoint::Make(0, 0); switch (layer->type) { - case kFlutterLayerContentTypeBackingStore: - layer_image = - reinterpret_cast(layer->backing_store->user_data) - ->makeImageSnapshot(); - + case kFlutterLayerContentTypeBackingStore: { + auto gl_user_data = + reinterpret_cast( + layer->backing_store->user_data); + + if (gl_user_data->gl_surface != nullptr) { + // This backing store is a OpenGL Surface. + // We need to make it current so we can snapshot it. + + gl_user_data->gl_surface->MakeCurrent(); + + // GetRasterSurfaceSnapshot() does two + // gl_surface->makeImageSnapshot()'s. Doing a single + // ->makeImageSnapshot() will not work. + layer_image = gl_user_data->gl_surface->GetRasterSurfaceSnapshot(); + } else { + layer_image = gl_user_data->surface->makeImageSnapshot(); + } + + // We don't clear the current surface here because we need the + // EGL context to be current for surface->makeImageSnapshot() below. break; + } case kFlutterLayerContentTypePlatformView: layer_image = platform_view_renderer_callback_ diff --git a/shell/platform/embedder/tests/embedder_test_compositor_software.cc b/shell/platform/embedder/tests/embedder_test_compositor_software.cc index 0a9143fc76ce5..309ca01d6b64d 100644 --- a/shell/platform/embedder/tests/embedder_test_compositor_software.cc +++ b/shell/platform/embedder/tests/embedder_test_compositor_software.cc @@ -48,7 +48,8 @@ bool EmbedderTestCompositorSoftware::UpdateOffscrenComposition( switch (layer->type) { case kFlutterLayerContentTypeBackingStore: layer_image = - reinterpret_cast(layer->backing_store->user_data) + reinterpret_cast(layer->backing_store->user_data) + ->surface ->makeImageSnapshot(); break; diff --git a/shell/platform/embedder/tests/embedder_test_context.h b/shell/platform/embedder/tests/embedder_test_context.h index 6f1a1568232d6..b99d6b3534201 100644 --- a/shell/platform/embedder/tests/embedder_test_context.h +++ b/shell/platform/embedder/tests/embedder_test_context.h @@ -122,6 +122,10 @@ class EmbedderTestContext { fml::RefPtr vulkan_context_ = nullptr; #endif +#ifdef SHELL_ENABLE_GL + std::shared_ptr egl_context_ = nullptr; +#endif + std::string assets_path_; ELFAOTSymbols aot_symbols_; std::unique_ptr vm_snapshot_data_; diff --git a/shell/platform/embedder/tests/embedder_test_context_gl.cc b/shell/platform/embedder/tests/embedder_test_context_gl.cc index ed3dabcfe8c71..ac916ead94f87 100644 --- a/shell/platform/embedder/tests/embedder_test_context_gl.cc +++ b/shell/platform/embedder/tests/embedder_test_context_gl.cc @@ -20,7 +20,9 @@ namespace flutter { namespace testing { EmbedderTestContextGL::EmbedderTestContextGL(std::string assets_path) - : EmbedderTestContext(std::move(assets_path)) {} + : EmbedderTestContext(std::move(assets_path)) { + egl_context_ = std::make_shared(); +} EmbedderTestContextGL::~EmbedderTestContextGL() { SetGLGetFBOCallback(nullptr); @@ -28,7 +30,7 @@ EmbedderTestContextGL::~EmbedderTestContextGL() { void EmbedderTestContextGL::SetupSurface(SkISize surface_size) { FML_CHECK(!gl_surface_); - gl_surface_ = std::make_unique(surface_size); + gl_surface_ = std::make_unique(egl_context_, surface_size); } bool EmbedderTestContextGL::GLMakeCurrent() { diff --git a/shell/platform/embedder/tests/embedder_test_context_gl.h b/shell/platform/embedder/tests/embedder_test_context_gl.h index f23f98faeffe6..a7c880803851c 100644 --- a/shell/platform/embedder/tests/embedder_test_context_gl.h +++ b/shell/platform/embedder/tests/embedder_test_context_gl.h @@ -67,6 +67,8 @@ class EmbedderTestContextGL : public EmbedderTestContext { protected: virtual void SetupCompositor() override; + void SetupCompositorUsingGLSurfaces(); + private: // This allows the builder to access the hooks. friend class EmbedderConfigBuilder; diff --git a/shell/platform/embedder/tests/embedder_unittests.cc b/shell/platform/embedder/tests/embedder_unittests.cc index 2b03e5105ed47..11086400adf65 100644 --- a/shell/platform/embedder/tests/embedder_unittests.cc +++ b/shell/platform/embedder/tests/embedder_unittests.cc @@ -2735,8 +2735,9 @@ static void expectSoftwareRenderingOutputMatches( ASSERT_EQ(layers[0]->backing_store->type, kFlutterBackingStoreTypeSoftware2); matches = SurfacePixelDataMatchesBytes( - static_cast( - layers[0]->backing_store->software2.user_data), + reinterpret_cast( + layers[0]->backing_store->software2.user_data) + ->surface.get(), bytes); latch.Signal(); }); diff --git a/testing/test_gl_surface.cc b/testing/test_gl_surface.cc index c70364696277a..48d85505bb39b 100644 --- a/testing/test_gl_surface.cc +++ b/testing/test_gl_surface.cc @@ -130,15 +130,14 @@ static EGLDisplay CreateSwangleDisplay() { display_config); } -TestGLSurface::TestGLSurface(SkISize surface_size) - : surface_size_(surface_size) { - display_ = CreateSwangleDisplay(); - FML_CHECK(display_ != EGL_NO_DISPLAY); +TestEGLContext::TestEGLContext() { + display = CreateSwangleDisplay(); + FML_CHECK(display != EGL_NO_DISPLAY); - auto result = ::eglInitialize(display_, nullptr, nullptr); + auto result = ::eglInitialize(display, nullptr, nullptr); FML_CHECK(result == EGL_TRUE) << GetEGLError(); - EGLConfig config = {0}; + config = {0}; EGLint num_config = 0; const EGLint attribute_list[] = {EGL_RED_SIZE, @@ -157,39 +156,10 @@ TestGLSurface::TestGLSurface(SkISize surface_size) EGL_OPENGL_ES2_BIT, EGL_NONE}; - result = ::eglChooseConfig(display_, attribute_list, &config, 1, &num_config); + result = ::eglChooseConfig(display, attribute_list, &config, 1, &num_config); FML_CHECK(result == EGL_TRUE) << GetEGLError(); FML_CHECK(num_config == 1) << GetEGLError(); - { - const EGLint onscreen_surface_attributes[] = { - EGL_WIDTH, surface_size_.width(), // - EGL_HEIGHT, surface_size_.height(), // - EGL_NONE, - }; - - onscreen_surface_ = ::eglCreatePbufferSurface( - display_, // display connection - config, // config - onscreen_surface_attributes // surface attributes - ); - FML_CHECK(onscreen_surface_ != EGL_NO_SURFACE) << GetEGLError(); - } - - { - const EGLint offscreen_surface_attributes[] = { - EGL_WIDTH, 1, // - EGL_HEIGHT, 1, // - EGL_NONE, - }; - offscreen_surface_ = ::eglCreatePbufferSurface( - display_, // display connection - config, // config - offscreen_surface_attributes // surface attributes - ); - FML_CHECK(offscreen_surface_ != EGL_NO_SURFACE) << GetEGLError(); - } - { const EGLint context_attributes[] = { EGL_CONTEXT_CLIENT_VERSION, // @@ -197,50 +167,68 @@ TestGLSurface::TestGLSurface(SkISize surface_size) EGL_NONE // }; - onscreen_context_ = - ::eglCreateContext(display_, // display connection + onscreen_context = + ::eglCreateContext(display, // display connection config, // config EGL_NO_CONTEXT, // sharegroup context_attributes // context attributes ); - FML_CHECK(onscreen_context_ != EGL_NO_CONTEXT) << GetEGLError(); + FML_CHECK(onscreen_context != EGL_NO_CONTEXT) << GetEGLError(); - offscreen_context_ = - ::eglCreateContext(display_, // display connection + offscreen_context = + ::eglCreateContext(display, // display connection config, // config - onscreen_context_, // sharegroup + onscreen_context, // sharegroup context_attributes // context attributes ); - FML_CHECK(offscreen_context_ != EGL_NO_CONTEXT) << GetEGLError(); + FML_CHECK(offscreen_context != EGL_NO_CONTEXT) << GetEGLError(); } } -TestGLSurface::~TestGLSurface() { - context_ = nullptr; - - auto result = ::eglDestroyContext(display_, onscreen_context_); +TestEGLContext::~TestEGLContext() { + auto result = ::eglDestroyContext(display, onscreen_context); FML_CHECK(result == EGL_TRUE) << GetEGLError(); - result = ::eglDestroyContext(display_, offscreen_context_); + result = ::eglDestroyContext(display, offscreen_context); FML_CHECK(result == EGL_TRUE) << GetEGLError(); - result = ::eglDestroySurface(display_, onscreen_surface_); - FML_CHECK(result == EGL_TRUE) << GetEGLError(); + result = ::eglTerminate(display); + FML_CHECK(result == EGL_TRUE); +} - result = ::eglDestroySurface(display_, offscreen_surface_); - FML_CHECK(result == EGL_TRUE) << GetEGLError(); +TestGLOnscreenOnlySurface::TestGLOnscreenOnlySurface( + std::shared_ptr context, + SkISize size) + : surface_size_(size), egl_context_(context) { + const EGLint attributes[] = { + EGL_WIDTH, size.width(), // + EGL_HEIGHT, size.height(), // + EGL_NONE, + }; - result = ::eglTerminate(display_); - FML_CHECK(result == EGL_TRUE); + onscreen_surface_ = + ::eglCreatePbufferSurface(egl_context_->display, // display connection + egl_context_->config, // config + attributes // surface attributes + ); + FML_CHECK(onscreen_surface_ != EGL_NO_SURFACE) << GetEGLError(); } -const SkISize& TestGLSurface::GetSurfaceSize() const { +TestGLOnscreenOnlySurface::~TestGLOnscreenOnlySurface() { + skia_context_ = nullptr; + + auto result = ::eglDestroySurface(egl_context_->display, onscreen_surface_); + FML_CHECK(result == EGL_TRUE) << GetEGLError(); +} + +const SkISize& TestGLOnscreenOnlySurface::GetSurfaceSize() const { return surface_size_; } -bool TestGLSurface::MakeCurrent() { - auto result = ::eglMakeCurrent(display_, onscreen_surface_, onscreen_surface_, - onscreen_context_); +bool TestGLOnscreenOnlySurface::MakeCurrent() { + auto result = + ::eglMakeCurrent(egl_context_->display, onscreen_surface_, + onscreen_surface_, egl_context_->onscreen_context); if (result == EGL_FALSE) { FML_LOG(ERROR) << "Could not make the context current. " << GetEGLError(); @@ -249,9 +237,9 @@ bool TestGLSurface::MakeCurrent() { return result == EGL_TRUE; } -bool TestGLSurface::ClearCurrent() { - auto result = ::eglMakeCurrent(display_, EGL_NO_SURFACE, EGL_NO_SURFACE, - EGL_NO_CONTEXT); +bool TestGLOnscreenOnlySurface::ClearCurrent() { + auto result = ::eglMakeCurrent(egl_context_->display, EGL_NO_SURFACE, + EGL_NO_SURFACE, EGL_NO_CONTEXT); if (result == EGL_FALSE) { FML_LOG(ERROR) << "Could not clear the current context. " << GetEGLError(); @@ -260,8 +248,8 @@ bool TestGLSurface::ClearCurrent() { return result == EGL_TRUE; } -bool TestGLSurface::Present() { - auto result = ::eglSwapBuffers(display_, onscreen_surface_); +bool TestGLOnscreenOnlySurface::Present() { + auto result = ::eglSwapBuffers(egl_context_->display, onscreen_surface_); if (result == EGL_FALSE) { FML_LOG(ERROR) << "Could not swap buffers. " << GetEGLError(); @@ -270,23 +258,12 @@ bool TestGLSurface::Present() { return result == EGL_TRUE; } -uint32_t TestGLSurface::GetFramebuffer(uint32_t width, uint32_t height) const { +uint32_t TestGLOnscreenOnlySurface::GetFramebuffer(uint32_t width, + uint32_t height) const { return GetWindowFBOId(); } -bool TestGLSurface::MakeResourceCurrent() { - auto result = ::eglMakeCurrent(display_, offscreen_surface_, - offscreen_surface_, offscreen_context_); - - if (result == EGL_FALSE) { - FML_LOG(ERROR) << "Could not make the resource context current. " - << GetEGLError(); - } - - return result == EGL_TRUE; -} - -void* TestGLSurface::GetProcAddress(const char* name) const { +void* TestGLOnscreenOnlySurface::GetProcAddress(const char* name) const { if (name == nullptr) { return nullptr; } @@ -297,15 +274,15 @@ void* TestGLSurface::GetProcAddress(const char* name) const { return reinterpret_cast(symbol); } -sk_sp TestGLSurface::GetGrContext() { - if (context_) { - return context_; +sk_sp TestGLOnscreenOnlySurface::GetGrContext() { + if (skia_context_) { + return skia_context_; } return CreateGrContext(); } -sk_sp TestGLSurface::CreateGrContext() { +sk_sp TestGLOnscreenOnlySurface::CreateGrContext() { if (!MakeCurrent()) { return nullptr; } @@ -325,7 +302,8 @@ sk_sp TestGLSurface::CreateGrContext() { GrGLGetProc get_proc = [](void* context, const char name[]) -> GrGLFuncPtr { return reinterpret_cast( - reinterpret_cast(context)->GetProcAddress(name)); + reinterpret_cast(context)->GetProcAddress( + name)); }; std::string version(c_version); @@ -337,11 +315,11 @@ sk_sp TestGLSurface::CreateGrContext() { return nullptr; } - context_ = GrDirectContexts::MakeGL(interface); - return context_; + skia_context_ = GrDirectContexts::MakeGL(interface); + return skia_context_; } -sk_sp TestGLSurface::GetOnscreenSurface() { +sk_sp TestGLOnscreenOnlySurface::GetOnscreenSurface() { FML_CHECK(::eglGetCurrentContext() != EGL_NO_CONTEXT); GrGLFramebufferInfo framebuffer_info = {}; @@ -384,7 +362,7 @@ sk_sp TestGLSurface::GetOnscreenSurface() { return surface; } -sk_sp TestGLSurface::GetRasterSurfaceSnapshot() { +sk_sp TestGLOnscreenOnlySurface::GetRasterSurfaceSnapshot() { auto surface = GetOnscreenSurface(); if (!surface) { @@ -412,9 +390,48 @@ sk_sp TestGLSurface::GetRasterSurfaceSnapshot() { return host_snapshot; } -uint32_t TestGLSurface::GetWindowFBOId() const { +uint32_t TestGLOnscreenOnlySurface::GetWindowFBOId() const { return 0u; } +TestGLSurface::TestGLSurface(SkISize surface_size) + : TestGLSurface(std::make_shared(), surface_size) {} + +TestGLSurface::TestGLSurface(std::shared_ptr egl_context, + SkISize surface_size) + : TestGLOnscreenOnlySurface(egl_context, surface_size) { + { + const EGLint offscreen_surface_attributes[] = { + EGL_WIDTH, 1, // + EGL_HEIGHT, 1, // + EGL_NONE, + }; + offscreen_surface_ = ::eglCreatePbufferSurface( + egl_context_->display, // display connection + egl_context_->config, // config + offscreen_surface_attributes // surface attributes + ); + FML_CHECK(offscreen_surface_ != EGL_NO_SURFACE) << GetEGLError(); + } +} + +TestGLSurface::~TestGLSurface() { + auto result = ::eglDestroySurface(egl_context_->display, offscreen_surface_); + FML_CHECK(result == EGL_TRUE) << GetEGLError(); +} + +bool TestGLSurface::MakeResourceCurrent() { + auto result = + ::eglMakeCurrent(egl_context_->display, offscreen_surface_, + offscreen_surface_, egl_context_->offscreen_context); + + if (result == EGL_FALSE) { + FML_LOG(ERROR) << "Could not make the resource context current. " + << GetEGLError(); + } + + return result == EGL_TRUE; +} + } // namespace testing } // namespace flutter diff --git a/testing/test_gl_surface.h b/testing/test_gl_surface.h index 32bd732f2c8b9..654d2cdbd0747 100644 --- a/testing/test_gl_surface.h +++ b/testing/test_gl_surface.h @@ -15,11 +15,33 @@ namespace flutter { namespace testing { -class TestGLSurface { +struct TestEGLContext { + explicit TestEGLContext(); + + ~TestEGLContext(); + + using EGLDisplay = void*; + using EGLContext = void*; + using EGLConfig = void*; + + EGLDisplay display; + EGLContext onscreen_context; + EGLContext offscreen_context; + + // EGLConfig is technically a property of the surfaces, no the context, + // but it's not that well separated in EGL (e.g. when + // EGL_KHR_no_config_context is not supported), so we just store it here. + EGLConfig config; +}; + +class TestGLOnscreenOnlySurface { public: - explicit TestGLSurface(SkISize surface_size); + explicit TestGLOnscreenOnlySurface(SkISize surface_size); - ~TestGLSurface(); + explicit TestGLOnscreenOnlySurface(std::shared_ptr context, + SkISize size); + + ~TestGLOnscreenOnlySurface(); const SkISize& GetSurfaceSize() const; @@ -31,8 +53,6 @@ class TestGLSurface { uint32_t GetFramebuffer(uint32_t width, uint32_t height) const; - bool MakeResourceCurrent(); - void* GetProcAddress(const char* name) const; sk_sp GetOnscreenSurface(); @@ -45,22 +65,33 @@ class TestGLSurface { uint32_t GetWindowFBOId() const; - private: - // Importing the EGL.h pulls in platform headers which are problematic - // (especially X11 which #defineds types like Bool). Any TUs importing - // this header then become susceptible to failures because of platform - // specific craziness. Don't expose EGL internals via this header. - using EGLDisplay = void*; - using EGLContext = void*; + protected: using EGLSurface = void*; const SkISize surface_size_; - EGLDisplay display_; - EGLContext onscreen_context_; - EGLContext offscreen_context_; + std::shared_ptr egl_context_; EGLSurface onscreen_surface_; + + sk_sp skia_context_; + + FML_DISALLOW_COPY_AND_ASSIGN(TestGLOnscreenOnlySurface); +}; + +class TestGLSurface : public TestGLOnscreenOnlySurface { + public: + explicit TestGLSurface(SkISize surface_size); + + explicit TestGLSurface(std::shared_ptr egl_context, + SkISize surface_size); + + ~TestGLSurface(); + + bool MakeResourceCurrent(); + + private: + using EGLSurface = void*; + EGLSurface offscreen_surface_; - sk_sp context_; FML_DISALLOW_COPY_AND_ASSIGN(TestGLSurface); }; From 7485655bd7c24424e5c758785101bd59c37f5aaf Mon Sep 17 00:00:00 2001 From: Hannes Winkler Date: Mon, 8 Jan 2024 15:14:35 +0100 Subject: [PATCH 06/30] fix formatting --- shell/platform/embedder/embedder.cc | 14 +++++++------- .../tests/embedder_test_compositor_software.cc | 6 +++--- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/shell/platform/embedder/embedder.cc b/shell/platform/embedder/embedder.cc index 04f1c78cbc5eb..bc5e30ea13ecb 100644 --- a/shell/platform/embedder/embedder.cc +++ b/shell/platform/embedder/embedder.cc @@ -842,13 +842,13 @@ static sk_sp MakeSkSurfaceFromBackingStore( framebuffer_info.fFormat = GL_BGRA8_EXT; framebuffer_info.fFBOID = 0; - auto backend_render_target = GrBackendRenderTargets::MakeGL( - config.size.width, // width - config.size.height, // height - 1, // sample count - 0, // stencil bits - framebuffer_info // framebuffer info - ); + auto backend_render_target = + GrBackendRenderTargets::MakeGL(config.size.width, // width + config.size.height, // height + 1, // sample count + 0, // stencil bits + framebuffer_info // framebuffer info + ); SkSurfaceProps surface_properties(0, kUnknown_SkPixelGeometry); diff --git a/shell/platform/embedder/tests/embedder_test_compositor_software.cc b/shell/platform/embedder/tests/embedder_test_compositor_software.cc index 309ca01d6b64d..55e22a940a7fe 100644 --- a/shell/platform/embedder/tests/embedder_test_compositor_software.cc +++ b/shell/platform/embedder/tests/embedder_test_compositor_software.cc @@ -48,9 +48,9 @@ bool EmbedderTestCompositorSoftware::UpdateOffscrenComposition( switch (layer->type) { case kFlutterLayerContentTypeBackingStore: layer_image = - reinterpret_cast(layer->backing_store->user_data) - ->surface - ->makeImageSnapshot(); + reinterpret_cast( + layer->backing_store->user_data) + ->surface->makeImageSnapshot(); break; case kFlutterLayerContentTypePlatformView: From ce1e545710cacda1bd1ed60454db1ae89a6b03ff Mon Sep 17 00:00:00 2001 From: Hannes Winkler Date: Tue, 20 Feb 2024 16:57:08 +0100 Subject: [PATCH 07/30] Don't use GLUserData for software tests --- .../embedder/tests/embedder_test_backingstore_producer.cc | 8 ++++---- .../embedder/tests/embedder_test_backingstore_producer.h | 8 ++++++++ .../embedder/tests/embedder_test_compositor_software.cc | 2 +- 3 files changed, 13 insertions(+), 5 deletions(-) diff --git a/shell/platform/embedder/tests/embedder_test_backingstore_producer.cc b/shell/platform/embedder/tests/embedder_test_backingstore_producer.cc index 074a00081033e..e1e9870fdcb4c 100644 --- a/shell/platform/embedder/tests/embedder_test_backingstore_producer.cc +++ b/shell/platform/embedder/tests/embedder_test_backingstore_producer.cc @@ -288,7 +288,7 @@ bool EmbedderTestBackingStoreProducer::CreateSoftware( return false; } - auto userdata = new GLUserData{nullptr, surface}; + auto userdata = new SWUserData{surface}; backing_store_out->type = kFlutterBackingStoreTypeSoftware; backing_store_out->user_data = userdata; @@ -297,7 +297,7 @@ bool EmbedderTestBackingStoreProducer::CreateSoftware( backing_store_out->software.height = pixmap.height(); backing_store_out->software.user_data = userdata; backing_store_out->software.destruction_callback = [](void* user_data) { - delete reinterpret_cast(user_data); + delete reinterpret_cast(user_data); }; return true; @@ -325,7 +325,7 @@ bool EmbedderTestBackingStoreProducer::CreateSoftware2( return false; } - auto userdata = new GLUserData{nullptr, surface}; + auto userdata = new SWUserData{surface}; backing_store_out->type = kFlutterBackingStoreTypeSoftware2; backing_store_out->user_data = userdata; @@ -336,7 +336,7 @@ bool EmbedderTestBackingStoreProducer::CreateSoftware2( backing_store_out->software2.height = pixmap.height(); backing_store_out->software2.user_data = userdata; backing_store_out->software2.destruction_callback = [](void* user_data) { - delete reinterpret_cast(user_data); + delete reinterpret_cast(user_data); }; backing_store_out->software2.pixel_format = software_pixfmt_; diff --git a/shell/platform/embedder/tests/embedder_test_backingstore_producer.h b/shell/platform/embedder/tests/embedder_test_backingstore_producer.h index 18e39f4eec325..7bde20182640e 100644 --- a/shell/platform/embedder/tests/embedder_test_backingstore_producer.h +++ b/shell/platform/embedder/tests/embedder_test_backingstore_producer.h @@ -35,10 +35,16 @@ class EmbedderTestBackingStoreProducer { FlutterVulkanImage* image; }; + struct SWUserData { + sk_sp surface; + }; + +#ifdef SHELL_ENABLE_GL struct GLUserData { std::unique_ptr gl_surface; sk_sp surface; }; +#endif enum class RenderTargetType { kSoftwareBuffer, @@ -56,7 +62,9 @@ class EmbedderTestBackingStoreProducer { kFlutterSoftwarePixelFormatNative32); ~EmbedderTestBackingStoreProducer(); +#ifdef SHELL_ENABLE_GL void SetEGLContext(std::shared_ptr context); +#endif bool Create(const FlutterBackingStoreConfig* config, FlutterBackingStore* renderer_out); diff --git a/shell/platform/embedder/tests/embedder_test_compositor_software.cc b/shell/platform/embedder/tests/embedder_test_compositor_software.cc index 55e22a940a7fe..391d794c9cd2c 100644 --- a/shell/platform/embedder/tests/embedder_test_compositor_software.cc +++ b/shell/platform/embedder/tests/embedder_test_compositor_software.cc @@ -48,7 +48,7 @@ bool EmbedderTestCompositorSoftware::UpdateOffscrenComposition( switch (layer->type) { case kFlutterLayerContentTypeBackingStore: layer_image = - reinterpret_cast( + reinterpret_cast( layer->backing_store->user_data) ->surface->makeImageSnapshot(); From 177024013fe988be579de9b0471d9f3cdd971806 Mon Sep 17 00:00:00 2001 From: Hannes Winkler Date: Tue, 20 Feb 2024 17:10:43 +0100 Subject: [PATCH 08/30] fix one remaining use of GLUserData in software context --- shell/platform/embedder/tests/embedder_unittests.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shell/platform/embedder/tests/embedder_unittests.cc b/shell/platform/embedder/tests/embedder_unittests.cc index 11086400adf65..68454f325c8aa 100644 --- a/shell/platform/embedder/tests/embedder_unittests.cc +++ b/shell/platform/embedder/tests/embedder_unittests.cc @@ -2735,7 +2735,7 @@ static void expectSoftwareRenderingOutputMatches( ASSERT_EQ(layers[0]->backing_store->type, kFlutterBackingStoreTypeSoftware2); matches = SurfacePixelDataMatchesBytes( - reinterpret_cast( + reinterpret_cast( layers[0]->backing_store->software2.user_data) ->surface.get(), bytes); From 0c9c5ba668b66d763fe94ad32584bf72d3915a5f Mon Sep 17 00:00:00 2001 From: Hannes Winkler Date: Tue, 20 Feb 2024 17:43:24 +0100 Subject: [PATCH 09/30] fix memory leak Don't take a reference on the backing store SkSurface because we're manually destroying it later. --- .../embedder/tests/embedder_test_backingstore_producer.cc | 2 -- 1 file changed, 2 deletions(-) diff --git a/shell/platform/embedder/tests/embedder_test_backingstore_producer.cc b/shell/platform/embedder/tests/embedder_test_backingstore_producer.cc index e1e9870fdcb4c..62a57a382053b 100644 --- a/shell/platform/embedder/tests/embedder_test_backingstore_producer.cc +++ b/shell/platform/embedder/tests/embedder_test_backingstore_producer.cc @@ -157,8 +157,6 @@ bool EmbedderTestBackingStoreProducer::CreateFramebuffer( backing_store_out->open_gl.type = kFlutterOpenGLTargetTypeFramebuffer; backing_store_out->open_gl.framebuffer.target = framebuffer_info.fFormat; backing_store_out->open_gl.framebuffer.name = framebuffer_info.fFBOID; - // The balancing unref is in the destruction callback. - surface->ref(); backing_store_out->open_gl.framebuffer.user_data = userdata; backing_store_out->open_gl.framebuffer.destruction_callback = [](void* user_data) { delete reinterpret_cast(user_data); }; From e8b7cae17384469aac3d5c7aa5caff7757d4ff3a Mon Sep 17 00:00:00 2001 From: Hannes Winkler Date: Tue, 20 Feb 2024 18:35:42 +0100 Subject: [PATCH 10/30] don't use different backing store userdatas per graphics API Apparently an OpenGL-rendering engine should be able to render into software backing stores, so the userdata for a sw backing store can't be different to a gl backing store. --- .../embedder_test_backingstore_producer.cc | 34 +++++++------------ .../embedder_test_backingstore_producer.h | 24 ++++++++----- .../tests/embedder_test_compositor_gl.cc | 2 +- .../embedder_test_compositor_software.cc | 2 +- .../embedder/tests/embedder_unittests.cc | 2 +- 5 files changed, 30 insertions(+), 34 deletions(-) diff --git a/shell/platform/embedder/tests/embedder_test_backingstore_producer.cc b/shell/platform/embedder/tests/embedder_test_backingstore_producer.cc index 62a57a382053b..387973b1d5163 100644 --- a/shell/platform/embedder/tests/embedder_test_backingstore_producer.cc +++ b/shell/platform/embedder/tests/embedder_test_backingstore_producer.cc @@ -150,7 +150,7 @@ bool EmbedderTestBackingStoreProducer::CreateFramebuffer( return false; } - auto userdata = new GLUserData{nullptr, surface}; + auto userdata = new UserData(surface); backing_store_out->type = kFlutterBackingStoreTypeOpenGL; backing_store_out->user_data = userdata; @@ -159,7 +159,7 @@ bool EmbedderTestBackingStoreProducer::CreateFramebuffer( backing_store_out->open_gl.framebuffer.name = framebuffer_info.fFBOID; backing_store_out->open_gl.framebuffer.user_data = userdata; backing_store_out->open_gl.framebuffer.destruction_callback = - [](void* user_data) { delete reinterpret_cast(user_data); }; + [](void* user_data) { delete reinterpret_cast(user_data); }; return true; #else @@ -203,7 +203,7 @@ bool EmbedderTestBackingStoreProducer::CreateTexture( return false; } - auto userdata = new GLUserData{nullptr, surface}; + auto userdata = new UserData(surface); backing_store_out->type = kFlutterBackingStoreTypeOpenGL; backing_store_out->user_data = userdata; @@ -213,7 +213,7 @@ bool EmbedderTestBackingStoreProducer::CreateTexture( backing_store_out->open_gl.texture.format = texture_info.fFormat; backing_store_out->open_gl.texture.user_data = userdata; backing_store_out->open_gl.texture.destruction_callback = - [](void* user_data) { delete reinterpret_cast(user_data); }; + [](void* user_data) { delete reinterpret_cast(user_data); }; return true; #else @@ -232,7 +232,7 @@ bool EmbedderTestBackingStoreProducer::CreateSurface( auto make_current = [](void* userdata, bool* invalidate_state) -> bool { *invalidate_state = false; - return reinterpret_cast(userdata)->gl_surface->MakeCurrent(); + return reinterpret_cast(userdata)->gl_surface->MakeCurrent(); }; auto clear_current = [](void* userdata, bool* invalidate_state) -> bool { @@ -243,15 +243,12 @@ bool EmbedderTestBackingStoreProducer::CreateSurface( }; auto destruction_callback = [](void* userdata) { - delete reinterpret_cast(userdata); + delete reinterpret_cast(userdata); }; auto sk_surface = surface->GetOnscreenSurface(); - auto userdata = new GLUserData{ - std::move(surface), - sk_surface, - }; + auto userdata = new UserData(sk_surface, nullptr, std::move(surface)); backing_store_out->type = kFlutterBackingStoreTypeOpenGL; backing_store_out->user_data = userdata; @@ -286,7 +283,7 @@ bool EmbedderTestBackingStoreProducer::CreateSoftware( return false; } - auto userdata = new SWUserData{surface}; + auto userdata = new UserData(surface); backing_store_out->type = kFlutterBackingStoreTypeSoftware; backing_store_out->user_data = userdata; @@ -295,7 +292,7 @@ bool EmbedderTestBackingStoreProducer::CreateSoftware( backing_store_out->software.height = pixmap.height(); backing_store_out->software.user_data = userdata; backing_store_out->software.destruction_callback = [](void* user_data) { - delete reinterpret_cast(user_data); + delete reinterpret_cast(user_data); }; return true; @@ -323,7 +320,7 @@ bool EmbedderTestBackingStoreProducer::CreateSoftware2( return false; } - auto userdata = new SWUserData{surface}; + auto userdata = new UserData(surface); backing_store_out->type = kFlutterBackingStoreTypeSoftware2; backing_store_out->user_data = userdata; @@ -334,7 +331,7 @@ bool EmbedderTestBackingStoreProducer::CreateSoftware2( backing_store_out->software2.height = pixmap.height(); backing_store_out->software2.user_data = userdata; backing_store_out->software2.destruction_callback = [](void* user_data) { - delete reinterpret_cast(user_data); + delete reinterpret_cast(user_data); }; backing_store_out->software2.pixel_format = software_pixfmt_; @@ -440,21 +437,14 @@ bool EmbedderTestBackingStoreProducer::CreateVulkanImage( // Collect all allocated resources in the destruction_callback. { - UserData* user_data = new UserData(); - user_data->image = image; - user_data->surface = surface.get(); - + auto user_data = new UserData(surface, image); backing_store_out->user_data = user_data; backing_store_out->vulkan.user_data = user_data; backing_store_out->vulkan.destruction_callback = [](void* user_data) { UserData* d = reinterpret_cast(user_data); - d->surface->unref(); delete d->image; delete d; }; - - // The balancing unref is in the destruction callback. - surface->ref(); } return true; diff --git a/shell/platform/embedder/tests/embedder_test_backingstore_producer.h b/shell/platform/embedder/tests/embedder_test_backingstore_producer.h index 7bde20182640e..a26be2c64763f 100644 --- a/shell/platform/embedder/tests/embedder_test_backingstore_producer.h +++ b/shell/platform/embedder/tests/embedder_test_backingstore_producer.h @@ -31,20 +31,26 @@ namespace testing { class EmbedderTestBackingStoreProducer { public: struct UserData { - SkSurface* surface; - FlutterVulkanImage* image; - }; + UserData() : surface(nullptr), image(nullptr){}; - struct SWUserData { - sk_sp surface; - }; + UserData(sk_sp surface) : surface(surface), image(nullptr){}; + + UserData(sk_sp surface, FlutterVulkanImage* vk_image) + : surface(surface), image(vk_image){}; + sk_sp surface; + FlutterVulkanImage* image; #ifdef SHELL_ENABLE_GL - struct GLUserData { + UserData(sk_sp surface, + FlutterVulkanImage* vk_image, + std::unique_ptr gl_surface) + : surface(surface), + image(vk_image), + gl_surface(std::move(gl_surface)){}; + std::unique_ptr gl_surface; - sk_sp surface; - }; #endif + }; enum class RenderTargetType { kSoftwareBuffer, diff --git a/shell/platform/embedder/tests/embedder_test_compositor_gl.cc b/shell/platform/embedder/tests/embedder_test_compositor_gl.cc index 9c05c21e857ee..29daf8da59969 100644 --- a/shell/platform/embedder/tests/embedder_test_compositor_gl.cc +++ b/shell/platform/embedder/tests/embedder_test_compositor_gl.cc @@ -60,7 +60,7 @@ bool EmbedderTestCompositorGL::UpdateOffscrenComposition( switch (layer->type) { case kFlutterLayerContentTypeBackingStore: { auto gl_user_data = - reinterpret_cast( + reinterpret_cast( layer->backing_store->user_data); if (gl_user_data->gl_surface != nullptr) { diff --git a/shell/platform/embedder/tests/embedder_test_compositor_software.cc b/shell/platform/embedder/tests/embedder_test_compositor_software.cc index 391d794c9cd2c..46938205e2c13 100644 --- a/shell/platform/embedder/tests/embedder_test_compositor_software.cc +++ b/shell/platform/embedder/tests/embedder_test_compositor_software.cc @@ -48,7 +48,7 @@ bool EmbedderTestCompositorSoftware::UpdateOffscrenComposition( switch (layer->type) { case kFlutterLayerContentTypeBackingStore: layer_image = - reinterpret_cast( + reinterpret_cast( layer->backing_store->user_data) ->surface->makeImageSnapshot(); diff --git a/shell/platform/embedder/tests/embedder_unittests.cc b/shell/platform/embedder/tests/embedder_unittests.cc index 68454f325c8aa..22cdb54cdcf95 100644 --- a/shell/platform/embedder/tests/embedder_unittests.cc +++ b/shell/platform/embedder/tests/embedder_unittests.cc @@ -2735,7 +2735,7 @@ static void expectSoftwareRenderingOutputMatches( ASSERT_EQ(layers[0]->backing_store->type, kFlutterBackingStoreTypeSoftware2); matches = SurfacePixelDataMatchesBytes( - reinterpret_cast( + reinterpret_cast( layers[0]->backing_store->software2.user_data) ->surface.get(), bytes); From 3e85028953420a75f16501a4180cb3b52304e37a Mon Sep 17 00:00:00 2001 From: Hannes Winkler Date: Tue, 20 Feb 2024 19:13:31 +0100 Subject: [PATCH 11/30] fix lints Fixes for lints `google-explicit-constructor`, `performance-unnecessary-value-param`. --- .../embedder/tests/embedder_test_backingstore_producer.h | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/shell/platform/embedder/tests/embedder_test_backingstore_producer.h b/shell/platform/embedder/tests/embedder_test_backingstore_producer.h index a26be2c64763f..613b824c48dbc 100644 --- a/shell/platform/embedder/tests/embedder_test_backingstore_producer.h +++ b/shell/platform/embedder/tests/embedder_test_backingstore_producer.h @@ -33,10 +33,11 @@ class EmbedderTestBackingStoreProducer { struct UserData { UserData() : surface(nullptr), image(nullptr){}; - UserData(sk_sp surface) : surface(surface), image(nullptr){}; + explicit UserData(sk_sp surface) + : surface(std::move(surface)), image(nullptr){}; UserData(sk_sp surface, FlutterVulkanImage* vk_image) - : surface(surface), image(vk_image){}; + : surface(std::move(surface)), image(vk_image){}; sk_sp surface; FlutterVulkanImage* image; @@ -44,7 +45,7 @@ class EmbedderTestBackingStoreProducer { UserData(sk_sp surface, FlutterVulkanImage* vk_image, std::unique_ptr gl_surface) - : surface(surface), + : surface(std::move(surface)), image(vk_image), gl_surface(std::move(gl_surface)){}; From 4e8c82857705e7e5fc954ec67ad697f41de04400 Mon Sep 17 00:00:00 2001 From: Hannes Winkler Date: Tue, 20 Feb 2024 19:45:13 +0100 Subject: [PATCH 12/30] fix more lints --- shell/platform/embedder/embedder.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/shell/platform/embedder/embedder.cc b/shell/platform/embedder/embedder.cc index bc5e30ea13ecb..6dcf2a5e53010 100644 --- a/shell/platform/embedder/embedder.cc +++ b/shell/platform/embedder/embedder.cc @@ -1220,8 +1220,8 @@ static std::unique_ptr MakeRenderTargetFromSkSurface(FlutterBackingStore backing_store, sk_sp skia_surface, fml::closure on_release) { - return MakeRenderTargetFromSkSurface(backing_store, skia_surface, on_release, - nullptr, nullptr); + return MakeRenderTargetFromSkSurface(backing_store, std::move(skia_surface), + std::move(on_release), nullptr, nullptr); } static std::unique_ptr From 3b08cc54dae2c7b46a17d42515da502a0e31a726 Mon Sep 17 00:00:00 2001 From: Hannes Winkler Date: Wed, 21 Feb 2024 16:09:29 +0100 Subject: [PATCH 13/30] fix more lints again --- .../embedder/tests/embedder_test_backingstore_producer.cc | 2 +- testing/test_gl_surface.cc | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/shell/platform/embedder/tests/embedder_test_backingstore_producer.cc b/shell/platform/embedder/tests/embedder_test_backingstore_producer.cc index 387973b1d5163..379eb5461ef00 100644 --- a/shell/platform/embedder/tests/embedder_test_backingstore_producer.cc +++ b/shell/platform/embedder/tests/embedder_test_backingstore_producer.cc @@ -80,7 +80,7 @@ void EmbedderTestBackingStoreProducer::SetEGLContext( // Ideally this would be set in the constructor, however we can't do that // without introducing different constructors depending on different graphics // APIs, which is a bit ugly. - test_egl_context_ = context; + test_egl_context_ = std::move(context); } #endif diff --git a/testing/test_gl_surface.cc b/testing/test_gl_surface.cc index 48d85505bb39b..25083c113a94b 100644 --- a/testing/test_gl_surface.cc +++ b/testing/test_gl_surface.cc @@ -199,7 +199,7 @@ TestEGLContext::~TestEGLContext() { TestGLOnscreenOnlySurface::TestGLOnscreenOnlySurface( std::shared_ptr context, SkISize size) - : surface_size_(size), egl_context_(context) { + : surface_size_(size), egl_context_(std::move(context)) { const EGLint attributes[] = { EGL_WIDTH, size.width(), // EGL_HEIGHT, size.height(), // @@ -399,7 +399,7 @@ TestGLSurface::TestGLSurface(SkISize surface_size) TestGLSurface::TestGLSurface(std::shared_ptr egl_context, SkISize surface_size) - : TestGLOnscreenOnlySurface(egl_context, surface_size) { + : TestGLOnscreenOnlySurface(std::move(egl_context), surface_size) { { const EGLint offscreen_surface_attributes[] = { EGL_WIDTH, 1, // From b903405979bd812690fa7c6a8f8e795cf9018c5a Mon Sep 17 00:00:00 2001 From: Hannes Winkler Date: Thu, 11 Jul 2024 22:50:10 +0200 Subject: [PATCH 14/30] Update shell/platform/embedder/embedder.cc Co-authored-by: Chris Bracken --- shell/platform/embedder/embedder.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shell/platform/embedder/embedder.cc b/shell/platform/embedder/embedder.cc index 6dcf2a5e53010..de742eba25246 100644 --- a/shell/platform/embedder/embedder.cc +++ b/shell/platform/embedder/embedder.cc @@ -1314,7 +1314,7 @@ CreateEmbedderRenderTarget( }; if (enable_impeller) { - /// TODO: Implement + // TODO: Implement. FML_LOG(ERROR) << "Unimplemented"; break; } else { From 4214fa222a1d4741bad31b4ab401dd8fed06d140 Mon Sep 17 00:00:00 2001 From: Hannes Winkler Date: Thu, 11 Jul 2024 22:50:29 +0200 Subject: [PATCH 15/30] Update shell/platform/embedder/embedder.h Co-authored-by: Chris Bracken --- shell/platform/embedder/embedder.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shell/platform/embedder/embedder.h b/shell/platform/embedder/embedder.h index dfdf3fe1cb614..2600682d750fb 100644 --- a/shell/platform/embedder/embedder.h +++ b/shell/platform/embedder/embedder.h @@ -419,7 +419,7 @@ typedef struct { size_t struct_size; /// User data to be passed to the make_current, clear_current and - /// destruction callback. + /// destruction callbacks. void* user_data; /// Callback invoked (on an engine managed thread) that asks the embedder to From 961ba09f2cbc2c2d7f63ef3e97763fbd96562b62 Mon Sep 17 00:00:00 2001 From: Hannes Winkler Date: Thu, 11 Jul 2024 22:50:38 +0200 Subject: [PATCH 16/30] Update shell/platform/embedder/embedder.h Co-authored-by: Chris Bracken --- shell/platform/embedder/embedder.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shell/platform/embedder/embedder.h b/shell/platform/embedder/embedder.h index 2600682d750fb..da86c412eea5f 100644 --- a/shell/platform/embedder/embedder.h +++ b/shell/platform/embedder/embedder.h @@ -422,7 +422,7 @@ typedef struct { /// destruction callbacks. void* user_data; - /// Callback invoked (on an engine managed thread) that asks the embedder to + /// Callback invoked (on an engine-managed thread) that asks the embedder to /// make the surface current. /// /// Should return true if the operation succeeded, false if the surface could From 759d78ce87e93896d6bfb5f06830e83853ab1dc6 Mon Sep 17 00:00:00 2001 From: Hannes Winkler Date: Thu, 11 Jul 2024 22:50:58 +0200 Subject: [PATCH 17/30] Update shell/platform/embedder/embedder.h Co-authored-by: Chris Bracken --- shell/platform/embedder/embedder.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shell/platform/embedder/embedder.h b/shell/platform/embedder/embedder.h index da86c412eea5f..edac15f4b9ead 100644 --- a/shell/platform/embedder/embedder.h +++ b/shell/platform/embedder/embedder.h @@ -432,7 +432,7 @@ typedef struct { /// any OpenGL API state has changed and flutter should not assume any native /// API state is the same as before this callback was called. /// - /// In that case, flutter will invalidate the internal OpenGL API state cache, + /// In that case, Flutter will invalidate the internal OpenGL API state cache, /// which is a somewhat expensive operation. /// /// @attention required. (non-null) From 0d41d3ddbc3931cd94f63b2763ded461b2034900 Mon Sep 17 00:00:00 2001 From: Hannes Winkler Date: Thu, 11 Jul 2024 22:51:05 +0200 Subject: [PATCH 18/30] Update shell/platform/embedder/embedder.h Co-authored-by: Chris Bracken --- shell/platform/embedder/embedder.h | 1 - 1 file changed, 1 deletion(-) diff --git a/shell/platform/embedder/embedder.h b/shell/platform/embedder/embedder.h index edac15f4b9ead..62d50a7240570 100644 --- a/shell/platform/embedder/embedder.h +++ b/shell/platform/embedder/embedder.h @@ -431,7 +431,6 @@ typedef struct { /// The second parameter 'opengl state changed' should be set to true if /// any OpenGL API state has changed and flutter should not assume any native /// API state is the same as before this callback was called. - /// /// In that case, Flutter will invalidate the internal OpenGL API state cache, /// which is a somewhat expensive operation. /// From 3a6c54706f162eeeb1d6de13422806e923b113d1 Mon Sep 17 00:00:00 2001 From: Hannes Winkler Date: Thu, 11 Jul 2024 22:51:11 +0200 Subject: [PATCH 19/30] Update shell/platform/embedder/embedder.h Co-authored-by: Chris Bracken --- shell/platform/embedder/embedder.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shell/platform/embedder/embedder.h b/shell/platform/embedder/embedder.h index 62d50a7240570..c67b7a9faa5fb 100644 --- a/shell/platform/embedder/embedder.h +++ b/shell/platform/embedder/embedder.h @@ -437,7 +437,7 @@ typedef struct { /// @attention required. (non-null) FlutterOpenGLSurfaceCallback make_current_callback; - /// Callback invoked (on an engine managed thread) when the current surface + /// Callback invoked (on an engine-managed thread) when the current surface /// can be cleared. /// /// Should return true if the operation succeeded, false if an error ocurred. From 03510cba1ae4eb32133c74133e1bbef2a881f4f8 Mon Sep 17 00:00:00 2001 From: Hannes Winkler Date: Thu, 11 Jul 2024 22:51:23 +0200 Subject: [PATCH 20/30] Update shell/platform/embedder/embedder.h Co-authored-by: Chris Bracken --- shell/platform/embedder/embedder.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shell/platform/embedder/embedder.h b/shell/platform/embedder/embedder.h index c67b7a9faa5fb..f161b468e9f05 100644 --- a/shell/platform/embedder/embedder.h +++ b/shell/platform/embedder/embedder.h @@ -448,7 +448,7 @@ typedef struct { /// /// The embedder might clear the surface here after it was previously made /// current. That's not required however, it's also possible to clear it in - /// the destruction callback. There's no way to signal opengl state + /// the destruction callback. There's no way to signal OpenGL state /// changes in the destruction callback though. /// /// @attention required. (non-null) From e55c4c583d8ff9431baa01496972e549209f4b12 Mon Sep 17 00:00:00 2001 From: Hannes Winkler Date: Thu, 11 Jul 2024 22:51:31 +0200 Subject: [PATCH 21/30] Update shell/platform/embedder/embedder.h Co-authored-by: Chris Bracken --- shell/platform/embedder/embedder.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shell/platform/embedder/embedder.h b/shell/platform/embedder/embedder.h index f161b468e9f05..83163b4b1937b 100644 --- a/shell/platform/embedder/embedder.h +++ b/shell/platform/embedder/embedder.h @@ -454,7 +454,7 @@ typedef struct { /// @attention required. (non-null) FlutterOpenGLSurfaceCallback clear_current_callback; - /// Callback invoked (on an engine managed thread) that asks the embedder to + /// Callback invoked (on an engine-managed thread) that asks the embedder to /// collect the surface. /// /// @attention required. (non-null) From 83eba98627ad7167ca51736189de3ea5e1acf9e7 Mon Sep 17 00:00:00 2001 From: Hannes Winkler Date: Thu, 11 Jul 2024 22:51:56 +0200 Subject: [PATCH 22/30] Update shell/platform/embedder/embedder_external_view.cc Co-authored-by: Chris Bracken --- shell/platform/embedder/embedder_external_view.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shell/platform/embedder/embedder_external_view.cc b/shell/platform/embedder/embedder_external_view.cc index 6cf7a7fa5dd6b..b7ec58bf53684 100644 --- a/shell/platform/embedder/embedder_external_view.cc +++ b/shell/platform/embedder/embedder_external_view.cc @@ -101,7 +101,7 @@ static void InvalidateApiState(SkSurface& skia_surface) { // Can happen when using software rendering. // Print an error but otherwise continue in that case. FML_LOG(ERROR) << "Embedder asked to invalidate cached graphics API state " - "but flutter is not using a graphics API."; + "but Flutter is not using a graphics API."; } else { direct_context->resetContext(kAll_GrBackendState); } From 2c0a61ad27ca87a5bb87d952b8b5dc7766a5fbe1 Mon Sep 17 00:00:00 2001 From: Hannes Winkler Date: Thu, 11 Jul 2024 22:52:03 +0200 Subject: [PATCH 23/30] Update shell/platform/embedder/embedder_external_view.cc Co-authored-by: Chris Bracken --- shell/platform/embedder/embedder_external_view.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shell/platform/embedder/embedder_external_view.cc b/shell/platform/embedder/embedder_external_view.cc index b7ec58bf53684..a323fe1165d09 100644 --- a/shell/platform/embedder/embedder_external_view.cc +++ b/shell/platform/embedder/embedder_external_view.cc @@ -172,7 +172,7 @@ bool EmbedderExternalView::Render(const EmbedderRenderTarget& render_target, return false; } - // clear the current render target (most likely EGLSurface) at the + // Clear the current render target (most likely EGLSurface) at the // end of this scope. fml::ScopedCleanupClosure clear_current_surface([&]() { auto [ok, invalidate_api_state] = render_target.MaybeClearCurrent(); From afd99b9715f7c232eedd85dbf09f38b1bacc9080 Mon Sep 17 00:00:00 2001 From: Hannes Winkler Date: Thu, 11 Jul 2024 20:59:02 +0000 Subject: [PATCH 24/30] replace all ocurrences of `userdata` with `user_data` --- .../embedder_test_backingstore_producer.cc | 42 +++++++++---------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/shell/platform/embedder/tests/embedder_test_backingstore_producer.cc b/shell/platform/embedder/tests/embedder_test_backingstore_producer.cc index 379eb5461ef00..c58f4dd410d75 100644 --- a/shell/platform/embedder/tests/embedder_test_backingstore_producer.cc +++ b/shell/platform/embedder/tests/embedder_test_backingstore_producer.cc @@ -150,14 +150,14 @@ bool EmbedderTestBackingStoreProducer::CreateFramebuffer( return false; } - auto userdata = new UserData(surface); + auto user_data = new UserData(surface); backing_store_out->type = kFlutterBackingStoreTypeOpenGL; - backing_store_out->user_data = userdata; + backing_store_out->user_data = user_data; backing_store_out->open_gl.type = kFlutterOpenGLTargetTypeFramebuffer; backing_store_out->open_gl.framebuffer.target = framebuffer_info.fFormat; backing_store_out->open_gl.framebuffer.name = framebuffer_info.fFBOID; - backing_store_out->open_gl.framebuffer.user_data = userdata; + backing_store_out->open_gl.framebuffer.user_data = user_data; backing_store_out->open_gl.framebuffer.destruction_callback = [](void* user_data) { delete reinterpret_cast(user_data); }; @@ -203,15 +203,15 @@ bool EmbedderTestBackingStoreProducer::CreateTexture( return false; } - auto userdata = new UserData(surface); + auto user_data = new UserData(surface); backing_store_out->type = kFlutterBackingStoreTypeOpenGL; - backing_store_out->user_data = userdata; + backing_store_out->user_data = user_data; backing_store_out->open_gl.type = kFlutterOpenGLTargetTypeTexture; backing_store_out->open_gl.texture.target = texture_info.fTarget; backing_store_out->open_gl.texture.name = texture_info.fID; backing_store_out->open_gl.texture.format = texture_info.fFormat; - backing_store_out->open_gl.texture.user_data = userdata; + backing_store_out->open_gl.texture.user_data = user_data; backing_store_out->open_gl.texture.destruction_callback = [](void* user_data) { delete reinterpret_cast(user_data); }; @@ -230,30 +230,30 @@ bool EmbedderTestBackingStoreProducer::CreateSurface( test_egl_context_, SkSize::Make(config->size.width, config->size.height).toRound()); - auto make_current = [](void* userdata, bool* invalidate_state) -> bool { + auto make_current = [](void* user_data, bool* invalidate_state) -> bool { *invalidate_state = false; - return reinterpret_cast(userdata)->gl_surface->MakeCurrent(); + return reinterpret_cast(user_data)->gl_surface->MakeCurrent(); }; - auto clear_current = [](void* userdata, bool* invalidate_state) -> bool { + auto clear_current = [](void* user_data, bool* invalidate_state) -> bool { *invalidate_state = false; // return - // reinterpret_cast(userdata)->gl_surface->ClearCurrent(); + // reinterpret_cast(user_data)->gl_surface->ClearCurrent(); return true; }; - auto destruction_callback = [](void* userdata) { - delete reinterpret_cast(userdata); + auto destruction_callback = [](void* user_data) { + delete reinterpret_cast(user_data); }; auto sk_surface = surface->GetOnscreenSurface(); - auto userdata = new UserData(sk_surface, nullptr, std::move(surface)); + auto user_data = new UserData(sk_surface, nullptr, std::move(surface)); backing_store_out->type = kFlutterBackingStoreTypeOpenGL; - backing_store_out->user_data = userdata; + backing_store_out->user_data = user_data; backing_store_out->open_gl.type = kFlutterOpenGLTargetTypeSurface; - backing_store_out->open_gl.surface.user_data = userdata; + backing_store_out->open_gl.surface.user_data = user_data; backing_store_out->open_gl.surface.make_current_callback = make_current; backing_store_out->open_gl.surface.clear_current_callback = clear_current; backing_store_out->open_gl.surface.destruction_callback = @@ -283,14 +283,14 @@ bool EmbedderTestBackingStoreProducer::CreateSoftware( return false; } - auto userdata = new UserData(surface); + auto user_data = new UserData(surface); backing_store_out->type = kFlutterBackingStoreTypeSoftware; - backing_store_out->user_data = userdata; + backing_store_out->user_data = user_data; backing_store_out->software.allocation = pixmap.addr(); backing_store_out->software.row_bytes = pixmap.rowBytes(); backing_store_out->software.height = pixmap.height(); - backing_store_out->software.user_data = userdata; + backing_store_out->software.user_data = user_data; backing_store_out->software.destruction_callback = [](void* user_data) { delete reinterpret_cast(user_data); }; @@ -320,16 +320,16 @@ bool EmbedderTestBackingStoreProducer::CreateSoftware2( return false; } - auto userdata = new UserData(surface); + auto user_data = new UserData(surface); backing_store_out->type = kFlutterBackingStoreTypeSoftware2; - backing_store_out->user_data = userdata; + backing_store_out->user_data = user_data; backing_store_out->software2.struct_size = sizeof(FlutterSoftwareBackingStore2); backing_store_out->software2.allocation = pixmap.writable_addr(); backing_store_out->software2.row_bytes = pixmap.rowBytes(); backing_store_out->software2.height = pixmap.height(); - backing_store_out->software2.user_data = userdata; + backing_store_out->software2.user_data = user_data; backing_store_out->software2.destruction_callback = [](void* user_data) { delete reinterpret_cast(user_data); }; From 47440ea7004278b948d01ec08aa9985a7c499b85 Mon Sep 17 00:00:00 2001 From: Hannes Winkler Date: Thu, 11 Jul 2024 23:16:36 +0000 Subject: [PATCH 25/30] fix egl surface tests present callback type signature --- shell/platform/embedder/tests/embedder_gl_unittests.cc | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/shell/platform/embedder/tests/embedder_gl_unittests.cc b/shell/platform/embedder/tests/embedder_gl_unittests.cc index 9f3d82da09654..54ca779ebfa18 100644 --- a/shell/platform/embedder/tests/embedder_gl_unittests.cc +++ b/shell/platform/embedder/tests/embedder_gl_unittests.cc @@ -4819,7 +4819,8 @@ TEST_F(EmbedderTest, CompositorMustBeAbleToRenderToOpenGLSurface) { fml::CountDownLatch latch(3); context.GetCompositor().SetNextPresentCallback( - [&](const FlutterLayer** layers, size_t layers_count) { + [&](FlutterViewId view_id, const FlutterLayer** layers, + size_t layers_count) { ASSERT_EQ(layers_count, 3u); { @@ -4937,7 +4938,8 @@ TEST_F(EmbedderTest, CompositorMustBeAbleToRenderKnownSceneToOpenGLSurfaces) { auto scene_image = context.GetNextSceneImage(); context.GetCompositor().SetNextPresentCallback( - [&](const FlutterLayer** layers, size_t layers_count) { + [&](FlutterViewId view_id, const FlutterLayer** layers, + size_t layers_count) { ASSERT_EQ(layers_count, 5u); // Layer Root From b9b71f1a39d8879379da588346b7ba6f8f2df179 Mon Sep 17 00:00:00 2001 From: Hannes Winkler Date: Fri, 12 Jul 2024 01:51:18 +0000 Subject: [PATCH 26/30] fix formatting in embedder_text_backingstore_producer.cc --- .../embedder/tests/embedder_test_backingstore_producer.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/shell/platform/embedder/tests/embedder_test_backingstore_producer.cc b/shell/platform/embedder/tests/embedder_test_backingstore_producer.cc index c58f4dd410d75..c93838a46eac1 100644 --- a/shell/platform/embedder/tests/embedder_test_backingstore_producer.cc +++ b/shell/platform/embedder/tests/embedder_test_backingstore_producer.cc @@ -29,11 +29,11 @@ #ifdef SHELL_ENABLE_METAL #include "third_party/skia/include/gpu/ganesh/mtl/GrMtlBackendSurface.h" #include "third_party/skia/include/gpu/ganesh/mtl/GrMtlTypes.h" -#endif // SHELL_ENABLE_METAL +#endif // SHELL_ENABLE_METAL #ifdef SHELL_ENABLE_GL #include "flutter/testing/test_gl_surface.h" -#endif // SHELL_ENABLE_GL +#endif // SHELL_ENABLE_GL // TODO(zanderso): https://github.com/flutter/flutter/issues/127701 // NOLINTBEGIN(bugprone-unchecked-optional-access) From e56486634507a7f4d5a5f60fbda4cabb6e34a446 Mon Sep 17 00:00:00 2001 From: Hannes Winkler Date: Fri, 12 Jul 2024 17:19:21 +0000 Subject: [PATCH 27/30] add github issue TODO comment --- shell/platform/embedder/embedder.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/shell/platform/embedder/embedder.cc b/shell/platform/embedder/embedder.cc index de742eba25246..5c1590cb1e761 100644 --- a/shell/platform/embedder/embedder.cc +++ b/shell/platform/embedder/embedder.cc @@ -1314,7 +1314,8 @@ CreateEmbedderRenderTarget( }; if (enable_impeller) { - // TODO: Implement. + // TODO(https://github.com/flutter/flutter/issues/151670): Implement + // GL Surface backing stores for Impeller. FML_LOG(ERROR) << "Unimplemented"; break; } else { From 2f6c32609bdbecdb889b7c94a07a2f448d9b9084 Mon Sep 17 00:00:00 2001 From: Hannes Winkler Date: Fri, 12 Jul 2024 19:19:51 +0000 Subject: [PATCH 28/30] only include `InvalidateApiState` when building with Skia --- shell/platform/embedder/embedder_external_view.cc | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/shell/platform/embedder/embedder_external_view.cc b/shell/platform/embedder/embedder_external_view.cc index a323fe1165d09..349f77fcc9cec 100644 --- a/shell/platform/embedder/embedder_external_view.cc +++ b/shell/platform/embedder/embedder_external_view.cc @@ -90,6 +90,9 @@ const EmbeddedViewParams* EmbedderExternalView::GetEmbeddedViewParams() const { return embedded_view_params_.get(); } +// TODO(https://github.com/flutter/flutter/issues/151670): Implement this for +// Impeller as well. +#if !SLIMPELLER static void InvalidateApiState(SkSurface& skia_surface) { auto recording_context = skia_surface.recordingContext(); @@ -106,6 +109,7 @@ static void InvalidateApiState(SkSurface& skia_surface) { direct_context->resetContext(kAll_GrBackendState); } } +#endif bool EmbedderExternalView::Render(const EmbedderRenderTarget& render_target, bool clear_surface) { From 81743732a26c9c0ae720f28e8982b4cc3c9a6d36 Mon Sep 17 00:00:00 2001 From: Hannes Winkler Date: Fri, 12 Jul 2024 19:52:04 +0000 Subject: [PATCH 29/30] improve documentation for make_current_callback --- shell/platform/embedder/embedder.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/shell/platform/embedder/embedder.h b/shell/platform/embedder/embedder.h index 83163b4b1937b..6c59c2c990e3f 100644 --- a/shell/platform/embedder/embedder.h +++ b/shell/platform/embedder/embedder.h @@ -429,8 +429,7 @@ typedef struct { /// not be made current and rendering should be cancelled. /// /// The second parameter 'opengl state changed' should be set to true if - /// any OpenGL API state has changed and flutter should not assume any native - /// API state is the same as before this callback was called. + /// any OpenGL API state is different than before this callback was called. /// In that case, Flutter will invalidate the internal OpenGL API state cache, /// which is a somewhat expensive operation. /// From d315fdc940eaccf392ebd747d6b5f04e05c632eb Mon Sep 17 00:00:00 2001 From: Hannes Winkler Date: Tue, 6 Aug 2024 10:50:52 +0000 Subject: [PATCH 30/30] capture result of surface ops in a struct instead of a std::pair --- shell/platform/embedder/embedder.cc | 4 +- .../embedder/embedder_render_target.h | 38 +++++++++---------- .../embedder/embedder_render_target_skia.cc | 6 ++- .../embedder/embedder_render_target_skia.h | 4 +- 4 files changed, 25 insertions(+), 27 deletions(-) diff --git a/shell/platform/embedder/embedder.cc b/shell/platform/embedder/embedder.cc index 5c1590cb1e761..cb75f4138bbde 100644 --- a/shell/platform/embedder/embedder.cc +++ b/shell/platform/embedder/embedder.cc @@ -1298,7 +1298,7 @@ CreateEmbedderRenderTarget( auto on_make_current = [callback = backing_store.open_gl.surface.make_current_callback, context = backing_store.open_gl.surface.user_data]() - -> std::pair { + -> flutter::EmbedderRenderTarget::SetCurrentResult { bool invalidate_api_state = false; bool ok = callback(context, &invalidate_api_state); return {ok, invalidate_api_state}; @@ -1307,7 +1307,7 @@ CreateEmbedderRenderTarget( auto on_clear_current = [callback = backing_store.open_gl.surface.clear_current_callback, context = backing_store.open_gl.surface.user_data]() - -> std::pair { + -> flutter::EmbedderRenderTarget::SetCurrentResult { bool invalidate_api_state = false; bool ok = callback(context, &invalidate_api_state); return {ok, invalidate_api_state}; diff --git a/shell/platform/embedder/embedder_render_target.h b/shell/platform/embedder/embedder_render_target.h index 87120a52d6697..245ec85d0998d 100644 --- a/shell/platform/embedder/embedder_render_target.h +++ b/shell/platform/embedder/embedder_render_target.h @@ -27,7 +27,19 @@ namespace flutter { /// class EmbedderRenderTarget { public: - using MakeOrClearCurrentCallback = std::function()>; + struct SetCurrentResult { + /// This is true if the operation succeeded (even if it was a no-op), + /// false if the surface could not be made current / the current surface + /// could not be cleared. + bool success; + + /// This is true if any native graphics API (e.g. GL, but not EGL) state has + /// changed and Skia/Impeller should not assume any GL state values are the + /// same as before the context change operation was attempted. + bool gl_state_trampled; + }; + + using MakeOrClearCurrentCallback = std::function; //---------------------------------------------------------------------------- /// @brief Destroys this instance of the render target and invokes the @@ -88,30 +100,14 @@ class EmbedderRenderTarget { /// the embedder will provide a callback that should be called /// when the target surface should be made current. /// - /// @return A pair of booleans. The first bool is true if the operation - /// succeeded (even if it was a no-op), false if the target could - /// not be make current. - /// The second boolean is true if any native graphics API - /// (e.g. GL, but not EGL) state has changed and skia/impeller - /// should not assume any GL state values are the same as before - /// MaybeMakeCurrent was called. - virtual std::pair MaybeMakeCurrent() const { - return {true, false}; - } + /// @return The result of the operation. + virtual SetCurrentResult MaybeMakeCurrent() const { return {true, false}; } //---------------------------------------------------------------------------- /// @brief Clear the current render target. @see MaybeMakeCurrent /// - /// @return A pair of booleans. The first bool is true if the operation - /// succeeded (even if it was a no-op), false if the target could - /// not be cleared. - /// The second boolean is true if any native graphics API - /// (e.g. GL, but not EGL) state has changed and skia/impeller - /// should not assume any GL state values are the same as before - /// MaybeClearCurrent was called. - virtual std::pair MaybeClearCurrent() const { - return {true, false}; - } + /// @return The result of the operation. + virtual SetCurrentResult MaybeClearCurrent() const { return {true, false}; } protected: //---------------------------------------------------------------------------- diff --git a/shell/platform/embedder/embedder_render_target_skia.cc b/shell/platform/embedder/embedder_render_target_skia.cc index 2d5167db4958a..25bc9277169c8 100644 --- a/shell/platform/embedder/embedder_render_target_skia.cc +++ b/shell/platform/embedder/embedder_render_target_skia.cc @@ -41,7 +41,8 @@ SkISize EmbedderRenderTargetSkia::GetRenderTargetSize() const { return SkISize::Make(render_surface_->width(), render_surface_->height()); } -std::pair EmbedderRenderTargetSkia::MaybeMakeCurrent() const { +EmbedderRenderTarget::SetCurrentResult +EmbedderRenderTargetSkia::MaybeMakeCurrent() const { if (on_make_current_ != nullptr) { return on_make_current_(); } @@ -49,7 +50,8 @@ std::pair EmbedderRenderTargetSkia::MaybeMakeCurrent() const { return {true, false}; } -std::pair EmbedderRenderTargetSkia::MaybeClearCurrent() const { +EmbedderRenderTarget::SetCurrentResult +EmbedderRenderTargetSkia::MaybeClearCurrent() const { if (on_clear_current_ != nullptr) { return on_clear_current_(); } diff --git a/shell/platform/embedder/embedder_render_target_skia.h b/shell/platform/embedder/embedder_render_target_skia.h index 48e2c8604b980..6331f2165d4ee 100644 --- a/shell/platform/embedder/embedder_render_target_skia.h +++ b/shell/platform/embedder/embedder_render_target_skia.h @@ -33,10 +33,10 @@ class EmbedderRenderTargetSkia final : public EmbedderRenderTarget { SkISize GetRenderTargetSize() const override; // |EmbedderRenderTarget| - std::pair MaybeMakeCurrent() const override; + SetCurrentResult MaybeMakeCurrent() const override; // |EmbedderRenderTarget| - std::pair MaybeClearCurrent() const override; + SetCurrentResult MaybeClearCurrent() const override; private: sk_sp render_surface_;