From ca272e68038d966a23a9876041ae4620434c87ff Mon Sep 17 00:00:00 2001 From: Bernardo Eilert Trevisan Date: Wed, 6 Jul 2022 20:28:46 +0000 Subject: [PATCH 1/5] Partial repaint prototype. --- shell/common/rasterizer.cc | 3 +++ shell/gpu/gpu_surface_gl_delegate.cc | 6 ++++++ shell/gpu/gpu_surface_gl_skia.cc | 9 ++++++++- shell/platform/embedder/embedder.cc | 6 ++++-- shell/platform/embedder/embedder.h | 3 +++ shell/platform/embedder/embedder_surface_gl.cc | 14 +++++++++++++- shell/platform/embedder/embedder_surface_gl.h | 5 ++++- 7 files changed, 41 insertions(+), 5 deletions(-) diff --git a/shell/common/rasterizer.cc b/shell/common/rasterizer.cc index 077bb33365824..bea12cbc2d2fa 100644 --- a/shell/common/rasterizer.cc +++ b/shell/common/rasterizer.cc @@ -571,12 +571,15 @@ RasterStatus Rasterizer::DrawToSurfaceUnsafe( std::unique_ptr damage; // when leaf layer tracing is enabled we wish to repaing the whole frame for // accurate performance metrics. + // TODO(btrevisan): make sure that leaf layer tracing is disabled when partial + // repaint is desired. if (frame->framebuffer_info().supports_partial_repaint && !layer_tree.is_leaf_layer_tracing_enabled()) { // Disable partial repaint if external_view_embedder_ SubmitFrame is // involved - ExternalViewEmbedder unconditionally clears the entire // surface and also partial repaint with platform view present is // something that still need to be figured out. + // TODO(btrevisan): make sure that no external_view_embedder_ exists. bool force_full_repaint = external_view_embedder_ && (!raster_thread_merger_ || raster_thread_merger_->IsMerged()); diff --git a/shell/gpu/gpu_surface_gl_delegate.cc b/shell/gpu/gpu_surface_gl_delegate.cc index 2aea45cf21d52..d4640a9b52b96 100644 --- a/shell/gpu/gpu_surface_gl_delegate.cc +++ b/shell/gpu/gpu_surface_gl_delegate.cc @@ -20,6 +20,12 @@ SurfaceFrame::FramebufferInfo GPUSurfaceGLDelegate::GLContextFramebufferInfo() const { SurfaceFrame::FramebufferInfo res; res.supports_readback = true; + // By default, partial repaint is enabled for every frame. + // TODO(btrevisan): Implement deactivation of partial repaint in cases that + // do not benefit from it (e.g. when damage takes over most of the frame). + res.supports_partial_repaint = true; + // TODO(btrevisan): make sure that when there is existing damage, it is set. + res.existing_damage = SkIRect::MakeEmpty(); return res; } diff --git a/shell/gpu/gpu_surface_gl_skia.cc b/shell/gpu/gpu_surface_gl_skia.cc index 60acb9d8b901e..7ad480b7d3076 100644 --- a/shell/gpu/gpu_surface_gl_skia.cc +++ b/shell/gpu/gpu_surface_gl_skia.cc @@ -247,6 +247,11 @@ std::unique_ptr GPUSurfaceGLSkia::AcquireFrame( return weak ? weak->PresentSurface(surface_frame, canvas) : false; }; + // TODO(btrevisan): Make sure framebuffer_info has the necesssary fields for + // partial repaint. + // TODO(btrevisan): framebuffer_info.supports_partial_repaint must be true. + // TODO(btrevisan): framebuffer_info.existing_damage must a valid SkIRect or + // an empty SkIRect (if there is no existing damage). framebuffer_info = delegate_->GLContextFramebufferInfo(); return std::make_unique(surface, std::move(framebuffer_info), submit_callback, @@ -268,7 +273,7 @@ bool GPUSurfaceGLSkia::PresentSurface(const SurfaceFrame& frame, GLPresentInfo present_info = { .fbo_id = fbo_id_, - .damage = frame.submit_info().frame_damage, + .damage = frame.submit_info().buffer_damage, .presentation_time = frame.submit_info().presentation_time, }; if (!delegate_->GLContextPresent(present_info)) { @@ -279,11 +284,13 @@ bool GPUSurfaceGLSkia::PresentSurface(const SurfaceFrame& frame, auto current_size = SkISize::Make(onscreen_surface_->width(), onscreen_surface_->height()); + // TODO(btrevisan): this needs to be updated to have the buffer damage. GLFrameInfo frame_info = {static_cast(current_size.width()), static_cast(current_size.height())}; // The FBO has changed, ask the delegate for the new FBO and do a surface // re-wrap. + // TODO(btrevisan): Make sure this returns the fbo_id as well as its damage. const uint32_t fbo_id = delegate_->GLContextFBO(frame_info); auto new_onscreen_surface = WrapOnscreenSurface(context_.get(), // GL context diff --git a/shell/platform/embedder/embedder.cc b/shell/platform/embedder/embedder.cc index 759f649f3c64b..9a6660ae71f75 100644 --- a/shell/platform/embedder/embedder.cc +++ b/shell/platform/embedder/embedder.cc @@ -242,13 +242,15 @@ InferOpenGLPlatformViewCreationCallback( auto gl_present = [present = config->open_gl.present, present_with_info = config->open_gl.present_with_info, - user_data](uint32_t fbo_id) -> bool { + 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; + present_info.fbo_id = gl_present_info.fbo_id; + // TODO(btrevisan): translate const std::optional to const FlutterRect *? + present_info.damage = gl_present_info.damage; return present_with_info(user_data, &present_info); } }; diff --git a/shell/platform/embedder/embedder.h b/shell/platform/embedder/embedder.h index 66a25c7f30c5b..6b3c2d8f73e3f 100644 --- a/shell/platform/embedder/embedder.h +++ b/shell/platform/embedder/embedder.h @@ -401,6 +401,9 @@ typedef struct { size_t struct_size; /// Id of the fbo backing the surface that was presented. uint32_t fbo_id; + // An array of rectangles representing the areas that need to be repainted + // in this buffer. + const FlutterRect* damage; } FlutterPresentInfo; /// Callback for when a surface is presented. diff --git a/shell/platform/embedder/embedder_surface_gl.cc b/shell/platform/embedder/embedder_surface_gl.cc index e406ee9ed10fa..d8186b53aa730 100644 --- a/shell/platform/embedder/embedder_surface_gl.cc +++ b/shell/platform/embedder/embedder_surface_gl.cc @@ -46,7 +46,7 @@ bool EmbedderSurfaceGL::GLContextClearCurrent() { // |GPUSurfaceGLDelegate| bool EmbedderSurfaceGL::GLContextPresent(const GLPresentInfo& present_info) { - return gl_dispatch_table_.gl_present_callback(present_info.fbo_id); + return gl_dispatch_table_.gl_present_callback(present_info); } // |GPUSurfaceGLDelegate| @@ -59,6 +59,18 @@ bool EmbedderSurfaceGL::GLContextFBOResetAfterPresent() const { return fbo_reset_after_present_; } +// |GPUSurfaceGLDelegate| +void EmbedderSurfaceGL::GLContextSetDamageRegion(const std::optional& region) { + // TODO(btrevisan): add checks. + + // The region given here is the buffer damage. + + // Update GL Context such that it is restricted to the buffer damage. + + + return; +} + // |GPUSurfaceGLDelegate| SkMatrix EmbedderSurfaceGL::GLContextSurfaceTransformation() const { auto callback = gl_dispatch_table_.gl_surface_transformation_callback; diff --git a/shell/platform/embedder/embedder_surface_gl.h b/shell/platform/embedder/embedder_surface_gl.h index c0a2b6a55d98f..09b52e3795bd9 100644 --- a/shell/platform/embedder/embedder_surface_gl.h +++ b/shell/platform/embedder/embedder_surface_gl.h @@ -18,7 +18,7 @@ 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 @@ -64,6 +64,9 @@ class EmbedderSurfaceGL final : public EmbedderSurface, // |GPUSurfaceGLDelegate| bool GLContextFBOResetAfterPresent() const override; + // |GPUSurfaceGLDelegate| + void GLContextSetDamageRegion(const std::optional& region) override; + // |GPUSurfaceGLDelegate| SkMatrix GLContextSurfaceTransformation() const override; From b172c9b3117c5385d9c1209612b08ca58cba4d41 Mon Sep 17 00:00:00 2001 From: Bernardo Eilert Trevisan Date: Mon, 11 Jul 2022 10:38:31 -0700 Subject: [PATCH 2/5] updates to prototype. --- Reload | 0 shell/common/rasterizer.cc | 3 --- shell/gpu/gpu_surface_gl_skia.cc | 9 +-------- shell/platform/embedder/embedder.cc | 10 ++++++++-- shell/platform/embedder/embedder.h | 14 +++++++++++--- shell/platform/embedder/embedder_surface_gl.cc | 14 +++++++++----- shell/platform/embedder/embedder_surface_gl.h | 1 + 7 files changed, 30 insertions(+), 21 deletions(-) create mode 100644 Reload diff --git a/Reload b/Reload new file mode 100644 index 0000000000000..e69de29bb2d1d diff --git a/shell/common/rasterizer.cc b/shell/common/rasterizer.cc index baad27bf3442c..8587d253d1f4d 100644 --- a/shell/common/rasterizer.cc +++ b/shell/common/rasterizer.cc @@ -643,15 +643,12 @@ RasterStatus Rasterizer::DrawToSurfaceUnsafe( std::unique_ptr damage; // when leaf layer tracing is enabled we wish to repaing the whole frame for // accurate performance metrics. - // TODO(btrevisan): make sure that leaf layer tracing is disabled when partial - // repaint is desired. if (frame->framebuffer_info().supports_partial_repaint && !layer_tree.is_leaf_layer_tracing_enabled()) { // Disable partial repaint if external_view_embedder_ SubmitFrame is // involved - ExternalViewEmbedder unconditionally clears the entire // surface and also partial repaint with platform view present is // something that still need to be figured out. - // TODO(btrevisan): make sure that no external_view_embedder_ exists. bool force_full_repaint = external_view_embedder_ && (!raster_thread_merger_ || raster_thread_merger_->IsMerged()); diff --git a/shell/gpu/gpu_surface_gl_skia.cc b/shell/gpu/gpu_surface_gl_skia.cc index eb0ba9ee49657..66bb6636d95d8 100644 --- a/shell/gpu/gpu_surface_gl_skia.cc +++ b/shell/gpu/gpu_surface_gl_skia.cc @@ -247,11 +247,6 @@ std::unique_ptr GPUSurfaceGLSkia::AcquireFrame( return weak ? weak->PresentSurface(surface_frame, canvas) : false; }; - // TODO(btrevisan): Make sure framebuffer_info has the necesssary fields for - // partial repaint. - // TODO(btrevisan): framebuffer_info.supports_partial_repaint must be true. - // TODO(btrevisan): framebuffer_info.existing_damage must a valid SkIRect or - // an empty SkIRect (if there is no existing damage). framebuffer_info = delegate_->GLContextFramebufferInfo(); return std::make_unique(surface, std::move(framebuffer_info), submit_callback, @@ -273,7 +268,7 @@ bool GPUSurfaceGLSkia::PresentSurface(const SurfaceFrame& frame, GLPresentInfo present_info = { .fbo_id = fbo_id_, - .damage = frame.submit_info().buffer_damage, + .damage = frame.submit_info().frame_damage, .presentation_time = frame.submit_info().presentation_time, }; if (!delegate_->GLContextPresent(present_info)) { @@ -284,13 +279,11 @@ bool GPUSurfaceGLSkia::PresentSurface(const SurfaceFrame& frame, auto current_size = SkISize::Make(onscreen_surface_->width(), onscreen_surface_->height()); - // TODO(btrevisan): this needs to be updated to have the buffer damage. GLFrameInfo frame_info = {static_cast(current_size.width()), static_cast(current_size.height())}; // The FBO has changed, ask the delegate for the new FBO and do a surface // re-wrap. - // TODO(btrevisan): Make sure this returns the fbo_id as well as its damage. const uint32_t fbo_id = delegate_->GLContextFBO(frame_info); auto new_onscreen_surface = WrapOnscreenSurface(context_.get(), // GL context diff --git a/shell/platform/embedder/embedder.cc b/shell/platform/embedder/embedder.cc index c67ecfac92f73..727cd3634e298 100644 --- a/shell/platform/embedder/embedder.cc +++ b/shell/platform/embedder/embedder.cc @@ -249,8 +249,14 @@ InferOpenGLPlatformViewCreationCallback( FlutterPresentInfo present_info = {}; present_info.struct_size = sizeof(FlutterPresentInfo); present_info.fbo_id = gl_present_info.fbo_id; - // TODO(btrevisan): translate const std::optional to const FlutterRect *? - present_info.damage = gl_present_info.damage; + + FlutterDamage fbo_damage = {}; + fbo_damage.struct_size = sizeof(FlutterDamage); + fbo_damage.damage = std::vector {FlutterRect{static_cast(gl_present_info.damage->fLeft), + static_cast(gl_present_info.damage->fRight), + static_cast(gl_present_info.damage->fTop), + static_cast(gl_present_info.damage->fBottom)}}; + present_info.fbo_damage = fbo_damage; return present_with_info(user_data, &present_info); } }; diff --git a/shell/platform/embedder/embedder.h b/shell/platform/embedder/embedder.h index 6b3c2d8f73e3f..cda8003f7fd69 100644 --- a/shell/platform/embedder/embedder.h +++ b/shell/platform/embedder/embedder.h @@ -5,6 +5,7 @@ #ifndef FLUTTER_EMBEDDER_H_ #define FLUTTER_EMBEDDER_H_ +#include #include #include #include @@ -377,6 +378,14 @@ typedef struct { FlutterSize lower_left_corner_radius; } FlutterRoundedRect; +/// A strucutre to represent damage as rectangles. +typedef struct { + /// The size of this struct. Must be sizeof(FlutterDamage). + size_t struct_size; + // Vector of rectangles representing the areas that need to be repainted. + std::vector damage; +} FlutterDamage; + /// This information is passed to the embedder when requesting a frame buffer /// object. /// @@ -401,9 +410,8 @@ typedef struct { size_t struct_size; /// Id of the fbo backing the surface that was presented. uint32_t fbo_id; - // An array of rectangles representing the areas that need to be repainted - // in this buffer. - const FlutterRect* damage; + // A vector of rectangles representing the areas that need to be repainted. + FlutterDamage fbo_damage; } FlutterPresentInfo; /// Callback for when a surface is presented. diff --git a/shell/platform/embedder/embedder_surface_gl.cc b/shell/platform/embedder/embedder_surface_gl.cc index d8186b53aa730..b58fb2312ed23 100644 --- a/shell/platform/embedder/embedder_surface_gl.cc +++ b/shell/platform/embedder/embedder_surface_gl.cc @@ -24,6 +24,13 @@ EmbedderSurfaceGL::EmbedderSurfaceGL( } valid_ = true; + + + + // if (HasExtension(extensions, "EGL_KHR_partial_update")) { + // set_damage_region_ = reinterpret_cast( + // eglGetProcAddress("eglSetDamageRegionKHR")); + // } } EmbedderSurfaceGL::~EmbedderSurfaceGL() = default; @@ -62,13 +69,10 @@ bool EmbedderSurfaceGL::GLContextFBOResetAfterPresent() const { // |GPUSurfaceGLDelegate| void EmbedderSurfaceGL::GLContextSetDamageRegion(const std::optional& region) { // TODO(btrevisan): add checks. + FML_DCHECK(IsValid()); // The region given here is the buffer damage. - - // Update GL Context such that it is restricted to the buffer damage. - - - return; + // onscreen_surface_->SetDamageRegion(region); } // |GPUSurfaceGLDelegate| diff --git a/shell/platform/embedder/embedder_surface_gl.h b/shell/platform/embedder/embedder_surface_gl.h index 09b52e3795bd9..d58dd748c760b 100644 --- a/shell/platform/embedder/embedder_surface_gl.h +++ b/shell/platform/embedder/embedder_surface_gl.h @@ -37,6 +37,7 @@ class EmbedderSurfaceGL final : public EmbedderSurface, bool valid_ = false; GLDispatchTable gl_dispatch_table_; bool fbo_reset_after_present_; + // auto set_damage_region_ = nullptr; std::shared_ptr external_view_embedder_; From 4b9a381a1a4fdfd72a31e82930253dd36b815bd9 Mon Sep 17 00:00:00 2001 From: Bernardo Eilert Trevisan Date: Mon, 11 Jul 2022 10:41:11 -0700 Subject: [PATCH 3/5] Formatting. --- shell/platform/embedder/embedder.cc | 16 +++++++++------- shell/platform/embedder/embedder.h | 2 +- shell/platform/embedder/embedder_surface_gl.cc | 5 ++--- shell/platform/embedder/embedder_surface_gl.h | 2 +- 4 files changed, 13 insertions(+), 12 deletions(-) diff --git a/shell/platform/embedder/embedder.cc b/shell/platform/embedder/embedder.cc index 727cd3634e298..6e23d3c488d69 100644 --- a/shell/platform/embedder/embedder.cc +++ b/shell/platform/embedder/embedder.cc @@ -240,9 +240,10 @@ 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](flutter::GLPresentInfo gl_present_info) -> 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 { @@ -252,10 +253,11 @@ InferOpenGLPlatformViewCreationCallback( FlutterDamage fbo_damage = {}; fbo_damage.struct_size = sizeof(FlutterDamage); - fbo_damage.damage = std::vector {FlutterRect{static_cast(gl_present_info.damage->fLeft), - static_cast(gl_present_info.damage->fRight), - static_cast(gl_present_info.damage->fTop), - static_cast(gl_present_info.damage->fBottom)}}; + fbo_damage.damage = std::vector{ + FlutterRect{static_cast(gl_present_info.damage->fLeft), + static_cast(gl_present_info.damage->fRight), + static_cast(gl_present_info.damage->fTop), + static_cast(gl_present_info.damage->fBottom)}}; present_info.fbo_damage = fbo_damage; return present_with_info(user_data, &present_info); } diff --git a/shell/platform/embedder/embedder.h b/shell/platform/embedder/embedder.h index cda8003f7fd69..daadc0cc80591 100644 --- a/shell/platform/embedder/embedder.h +++ b/shell/platform/embedder/embedder.h @@ -5,10 +5,10 @@ #ifndef FLUTTER_EMBEDDER_H_ #define FLUTTER_EMBEDDER_H_ -#include #include #include #include +#include // This file defines an Application Binary Interface (ABI), which requires more // stability than regular code to remain functional for exchanging messages diff --git a/shell/platform/embedder/embedder_surface_gl.cc b/shell/platform/embedder/embedder_surface_gl.cc index b58fb2312ed23..b4eae40ce8f9d 100644 --- a/shell/platform/embedder/embedder_surface_gl.cc +++ b/shell/platform/embedder/embedder_surface_gl.cc @@ -25,8 +25,6 @@ EmbedderSurfaceGL::EmbedderSurfaceGL( valid_ = true; - - // if (HasExtension(extensions, "EGL_KHR_partial_update")) { // set_damage_region_ = reinterpret_cast( // eglGetProcAddress("eglSetDamageRegionKHR")); @@ -67,7 +65,8 @@ bool EmbedderSurfaceGL::GLContextFBOResetAfterPresent() const { } // |GPUSurfaceGLDelegate| -void EmbedderSurfaceGL::GLContextSetDamageRegion(const std::optional& region) { +void EmbedderSurfaceGL::GLContextSetDamageRegion( + const std::optional& region) { // TODO(btrevisan): add checks. FML_DCHECK(IsValid()); diff --git a/shell/platform/embedder/embedder_surface_gl.h b/shell/platform/embedder/embedder_surface_gl.h index d58dd748c760b..6c7d19eba7767 100644 --- a/shell/platform/embedder/embedder_surface_gl.h +++ b/shell/platform/embedder/embedder_surface_gl.h @@ -18,7 +18,7 @@ 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 From 987ed41eeb70a6eb303213dadddee4446cd7eb6f Mon Sep 17 00:00:00 2001 From: Bernardo Eilert Trevisan Date: Mon, 11 Jul 2022 12:00:24 -0700 Subject: [PATCH 4/5] updates. --- examples/glfw/FlutterEmbedderGLFW.cc | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/examples/glfw/FlutterEmbedderGLFW.cc b/examples/glfw/FlutterEmbedderGLFW.cc index 8b17b25958fc1..a3d3c5ab8e676 100644 --- a/examples/glfw/FlutterEmbedderGLFW.cc +++ b/examples/glfw/FlutterEmbedderGLFW.cc @@ -96,8 +96,16 @@ bool RunFlutter(GLFWwindow* window, glfwMakeContextCurrent(nullptr); // is this even a thing? return true; }; - config.open_gl.present = [](void* userdata) -> bool { - glfwSwapBuffers(static_cast(userdata)); + config.open_gl.present_with_info = [](void* userdata, FlutterPresentInfo* present_info) -> bool { + // Check if damage exists + if (present_info->fbo_damage.empty()) { + // Render only damaged regions + // TODO(btrevisan): verify extensions. + eglSwapBuffersWithDamageKHR(static_cast(userdata), present_info->fbo_damage); + } else { + // Otherwise, render the entire screen + glfwSwapBuffers(static_cast(userdata)); + } return true; }; config.open_gl.fbo_callback = [](void*) -> uint32_t { From a8f4982c767678089e09907ac2fb6e0c5cead60a Mon Sep 17 00:00:00 2001 From: Bernardo Eilert Trevisan Date: Mon, 11 Jul 2022 19:33:30 -0700 Subject: [PATCH 5/5] todos --- shell/gpu/gpu_surface_gl_skia.cc | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/shell/gpu/gpu_surface_gl_skia.cc b/shell/gpu/gpu_surface_gl_skia.cc index 66bb6636d95d8..ba530007c9af7 100644 --- a/shell/gpu/gpu_surface_gl_skia.cc +++ b/shell/gpu/gpu_surface_gl_skia.cc @@ -248,6 +248,10 @@ std::unique_ptr GPUSurfaceGLSkia::AcquireFrame( }; framebuffer_info = delegate_->GLContextFramebufferInfo(); + // CHECK IF PARTIAL REPAINT IS ENABLED + // CALCULATE ACCUMULATED DAMAGE (I.E. ARE IN CURRENT FBO THAT LAGS BEHIND FRONT BUFFER) + // SET ACCUMULATED DAMAGE TO FBO_INFO EXISTING_DAMAGE + // SET SUPPORTS_PARTIAL_REPAINT TO TRUE return std::make_unique(surface, std::move(framebuffer_info), submit_callback, std::move(context_switch));