From 62a1ddeaeeb5577359b5c64fb5ca54510560beac Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Thu, 8 Aug 2024 12:46:04 -0700 Subject: [PATCH] [Impeller] perform final blit and gpu end frame tracing earlier. (flutter/engine#54452) Fixes the reported GPU time regression when rendering platform views. The problem was that we delayed the tracing of the end of the frame until the CATransaction, which artificially stretches out GPU frame time. Instead add a new method SurfaceMTL::PreparePResent which performs the final blit and tracing, call this in the encode callback. Fixes: https://flutter-flutter-perf.skia.org/e/?queries=sub_result%3Daverage_gpu_frame_time%26test%3Dplatform_views_scroll_perf_ios__timeline_summary&selected=commit%3D41853%26name%3D%252Carch%253Dintel%252Cbranch%253Dmaster%252Cconfig%253Ddefault%252Cdevice_type%253DiPhone_11%252Cdevice_version%253Dnone%252Chost_type%253Dmac%252Csub_result%253Daverage_gpu_frame_time%252Ctest%253Dplatform_views_scroll_perf_ios__timeline_summary%252C Test: benchmarks. --- .../renderer/backend/metal/surface_mtl.h | 4 +++ .../renderer/backend/metal/surface_mtl.mm | 14 ++++++++-- .../shell/gpu/gpu_surface_metal_impeller.mm | 27 ++++++++++++++++--- 3 files changed, 39 insertions(+), 6 deletions(-) diff --git a/engine/src/flutter/impeller/renderer/backend/metal/surface_mtl.h b/engine/src/flutter/impeller/renderer/backend/metal/surface_mtl.h index c5dd5a61eef..bff5072f8fb 100644 --- a/engine/src/flutter/impeller/renderer/backend/metal/surface_mtl.h +++ b/engine/src/flutter/impeller/renderer/backend/metal/surface_mtl.h @@ -65,6 +65,9 @@ class SurfaceMTL final : public Surface { present_with_transaction_ = present_with_transaction; } + /// @brief Perform the final blit and trigger end of frame workloads. + bool PreparePresent(); + // |Surface| bool Present() const override; @@ -82,6 +85,7 @@ class SurfaceMTL final : public Surface { std::optional clip_rect_; bool frame_boundary_ = false; bool present_with_transaction_ = false; + bool prepared_ = false; static bool ShouldPerformPartialRepaint(std::optional damage_rect); diff --git a/engine/src/flutter/impeller/renderer/backend/metal/surface_mtl.mm b/engine/src/flutter/impeller/renderer/backend/metal/surface_mtl.mm index c94ad6cd4c0..caa6804c8a4 100644 --- a/engine/src/flutter/impeller/renderer/backend/metal/surface_mtl.mm +++ b/engine/src/flutter/impeller/renderer/backend/metal/surface_mtl.mm @@ -222,8 +222,7 @@ IRect SurfaceMTL::coverage() const { return IRect::MakeSize(resolve_texture_->GetSize()); } -// |Surface| -bool SurfaceMTL::Present() const { +bool SurfaceMTL::PreparePresent() { auto context = context_.lock(); if (!context) { return false; @@ -260,6 +259,17 @@ bool SurfaceMTL::Present() const { #ifdef IMPELLER_DEBUG ContextMTL::Cast(context.get())->GetGPUTracer()->MarkFrameEnd(); #endif // IMPELLER_DEBUG + prepared_ = true; + return true; +} + +// |Surface| +bool SurfaceMTL::Present() const { + FML_CHECK(prepared_); + auto context = context_.lock(); + if (!context) { + return false; + } if (drawable_) { id command_buffer = diff --git a/engine/src/flutter/shell/gpu/gpu_surface_metal_impeller.mm b/engine/src/flutter/shell/gpu/gpu_surface_metal_impeller.mm index 5041fc4092c..f9cb5dd30b3 100644 --- a/engine/src/flutter/shell/gpu/gpu_surface_metal_impeller.mm +++ b/engine/src/flutter/shell/gpu/gpu_surface_metal_impeller.mm @@ -157,6 +157,9 @@ std::unique_ptr GPUSurfaceMetalImpeller::AcquireFrameFromCAMetalLa surface->PresentWithTransaction(surface_frame.submit_info().present_with_transaction); if (clip_rect && clip_rect->IsEmpty()) { + if (!surface->PreparePresent()) { + return false; + } surface_frame.set_user_data(std::move(surface)); return true; } @@ -167,7 +170,6 @@ std::unique_ptr GPUSurfaceMetalImpeller::AcquireFrameFromCAMetalLa const impeller::RenderTarget& render_target = surface->GetTargetRenderPassDescriptor(); surface->SetFrameBoundary(surface_frame.submit_info().frame_boundary); - surface_frame.set_user_data(std::move(surface)); #if EXPERIMENTAL_CANVAS impeller::TextFrameDispatcher collector(aiks_context->GetContentContext(), impeller::Matrix()); @@ -181,13 +183,25 @@ std::unique_ptr GPUSurfaceMetalImpeller::AcquireFrameFromCAMetalLa impeller_dispatcher.FinishRecording(); aiks_context->GetContentContext().GetTransientsBuffer().Reset(); aiks_context->GetContentContext().GetLazyGlyphAtlas()->ResetTextFrames(); + + if (!surface->PreparePresent()) { + return false; + } + surface_frame.set_user_data(std::move(surface)); return true; #else impeller::DlDispatcher impeller_dispatcher(cull_rect); display_list->Dispatch(impeller_dispatcher, sk_cull_rect); auto picture = impeller_dispatcher.EndRecordingAsPicture(); const bool reset_host_buffer = surface_frame.submit_info().frame_boundary; - return aiks_context->Render(picture, render_target, reset_host_buffer); + + auto result = aiks_context->Render(picture, render_target, reset_host_buffer); + + if (!surface->PreparePresent()) { + return false; + } + surface_frame.set_user_data(std::move(surface)); + return result; #endif }); @@ -278,6 +292,9 @@ std::unique_ptr GPUSurfaceMetalImpeller::AcquireFrameFromMTLTextur surface->PresentWithTransaction(surface_frame.submit_info().present_with_transaction); if (clip_rect && clip_rect->IsEmpty()) { + if (!surface->PreparePresent()) { + return false; + } return surface->Present(); } @@ -296,6 +313,9 @@ std::unique_ptr GPUSurfaceMetalImpeller::AcquireFrameFromMTLTextur impeller_dispatcher.FinishRecording(); aiks_context->GetContentContext().GetTransientsBuffer().Reset(); aiks_context->GetContentContext().GetLazyGlyphAtlas()->ResetTextFrames(); + if (!surface->PreparePresent()) { + return false; + } bool render_result = true; #else impeller::DlDispatcher impeller_dispatcher(cull_rect); @@ -310,8 +330,7 @@ std::unique_ptr GPUSurfaceMetalImpeller::AcquireFrameFromMTLTextur FML_LOG(ERROR) << "Failed to render Impeller frame"; return false; } - - return true; + return surface->PreparePresent(); }); SurfaceFrame::SubmitCallback submit_callback =