From a69e0f3f67931a8439b551f750ae20f3b7a6a4d8 Mon Sep 17 00:00:00 2001 From: Bernardo Eilert Trevisan Date: Fri, 29 Jul 2022 17:59:24 -0700 Subject: [PATCH 01/34] Create GLFBOInfo. --- shell/gpu/gpu_surface_gl_delegate.h | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/shell/gpu/gpu_surface_gl_delegate.h b/shell/gpu/gpu_surface_gl_delegate.h index f9da6a11120fd..484c38bc9a5df 100644 --- a/shell/gpu/gpu_surface_gl_delegate.h +++ b/shell/gpu/gpu_surface_gl_delegate.h @@ -22,6 +22,17 @@ struct GLFrameInfo { uint32_t height; }; +// A structure to represent the frame buffer information which is returned to +// the rendering backend after requesting a frame buffer object. +struct GLFBOInfo { + // The frame buffer's ID. + uint32_t fbo_id; + // This boolean flags whether the returned FBO supports partial repaint. + const bool partial_repaint_enabled; + // The frame buffer's existing damage (i.e. damage since it was last used). + const SkIRect existing_damage; +}; + // Information passed during presentation of a frame. struct GLPresentInfo { uint32_t fbo_id; From a28f3a9fe8758f12cba9df8ae5a9084a96aea524 Mon Sep 17 00:00:00 2001 From: Bernardo Eilert Trevisan Date: Fri, 29 Jul 2022 18:00:33 -0700 Subject: [PATCH 02/34] Update FlutterPresentInfo with frame and buffer damage --- shell/gpu/gpu_surface_gl_delegate.h | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/shell/gpu/gpu_surface_gl_delegate.h b/shell/gpu/gpu_surface_gl_delegate.h index 484c38bc9a5df..e71c4a4021559 100644 --- a/shell/gpu/gpu_surface_gl_delegate.h +++ b/shell/gpu/gpu_surface_gl_delegate.h @@ -37,13 +37,17 @@ struct GLFBOInfo { struct GLPresentInfo { uint32_t fbo_id; - // Damage is a hint to compositor telling it which parts of front buffer - // need to be updated - const std::optional& damage; + // The frame damage is a hint to compositor telling it which parts of front + // buffer need to be updated. + const std::optional& frame_damage; // Time at which this frame is scheduled to be presented. This is a hint // that can be passed to the platform to drop queued frames. std::optional presentation_time = std::nullopt; + + // The buffer damage refers to the region that needs to be set as damaged + // within the frame buffer. + const std::optional& buffer_damage; }; class GPUSurfaceGLDelegate { From 030e54e0903744650a410544efa9ee7e9b8a6d90 Mon Sep 17 00:00:00 2001 From: Bernardo Eilert Trevisan Date: Fri, 29 Jul 2022 18:01:43 -0700 Subject: [PATCH 03/34] Update GLContextFBO prototype to return a GLFBOInfo. --- shell/gpu/gpu_surface_gl_delegate.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/shell/gpu/gpu_surface_gl_delegate.h b/shell/gpu/gpu_surface_gl_delegate.h index e71c4a4021559..d1e31b96559f5 100644 --- a/shell/gpu/gpu_surface_gl_delegate.h +++ b/shell/gpu/gpu_surface_gl_delegate.h @@ -69,8 +69,9 @@ class GPUSurfaceGLDelegate { // context and not any of the contexts dedicated for IO. virtual bool GLContextPresent(const GLPresentInfo& present_info) = 0; - // The ID of the main window bound framebuffer. Typically FBO0. - virtual intptr_t GLContextFBO(GLFrameInfo frame_info) const = 0; + // The information about the main window bound framebuffer. ID is Typically + // FBO0. + virtual GLFBOInfo GLContextFBO(GLFrameInfo frame_info) const = 0; // The rendering subsystem assumes that the ID of the main window bound // framebuffer remains constant throughout. If this assumption in incorrect, From dde31a1ca3bebbeb62034728019dd9796c92e324 Mon Sep 17 00:00:00 2001 From: Bernardo Eilert Trevisan Date: Fri, 29 Jul 2022 18:04:50 -0700 Subject: [PATCH 04/34] Update affected functions by updates to GLPresentInfo --- shell/gpu/gpu_surface_gl_impeller.cc | 3 ++- shell/platform/android/android_surface_gl_skia.cc | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/shell/gpu/gpu_surface_gl_impeller.cc b/shell/gpu/gpu_surface_gl_impeller.cc index 24413147274a6..3fc7c441b42bb 100644 --- a/shell/gpu/gpu_surface_gl_impeller.cc +++ b/shell/gpu/gpu_surface_gl_impeller.cc @@ -62,10 +62,11 @@ std::unique_ptr GPUSurfaceGLImpeller::AcquireFrame( if (weak) { GLPresentInfo present_info = { .fbo_id = 0, - .damage = std::nullopt, + .frame_damage = std::nullopt, // TODO (https://github.com/flutter/flutter/issues/105597): wire-up // presentation time to impeller backend. .presentation_time = std::nullopt, + .buffer_damage = std::nullopt, }; delegate->GLContextPresent(present_info); } diff --git a/shell/platform/android/android_surface_gl_skia.cc b/shell/platform/android/android_surface_gl_skia.cc index 23242c9b3bad4..501086f17c55d 100644 --- a/shell/platform/android/android_surface_gl_skia.cc +++ b/shell/platform/android/android_surface_gl_skia.cc @@ -159,7 +159,7 @@ bool AndroidSurfaceGLSkia::GLContextPresent(const GLPresentInfo& present_info) { if (present_info.presentation_time) { onscreen_surface_->SetPresentationTime(*present_info.presentation_time); } - return onscreen_surface_->SwapBuffers(present_info.damage); + return onscreen_surface_->SwapBuffers(present_info.frame_damage); } intptr_t AndroidSurfaceGLSkia::GLContextFBO(GLFrameInfo frame_info) const { From b035be5eba07e9bd53b2e1f2911f2efb02d4b16f Mon Sep 17 00:00:00 2001 From: Bernardo Eilert Trevisan Date: Fri, 29 Jul 2022 18:14:08 -0700 Subject: [PATCH 05/34] Update the behavior of GL Skia to account for dirty region management. --- shell/gpu/gpu_surface_gl_skia.cc | 22 +++++++++++++++------- shell/gpu/gpu_surface_gl_skia.h | 4 ++++ 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/shell/gpu/gpu_surface_gl_skia.cc b/shell/gpu/gpu_surface_gl_skia.cc index 66bb6636d95d8..eafb6120b3774 100644 --- a/shell/gpu/gpu_surface_gl_skia.cc +++ b/shell/gpu/gpu_surface_gl_skia.cc @@ -181,10 +181,10 @@ bool GPUSurfaceGLSkia::CreateOrUpdateSurfaces(const SkISize& size) { GLFrameInfo frame_info = {static_cast(size.width()), static_cast(size.height())}; - const uint32_t fbo_id = delegate_->GLContextFBO(frame_info); + const GLFBOInfo fbo_info = delegate_->GLContextFBO(frame_info); onscreen_surface = WrapOnscreenSurface(context_.get(), // GL context size, // root surface size - fbo_id // window FBO ID + fbo_info.fbo_id // window FBO ID ); if (onscreen_surface == nullptr) { @@ -195,7 +195,9 @@ bool GPUSurfaceGLSkia::CreateOrUpdateSurfaces(const SkISize& size) { } onscreen_surface_ = std::move(onscreen_surface); - fbo_id_ = fbo_id; + fbo_id_ = fbo_info.fbo_id; + supports_partial_repaint_ = fbo_info.partial_repaint_enabled; + existing_damage_ = fbo_info.existing_damage; return true; } @@ -248,6 +250,9 @@ std::unique_ptr GPUSurfaceGLSkia::AcquireFrame( }; framebuffer_info = delegate_->GLContextFramebufferInfo(); + // Partial repaint is enabled by default + framebuffer_info.supports_partial_repaint = supports_partial_repaint_; + framebuffer_info.existing_damage = existing_damage_; return std::make_unique(surface, std::move(framebuffer_info), submit_callback, std::move(context_switch)); @@ -268,8 +273,9 @@ bool GPUSurfaceGLSkia::PresentSurface(const SurfaceFrame& frame, GLPresentInfo present_info = { .fbo_id = fbo_id_, - .damage = frame.submit_info().frame_damage, + .frame_damage = frame.submit_info().frame_damage, .presentation_time = frame.submit_info().presentation_time, + .buffer_damage = frame.submit_info().buffer_damage, }; if (!delegate_->GLContextPresent(present_info)) { return false; @@ -284,11 +290,11 @@ bool GPUSurfaceGLSkia::PresentSurface(const SurfaceFrame& frame, // The FBO has changed, ask the delegate for the new FBO and do a surface // re-wrap. - const uint32_t fbo_id = delegate_->GLContextFBO(frame_info); + const GLFBOInfo fbo_info = delegate_->GLContextFBO(frame_info); auto new_onscreen_surface = WrapOnscreenSurface(context_.get(), // GL context current_size, // root surface size - fbo_id // window FBO ID + fbo_info.fbo_id // window FBO ID ); if (!new_onscreen_surface) { @@ -296,7 +302,9 @@ bool GPUSurfaceGLSkia::PresentSurface(const SurfaceFrame& frame, } onscreen_surface_ = std::move(new_onscreen_surface); - fbo_id_ = fbo_id; + fbo_id_ = fbo_info.fbo_id; + supports_partial_repaint_ = fbo_info.partial_repaint_enabled; + existing_damage_ = fbo_info.existing_damage; } return true; diff --git a/shell/gpu/gpu_surface_gl_skia.h b/shell/gpu/gpu_surface_gl_skia.h index 90cad241945fd..39f6e3f607dfe 100644 --- a/shell/gpu/gpu_surface_gl_skia.h +++ b/shell/gpu/gpu_surface_gl_skia.h @@ -67,6 +67,8 @@ class GPUSurfaceGLSkia : public Surface { sk_sp onscreen_surface_; /// FBO backing the current `onscreen_surface_`. uint32_t fbo_id_ = 0; + // Private variable used to keep track of the current FBO's existing damage. + SkIRect existing_damage_ = SkIRect::MakeEmpty(); bool context_owner_ = false; // TODO(38466): Refactor GPU surface APIs take into account the fact that an // external view embedder may want to render to the root surface. This is a @@ -74,6 +76,8 @@ class GPUSurfaceGLSkia : public Surface { // external view embedder is present. const bool render_to_surface_ = true; bool valid_ = false; + // Partial repaint is on by default. + bool supports_partial_repaint_ = true; // WeakPtrFactory must be the last member. fml::TaskRunnerAffineWeakPtrFactory weak_factory_; From 7f4bd472eb6b644a8121ac5e11c6db4cfe8faf2e Mon Sep 17 00:00:00 2001 From: Bernardo Eilert Trevisan Date: Fri, 29 Jul 2022 18:14:45 -0700 Subject: [PATCH 06/34] Updated affected functions by the changes to the return value of GLContextFBO. --- shell/common/shell_test_platform_view_gl.cc | 6 ++++-- shell/common/shell_test_platform_view_gl.h | 2 +- shell/platform/android/android_surface_gl_impeller.cc | 6 ++++-- shell/platform/android/android_surface_gl_impeller.h | 2 +- shell/platform/android/android_surface_gl_skia.cc | 6 ++++-- shell/platform/android/android_surface_gl_skia.h | 2 +- shell/platform/android/surface/android_surface_mock.cc | 7 ++++--- shell/platform/android/surface/android_surface_mock.h | 2 +- 8 files changed, 20 insertions(+), 13 deletions(-) diff --git a/shell/common/shell_test_platform_view_gl.cc b/shell/common/shell_test_platform_view_gl.cc index 492354230e8dc..ac68ff3971d0b 100644 --- a/shell/common/shell_test_platform_view_gl.cc +++ b/shell/common/shell_test_platform_view_gl.cc @@ -69,8 +69,10 @@ bool ShellTestPlatformViewGL::GLContextPresent( } // |GPUSurfaceGLDelegate| -intptr_t ShellTestPlatformViewGL::GLContextFBO(GLFrameInfo frame_info) const { - return gl_surface_.GetFramebuffer(frame_info.width, frame_info.height); +GLFBOInfo ShellTestPlatformViewGL::GLContextFBO(GLFrameInfo frame_info) const { + return GLFBOInfo{ + .fbo_id = gl_surface_.GetFramebuffer(frame_info.width, frame_info.height), + }; } // |GPUSurfaceGLDelegate| diff --git a/shell/common/shell_test_platform_view_gl.h b/shell/common/shell_test_platform_view_gl.h index c890c1e0fc6ec..b4afc1266975b 100644 --- a/shell/common/shell_test_platform_view_gl.h +++ b/shell/common/shell_test_platform_view_gl.h @@ -61,7 +61,7 @@ class ShellTestPlatformViewGL : public ShellTestPlatformView, bool GLContextPresent(const GLPresentInfo& present_info) override; // |GPUSurfaceGLDelegate| - intptr_t GLContextFBO(GLFrameInfo frame_info) const override; + GLFBOInfo GLContextFBO(GLFrameInfo frame_info) const override; // |GPUSurfaceGLDelegate| GLProcResolver GetGLProcResolver() const override; diff --git a/shell/platform/android/android_surface_gl_impeller.cc b/shell/platform/android/android_surface_gl_impeller.cc index 28c1cfe55f19e..635acc77a9502 100644 --- a/shell/platform/android/android_surface_gl_impeller.cc +++ b/shell/platform/android/android_surface_gl_impeller.cc @@ -295,9 +295,11 @@ bool AndroidSurfaceGLImpeller::GLContextPresent( } // |GPUSurfaceGLDelegate| -intptr_t AndroidSurfaceGLImpeller::GLContextFBO(GLFrameInfo frame_info) const { +GLFBOInfo AndroidSurfaceGLImpeller::GLContextFBO(GLFrameInfo frame_info) const { // FBO0 is the default window bound framebuffer in EGL environments. - return 0; + return GLFBOInfo{ + .fbo_id = 0, + }; } // |GPUSurfaceGLDelegate| diff --git a/shell/platform/android/android_surface_gl_impeller.h b/shell/platform/android/android_surface_gl_impeller.h index 8bcf14dbee27b..9d2399868da97 100644 --- a/shell/platform/android/android_surface_gl_impeller.h +++ b/shell/platform/android/android_surface_gl_impeller.h @@ -68,7 +68,7 @@ class AndroidSurfaceGLImpeller final : public GPUSurfaceGLDelegate, bool GLContextPresent(const GLPresentInfo& present_info) override; // |GPUSurfaceGLDelegate| - intptr_t GLContextFBO(GLFrameInfo frame_info) const override; + GLFBOInfo GLContextFBO(GLFrameInfo frame_info) const override; // |GPUSurfaceGLDelegate| sk_sp GetGLInterface() const override; diff --git a/shell/platform/android/android_surface_gl_skia.cc b/shell/platform/android/android_surface_gl_skia.cc index 501086f17c55d..50ac30835bb1f 100644 --- a/shell/platform/android/android_surface_gl_skia.cc +++ b/shell/platform/android/android_surface_gl_skia.cc @@ -162,10 +162,12 @@ bool AndroidSurfaceGLSkia::GLContextPresent(const GLPresentInfo& present_info) { return onscreen_surface_->SwapBuffers(present_info.frame_damage); } -intptr_t AndroidSurfaceGLSkia::GLContextFBO(GLFrameInfo frame_info) const { +GLFBOInfo AndroidSurfaceGLSkia::GLContextFBO(GLFrameInfo frame_info) const { FML_DCHECK(IsValid()); // The default window bound framebuffer on Android. - return 0; + return GLFBOInfo{ + .fbo_id = 0, + }; } // |GPUSurfaceGLDelegate| diff --git a/shell/platform/android/android_surface_gl_skia.h b/shell/platform/android/android_surface_gl_skia.h index 99a910b69b76d..73f36945127f8 100644 --- a/shell/platform/android/android_surface_gl_skia.h +++ b/shell/platform/android/android_surface_gl_skia.h @@ -67,7 +67,7 @@ class AndroidSurfaceGLSkia final : public GPUSurfaceGLDelegate, bool GLContextPresent(const GLPresentInfo& present_info) override; // |GPUSurfaceGLDelegate| - intptr_t GLContextFBO(GLFrameInfo frame_info) const override; + GLFBOInfo GLContextFBO(GLFrameInfo frame_info) const override; // |GPUSurfaceGLDelegate| sk_sp GetGLInterface() const override; diff --git a/shell/platform/android/surface/android_surface_mock.cc b/shell/platform/android/surface/android_surface_mock.cc index c3a9f38fc8f0c..9dd367e285d46 100644 --- a/shell/platform/android/surface/android_surface_mock.cc +++ b/shell/platform/android/surface/android_surface_mock.cc @@ -22,8 +22,9 @@ bool AndroidSurfaceMock::GLContextPresent(const GLPresentInfo& present_info) { return true; } -intptr_t AndroidSurfaceMock::GLContextFBO(GLFrameInfo frame_info) const { - return 0; -} +GLFBOInfo AndroidSurfaceMock::GLContextFBO(GLFrameInfo frame_info) const { + return GLFBOInfo{ + .fbo_id = 0, + }; } // namespace flutter diff --git a/shell/platform/android/surface/android_surface_mock.h b/shell/platform/android/surface/android_surface_mock.h index d7363873a9a31..3f326a21920b7 100644 --- a/shell/platform/android/surface/android_surface_mock.h +++ b/shell/platform/android/surface/android_surface_mock.h @@ -51,7 +51,7 @@ class AndroidSurfaceMock final : public GPUSurfaceGLDelegate, bool GLContextPresent(const GLPresentInfo& present_info) override; // |GPUSurfaceGLDelegate| - intptr_t GLContextFBO(GLFrameInfo frame_info) const override; + GLFBOInfo GLContextFBO(GLFrameInfo frame_info) const override; }; } // namespace flutter From 89b112ca72ff19fec4dcfe604b6fe0c613789a64 Mon Sep 17 00:00:00 2001 From: Bernardo Eilert Trevisan Date: Fri, 29 Jul 2022 18:16:13 -0700 Subject: [PATCH 07/34] Create FlutterDamage. --- shell/platform/embedder/embedder.h | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/shell/platform/embedder/embedder.h b/shell/platform/embedder/embedder.h index 43fd5bd5b1cde..3ba2c7d0d1751 100644 --- a/shell/platform/embedder/embedder.h +++ b/shell/platform/embedder/embedder.h @@ -433,6 +433,16 @@ typedef struct { FlutterSize lower_left_corner_radius; } FlutterRoundedRect; +/// A structure to represent a damage region. +typedef struct { + /// The size of this struct. Must be sizeof(FlutterDamage). + size_t struct_size; + /// The number of rectangles within the damage region. + size_t num_rects; + /// The actual damage region(s) in question. + FlutterRect* damage; +} FlutterDamage; + /// This information is passed to the embedder when requesting a frame buffer /// object. /// From 10df5e39da8eec9ef3eb4b7625d0f9aa4d2721b1 Mon Sep 17 00:00:00 2001 From: Bernardo Eilert Trevisan Date: Fri, 29 Jul 2022 18:16:57 -0700 Subject: [PATCH 08/34] Update FlutterPresentInfo fields. --- shell/platform/embedder/embedder.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/shell/platform/embedder/embedder.h b/shell/platform/embedder/embedder.h index 3ba2c7d0d1751..51eae33f7b261 100644 --- a/shell/platform/embedder/embedder.h +++ b/shell/platform/embedder/embedder.h @@ -467,6 +467,10 @@ typedef struct { size_t struct_size; /// Id of the fbo backing the surface that was presented. uint32_t fbo_id; + /// Damage representing the area that the compositor needs to render. + FlutterDamage frame_damage; + /// Damage used to set the buffer's damage region. + FlutterDamage buffer_damage; } FlutterPresentInfo; /// Callback for when a surface is presented. From ab17275e918680d76288f008ee176dea871b2779 Mon Sep 17 00:00:00 2001 From: Bernardo Eilert Trevisan Date: Fri, 29 Jul 2022 18:17:59 -0700 Subject: [PATCH 09/34] Add new callback fbo_with_damage_callback. --- shell/platform/embedder/embedder.h | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/shell/platform/embedder/embedder.h b/shell/platform/embedder/embedder.h index 51eae33f7b261..0c091429b47e0 100644 --- a/shell/platform/embedder/embedder.h +++ b/shell/platform/embedder/embedder.h @@ -459,6 +459,13 @@ typedef uint32_t (*UIntFrameInfoCallback)( void* /* user data */, const FlutterFrameInfo* /* frame info */); +/// Callback for when a frame buffer object is requested with necessary +/// information for partial repaint. +typedef void (*FlutterFrameBufferWithDamageCallback)( + void* /* user data */, + const intptr_t id, + FlutterDamage* existing_damage); + /// This information is passed to the embedder when a surface is presented. /// /// See: \ref FlutterOpenGLRendererConfig.present_with_info. @@ -536,6 +543,10 @@ typedef struct { /// `FlutterPresentInfo` struct that the embedder can use to release any /// resources. The return value indicates success of the present call. BoolPresentInfoCallback present_with_info; + /// Specifying this callback is a requirement for dirty region management. The + /// callback will return a FlutterFrameBuffer containing information about the + /// buffer whose ID was passed to it. + FlutterFrameBufferWithDamageCallback fbo_with_damage_callback; } FlutterOpenGLRendererConfig; /// Alias for id. From a30e0164052d22c7f51eed98f1f2777c97b2c25b Mon Sep 17 00:00:00 2001 From: Bernardo Eilert Trevisan Date: Fri, 29 Jul 2022 18:20:46 -0700 Subject: [PATCH 10/34] Update dispatch table. --- shell/platform/embedder/embedder_surface_gl.h | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/shell/platform/embedder/embedder_surface_gl.h b/shell/platform/embedder/embedder_surface_gl.h index c0a2b6a55d98f..98d1cf975018c 100644 --- a/shell/platform/embedder/embedder_surface_gl.h +++ b/shell/platform/embedder/embedder_surface_gl.h @@ -18,12 +18,13 @@ class EmbedderSurfaceGL final : public EmbedderSurface, struct GLDispatchTable { std::function gl_make_current_callback; // required std::function gl_clear_current_callback; // required - std::function gl_present_callback; // required + std::function gl_present_callback; // required std::function gl_fbo_callback; // required std::function gl_make_resource_current_callback; // optional std::function - gl_surface_transformation_callback; // optional - std::function gl_proc_resolver; // optional + gl_surface_transformation_callback; // optional + std::function gl_proc_resolver; // optional + std::function gl_fbo_with_damage_callback; // required }; EmbedderSurfaceGL( @@ -59,7 +60,7 @@ class EmbedderSurfaceGL final : public EmbedderSurface, bool GLContextPresent(const GLPresentInfo& present_info) override; // |GPUSurfaceGLDelegate| - intptr_t GLContextFBO(GLFrameInfo frame_info) const override; + GLFBOInfo GLContextFBO(GLFrameInfo frame_info) const override; // |GPUSurfaceGLDelegate| bool GLContextFBOResetAfterPresent() const override; From e532950d333d84d6b55a001ccc663d1e3933b8d4 Mon Sep 17 00:00:00 2001 From: Bernardo Eilert Trevisan Date: Fri, 29 Jul 2022 18:21:50 -0700 Subject: [PATCH 11/34] Add check to make sure gl_fbo_with_damage_callback was passed. --- shell/platform/embedder/embedder_surface_gl.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/shell/platform/embedder/embedder_surface_gl.cc b/shell/platform/embedder/embedder_surface_gl.cc index e406ee9ed10fa..400efda802c35 100644 --- a/shell/platform/embedder/embedder_surface_gl.cc +++ b/shell/platform/embedder/embedder_surface_gl.cc @@ -19,7 +19,8 @@ EmbedderSurfaceGL::EmbedderSurfaceGL( if (!gl_dispatch_table_.gl_make_current_callback || !gl_dispatch_table_.gl_clear_current_callback || !gl_dispatch_table_.gl_present_callback || - !gl_dispatch_table_.gl_fbo_callback) { + !gl_dispatch_table_.gl_fbo_callback || + !gl_dispatch_table_.gl_fbo_with_damage_callback) { return; } From 2b236377958bee7f73430baf34ce7b9bac2984fd Mon Sep 17 00:00:00 2001 From: Bernardo Eilert Trevisan Date: Fri, 29 Jul 2022 18:22:44 -0700 Subject: [PATCH 12/34] Update GLContextPresent and GLContextFBO. --- shell/platform/embedder/embedder_surface_gl.cc | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/shell/platform/embedder/embedder_surface_gl.cc b/shell/platform/embedder/embedder_surface_gl.cc index 400efda802c35..504ccf9316ce0 100644 --- a/shell/platform/embedder/embedder_surface_gl.cc +++ b/shell/platform/embedder/embedder_surface_gl.cc @@ -47,12 +47,16 @@ bool EmbedderSurfaceGL::GLContextClearCurrent() { // |GPUSurfaceGLDelegate| bool EmbedderSurfaceGL::GLContextPresent(const GLPresentInfo& present_info) { - return gl_dispatch_table_.gl_present_callback(present_info.fbo_id); + // Pass the present information to the embedder present callback. + return gl_dispatch_table_.gl_present_callback(present_info); } // |GPUSurfaceGLDelegate| -intptr_t EmbedderSurfaceGL::GLContextFBO(GLFrameInfo frame_info) const { - return gl_dispatch_table_.gl_fbo_callback(frame_info); +GLFBOInfo EmbedderSurfaceGL::GLContextFBO(GLFrameInfo frame_info) const { + // Get the FBO ID using the gl_fbo_callback and then get exiting damage by + // passing that ID to the gl_fbo_with_damage_callback. + return gl_dispatch_table_.gl_fbo_with_damage_callback( + gl_dispatch_table_.gl_fbo_callback(frame_info)); } // |GPUSurfaceGLDelegate| From de9599009e01f8dc455e38c352c906ac2c1d46a7 Mon Sep 17 00:00:00 2001 From: Bernardo Eilert Trevisan Date: Fri, 29 Jul 2022 18:27:01 -0700 Subject: [PATCH 13/34] Update open gl default renderer config for tests. --- shell/platform/embedder/tests/embedder_config_builder.cc | 9 ++++++--- shell/platform/embedder/tests/embedder_test_context_gl.h | 2 +- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/shell/platform/embedder/tests/embedder_config_builder.cc b/shell/platform/embedder/tests/embedder_config_builder.cc index 918abff4e261e..1666f84750c60 100644 --- a/shell/platform/embedder/tests/embedder_config_builder.cc +++ b/shell/platform/embedder/tests/embedder_config_builder.cc @@ -52,13 +52,14 @@ EmbedderConfigBuilder::EmbedderConfigBuilder( opengl_renderer_config_.present_with_info = [](void* context, const FlutterPresentInfo* present_info) -> bool { return reinterpret_cast(context)->GLPresent( - present_info->fbo_id); + *present_info); }; opengl_renderer_config_.fbo_with_frame_info_callback = [](void* context, const FlutterFrameInfo* frame_info) -> uint32_t { return reinterpret_cast(context)->GLGetFramebuffer( *frame_info); }; + opengl_renderer_config_.fbo_with_damage_callback = nullptr; opengl_renderer_config_.make_resource_current = [](void* context) -> bool { return reinterpret_cast(context) ->GLMakeResourceCurrent(); @@ -159,8 +160,10 @@ void EmbedderConfigBuilder::SetOpenGLPresentCallBack() { // SetOpenGLRendererConfig must be called before this. FML_CHECK(renderer_config_.type == FlutterRendererType::kOpenGL); renderer_config_.open_gl.present = [](void* context) -> bool { - // passing a placeholder fbo_id. - return reinterpret_cast(context)->GLPresent(0); + return reinterpret_cast(context)->GLPresent( + FlutterPresentInfo{ + .fbo_id = 0, + }); }; #endif } diff --git a/shell/platform/embedder/tests/embedder_test_context_gl.h b/shell/platform/embedder/tests/embedder_test_context_gl.h index 65e2c34c355c2..ddbed15003769 100644 --- a/shell/platform/embedder/tests/embedder_test_context_gl.h +++ b/shell/platform/embedder/tests/embedder_test_context_gl.h @@ -72,7 +72,7 @@ class EmbedderTestContextGL : public EmbedderTestContext { bool GLClearCurrent(); - bool GLPresent(uint32_t fbo_id); + bool GLPresent(FlutterPresentInfo present_info); uint32_t GLGetFramebuffer(FlutterFrameInfo frame_info); From 98d2d6a507f04bea563aede78857525741e5e5a7 Mon Sep 17 00:00:00 2001 From: Bernardo Eilert Trevisan Date: Fri, 29 Jul 2022 18:29:37 -0700 Subject: [PATCH 14/34] Add GetRendererConfig function. --- shell/platform/embedder/tests/embedder_config_builder.cc | 5 +++++ shell/platform/embedder/tests/embedder_config_builder.h | 2 ++ 2 files changed, 7 insertions(+) diff --git a/shell/platform/embedder/tests/embedder_config_builder.cc b/shell/platform/embedder/tests/embedder_config_builder.cc index 1666f84750c60..2fc8c2885018e 100644 --- a/shell/platform/embedder/tests/embedder_config_builder.cc +++ b/shell/platform/embedder/tests/embedder_config_builder.cc @@ -160,6 +160,7 @@ void EmbedderConfigBuilder::SetOpenGLPresentCallBack() { // SetOpenGLRendererConfig must be called before this. FML_CHECK(renderer_config_.type == FlutterRendererType::kOpenGL); renderer_config_.open_gl.present = [](void* context) -> bool { + // passing a placeholder fbo_id. return reinterpret_cast(context)->GLPresent( FlutterPresentInfo{ .fbo_id = 0, @@ -315,6 +316,10 @@ void EmbedderConfigBuilder::SetupVsyncCallback() { }; } +FlutterRendererConfig& EmbedderConfigBuilder::GetRendererConfig() { + return renderer_config_; +} + void EmbedderConfigBuilder::SetRenderTaskRunner( const FlutterTaskRunnerDescription* runner) { if (runner == nullptr) { diff --git a/shell/platform/embedder/tests/embedder_config_builder.h b/shell/platform/embedder/tests/embedder_config_builder.h index 3ce230c0146b8..2727c85fdadbd 100644 --- a/shell/platform/embedder/tests/embedder_config_builder.h +++ b/shell/platform/embedder/tests/embedder_config_builder.h @@ -104,6 +104,8 @@ class EmbedderConfigBuilder { FlutterCompositor& GetCompositor(); + FlutterRendererConfig& GetRendererConfig(); + void SetRenderTargetType( EmbedderTestBackingStoreProducer::RenderTargetType type, FlutterSoftwarePixelFormat software_pixfmt = kNative32); From c6b86681a1282641d04e2f9506fbc45e6b88afab Mon Sep 17 00:00:00 2001 From: Bernardo Eilert Trevisan Date: Fri, 29 Jul 2022 18:31:34 -0700 Subject: [PATCH 15/34] Update GLPresent used in tests. --- shell/platform/embedder/tests/embedder_test_context_gl.cc | 4 ++-- shell/platform/embedder/tests/embedder_test_context_gl.h | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/shell/platform/embedder/tests/embedder_test_context_gl.cc b/shell/platform/embedder/tests/embedder_test_context_gl.cc index 9a7bc9bcf1c26..0dc1e42f84534 100644 --- a/shell/platform/embedder/tests/embedder_test_context_gl.cc +++ b/shell/platform/embedder/tests/embedder_test_context_gl.cc @@ -39,7 +39,7 @@ bool EmbedderTestContextGL::GLClearCurrent() { return gl_surface_->ClearCurrent(); } -bool EmbedderTestContextGL::GLPresent(uint32_t fbo_id) { +bool EmbedderTestContextGL::GLPresent(FlutterPresentInfo present_info) { FML_CHECK(gl_surface_) << "GL surface must be initialized."; gl_surface_present_count_++; @@ -50,7 +50,7 @@ bool EmbedderTestContextGL::GLPresent(uint32_t fbo_id) { } if (callback) { - callback(fbo_id); + callback(present_info); } FireRootSurfacePresentCallbackIfPresent( diff --git a/shell/platform/embedder/tests/embedder_test_context_gl.h b/shell/platform/embedder/tests/embedder_test_context_gl.h index ddbed15003769..9342a4ee62a61 100644 --- a/shell/platform/embedder/tests/embedder_test_context_gl.h +++ b/shell/platform/embedder/tests/embedder_test_context_gl.h @@ -14,7 +14,8 @@ namespace testing { class EmbedderTestContextGL : public EmbedderTestContext { public: using GLGetFBOCallback = std::function; - using GLPresentCallback = std::function; + using GLPresentCallback = + std::function; explicit EmbedderTestContextGL(std::string assets_path = ""); From b50677d89c50817879d2dcb8177d286e633f63eb Mon Sep 17 00:00:00 2001 From: Bernardo Eilert Trevisan Date: Fri, 29 Jul 2022 18:34:51 -0700 Subject: [PATCH 16/34] Add fbo_with_damage_callback to tests. --- .../tests/embedder_test_context_gl.cc | 21 +++++++++++++++++++ .../embedder/tests/embedder_test_context_gl.h | 7 +++++++ 2 files changed, 28 insertions(+) diff --git a/shell/platform/embedder/tests/embedder_test_context_gl.cc b/shell/platform/embedder/tests/embedder_test_context_gl.cc index 0dc1e42f84534..63567a6f82410 100644 --- a/shell/platform/embedder/tests/embedder_test_context_gl.cc +++ b/shell/platform/embedder/tests/embedder_test_context_gl.cc @@ -64,6 +64,12 @@ void EmbedderTestContextGL::SetGLGetFBOCallback(GLGetFBOCallback callback) { gl_get_fbo_callback_ = callback; } +void EmbedderTestContextGL::SetGLGetFBOWithDamageCallback( + GLGetFBOWithDamageCallback callback) { + std::scoped_lock lock(gl_callback_mutex_); + gl_get_fbo_with_damage_callback_ = callback; +} + void EmbedderTestContextGL::SetGLPresentCallback(GLPresentCallback callback) { std::scoped_lock lock(gl_callback_mutex_); gl_present_callback_ = callback; @@ -86,6 +92,21 @@ uint32_t EmbedderTestContextGL::GLGetFramebuffer(FlutterFrameInfo frame_info) { return gl_surface_->GetFramebuffer(size.width, size.height); } +void EmbedderTestContextGL::GLGetFBOWithDamage(const intptr_t id, + FlutterDamage* existing_damage) { + FML_CHECK(gl_surface_) << "GL surface must be initialized."; + + GLGetFBOWithDamageCallback callback; + { + std::scoped_lock lock(gl_callback_mutex_); + callback = gl_get_fbo_with_damage_callback_; + } + + if (callback) { + callback(id, existing_damage); + } +} + bool EmbedderTestContextGL::GLMakeResourceCurrent() { FML_CHECK(gl_surface_) << "GL surface must be initialized."; return gl_surface_->MakeResourceCurrent(); diff --git a/shell/platform/embedder/tests/embedder_test_context_gl.h b/shell/platform/embedder/tests/embedder_test_context_gl.h index 9342a4ee62a61..720a9ed5d467b 100644 --- a/shell/platform/embedder/tests/embedder_test_context_gl.h +++ b/shell/platform/embedder/tests/embedder_test_context_gl.h @@ -14,6 +14,8 @@ namespace testing { class EmbedderTestContextGL : public EmbedderTestContext { public: using GLGetFBOCallback = std::function; + using GLGetFBOWithDamageCallback = + std::function; using GLPresentCallback = std::function; @@ -39,6 +41,8 @@ class EmbedderTestContextGL : public EmbedderTestContext { /// void SetGLGetFBOCallback(GLGetFBOCallback callback); + void SetGLGetFBOWithDamageCallback(GLGetFBOWithDamageCallback callback); + uint32_t GetWindowFBOId() const; //---------------------------------------------------------------------------- @@ -54,6 +58,8 @@ class EmbedderTestContextGL : public EmbedderTestContext { /// void SetGLPresentCallback(GLPresentCallback callback); + void GLGetFBOWithDamage(const intptr_t id, FlutterDamage* existing_damage); + protected: virtual void SetupCompositor() override; @@ -66,6 +72,7 @@ class EmbedderTestContextGL : public EmbedderTestContext { std::mutex gl_callback_mutex_; GLGetFBOCallback gl_get_fbo_callback_; GLPresentCallback gl_present_callback_; + GLGetFBOWithDamageCallback gl_get_fbo_with_damage_callback_; void SetupSurface(SkISize surface_size) override; From a865ab3fe3224f0b5d259607dd8b43ef82b99086 Mon Sep 17 00:00:00 2001 From: Bernardo Eilert Trevisan Date: Fri, 29 Jul 2022 18:36:45 -0700 Subject: [PATCH 17/34] Add unittests to check valid fbo_with_damage callback. --- .../embedder/tests/embedder_unittests_gl.cc | 69 ++++++++++++++++++- 1 file changed, 67 insertions(+), 2 deletions(-) diff --git a/shell/platform/embedder/tests/embedder_unittests_gl.cc b/shell/platform/embedder/tests/embedder_unittests_gl.cc index 9174461529058..1ca67c6e1575c 100644 --- a/shell/platform/embedder/tests/embedder_unittests_gl.cc +++ b/shell/platform/embedder/tests/embedder_unittests_gl.cc @@ -3106,6 +3106,71 @@ TEST_F(EmbedderTest, MustNotRunWithBothPresentCallbacksSet) { ASSERT_FALSE(engine.is_valid()); } +TEST_F(EmbedderTest, MustStillRunWhenFBOWithDamageIsNotProvided) { + auto& context = GetEmbedderContext(EmbedderTestContextType::kOpenGLContext); + + EmbedderConfigBuilder builder(context); + builder.SetOpenGLRendererConfig(SkISize::Make(1, 1)); + builder.GetRendererConfig().open_gl.fbo_with_damage_callback = nullptr; + + auto engine = builder.LaunchEngine(); + ASSERT_TRUE(engine.is_valid()); +} + +TEST_F(EmbedderTest, MustRunWhenFBOWithDamageIsProvided) { + auto& context = GetEmbedderContext(EmbedderTestContextType::kOpenGLContext); + + EmbedderConfigBuilder builder(context); + builder.SetOpenGLRendererConfig(SkISize::Make(1, 1)); + + builder.GetRendererConfig().open_gl.fbo_with_damage_callback = + [](void* context, const intptr_t id, + FlutterDamage* existing_damage) -> void { + return reinterpret_cast(context) + ->GLGetFBOWithDamage(id, existing_damage); + }; + + auto engine = builder.LaunchEngine(); + ASSERT_TRUE(engine.is_valid()); +} + +TEST_F(EmbedderTest, MustRunWithFBOWithDamageAndFBOCallback) { + auto& context = GetEmbedderContext(EmbedderTestContextType::kOpenGLContext); + + EmbedderConfigBuilder builder(context); + builder.SetOpenGLRendererConfig(SkISize::Make(1, 1)); + builder.GetRendererConfig().open_gl.fbo_callback = + [](void* context) -> uint32_t { return 0; }; + builder.GetRendererConfig().open_gl.fbo_with_frame_info_callback = nullptr; + builder.GetRendererConfig().open_gl.fbo_with_damage_callback = + [](void* context, const intptr_t id, + FlutterDamage* existing_damage) -> void { + return reinterpret_cast(context) + ->GLGetFBOWithDamage(id, existing_damage); + }; + + auto engine = builder.LaunchEngine(); + ASSERT_TRUE(engine.is_valid()); +} + +TEST_F(EmbedderTest, MustNotRunWhenFBOWithDamageButNoOtherFBOCallback) { + auto& context = GetEmbedderContext(EmbedderTestContextType::kOpenGLContext); + + EmbedderConfigBuilder builder(context); + builder.SetOpenGLRendererConfig(SkISize::Make(1, 1)); + builder.GetRendererConfig().open_gl.fbo_callback = nullptr; + builder.GetRendererConfig().open_gl.fbo_with_frame_info_callback = nullptr; + builder.GetRendererConfig().open_gl.fbo_with_damage_callback = + [](void* context, const intptr_t id, + FlutterDamage* existing_damage) -> void { + return reinterpret_cast(context) + ->GLGetFBOWithDamage(id, existing_damage); + }; + + auto engine = builder.LaunchEngine(); + ASSERT_FALSE(engine.is_valid()); +} + TEST_F(EmbedderTest, PresentInfoContainsValidFBOId) { auto& context = GetEmbedderContext(EmbedderTestContextType::kOpenGLContext); @@ -3140,8 +3205,8 @@ TEST_F(EmbedderTest, PresentInfoContainsValidFBOId) { const uint32_t window_fbo_id = static_cast(context).GetWindowFBOId(); static_cast(context).SetGLPresentCallback( - [window_fbo_id = window_fbo_id](uint32_t fbo_id) { - ASSERT_EQ(fbo_id, window_fbo_id); + [window_fbo_id = window_fbo_id](FlutterPresentInfo present_info) { + ASSERT_EQ(present_info.fbo_id, window_fbo_id); frame_latch.CountDown(); }); From 8f953710518da9e835270fd4e54ce01941b46697 Mon Sep 17 00:00:00 2001 From: Bernardo Eilert Trevisan Date: Fri, 29 Jul 2022 18:40:58 -0700 Subject: [PATCH 18/34] Add unittests for partial repaint. --- .../embedder/tests/embedder_unittests_gl.cc | 225 ++++++++++++++++++ 1 file changed, 225 insertions(+) diff --git a/shell/platform/embedder/tests/embedder_unittests_gl.cc b/shell/platform/embedder/tests/embedder_unittests_gl.cc index 1ca67c6e1575c..1e355fccaa811 100644 --- a/shell/platform/embedder/tests/embedder_unittests_gl.cc +++ b/shell/platform/embedder/tests/embedder_unittests_gl.cc @@ -3214,6 +3214,231 @@ TEST_F(EmbedderTest, PresentInfoContainsValidFBOId) { frame_latch.Wait(); } +TEST_F(EmbedderTest, + PresentInfoReceivesFullDamageWhenExistingDamageIsWholeScreen) { + auto& context = GetEmbedderContext(EmbedderTestContextType::kOpenGLContext); + + EmbedderConfigBuilder builder(context); + builder.SetOpenGLRendererConfig(SkISize::Make(800, 600)); + builder.SetDartEntrypoint("render_gradient"); + builder.GetRendererConfig().open_gl.fbo_with_damage_callback = + [](void* context, const intptr_t id, + FlutterDamage* existing_damage) -> void { + return reinterpret_cast(context) + ->GLGetFBOWithDamage(id, existing_damage); + }; + + // Return existing damage as the entire screen on purpose. + static_cast(context).SetGLGetFBOWithDamageCallback( + [](const intptr_t id, FlutterDamage* existing_damage_ptr) { + const size_t num_rects = 1; + FlutterRect existing_damage_rects[num_rects] = { + FlutterRect{0, 0, 800, 600}}; + existing_damage_ptr->num_rects = num_rects; + existing_damage_ptr->damage = existing_damage_rects; + }); + + auto engine = builder.LaunchEngine(); + ASSERT_TRUE(engine.is_valid()); + + // First frame should be entirely rerendered. + static_cast(context).SetGLPresentCallback( + [](FlutterPresentInfo present_info) { + const size_t num_rects = 1; + ASSERT_EQ(present_info.frame_damage.num_rects, num_rects); + ASSERT_EQ(present_info.frame_damage.damage->left, 0); + ASSERT_EQ(present_info.frame_damage.damage->top, 0); + ASSERT_EQ(present_info.frame_damage.damage->right, 800); + ASSERT_EQ(present_info.frame_damage.damage->bottom, 600); + + ASSERT_EQ(present_info.buffer_damage.num_rects, num_rects); + ASSERT_EQ(present_info.buffer_damage.damage->left, 0); + ASSERT_EQ(present_info.buffer_damage.damage->top, 0); + ASSERT_EQ(present_info.buffer_damage.damage->right, 800); + ASSERT_EQ(present_info.buffer_damage.damage->bottom, 600); + }); + + // 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); + + // Because it's the same as the first frame, the second frame damage should + // be empty but, because there was a full existing buffer damage, the buffer + // damage should be the entire screen. + static_cast(context).SetGLPresentCallback( + [](FlutterPresentInfo present_info) { + const size_t num_rects = 1; + ASSERT_EQ(present_info.frame_damage.num_rects, num_rects); + ASSERT_EQ(present_info.frame_damage.damage->left, 0); + ASSERT_EQ(present_info.frame_damage.damage->top, 0); + ASSERT_EQ(present_info.frame_damage.damage->right, 0); + ASSERT_EQ(present_info.frame_damage.damage->bottom, 0); + + ASSERT_EQ(present_info.buffer_damage.num_rects, num_rects); + ASSERT_EQ(present_info.buffer_damage.damage->left, 0); + ASSERT_EQ(present_info.buffer_damage.damage->top, 0); + ASSERT_EQ(present_info.buffer_damage.damage->right, 800); + ASSERT_EQ(present_info.buffer_damage.damage->bottom, 600); + }); + + ASSERT_EQ(FlutterEngineSendWindowMetricsEvent(engine.get(), &event), + kSuccess); +} + +TEST_F(EmbedderTest, PresentInfoReceivesEmptyDamage) { + auto& context = GetEmbedderContext(EmbedderTestContextType::kOpenGLContext); + + EmbedderConfigBuilder builder(context); + builder.SetOpenGLRendererConfig(SkISize::Make(800, 600)); + builder.SetDartEntrypoint("render_gradient"); + builder.GetRendererConfig().open_gl.fbo_with_damage_callback = + [](void* context, const intptr_t id, + FlutterDamage* existing_damage) -> void { + return reinterpret_cast(context) + ->GLGetFBOWithDamage(id, existing_damage); + }; + + // Return no existing damage on purpose. + static_cast(context).SetGLGetFBOWithDamageCallback( + [](const intptr_t id, FlutterDamage* existing_damage_ptr) { + const size_t num_rects = 1; + FlutterRect existing_damage_rects[num_rects] = { + FlutterRect{0, 0, 0, 0}}; + existing_damage_ptr->num_rects = num_rects; + existing_damage_ptr->damage = existing_damage_rects; + }); + + auto engine = builder.LaunchEngine(); + ASSERT_TRUE(engine.is_valid()); + + // First frame should be entirely rerendered. + static_cast(context).SetGLPresentCallback( + [](FlutterPresentInfo present_info) { + const size_t num_rects = 1; + ASSERT_EQ(present_info.frame_damage.num_rects, num_rects); + ASSERT_EQ(present_info.frame_damage.damage->left, 0); + ASSERT_EQ(present_info.frame_damage.damage->top, 0); + ASSERT_EQ(present_info.frame_damage.damage->right, 800); + ASSERT_EQ(present_info.frame_damage.damage->bottom, 600); + + ASSERT_EQ(present_info.buffer_damage.num_rects, num_rects); + ASSERT_EQ(present_info.buffer_damage.damage->left, 0); + ASSERT_EQ(present_info.buffer_damage.damage->top, 0); + ASSERT_EQ(present_info.buffer_damage.damage->right, 800); + ASSERT_EQ(present_info.buffer_damage.damage->bottom, 600); + }); + + // 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); + + // Because it's the same as the first frame, the second frame should not be + // rerendered assuming there is no existing damage. + static_cast(context).SetGLPresentCallback( + [](FlutterPresentInfo present_info) { + const size_t num_rects = 1; + ASSERT_EQ(present_info.frame_damage.num_rects, num_rects); + ASSERT_EQ(present_info.frame_damage.damage->left, 0); + ASSERT_EQ(present_info.frame_damage.damage->top, 0); + ASSERT_EQ(present_info.frame_damage.damage->right, 0); + ASSERT_EQ(present_info.frame_damage.damage->bottom, 0); + + ASSERT_EQ(present_info.buffer_damage.num_rects, num_rects); + ASSERT_EQ(present_info.buffer_damage.damage->left, 0); + ASSERT_EQ(present_info.buffer_damage.damage->top, 0); + ASSERT_EQ(present_info.buffer_damage.damage->right, 0); + ASSERT_EQ(present_info.buffer_damage.damage->bottom, 0); + }); + + ASSERT_EQ(FlutterEngineSendWindowMetricsEvent(engine.get(), &event), + kSuccess); +} + +TEST_F(EmbedderTest, PresentInfoReceivesPartialDamage) { + auto& context = GetEmbedderContext(EmbedderTestContextType::kOpenGLContext); + + EmbedderConfigBuilder builder(context); + builder.SetOpenGLRendererConfig(SkISize::Make(800, 600)); + builder.SetDartEntrypoint("render_gradient"); + builder.GetRendererConfig().open_gl.fbo_with_damage_callback = + [](void* context, const intptr_t id, + FlutterDamage* existing_damage) -> void { + return reinterpret_cast(context) + ->GLGetFBOWithDamage(id, existing_damage); + }; + + // Return existing damage as only part of the screen on purpose. + static_cast(context).SetGLGetFBOWithDamageCallback( + [](const intptr_t id, FlutterDamage* existing_damage_ptr) { + const size_t num_rects = 1; + FlutterRect existing_damage_rects[num_rects] = { + FlutterRect{200, 150, 400, 300}}; + existing_damage_ptr->num_rects = num_rects; + existing_damage_ptr->damage = existing_damage_rects; + }); + + auto engine = builder.LaunchEngine(); + ASSERT_TRUE(engine.is_valid()); + + // First frame should be entirely rerendered. + static_cast(context).SetGLPresentCallback( + [](FlutterPresentInfo present_info) { + const size_t num_rects = 1; + ASSERT_EQ(present_info.frame_damage.num_rects, num_rects); + ASSERT_EQ(present_info.frame_damage.damage->left, 0); + ASSERT_EQ(present_info.frame_damage.damage->top, 0); + ASSERT_EQ(present_info.frame_damage.damage->right, 800); + ASSERT_EQ(present_info.frame_damage.damage->bottom, 600); + + ASSERT_EQ(present_info.buffer_damage.num_rects, num_rects); + ASSERT_EQ(present_info.buffer_damage.damage->left, 0); + ASSERT_EQ(present_info.buffer_damage.damage->top, 0); + ASSERT_EQ(present_info.buffer_damage.damage->right, 800); + ASSERT_EQ(present_info.buffer_damage.damage->bottom, 600); + }); + + // 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); + + // Because it's the same as the first frame, the second frame damage should be + // empty but, because there was a partial existing damage, the buffer damage + // should represent that partial damage area. + static_cast(context).SetGLPresentCallback( + [](FlutterPresentInfo present_info) { + const size_t num_rects = 1; + ASSERT_EQ(present_info.frame_damage.num_rects, num_rects); + ASSERT_EQ(present_info.frame_damage.damage->left, 0); + ASSERT_EQ(present_info.frame_damage.damage->top, 0); + ASSERT_EQ(present_info.frame_damage.damage->right, 0); + ASSERT_EQ(present_info.frame_damage.damage->bottom, 0); + + ASSERT_EQ(present_info.buffer_damage.num_rects, num_rects); + ASSERT_EQ(present_info.buffer_damage.damage->left, 200); + ASSERT_EQ(present_info.buffer_damage.damage->top, 150); + ASSERT_EQ(present_info.buffer_damage.damage->right, 400); + ASSERT_EQ(present_info.buffer_damage.damage->bottom, 300); + }); + + ASSERT_EQ(FlutterEngineSendWindowMetricsEvent(engine.get(), &event), + kSuccess); +} + TEST_F(EmbedderTest, SetSingleDisplayConfigurationWithDisplayId) { auto& context = GetEmbedderContext(EmbedderTestContextType::kOpenGLContext); From 8fd3a9d70749f10b53667a5f7347e829f3d1664e Mon Sep 17 00:00:00 2001 From: Bernardo Eilert Trevisan Date: Fri, 29 Jul 2022 18:42:19 -0700 Subject: [PATCH 19/34] Add unittest to make sure fbo with damage receives the correct information. --- .../embedder/tests/embedder_unittests_gl.cc | 79 +++++++++++++++++++ 1 file changed, 79 insertions(+) diff --git a/shell/platform/embedder/tests/embedder_unittests_gl.cc b/shell/platform/embedder/tests/embedder_unittests_gl.cc index 1e355fccaa811..ee27d2262860c 100644 --- a/shell/platform/embedder/tests/embedder_unittests_gl.cc +++ b/shell/platform/embedder/tests/embedder_unittests_gl.cc @@ -3439,6 +3439,85 @@ TEST_F(EmbedderTest, PresentInfoReceivesPartialDamage) { kSuccess); } +TEST_F(EmbedderTest, FBOWithDamageReceivesValidID) { + auto& context = GetEmbedderContext(EmbedderTestContextType::kOpenGLContext); + + EmbedderConfigBuilder builder(context); + builder.SetOpenGLRendererConfig(SkISize::Make(800, 600)); + builder.SetDartEntrypoint("render_gradient"); + builder.GetRendererConfig().open_gl.fbo_with_damage_callback = + [](void* context, const intptr_t id, + FlutterDamage* existing_damage) -> void { + return reinterpret_cast(context) + ->GLGetFBOWithDamage(id, existing_damage); + }; + + auto engine = builder.LaunchEngine(); + ASSERT_TRUE(engine.is_valid()); + + const uint32_t window_fbo_id = + static_cast(context).GetWindowFBOId(); + static_cast(context).SetGLGetFBOWithDamageCallback( + [window_fbo_id = window_fbo_id](intptr_t id, + FlutterDamage* existing_damage) { + ASSERT_EQ(id, window_fbo_id); + }); + + // 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); +} + +TEST_F(EmbedderTest, FBOWithDamageReceivesInvalidID) { + auto& context = GetEmbedderContext(EmbedderTestContextType::kOpenGLContext); + + EmbedderConfigBuilder builder(context); + builder.SetOpenGLRendererConfig(SkISize::Make(800, 600)); + builder.SetDartEntrypoint("render_gradient"); + builder.GetRendererConfig().open_gl.fbo_with_damage_callback = + [](void* context, const intptr_t id, + FlutterDamage* existing_damage) -> void { + return reinterpret_cast(context) + ->GLGetFBOWithDamage(id, existing_damage); + }; + + // Return a bad FBO ID on purpose. + builder.GetRendererConfig().open_gl.fbo_with_frame_info_callback = + [](void* context, const FlutterFrameInfo* frame_info) -> uint32_t { + return 123; + }; + + auto engine = builder.LaunchEngine(); + ASSERT_TRUE(engine.is_valid()); + + context.AddNativeCallback("SignalNativeTest", + CREATE_NATIVE_ENTRY([&](Dart_NativeArguments args) { + /* Nothing to do. */ + })); + + const uint32_t window_fbo_id = + static_cast(context).GetWindowFBOId(); + static_cast(context).SetGLGetFBOWithDamageCallback( + [window_fbo_id = window_fbo_id](intptr_t id, + FlutterDamage* existing_damage) { + ASSERT_NE(id, window_fbo_id); + }); + + // 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); +} + TEST_F(EmbedderTest, SetSingleDisplayConfigurationWithDisplayId) { auto& context = GetEmbedderContext(EmbedderTestContextType::kOpenGLContext); From ae1481f040ed182efe809373af70faec2124cb0e Mon Sep 17 00:00:00 2001 From: Bernardo Eilert Trevisan Date: Fri, 29 Jul 2022 18:43:16 -0700 Subject: [PATCH 20/34] Add log message if the user did not define an fbo_with_damage callback. --- shell/platform/embedder/embedder.cc | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/shell/platform/embedder/embedder.cc b/shell/platform/embedder/embedder.cc index c733c1a44ec92..4b6daa56a6c2f 100644 --- a/shell/platform/embedder/embedder.cc +++ b/shell/platform/embedder/embedder.cc @@ -132,6 +132,12 @@ static bool IsOpenGLRendererConfigValid(const FlutterRendererConfig* config) { return false; } + if (!SAFE_EXISTS(open_gl_config, fbo_with_damage_callback)) { + FML_LOG(INFO) << "fbo_with_damage_callback was not defined, disabling " + "partial repaint. If you wish to enable partial repaint, " + "please define this callback."; + } + return true; } From e6e115479ad4a7d2ed232c510f85d4d87d120544 Mon Sep 17 00:00:00 2001 From: Bernardo Eilert Trevisan Date: Fri, 29 Jul 2022 18:43:44 -0700 Subject: [PATCH 21/34] Add auxiliar functions. --- shell/platform/embedder/embedder.cc | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/shell/platform/embedder/embedder.cc b/shell/platform/embedder/embedder.cc index 4b6daa56a6c2f..8e5d61261bfad 100644 --- a/shell/platform/embedder/embedder.cc +++ b/shell/platform/embedder/embedder.cc @@ -228,6 +228,26 @@ static void* DefaultGLProcResolver(const char* name) { } #endif // FML_OS_LINUX || FML_OS_WIN +// Auxiliar function used to translate rectangles of type SkIRect to +// FlutterRect. +FlutterRect SkIRectToFlutterRect(const SkIRect sk_rect) { + FlutterRect flutter_rect = {static_cast(sk_rect.fLeft), + static_cast(sk_rect.fTop), + static_cast(sk_rect.fRight), + static_cast(sk_rect.fBottom)}; + return flutter_rect; +} + +// Auxiliar function used to translate rectangles of type FlutterRect to +// SkIRect. +const SkIRect FlutterRectToSkIRect(FlutterRect flutter_rect) { + SkIRect rect = {static_cast(flutter_rect.left), + static_cast(flutter_rect.top), + static_cast(flutter_rect.right), + static_cast(flutter_rect.bottom)}; + return rect; +} + static flutter::Shell::CreateCallback InferOpenGLPlatformViewCreationCallback( const FlutterRendererConfig* config, From f4d5043fb535dfe571be10cffe76dd46795b9a2f Mon Sep 17 00:00:00 2001 From: Bernardo Eilert Trevisan Date: Fri, 29 Jul 2022 18:44:53 -0700 Subject: [PATCH 22/34] Update behavior of present callback within the embedder. --- shell/platform/embedder/embedder.cc | 42 ++++++++++++++++++++++++----- 1 file changed, 36 insertions(+), 6 deletions(-) diff --git a/shell/platform/embedder/embedder.cc b/shell/platform/embedder/embedder.cc index 8e5d61261bfad..557c8fb8ca8be 100644 --- a/shell/platform/embedder/embedder.cc +++ b/shell/platform/embedder/embedder.cc @@ -267,15 +267,45 @@ InferOpenGLPlatformViewCreationCallback( auto gl_clear_current = [ptr = config->open_gl.clear_current, user_data]() -> bool { return ptr(user_data); }; - auto gl_present = [present = config->open_gl.present, - present_with_info = config->open_gl.present_with_info, - user_data](uint32_t fbo_id) -> bool { + auto gl_present = + [present = config->open_gl.present, + present_with_info = config->open_gl.present_with_info, + user_data](flutter::GLPresentInfo gl_present_info) -> bool { if (present) { return present(user_data); } else { - FlutterPresentInfo present_info = {}; - present_info.struct_size = sizeof(FlutterPresentInfo); - present_info.fbo_id = fbo_id; + // Format the frame and buffer damages accordingly. Note that, since the + // current compute damage algorithm only returns one rectangle for damage + // we are assuming the number of rectangles provided in frame and buffer + // damage are always 1. Once the function that computes damage implements + // support for multiple damage rectangles, GLPresentInfo should also + // contain the number of damage rectangles. + const size_t num_rects = 1; + + FlutterRect frame_damage_rect[num_rects] = { + SkIRectToFlutterRect(*(gl_present_info.frame_damage))}; + FlutterRect buffer_damage_rect[num_rects] = { + SkIRectToFlutterRect(*(gl_present_info.buffer_damage))}; + + FlutterDamage frame_damage{ + .struct_size = sizeof(FlutterDamage), + .num_rects = num_rects, + .damage = frame_damage_rect, + }; + FlutterDamage buffer_damage{ + .struct_size = sizeof(FlutterDamage), + .num_rects = num_rects, + .damage = buffer_damage_rect, + }; + + // Construct the present information concerning the frame being rendered. + FlutterPresentInfo present_info = { + .struct_size = sizeof(FlutterPresentInfo), + .fbo_id = gl_present_info.fbo_id, + .frame_damage = frame_damage, + .buffer_damage = buffer_damage, + }; + return present_with_info(user_data, &present_info); } }; From c2ce2af47aee69093fd4b4da83770b8a039ee96f Mon Sep 17 00:00:00 2001 From: Bernardo Eilert Trevisan Date: Fri, 29 Jul 2022 18:47:03 -0700 Subject: [PATCH 23/34] Update behavior of fbo with damage callback within the embedder. --- .../android/surface/android_surface_mock.cc | 1 + shell/platform/embedder/embedder.cc | 42 +++++++++++++++++++ 2 files changed, 43 insertions(+) diff --git a/shell/platform/android/surface/android_surface_mock.cc b/shell/platform/android/surface/android_surface_mock.cc index 9dd367e285d46..8f718fcae667a 100644 --- a/shell/platform/android/surface/android_surface_mock.cc +++ b/shell/platform/android/surface/android_surface_mock.cc @@ -26,5 +26,6 @@ GLFBOInfo AndroidSurfaceMock::GLContextFBO(GLFrameInfo frame_info) const { return GLFBOInfo{ .fbo_id = 0, }; +} } // namespace flutter diff --git a/shell/platform/embedder/embedder.cc b/shell/platform/embedder/embedder.cc index 557c8fb8ca8be..accb9f26d7ee8 100644 --- a/shell/platform/embedder/embedder.cc +++ b/shell/platform/embedder/embedder.cc @@ -325,6 +325,47 @@ InferOpenGLPlatformViewCreationCallback( } }; + auto gl_fbo_with_damage_callback = + [fbo_with_damage_callback = config->open_gl.fbo_with_damage_callback, + user_data](intptr_t id) -> flutter::GLFBOInfo { + // If no fbo_with_damage_callback was provided, disable partial repaint. + if (!fbo_with_damage_callback) { + return flutter::GLFBOInfo{ + .fbo_id = static_cast(id), + .partial_repaint_enabled = false, + .existing_damage = SkIRect::MakeEmpty(), + }; + } + + // Given the FBO's ID, get its existing damage. + FlutterDamage existing_damage; + fbo_with_damage_callback(user_data, id, &existing_damage); + + SkIRect existing_damage_rect; + + // Verify that at least one damage rectangle was provided. + if (existing_damage.num_rects <= 0 || existing_damage.damage == nullptr) { + FML_LOG(INFO) << "No damage was provided. Setting the damage to an " + "empty rectangle."; + existing_damage_rect = SkIRect::MakeEmpty(); + } else if (existing_damage.num_rects > 1) { + // Log message notifying users that multi-damage is not yet available in + // case they try to make use of it. + FML_LOG(INFO) << "Damage with multiple rectangles not yet supported. " + "Setting first rectangle as default."; + existing_damage_rect = FlutterRectToSkIRect(*(existing_damage.damage)); + } else { + existing_damage_rect = FlutterRectToSkIRect(*(existing_damage.damage)); + } + + // Pass the information about this FBO to the rendering backend. + return flutter::GLFBOInfo{ + .fbo_id = static_cast(id), + .partial_repaint_enabled = true, + .existing_damage = existing_damage_rect, + }; + }; + const FlutterOpenGLRendererConfig* open_gl_config = &config->open_gl; std::function gl_make_resource_current_callback = nullptr; if (SAFE_ACCESS(open_gl_config, make_resource_current, nullptr) != nullptr) { @@ -382,6 +423,7 @@ InferOpenGLPlatformViewCreationCallback( gl_make_resource_current_callback, // gl_make_resource_current_callback gl_surface_transformation_callback, // gl_surface_transformation_callback gl_proc_resolver, // gl_proc_resolver + gl_fbo_with_damage_callback, // gl_fbo_with_damage_callback }; return fml::MakeCopyable( From ef0cc25b87c602f3e509c90b5f7dceb9d1db52b4 Mon Sep 17 00:00:00 2001 From: Bernardo Eilert Trevisan Date: Fri, 29 Jul 2022 18:47:48 -0700 Subject: [PATCH 24/34] Formatting. --- 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 accb9f26d7ee8..f620ecea6795d 100644 --- a/shell/platform/embedder/embedder.cc +++ b/shell/platform/embedder/embedder.cc @@ -305,7 +305,7 @@ InferOpenGLPlatformViewCreationCallback( .frame_damage = frame_damage, .buffer_damage = buffer_damage, }; - + return present_with_info(user_data, &present_info); } }; From 6a64daae49e2c16231c376597b743eb2dff3ff17 Mon Sep 17 00:00:00 2001 From: Bernardo Eilert Trevisan Date: Mon, 1 Aug 2022 09:26:06 -0700 Subject: [PATCH 25/34] Update the fbo_with_damage_callback. --- shell/platform/embedder/embedder.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/shell/platform/embedder/embedder.h b/shell/platform/embedder/embedder.h index 0c091429b47e0..84e6547a977e7 100644 --- a/shell/platform/embedder/embedder.h +++ b/shell/platform/embedder/embedder.h @@ -463,8 +463,8 @@ typedef uint32_t (*UIntFrameInfoCallback)( /// information for partial repaint. typedef void (*FlutterFrameBufferWithDamageCallback)( void* /* user data */, - const intptr_t id, - FlutterDamage* existing_damage); + const intptr_t /* fbo id */, + FlutterDamage* /* existing damage */); /// This information is passed to the embedder when a surface is presented. /// From 931e53ebf091414f0fd7ac4137ea9b555e40373c Mon Sep 17 00:00:00 2001 From: Bernardo Eilert Trevisan Date: Mon, 1 Aug 2022 09:43:45 -0700 Subject: [PATCH 26/34] Documenting. --- shell/platform/embedder/embedder.h | 28 +++++++++++++++++++++++----- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/shell/platform/embedder/embedder.h b/shell/platform/embedder/embedder.h index 84e6547a977e7..e8a3b7b8fdf85 100644 --- a/shell/platform/embedder/embedder.h +++ b/shell/platform/embedder/embedder.h @@ -492,7 +492,10 @@ typedef struct { BoolCallback clear_current; /// Specifying one (and only one) of `present` or `present_with_info` is /// required. Specifying both is an error and engine initialization will be - /// terminated. The return value indicates success of the present call. + /// terminated. The return value indicates success of the present call. If + /// the intent is to use dirty region management, present_with_info must be + /// defined as present will not succeed in communicating information about + /// damage. BoolCallback present; /// Specifying one (and only one) of the `fbo_callback` or /// `fbo_with_frame_info_callback` is required. Specifying both is an error @@ -541,11 +544,26 @@ typedef struct { /// required. Specifying both is an error and engine initialization will be /// terminated. When using this variant, the embedder is passed a /// `FlutterPresentInfo` struct that the embedder can use to release any - /// resources. The return value indicates success of the present call. + /// resources. The return value indicates success of the present call. This + /// callback is essential for dirty region management. If not defined, all the + /// pixels on the screen will be rendered at every frame (regardless of + /// whether damage is actually being computed or not). This is because the + /// information that is passed along to the callback contains the frame and + /// buffer damage that are essential for dirty region management. BoolPresentInfoCallback present_with_info; - /// Specifying this callback is a requirement for dirty region management. The - /// callback will return a FlutterFrameBuffer containing information about the - /// buffer whose ID was passed to it. + /// Specifying this callback is a requirement for dirty region management. + /// Dirty region management will only render the areas of the screen that have + /// changed in between frames, greatly reducing rendering times and energy + /// consumption. To take advantage of these benefits, it is necessary to + /// define fbo_with_damage_callback as a callback that takes user data, an FBO + /// ID, and an existing damage FlutterDamage. The callback should use the + /// given FBO ID to identify the FBO's exisiting damage (i.e. areas that have + /// changed since the FBO was last used) and use it to populate the given + /// existing damage variable. This callback is dependent on either + /// fbo_callback or fbo_with_frame_info_callback being defined as they are + /// responsible for providing fbo_with_damage_callback with the FBO's ID. Not + /// specifying fbo_with_damage_callback will result in full repaint (i.e. + /// rendering all the pixels on the screen at every frame). FlutterFrameBufferWithDamageCallback fbo_with_damage_callback; } FlutterOpenGLRendererConfig; From 3f974497b0256a888674ee1260e5d8bea49d31cd Mon Sep 17 00:00:00 2001 From: Bernardo Eilert Trevisan Date: Mon, 1 Aug 2022 09:47:15 -0700 Subject: [PATCH 27/34] Update auxiliary functions as static functions. --- shell/platform/embedder/embedder.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/shell/platform/embedder/embedder.cc b/shell/platform/embedder/embedder.cc index f620ecea6795d..da538e6bd18a4 100644 --- a/shell/platform/embedder/embedder.cc +++ b/shell/platform/embedder/embedder.cc @@ -228,9 +228,9 @@ static void* DefaultGLProcResolver(const char* name) { } #endif // FML_OS_LINUX || FML_OS_WIN -// Auxiliar function used to translate rectangles of type SkIRect to +// Auxiliary function used to translate rectangles of type SkIRect to // FlutterRect. -FlutterRect SkIRectToFlutterRect(const SkIRect sk_rect) { +static FlutterRect SkIRectToFlutterRect(const SkIRect sk_rect) { FlutterRect flutter_rect = {static_cast(sk_rect.fLeft), static_cast(sk_rect.fTop), static_cast(sk_rect.fRight), @@ -238,9 +238,9 @@ FlutterRect SkIRectToFlutterRect(const SkIRect sk_rect) { return flutter_rect; } -// Auxiliar function used to translate rectangles of type FlutterRect to +// Auxiliary function used to translate rectangles of type FlutterRect to // SkIRect. -const SkIRect FlutterRectToSkIRect(FlutterRect flutter_rect) { +static const SkIRect FlutterRectToSkIRect(FlutterRect flutter_rect) { SkIRect rect = {static_cast(flutter_rect.left), static_cast(flutter_rect.top), static_cast(flutter_rect.right), From 1092807cedb5af4391b2271b0e8c5e42ac3effe0 Mon Sep 17 00:00:00 2001 From: Bernardo Eilert Trevisan Date: Mon, 1 Aug 2022 09:56:34 -0700 Subject: [PATCH 28/34] Force full repaint when the user tries to do partial repaint with multiple damage rectangles. --- shell/platform/embedder/embedder.cc | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/shell/platform/embedder/embedder.cc b/shell/platform/embedder/embedder.cc index da538e6bd18a4..136637dc22041 100644 --- a/shell/platform/embedder/embedder.cc +++ b/shell/platform/embedder/embedder.cc @@ -341,6 +341,7 @@ InferOpenGLPlatformViewCreationCallback( FlutterDamage existing_damage; fbo_with_damage_callback(user_data, id, &existing_damage); + bool partial_repaint_enabled = true; SkIRect existing_damage_rect; // Verify that at least one damage rectangle was provided. @@ -352,8 +353,9 @@ InferOpenGLPlatformViewCreationCallback( // Log message notifying users that multi-damage is not yet available in // case they try to make use of it. FML_LOG(INFO) << "Damage with multiple rectangles not yet supported. " - "Setting first rectangle as default."; - existing_damage_rect = FlutterRectToSkIRect(*(existing_damage.damage)); + "Repainting the whole frame."; + existing_damage_rect = SkIRect::MakeEmpty(); + partial_repaint_enabled = false; } else { existing_damage_rect = FlutterRectToSkIRect(*(existing_damage.damage)); } @@ -361,7 +363,7 @@ InferOpenGLPlatformViewCreationCallback( // Pass the information about this FBO to the rendering backend. return flutter::GLFBOInfo{ .fbo_id = static_cast(id), - .partial_repaint_enabled = true, + .partial_repaint_enabled = partial_repaint_enabled, .existing_damage = existing_damage_rect, }; }; From ecec224e4b24e5f416f3e5d3deb5677864f34a42 Mon Sep 17 00:00:00 2001 From: Bernardo Eilert Trevisan Date: Mon, 1 Aug 2022 10:06:56 -0700 Subject: [PATCH 29/34] Update construction of information passed to the present callback. --- shell/platform/embedder/embedder.cc | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/shell/platform/embedder/embedder.cc b/shell/platform/embedder/embedder.cc index 136637dc22041..3021eb437ed2b 100644 --- a/shell/platform/embedder/embedder.cc +++ b/shell/platform/embedder/embedder.cc @@ -282,20 +282,20 @@ InferOpenGLPlatformViewCreationCallback( // contain the number of damage rectangles. const size_t num_rects = 1; - FlutterRect frame_damage_rect[num_rects] = { + std::array frame_damage_rect = { SkIRectToFlutterRect(*(gl_present_info.frame_damage))}; - FlutterRect buffer_damage_rect[num_rects] = { + std::array buffer_damage_rect = { SkIRectToFlutterRect(*(gl_present_info.buffer_damage))}; FlutterDamage frame_damage{ .struct_size = sizeof(FlutterDamage), - .num_rects = num_rects, - .damage = frame_damage_rect, + .num_rects = frame_damage_rect.size(), + .damage = frame_damage_rect.data(), }; FlutterDamage buffer_damage{ .struct_size = sizeof(FlutterDamage), - .num_rects = num_rects, - .damage = buffer_damage_rect, + .num_rects = buffer_damage_rect.size(), + .damage = buffer_damage_rect.data(), }; // Construct the present information concerning the frame being rendered. From c1be60413947bc9bf9d766d024d94735ee1db401 Mon Sep 17 00:00:00 2001 From: Bernardo Eilert Trevisan Date: Mon, 1 Aug 2022 10:13:51 -0700 Subject: [PATCH 30/34] Fix static function warning. --- shell/platform/embedder/embedder.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/shell/platform/embedder/embedder.cc b/shell/platform/embedder/embedder.cc index 3021eb437ed2b..f181fb983ccdf 100644 --- a/shell/platform/embedder/embedder.cc +++ b/shell/platform/embedder/embedder.cc @@ -240,7 +240,7 @@ static FlutterRect SkIRectToFlutterRect(const SkIRect sk_rect) { // Auxiliary function used to translate rectangles of type FlutterRect to // SkIRect. -static const SkIRect FlutterRectToSkIRect(FlutterRect flutter_rect) { +static inline const SkIRect FlutterRectToSkIRect(FlutterRect flutter_rect) { SkIRect rect = {static_cast(flutter_rect.left), static_cast(flutter_rect.top), static_cast(flutter_rect.right), @@ -248,7 +248,7 @@ static const SkIRect FlutterRectToSkIRect(FlutterRect flutter_rect) { return rect; } -static flutter::Shell::CreateCallback +static inline flutter::Shell::CreateCallback InferOpenGLPlatformViewCreationCallback( const FlutterRendererConfig* config, void* user_data, @@ -282,9 +282,9 @@ InferOpenGLPlatformViewCreationCallback( // contain the number of damage rectangles. const size_t num_rects = 1; - std::array frame_damage_rect = { + std::array frame_damage_rect = { SkIRectToFlutterRect(*(gl_present_info.frame_damage))}; - std::array buffer_damage_rect = { + std::array buffer_damage_rect = { SkIRectToFlutterRect(*(gl_present_info.buffer_damage))}; FlutterDamage frame_damage{ From bde9b097dddb6f081c9e72b699ea627c4fb8f87a Mon Sep 17 00:00:00 2001 From: Bernardo Eilert Trevisan Date: Mon, 1 Aug 2022 11:10:51 -0700 Subject: [PATCH 31/34] Fix static function error. --- shell/platform/embedder/embedder.cc | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/shell/platform/embedder/embedder.cc b/shell/platform/embedder/embedder.cc index f181fb983ccdf..809045781fcfb 100644 --- a/shell/platform/embedder/embedder.cc +++ b/shell/platform/embedder/embedder.cc @@ -228,6 +228,7 @@ static void* DefaultGLProcResolver(const char* name) { } #endif // FML_OS_LINUX || FML_OS_WIN +#ifdef SHELL_ENABLE_GL // Auxiliary function used to translate rectangles of type SkIRect to // FlutterRect. static FlutterRect SkIRectToFlutterRect(const SkIRect sk_rect) { @@ -240,13 +241,14 @@ static FlutterRect SkIRectToFlutterRect(const SkIRect sk_rect) { // Auxiliary function used to translate rectangles of type FlutterRect to // SkIRect. -static inline const SkIRect FlutterRectToSkIRect(FlutterRect flutter_rect) { +static const SkIRect FlutterRectToSkIRect(FlutterRect flutter_rect) { SkIRect rect = {static_cast(flutter_rect.left), static_cast(flutter_rect.top), static_cast(flutter_rect.right), static_cast(flutter_rect.bottom)}; return rect; } +#endif static inline flutter::Shell::CreateCallback InferOpenGLPlatformViewCreationCallback( From 104b7caa735cf144c08fc00c52e2648f524435f9 Mon Sep 17 00:00:00 2001 From: Bernardo Eilert Trevisan Date: Mon, 1 Aug 2022 13:29:56 -0700 Subject: [PATCH 32/34] Rename fbo_with_damage_callback to populate_existing_damage_callback. --- shell/platform/embedder/embedder.cc | 23 ++-- shell/platform/embedder/embedder.h | 18 +-- .../platform/embedder/embedder_surface_gl.cc | 6 +- shell/platform/embedder/embedder_surface_gl.h | 7 +- .../embedder/tests/embedder_config_builder.cc | 2 +- .../tests/embedder_test_context_gl.cc | 15 ++- .../embedder/tests/embedder_test_context_gl.h | 10 +- .../embedder/tests/embedder_unittests_gl.cc | 121 +++++++++--------- 8 files changed, 108 insertions(+), 94 deletions(-) diff --git a/shell/platform/embedder/embedder.cc b/shell/platform/embedder/embedder.cc index 809045781fcfb..3b7194eec0d3d 100644 --- a/shell/platform/embedder/embedder.cc +++ b/shell/platform/embedder/embedder.cc @@ -132,10 +132,11 @@ static bool IsOpenGLRendererConfigValid(const FlutterRendererConfig* config) { return false; } - if (!SAFE_EXISTS(open_gl_config, fbo_with_damage_callback)) { - FML_LOG(INFO) << "fbo_with_damage_callback was not defined, disabling " - "partial repaint. If you wish to enable partial repaint, " - "please define this callback."; + if (!SAFE_EXISTS(open_gl_config, populate_existing_damage_callback)) { + FML_LOG(INFO) + << "populate_existing_damage_callback was not defined, disabling " + "partial repaint. If you wish to enable partial repaint, " + "please define this callback."; } return true; @@ -327,11 +328,13 @@ InferOpenGLPlatformViewCreationCallback( } }; - auto gl_fbo_with_damage_callback = - [fbo_with_damage_callback = config->open_gl.fbo_with_damage_callback, + auto gl_populate_existing_damage_callback = + [populate_existing_damage_callback = + config->open_gl.populate_existing_damage_callback, user_data](intptr_t id) -> flutter::GLFBOInfo { - // If no fbo_with_damage_callback was provided, disable partial repaint. - if (!fbo_with_damage_callback) { + // If no populate_existing_damage_callback was provided, disable partial + // repaint. + if (!populate_existing_damage_callback) { return flutter::GLFBOInfo{ .fbo_id = static_cast(id), .partial_repaint_enabled = false, @@ -341,7 +344,7 @@ InferOpenGLPlatformViewCreationCallback( // Given the FBO's ID, get its existing damage. FlutterDamage existing_damage; - fbo_with_damage_callback(user_data, id, &existing_damage); + populate_existing_damage_callback(user_data, id, &existing_damage); bool partial_repaint_enabled = true; SkIRect existing_damage_rect; @@ -427,7 +430,7 @@ InferOpenGLPlatformViewCreationCallback( gl_make_resource_current_callback, // gl_make_resource_current_callback gl_surface_transformation_callback, // gl_surface_transformation_callback gl_proc_resolver, // gl_proc_resolver - gl_fbo_with_damage_callback, // gl_fbo_with_damage_callback + gl_populate_existing_damage_callback, // gl_populate_existing_damage_callback }; return fml::MakeCopyable( diff --git a/shell/platform/embedder/embedder.h b/shell/platform/embedder/embedder.h index e8a3b7b8fdf85..8cddec272fcee 100644 --- a/shell/platform/embedder/embedder.h +++ b/shell/platform/embedder/embedder.h @@ -555,16 +555,16 @@ typedef struct { /// Dirty region management will only render the areas of the screen that have /// changed in between frames, greatly reducing rendering times and energy /// consumption. To take advantage of these benefits, it is necessary to - /// define fbo_with_damage_callback as a callback that takes user data, an FBO - /// ID, and an existing damage FlutterDamage. The callback should use the - /// given FBO ID to identify the FBO's exisiting damage (i.e. areas that have - /// changed since the FBO was last used) and use it to populate the given - /// existing damage variable. This callback is dependent on either + /// define populate_existing_damage_callback as a callback that takes user + /// data, an FBO ID, and an existing damage FlutterDamage. The callback should + /// use the given FBO ID to identify the FBO's exisiting damage (i.e. areas + /// that have changed since the FBO was last used) and use it to populate the + /// given existing damage variable. This callback is dependent on either /// fbo_callback or fbo_with_frame_info_callback being defined as they are - /// responsible for providing fbo_with_damage_callback with the FBO's ID. Not - /// specifying fbo_with_damage_callback will result in full repaint (i.e. - /// rendering all the pixels on the screen at every frame). - FlutterFrameBufferWithDamageCallback fbo_with_damage_callback; + /// responsible for providing populate_existing_damage_callback with the FBO's + /// ID. Not specifying populate_existing_damage_callback will result in full + /// repaint (i.e. rendering all the pixels on the screen at every frame). + FlutterFrameBufferWithDamageCallback populate_existing_damage_callback; } FlutterOpenGLRendererConfig; /// Alias for id. diff --git a/shell/platform/embedder/embedder_surface_gl.cc b/shell/platform/embedder/embedder_surface_gl.cc index 504ccf9316ce0..9ca71a5c0dc66 100644 --- a/shell/platform/embedder/embedder_surface_gl.cc +++ b/shell/platform/embedder/embedder_surface_gl.cc @@ -20,7 +20,7 @@ EmbedderSurfaceGL::EmbedderSurfaceGL( !gl_dispatch_table_.gl_clear_current_callback || !gl_dispatch_table_.gl_present_callback || !gl_dispatch_table_.gl_fbo_callback || - !gl_dispatch_table_.gl_fbo_with_damage_callback) { + !gl_dispatch_table_.gl_populate_existing_damage_callback) { return; } @@ -54,8 +54,8 @@ bool EmbedderSurfaceGL::GLContextPresent(const GLPresentInfo& present_info) { // |GPUSurfaceGLDelegate| GLFBOInfo EmbedderSurfaceGL::GLContextFBO(GLFrameInfo frame_info) const { // Get the FBO ID using the gl_fbo_callback and then get exiting damage by - // passing that ID to the gl_fbo_with_damage_callback. - return gl_dispatch_table_.gl_fbo_with_damage_callback( + // passing that ID to the gl_populate_existing_damage_callback. + return gl_dispatch_table_.gl_populate_existing_damage_callback( gl_dispatch_table_.gl_fbo_callback(frame_info)); } diff --git a/shell/platform/embedder/embedder_surface_gl.h b/shell/platform/embedder/embedder_surface_gl.h index 98d1cf975018c..5d4d72cfdb705 100644 --- a/shell/platform/embedder/embedder_surface_gl.h +++ b/shell/platform/embedder/embedder_surface_gl.h @@ -22,9 +22,10 @@ class EmbedderSurfaceGL final : public EmbedderSurface, std::function gl_fbo_callback; // required std::function gl_make_resource_current_callback; // optional std::function - gl_surface_transformation_callback; // optional - std::function gl_proc_resolver; // optional - std::function gl_fbo_with_damage_callback; // required + gl_surface_transformation_callback; // optional + std::function gl_proc_resolver; // optional + std::function + gl_populate_existing_damage_callback; // required }; EmbedderSurfaceGL( diff --git a/shell/platform/embedder/tests/embedder_config_builder.cc b/shell/platform/embedder/tests/embedder_config_builder.cc index 2fc8c2885018e..9b3f5592596e1 100644 --- a/shell/platform/embedder/tests/embedder_config_builder.cc +++ b/shell/platform/embedder/tests/embedder_config_builder.cc @@ -59,7 +59,7 @@ EmbedderConfigBuilder::EmbedderConfigBuilder( return reinterpret_cast(context)->GLGetFramebuffer( *frame_info); }; - opengl_renderer_config_.fbo_with_damage_callback = nullptr; + opengl_renderer_config_.populate_existing_damage_callback = nullptr; opengl_renderer_config_.make_resource_current = [](void* context) -> bool { return reinterpret_cast(context) ->GLMakeResourceCurrent(); diff --git a/shell/platform/embedder/tests/embedder_test_context_gl.cc b/shell/platform/embedder/tests/embedder_test_context_gl.cc index 63567a6f82410..0215999814a0d 100644 --- a/shell/platform/embedder/tests/embedder_test_context_gl.cc +++ b/shell/platform/embedder/tests/embedder_test_context_gl.cc @@ -64,10 +64,10 @@ void EmbedderTestContextGL::SetGLGetFBOCallback(GLGetFBOCallback callback) { gl_get_fbo_callback_ = callback; } -void EmbedderTestContextGL::SetGLGetFBOWithDamageCallback( - GLGetFBOWithDamageCallback callback) { +void EmbedderTestContextGL::SetGLPopulateExistingDamageCallback( + GLPopulateExistingDamageCallback callback) { std::scoped_lock lock(gl_callback_mutex_); - gl_get_fbo_with_damage_callback_ = callback; + gl_populate_existing_damage_callback_ = callback; } void EmbedderTestContextGL::SetGLPresentCallback(GLPresentCallback callback) { @@ -92,14 +92,15 @@ uint32_t EmbedderTestContextGL::GLGetFramebuffer(FlutterFrameInfo frame_info) { return gl_surface_->GetFramebuffer(size.width, size.height); } -void EmbedderTestContextGL::GLGetFBOWithDamage(const intptr_t id, - FlutterDamage* existing_damage) { +void EmbedderTestContextGL::GLPopulateExistingDamage( + const intptr_t id, + FlutterDamage* existing_damage) { FML_CHECK(gl_surface_) << "GL surface must be initialized."; - GLGetFBOWithDamageCallback callback; + GLPopulateExistingDamageCallback callback; { std::scoped_lock lock(gl_callback_mutex_); - callback = gl_get_fbo_with_damage_callback_; + callback = gl_populate_existing_damage_callback_; } if (callback) { diff --git a/shell/platform/embedder/tests/embedder_test_context_gl.h b/shell/platform/embedder/tests/embedder_test_context_gl.h index 720a9ed5d467b..9b91ef5868868 100644 --- a/shell/platform/embedder/tests/embedder_test_context_gl.h +++ b/shell/platform/embedder/tests/embedder_test_context_gl.h @@ -14,7 +14,7 @@ namespace testing { class EmbedderTestContextGL : public EmbedderTestContext { public: using GLGetFBOCallback = std::function; - using GLGetFBOWithDamageCallback = + using GLPopulateExistingDamageCallback = std::function; using GLPresentCallback = std::function; @@ -41,7 +41,8 @@ class EmbedderTestContextGL : public EmbedderTestContext { /// void SetGLGetFBOCallback(GLGetFBOCallback callback); - void SetGLGetFBOWithDamageCallback(GLGetFBOWithDamageCallback callback); + void SetGLPopulateExistingDamageCallback( + GLPopulateExistingDamageCallback callback); uint32_t GetWindowFBOId() const; @@ -58,7 +59,8 @@ class EmbedderTestContextGL : public EmbedderTestContext { /// void SetGLPresentCallback(GLPresentCallback callback); - void GLGetFBOWithDamage(const intptr_t id, FlutterDamage* existing_damage); + void GLPopulateExistingDamage(const intptr_t id, + FlutterDamage* existing_damage); protected: virtual void SetupCompositor() override; @@ -72,7 +74,7 @@ class EmbedderTestContextGL : public EmbedderTestContext { std::mutex gl_callback_mutex_; GLGetFBOCallback gl_get_fbo_callback_; GLPresentCallback gl_present_callback_; - GLGetFBOWithDamageCallback gl_get_fbo_with_damage_callback_; + GLPopulateExistingDamageCallback gl_populate_existing_damage_callback_; void SetupSurface(SkISize surface_size) override; diff --git a/shell/platform/embedder/tests/embedder_unittests_gl.cc b/shell/platform/embedder/tests/embedder_unittests_gl.cc index ee27d2262860c..3b5ecea2fb5ce 100644 --- a/shell/platform/embedder/tests/embedder_unittests_gl.cc +++ b/shell/platform/embedder/tests/embedder_unittests_gl.cc @@ -3106,35 +3106,36 @@ TEST_F(EmbedderTest, MustNotRunWithBothPresentCallbacksSet) { ASSERT_FALSE(engine.is_valid()); } -TEST_F(EmbedderTest, MustStillRunWhenFBOWithDamageIsNotProvided) { +TEST_F(EmbedderTest, MustStillRunWhenPopulateExistingDamageIsNotProvided) { auto& context = GetEmbedderContext(EmbedderTestContextType::kOpenGLContext); EmbedderConfigBuilder builder(context); builder.SetOpenGLRendererConfig(SkISize::Make(1, 1)); - builder.GetRendererConfig().open_gl.fbo_with_damage_callback = nullptr; + builder.GetRendererConfig().open_gl.populate_existing_damage_callback = + nullptr; auto engine = builder.LaunchEngine(); ASSERT_TRUE(engine.is_valid()); } -TEST_F(EmbedderTest, MustRunWhenFBOWithDamageIsProvided) { +TEST_F(EmbedderTest, MustRunWhenPopulateExistingDamageIsProvided) { auto& context = GetEmbedderContext(EmbedderTestContextType::kOpenGLContext); EmbedderConfigBuilder builder(context); builder.SetOpenGLRendererConfig(SkISize::Make(1, 1)); - builder.GetRendererConfig().open_gl.fbo_with_damage_callback = + builder.GetRendererConfig().open_gl.populate_existing_damage_callback = [](void* context, const intptr_t id, FlutterDamage* existing_damage) -> void { return reinterpret_cast(context) - ->GLGetFBOWithDamage(id, existing_damage); + ->GLPopulateExistingDamage(id, existing_damage); }; auto engine = builder.LaunchEngine(); ASSERT_TRUE(engine.is_valid()); } -TEST_F(EmbedderTest, MustRunWithFBOWithDamageAndFBOCallback) { +TEST_F(EmbedderTest, MustRunWithPopulateExistingDamageAndFBOCallback) { auto& context = GetEmbedderContext(EmbedderTestContextType::kOpenGLContext); EmbedderConfigBuilder builder(context); @@ -3142,29 +3143,30 @@ TEST_F(EmbedderTest, MustRunWithFBOWithDamageAndFBOCallback) { builder.GetRendererConfig().open_gl.fbo_callback = [](void* context) -> uint32_t { return 0; }; builder.GetRendererConfig().open_gl.fbo_with_frame_info_callback = nullptr; - builder.GetRendererConfig().open_gl.fbo_with_damage_callback = + builder.GetRendererConfig().open_gl.populate_existing_damage_callback = [](void* context, const intptr_t id, FlutterDamage* existing_damage) -> void { return reinterpret_cast(context) - ->GLGetFBOWithDamage(id, existing_damage); + ->GLPopulateExistingDamage(id, existing_damage); }; auto engine = builder.LaunchEngine(); ASSERT_TRUE(engine.is_valid()); } -TEST_F(EmbedderTest, MustNotRunWhenFBOWithDamageButNoOtherFBOCallback) { +TEST_F(EmbedderTest, + MustNotRunWhenPopulateExistingDamageButNoOtherFBOCallback) { auto& context = GetEmbedderContext(EmbedderTestContextType::kOpenGLContext); EmbedderConfigBuilder builder(context); builder.SetOpenGLRendererConfig(SkISize::Make(1, 1)); builder.GetRendererConfig().open_gl.fbo_callback = nullptr; builder.GetRendererConfig().open_gl.fbo_with_frame_info_callback = nullptr; - builder.GetRendererConfig().open_gl.fbo_with_damage_callback = + builder.GetRendererConfig().open_gl.populate_existing_damage_callback = [](void* context, const intptr_t id, FlutterDamage* existing_damage) -> void { return reinterpret_cast(context) - ->GLGetFBOWithDamage(id, existing_damage); + ->GLPopulateExistingDamage(id, existing_damage); }; auto engine = builder.LaunchEngine(); @@ -3221,22 +3223,23 @@ TEST_F(EmbedderTest, EmbedderConfigBuilder builder(context); builder.SetOpenGLRendererConfig(SkISize::Make(800, 600)); builder.SetDartEntrypoint("render_gradient"); - builder.GetRendererConfig().open_gl.fbo_with_damage_callback = + builder.GetRendererConfig().open_gl.populate_existing_damage_callback = [](void* context, const intptr_t id, FlutterDamage* existing_damage) -> void { return reinterpret_cast(context) - ->GLGetFBOWithDamage(id, existing_damage); + ->GLPopulateExistingDamage(id, existing_damage); }; // Return existing damage as the entire screen on purpose. - static_cast(context).SetGLGetFBOWithDamageCallback( - [](const intptr_t id, FlutterDamage* existing_damage_ptr) { - const size_t num_rects = 1; - FlutterRect existing_damage_rects[num_rects] = { - FlutterRect{0, 0, 800, 600}}; - existing_damage_ptr->num_rects = num_rects; - existing_damage_ptr->damage = existing_damage_rects; - }); + static_cast(context) + .SetGLPopulateExistingDamageCallback( + [](const intptr_t id, FlutterDamage* existing_damage_ptr) { + const size_t num_rects = 1; + FlutterRect existing_damage_rects[num_rects] = { + FlutterRect{0, 0, 800, 600}}; + existing_damage_ptr->num_rects = num_rects; + existing_damage_ptr->damage = existing_damage_rects; + }); auto engine = builder.LaunchEngine(); ASSERT_TRUE(engine.is_valid()); @@ -3296,22 +3299,23 @@ TEST_F(EmbedderTest, PresentInfoReceivesEmptyDamage) { EmbedderConfigBuilder builder(context); builder.SetOpenGLRendererConfig(SkISize::Make(800, 600)); builder.SetDartEntrypoint("render_gradient"); - builder.GetRendererConfig().open_gl.fbo_with_damage_callback = + builder.GetRendererConfig().open_gl.populate_existing_damage_callback = [](void* context, const intptr_t id, FlutterDamage* existing_damage) -> void { return reinterpret_cast(context) - ->GLGetFBOWithDamage(id, existing_damage); + ->GLPopulateExistingDamage(id, existing_damage); }; // Return no existing damage on purpose. - static_cast(context).SetGLGetFBOWithDamageCallback( - [](const intptr_t id, FlutterDamage* existing_damage_ptr) { - const size_t num_rects = 1; - FlutterRect existing_damage_rects[num_rects] = { - FlutterRect{0, 0, 0, 0}}; - existing_damage_ptr->num_rects = num_rects; - existing_damage_ptr->damage = existing_damage_rects; - }); + static_cast(context) + .SetGLPopulateExistingDamageCallback( + [](const intptr_t id, FlutterDamage* existing_damage_ptr) { + const size_t num_rects = 1; + FlutterRect existing_damage_rects[num_rects] = { + FlutterRect{0, 0, 0, 0}}; + existing_damage_ptr->num_rects = num_rects; + existing_damage_ptr->damage = existing_damage_rects; + }); auto engine = builder.LaunchEngine(); ASSERT_TRUE(engine.is_valid()); @@ -3370,22 +3374,23 @@ TEST_F(EmbedderTest, PresentInfoReceivesPartialDamage) { EmbedderConfigBuilder builder(context); builder.SetOpenGLRendererConfig(SkISize::Make(800, 600)); builder.SetDartEntrypoint("render_gradient"); - builder.GetRendererConfig().open_gl.fbo_with_damage_callback = + builder.GetRendererConfig().open_gl.populate_existing_damage_callback = [](void* context, const intptr_t id, FlutterDamage* existing_damage) -> void { return reinterpret_cast(context) - ->GLGetFBOWithDamage(id, existing_damage); + ->GLPopulateExistingDamage(id, existing_damage); }; // Return existing damage as only part of the screen on purpose. - static_cast(context).SetGLGetFBOWithDamageCallback( - [](const intptr_t id, FlutterDamage* existing_damage_ptr) { - const size_t num_rects = 1; - FlutterRect existing_damage_rects[num_rects] = { - FlutterRect{200, 150, 400, 300}}; - existing_damage_ptr->num_rects = num_rects; - existing_damage_ptr->damage = existing_damage_rects; - }); + static_cast(context) + .SetGLPopulateExistingDamageCallback( + [](const intptr_t id, FlutterDamage* existing_damage_ptr) { + const size_t num_rects = 1; + FlutterRect existing_damage_rects[num_rects] = { + FlutterRect{200, 150, 400, 300}}; + existing_damage_ptr->num_rects = num_rects; + existing_damage_ptr->damage = existing_damage_rects; + }); auto engine = builder.LaunchEngine(); ASSERT_TRUE(engine.is_valid()); @@ -3439,17 +3444,17 @@ TEST_F(EmbedderTest, PresentInfoReceivesPartialDamage) { kSuccess); } -TEST_F(EmbedderTest, FBOWithDamageReceivesValidID) { +TEST_F(EmbedderTest, PopulateExistingDamageReceivesValidID) { auto& context = GetEmbedderContext(EmbedderTestContextType::kOpenGLContext); EmbedderConfigBuilder builder(context); builder.SetOpenGLRendererConfig(SkISize::Make(800, 600)); builder.SetDartEntrypoint("render_gradient"); - builder.GetRendererConfig().open_gl.fbo_with_damage_callback = + builder.GetRendererConfig().open_gl.populate_existing_damage_callback = [](void* context, const intptr_t id, FlutterDamage* existing_damage) -> void { return reinterpret_cast(context) - ->GLGetFBOWithDamage(id, existing_damage); + ->GLPopulateExistingDamage(id, existing_damage); }; auto engine = builder.LaunchEngine(); @@ -3457,11 +3462,12 @@ TEST_F(EmbedderTest, FBOWithDamageReceivesValidID) { const uint32_t window_fbo_id = static_cast(context).GetWindowFBOId(); - static_cast(context).SetGLGetFBOWithDamageCallback( - [window_fbo_id = window_fbo_id](intptr_t id, - FlutterDamage* existing_damage) { - ASSERT_EQ(id, window_fbo_id); - }); + static_cast(context) + .SetGLPopulateExistingDamageCallback( + [window_fbo_id = window_fbo_id](intptr_t id, + FlutterDamage* existing_damage) { + ASSERT_EQ(id, window_fbo_id); + }); // Send a window metrics events so frames may be scheduled. FlutterWindowMetricsEvent event = {}; @@ -3473,17 +3479,17 @@ TEST_F(EmbedderTest, FBOWithDamageReceivesValidID) { kSuccess); } -TEST_F(EmbedderTest, FBOWithDamageReceivesInvalidID) { +TEST_F(EmbedderTest, PopulateExistingDamageReceivesInvalidID) { auto& context = GetEmbedderContext(EmbedderTestContextType::kOpenGLContext); EmbedderConfigBuilder builder(context); builder.SetOpenGLRendererConfig(SkISize::Make(800, 600)); builder.SetDartEntrypoint("render_gradient"); - builder.GetRendererConfig().open_gl.fbo_with_damage_callback = + builder.GetRendererConfig().open_gl.populate_existing_damage_callback = [](void* context, const intptr_t id, FlutterDamage* existing_damage) -> void { return reinterpret_cast(context) - ->GLGetFBOWithDamage(id, existing_damage); + ->GLPopulateExistingDamage(id, existing_damage); }; // Return a bad FBO ID on purpose. @@ -3502,11 +3508,12 @@ TEST_F(EmbedderTest, FBOWithDamageReceivesInvalidID) { const uint32_t window_fbo_id = static_cast(context).GetWindowFBOId(); - static_cast(context).SetGLGetFBOWithDamageCallback( - [window_fbo_id = window_fbo_id](intptr_t id, - FlutterDamage* existing_damage) { - ASSERT_NE(id, window_fbo_id); - }); + static_cast(context) + .SetGLPopulateExistingDamageCallback( + [window_fbo_id = window_fbo_id](intptr_t id, + FlutterDamage* existing_damage) { + ASSERT_NE(id, window_fbo_id); + }); // Send a window metrics events so frames may be scheduled. FlutterWindowMetricsEvent event = {}; From 73434c3128d623fc100aafcbb87fcba7efddc2e2 Mon Sep 17 00:00:00 2001 From: Bernardo Eilert Trevisan Date: Thu, 4 Aug 2022 11:56:33 -0700 Subject: [PATCH 33/34] Force full repaint when no existing damage rectangle was given --- 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 3b7194eec0d3d..e65380c749fa5 100644 --- a/shell/platform/embedder/embedder.cc +++ b/shell/platform/embedder/embedder.cc @@ -351,9 +351,9 @@ InferOpenGLPlatformViewCreationCallback( // Verify that at least one damage rectangle was provided. if (existing_damage.num_rects <= 0 || existing_damage.damage == nullptr) { - FML_LOG(INFO) << "No damage was provided. Setting the damage to an " - "empty rectangle."; + FML_LOG(INFO) << "No damage was provided. Forcing full repaint."; existing_damage_rect = SkIRect::MakeEmpty(); + partial_repaint_enabled = false; } else if (existing_damage.num_rects > 1) { // Log message notifying users that multi-damage is not yet available in // case they try to make use of it. From 0136a188b18f4ba88942c84a69f2663809304eea Mon Sep 17 00:00:00 2001 From: Bernardo Eilert Trevisan Date: Fri, 12 Aug 2022 10:02:47 -0700 Subject: [PATCH 34/34] Update the callback name. --- shell/platform/embedder/embedder.cc | 22 +++++++++---------- shell/platform/embedder/embedder.h | 8 +++---- .../platform/embedder/embedder_surface_gl.cc | 6 ++--- shell/platform/embedder/embedder_surface_gl.h | 7 +++--- .../embedder/tests/embedder_config_builder.cc | 2 +- .../embedder/tests/embedder_unittests_gl.cc | 19 ++++++++-------- 6 files changed, 30 insertions(+), 34 deletions(-) diff --git a/shell/platform/embedder/embedder.cc b/shell/platform/embedder/embedder.cc index e65380c749fa5..04895e9087842 100644 --- a/shell/platform/embedder/embedder.cc +++ b/shell/platform/embedder/embedder.cc @@ -132,11 +132,10 @@ static bool IsOpenGLRendererConfigValid(const FlutterRendererConfig* config) { return false; } - if (!SAFE_EXISTS(open_gl_config, populate_existing_damage_callback)) { - FML_LOG(INFO) - << "populate_existing_damage_callback was not defined, disabling " - "partial repaint. If you wish to enable partial repaint, " - "please define this callback."; + if (!SAFE_EXISTS(open_gl_config, populate_existing_damage)) { + FML_LOG(INFO) << "populate_existing_damage was not defined, disabling " + "partial repaint. If you wish to enable partial repaint, " + "please define this callback."; } return true; @@ -328,13 +327,12 @@ InferOpenGLPlatformViewCreationCallback( } }; - auto gl_populate_existing_damage_callback = - [populate_existing_damage_callback = - config->open_gl.populate_existing_damage_callback, + auto gl_populate_existing_damage = + [populate_existing_damage = config->open_gl.populate_existing_damage, user_data](intptr_t id) -> flutter::GLFBOInfo { - // If no populate_existing_damage_callback was provided, disable partial + // If no populate_existing_damage was provided, disable partial // repaint. - if (!populate_existing_damage_callback) { + if (!populate_existing_damage) { return flutter::GLFBOInfo{ .fbo_id = static_cast(id), .partial_repaint_enabled = false, @@ -344,7 +342,7 @@ InferOpenGLPlatformViewCreationCallback( // Given the FBO's ID, get its existing damage. FlutterDamage existing_damage; - populate_existing_damage_callback(user_data, id, &existing_damage); + populate_existing_damage(user_data, id, &existing_damage); bool partial_repaint_enabled = true; SkIRect existing_damage_rect; @@ -430,7 +428,7 @@ InferOpenGLPlatformViewCreationCallback( gl_make_resource_current_callback, // gl_make_resource_current_callback gl_surface_transformation_callback, // gl_surface_transformation_callback gl_proc_resolver, // gl_proc_resolver - gl_populate_existing_damage_callback, // gl_populate_existing_damage_callback + gl_populate_existing_damage, // gl_populate_existing_damage }; return fml::MakeCopyable( diff --git a/shell/platform/embedder/embedder.h b/shell/platform/embedder/embedder.h index 8cddec272fcee..f9aa11b0f877d 100644 --- a/shell/platform/embedder/embedder.h +++ b/shell/platform/embedder/embedder.h @@ -555,16 +555,16 @@ typedef struct { /// Dirty region management will only render the areas of the screen that have /// changed in between frames, greatly reducing rendering times and energy /// consumption. To take advantage of these benefits, it is necessary to - /// define populate_existing_damage_callback as a callback that takes user + /// define populate_existing_damage as a callback that takes user /// data, an FBO ID, and an existing damage FlutterDamage. The callback should /// use the given FBO ID to identify the FBO's exisiting damage (i.e. areas /// that have changed since the FBO was last used) and use it to populate the /// given existing damage variable. This callback is dependent on either /// fbo_callback or fbo_with_frame_info_callback being defined as they are - /// responsible for providing populate_existing_damage_callback with the FBO's - /// ID. Not specifying populate_existing_damage_callback will result in full + /// responsible for providing populate_existing_damage with the FBO's + /// ID. Not specifying populate_existing_damage will result in full /// repaint (i.e. rendering all the pixels on the screen at every frame). - FlutterFrameBufferWithDamageCallback populate_existing_damage_callback; + FlutterFrameBufferWithDamageCallback populate_existing_damage; } FlutterOpenGLRendererConfig; /// Alias for id. diff --git a/shell/platform/embedder/embedder_surface_gl.cc b/shell/platform/embedder/embedder_surface_gl.cc index 9ca71a5c0dc66..88cdb316c5e5b 100644 --- a/shell/platform/embedder/embedder_surface_gl.cc +++ b/shell/platform/embedder/embedder_surface_gl.cc @@ -20,7 +20,7 @@ EmbedderSurfaceGL::EmbedderSurfaceGL( !gl_dispatch_table_.gl_clear_current_callback || !gl_dispatch_table_.gl_present_callback || !gl_dispatch_table_.gl_fbo_callback || - !gl_dispatch_table_.gl_populate_existing_damage_callback) { + !gl_dispatch_table_.gl_populate_existing_damage) { return; } @@ -54,8 +54,8 @@ bool EmbedderSurfaceGL::GLContextPresent(const GLPresentInfo& present_info) { // |GPUSurfaceGLDelegate| GLFBOInfo EmbedderSurfaceGL::GLContextFBO(GLFrameInfo frame_info) const { // Get the FBO ID using the gl_fbo_callback and then get exiting damage by - // passing that ID to the gl_populate_existing_damage_callback. - return gl_dispatch_table_.gl_populate_existing_damage_callback( + // passing that ID to the gl_populate_existing_damage. + return gl_dispatch_table_.gl_populate_existing_damage( gl_dispatch_table_.gl_fbo_callback(frame_info)); } diff --git a/shell/platform/embedder/embedder_surface_gl.h b/shell/platform/embedder/embedder_surface_gl.h index 5d4d72cfdb705..583f0f469a0e7 100644 --- a/shell/platform/embedder/embedder_surface_gl.h +++ b/shell/platform/embedder/embedder_surface_gl.h @@ -22,10 +22,9 @@ class EmbedderSurfaceGL final : public EmbedderSurface, std::function gl_fbo_callback; // required std::function gl_make_resource_current_callback; // optional std::function - gl_surface_transformation_callback; // optional - std::function gl_proc_resolver; // optional - std::function - gl_populate_existing_damage_callback; // required + gl_surface_transformation_callback; // optional + std::function gl_proc_resolver; // optional + std::function gl_populate_existing_damage; // required }; EmbedderSurfaceGL( diff --git a/shell/platform/embedder/tests/embedder_config_builder.cc b/shell/platform/embedder/tests/embedder_config_builder.cc index 9b3f5592596e1..58e48c4b594d8 100644 --- a/shell/platform/embedder/tests/embedder_config_builder.cc +++ b/shell/platform/embedder/tests/embedder_config_builder.cc @@ -59,7 +59,7 @@ EmbedderConfigBuilder::EmbedderConfigBuilder( return reinterpret_cast(context)->GLGetFramebuffer( *frame_info); }; - opengl_renderer_config_.populate_existing_damage_callback = nullptr; + opengl_renderer_config_.populate_existing_damage = nullptr; opengl_renderer_config_.make_resource_current = [](void* context) -> bool { return reinterpret_cast(context) ->GLMakeResourceCurrent(); diff --git a/shell/platform/embedder/tests/embedder_unittests_gl.cc b/shell/platform/embedder/tests/embedder_unittests_gl.cc index 3b5ecea2fb5ce..76cf6102cdf60 100644 --- a/shell/platform/embedder/tests/embedder_unittests_gl.cc +++ b/shell/platform/embedder/tests/embedder_unittests_gl.cc @@ -3111,8 +3111,7 @@ TEST_F(EmbedderTest, MustStillRunWhenPopulateExistingDamageIsNotProvided) { EmbedderConfigBuilder builder(context); builder.SetOpenGLRendererConfig(SkISize::Make(1, 1)); - builder.GetRendererConfig().open_gl.populate_existing_damage_callback = - nullptr; + builder.GetRendererConfig().open_gl.populate_existing_damage = nullptr; auto engine = builder.LaunchEngine(); ASSERT_TRUE(engine.is_valid()); @@ -3124,7 +3123,7 @@ TEST_F(EmbedderTest, MustRunWhenPopulateExistingDamageIsProvided) { EmbedderConfigBuilder builder(context); builder.SetOpenGLRendererConfig(SkISize::Make(1, 1)); - builder.GetRendererConfig().open_gl.populate_existing_damage_callback = + builder.GetRendererConfig().open_gl.populate_existing_damage = [](void* context, const intptr_t id, FlutterDamage* existing_damage) -> void { return reinterpret_cast(context) @@ -3143,7 +3142,7 @@ TEST_F(EmbedderTest, MustRunWithPopulateExistingDamageAndFBOCallback) { builder.GetRendererConfig().open_gl.fbo_callback = [](void* context) -> uint32_t { return 0; }; builder.GetRendererConfig().open_gl.fbo_with_frame_info_callback = nullptr; - builder.GetRendererConfig().open_gl.populate_existing_damage_callback = + builder.GetRendererConfig().open_gl.populate_existing_damage = [](void* context, const intptr_t id, FlutterDamage* existing_damage) -> void { return reinterpret_cast(context) @@ -3162,7 +3161,7 @@ TEST_F(EmbedderTest, builder.SetOpenGLRendererConfig(SkISize::Make(1, 1)); builder.GetRendererConfig().open_gl.fbo_callback = nullptr; builder.GetRendererConfig().open_gl.fbo_with_frame_info_callback = nullptr; - builder.GetRendererConfig().open_gl.populate_existing_damage_callback = + builder.GetRendererConfig().open_gl.populate_existing_damage = [](void* context, const intptr_t id, FlutterDamage* existing_damage) -> void { return reinterpret_cast(context) @@ -3223,7 +3222,7 @@ TEST_F(EmbedderTest, EmbedderConfigBuilder builder(context); builder.SetOpenGLRendererConfig(SkISize::Make(800, 600)); builder.SetDartEntrypoint("render_gradient"); - builder.GetRendererConfig().open_gl.populate_existing_damage_callback = + builder.GetRendererConfig().open_gl.populate_existing_damage = [](void* context, const intptr_t id, FlutterDamage* existing_damage) -> void { return reinterpret_cast(context) @@ -3299,7 +3298,7 @@ TEST_F(EmbedderTest, PresentInfoReceivesEmptyDamage) { EmbedderConfigBuilder builder(context); builder.SetOpenGLRendererConfig(SkISize::Make(800, 600)); builder.SetDartEntrypoint("render_gradient"); - builder.GetRendererConfig().open_gl.populate_existing_damage_callback = + builder.GetRendererConfig().open_gl.populate_existing_damage = [](void* context, const intptr_t id, FlutterDamage* existing_damage) -> void { return reinterpret_cast(context) @@ -3374,7 +3373,7 @@ TEST_F(EmbedderTest, PresentInfoReceivesPartialDamage) { EmbedderConfigBuilder builder(context); builder.SetOpenGLRendererConfig(SkISize::Make(800, 600)); builder.SetDartEntrypoint("render_gradient"); - builder.GetRendererConfig().open_gl.populate_existing_damage_callback = + builder.GetRendererConfig().open_gl.populate_existing_damage = [](void* context, const intptr_t id, FlutterDamage* existing_damage) -> void { return reinterpret_cast(context) @@ -3450,7 +3449,7 @@ TEST_F(EmbedderTest, PopulateExistingDamageReceivesValidID) { EmbedderConfigBuilder builder(context); builder.SetOpenGLRendererConfig(SkISize::Make(800, 600)); builder.SetDartEntrypoint("render_gradient"); - builder.GetRendererConfig().open_gl.populate_existing_damage_callback = + builder.GetRendererConfig().open_gl.populate_existing_damage = [](void* context, const intptr_t id, FlutterDamage* existing_damage) -> void { return reinterpret_cast(context) @@ -3485,7 +3484,7 @@ TEST_F(EmbedderTest, PopulateExistingDamageReceivesInvalidID) { EmbedderConfigBuilder builder(context); builder.SetOpenGLRendererConfig(SkISize::Make(800, 600)); builder.SetDartEntrypoint("render_gradient"); - builder.GetRendererConfig().open_gl.populate_existing_damage_callback = + builder.GetRendererConfig().open_gl.populate_existing_damage = [](void* context, const intptr_t id, FlutterDamage* existing_damage) -> void { return reinterpret_cast(context)