Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions flow/surface_frame.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

#include "flutter/common/graphics/gl_context_switch.h"
#include "flutter/fml/macros.h"
#include "flutter/fml/time/time_point.h"
#include "third_party/skia/include/core/SkCanvas.h"
#include "third_party/skia/include/core/SkSurface.h"

Expand Down Expand Up @@ -66,6 +67,12 @@ class SurfaceFrame {
//
// Corresponds to EGL_KHR_partial_update
std::optional<SkIRect> buffer_damage;

// The vsync target time.
//
// Backends may use this information to avoid overloading the GPU with
// multiple frames per vsync.
fml::TimePoint target_time;
};

bool Submit();
Expand Down
90 changes: 45 additions & 45 deletions shell/common/rasterizer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -536,60 +536,60 @@ RasterStatus Rasterizer::DrawToSurfaceUnsafe(
.supports_readback, // surface supports pixel reads
raster_thread_merger_ // thread merger
);
if (compositor_frame) {
compositor_context_->raster_cache().PrepareNewFrame();
frame_timings_recorder.RecordRasterStart(fml::TimePoint::Now());

// 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.
bool disable_partial_repaint =
external_view_embedder_ &&
(!raster_thread_merger_ || raster_thread_merger_->IsMerged());

FrameDamage damage;
if (!disable_partial_repaint && frame->framebuffer_info().existing_damage) {
damage.SetPreviousLayerTree(last_layer_tree_.get());
damage.AddAdditonalDamage(*frame->framebuffer_info().existing_damage);
}
if (!compositor_frame) {
return RasterStatus::kFailed;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This file is much easier to review if you choose the hide whitespace option in GitHub.

Copy link
Contributor

Choose a reason for hiding this comment

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

After googling it, an easy way to do this is to just add ?w=1 to the URL of this page and it will disable it for just this one visit.


RasterStatus raster_status =
compositor_frame->Raster(layer_tree, false, &damage);
if (raster_status == RasterStatus::kFailed ||
raster_status == RasterStatus::kSkipAndRetry) {
return raster_status;
}
compositor_context_->raster_cache().PrepareNewFrame();
frame_timings_recorder.RecordRasterStart(fml::TimePoint::Now());

SurfaceFrame::SubmitInfo submit_info;
submit_info.frame_damage = damage.GetFrameDamage();
submit_info.buffer_damage = damage.GetBufferDamage();
// 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.
bool disable_partial_repaint =
external_view_embedder_ &&
(!raster_thread_merger_ || raster_thread_merger_->IsMerged());

frame->set_submit_info(submit_info);
FrameDamage damage;
if (!disable_partial_repaint && frame->framebuffer_info().existing_damage) {
damage.SetPreviousLayerTree(last_layer_tree_.get());
damage.AddAdditonalDamage(*frame->framebuffer_info().existing_damage);
}

if (external_view_embedder_ &&
(!raster_thread_merger_ || raster_thread_merger_->IsMerged())) {
FML_DCHECK(!frame->IsSubmitted());
external_view_embedder_->SubmitFrame(surface_->GetContext(),
std::move(frame));
} else {
frame->Submit();
}
RasterStatus raster_status =
compositor_frame->Raster(layer_tree, false, &damage);
if (raster_status == RasterStatus::kFailed ||
raster_status == RasterStatus::kSkipAndRetry) {
return raster_status;
}

compositor_context_->raster_cache().CleanupAfterFrame();
frame_timings_recorder.RecordRasterEnd(
&compositor_context_->raster_cache());
FireNextFrameCallbackIfPresent();
SurfaceFrame::SubmitInfo submit_info;
submit_info.frame_damage = damage.GetFrameDamage();
submit_info.buffer_damage = damage.GetBufferDamage();
submit_info.target_time = frame_timings_recorder.GetVsyncTargetTime();
Copy link
Contributor

Choose a reason for hiding this comment

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

Since we're now depending on GetVsyncTargetTime for something other than metrics, I think we'll need to update VsyncWaiter to always get the current Display refresh rate instead of caching it. https://github.com/flutter/engine/blob/main/shell/platform/android/io/flutter/view/VsyncWaiter.java#L39

On Android S and later, Display.getRefreshRate() returns the current display refresh rate, instead of the app maximum. https://developer.android.com/about/versions/12/reference/compat-framework-changes#display_info_nr_advanced_supported

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jason-simmons FYI - since I think you recently changed this to not do that.

Copy link
Member

Choose a reason for hiding this comment

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

The call to getRefreshRate() was taking a significant amount of time on the platform thread in some cases (see flutter/flutter#90883)

Reintroducing that call may be risky if it needs to be done on every frame.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do we know if those devices were susceptible when running on Android S?

I guess the problem here is that some devices will scale the refresh rate automatically/dynamically.

It might also be interesting to see if the same delays exist when using the NDK API instead.

Copy link
Member

Choose a reason for hiding this comment

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

The Huawei device referenced in flutter/flutter#90883 (comment) was running Android P

I don't know if Android S ensures better performance for getRefreshRate

Copy link
Contributor Author

Choose a reason for hiding this comment

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

#29800 Should resolve this.


if (surface_->GetContext()) {
TRACE_EVENT0("flutter", "PerformDeferredSkiaCleanup");
surface_->GetContext()->performDeferredCleanup(kSkiaCleanupExpiration);
}
frame->set_submit_info(submit_info);

return raster_status;
if (external_view_embedder_ &&
(!raster_thread_merger_ || raster_thread_merger_->IsMerged())) {
FML_DCHECK(!frame->IsSubmitted());
external_view_embedder_->SubmitFrame(surface_->GetContext(),
std::move(frame));
} else {
frame->Submit();
}

return RasterStatus::kFailed;
compositor_context_->raster_cache().CleanupAfterFrame();
frame_timings_recorder.RecordRasterEnd(&compositor_context_->raster_cache());
FireNextFrameCallbackIfPresent();

if (surface_->GetContext()) {
TRACE_EVENT0("flutter", "PerformDeferredSkiaCleanup");
surface_->GetContext()->performDeferredCleanup(kSkiaCleanupExpiration);
}

return raster_status;
}

static sk_sp<SkData> ScreenshotLayerTreeAsPicture(
Expand Down
3 changes: 2 additions & 1 deletion shell/common/shell_test_platform_view_gl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,8 @@ bool ShellTestPlatformViewGL::GLContextClearCurrent() {
}

// |GPUSurfaceGLDelegate|
bool ShellTestPlatformViewGL::GLContextPresent(uint32_t fbo_id) {
bool ShellTestPlatformViewGL::GLContextPresent(fml::TimePoint target_time,
uint32_t fbo_id) {
return gl_surface_.Present();
}

Expand Down
2 changes: 1 addition & 1 deletion shell/common/shell_test_platform_view_gl.h
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ class ShellTestPlatformViewGL : public ShellTestPlatformView,
bool GLContextClearCurrent() override;

// |GPUSurfaceGLDelegate|
bool GLContextPresent(uint32_t fbo_id) override;
bool GLContextPresent(fml::TimePoint target_time, uint32_t fbo_id) override;

// |GPUSurfaceGLDelegate|
intptr_t GLContextFBO(GLFrameInfo frame_info) const override;
Expand Down
9 changes: 6 additions & 3 deletions shell/gpu/gpu_surface_gl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,9 @@ std::unique_ptr<SurfaceFrame> GPUSurfaceGL::AcquireFrame(const SkISize& size) {
SurfaceFrame::SubmitCallback submit_callback =
[weak = weak_factory_.GetWeakPtr()](const SurfaceFrame& surface_frame,
SkCanvas* canvas) {
return weak ? weak->PresentSurface(canvas) : false;
return weak ? weak->PresentSurface(
surface_frame.submit_info().target_time, canvas)
: false;
};

framebuffer_info = delegate_->GLContextFramebufferInfo();
Expand All @@ -251,7 +253,8 @@ std::unique_ptr<SurfaceFrame> GPUSurfaceGL::AcquireFrame(const SkISize& size) {
std::move(context_switch));
}

bool GPUSurfaceGL::PresentSurface(SkCanvas* canvas) {
bool GPUSurfaceGL::PresentSurface(fml::TimePoint target_time,
SkCanvas* canvas) {
if (delegate_ == nullptr || canvas == nullptr || context_ == nullptr) {
return false;
}
Expand All @@ -261,7 +264,7 @@ bool GPUSurfaceGL::PresentSurface(SkCanvas* canvas) {
onscreen_surface_->getCanvas()->flush();
}

if (!delegate_->GLContextPresent(fbo_id_)) {
if (!delegate_->GLContextPresent(target_time, fbo_id_)) {
return false;
}

Expand Down
3 changes: 2 additions & 1 deletion shell/gpu/gpu_surface_gl.h
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ class GPUSurfaceGL : public Surface {
const SkISize& untransformed_size,
const SkMatrix& root_surface_transformation);

bool PresentSurface(SkCanvas* canvas);
bool PresentSurface(fml::TimePoint target_time, SkCanvas* canvas);

GPUSurfaceGLDelegate* delegate_;
sk_sp<GrDirectContext> context_;
Expand All @@ -77,6 +77,7 @@ class GPUSurfaceGL : public Surface {

// WeakPtrFactory must be the last member.
fml::TaskRunnerAffineWeakPtrFactory<GPUSurfaceGL> weak_factory_;

FML_DISALLOW_COPY_AND_ASSIGN(GPUSurfaceGL);
};

Expand Down
3 changes: 2 additions & 1 deletion shell/gpu/gpu_surface_gl_delegate.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ class GPUSurfaceGLDelegate {

// Called to present the main GL surface. This is only called for the main GL
// context and not any of the contexts dedicated for IO.
virtual bool GLContextPresent(uint32_t fbo_id) = 0;
virtual bool GLContextPresent(fml::TimePoint target_time,
uint32_t fbo_id) = 0;

// The ID of the main window bound framebuffer. Typically FBO0.
virtual intptr_t GLContextFBO(GLFrameInfo frame_info) const = 0;
Expand Down
1 change: 1 addition & 0 deletions shell/platform/android/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ executable("flutter_shell_native_unittests") {
public_configs = [ "//flutter:config" ]
deps = [
":flutter_shell_native_src",
"//flutter/fml",
"//third_party/googletest:gmock",
"//third_party/googletest:gtest",
]
Expand Down
40 changes: 21 additions & 19 deletions shell/platform/android/android_context_gl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

#include <utility>

#include "android_environment_gl.h"
#include "flutter/fml/trace_event.h"

namespace flutter {
Expand Down Expand Up @@ -105,13 +106,14 @@ static bool TeardownContext(EGLDisplay display, EGLContext context) {
return true;
}

AndroidEGLSurface::AndroidEGLSurface(EGLSurface surface,
EGLDisplay display,
EGLContext context)
: surface_(surface), display_(display), context_(context) {}
AndroidEGLSurface::AndroidEGLSurface(
EGLSurface surface,
fml::RefPtr<AndroidEnvironmentGL> environment,
EGLContext context)
: surface_(surface), environment_(environment), context_(context) {}

AndroidEGLSurface::~AndroidEGLSurface() {
auto result = eglDestroySurface(display_, surface_);
auto result = eglDestroySurface(environment_->Display(), surface_);
FML_DCHECK(result == EGL_TRUE);
}

Expand All @@ -120,25 +122,28 @@ bool AndroidEGLSurface::IsValid() const {
}

bool AndroidEGLSurface::MakeCurrent() const {
if (eglMakeCurrent(display_, surface_, surface_, context_) != EGL_TRUE) {
if (eglMakeCurrent(environment_->Display(), surface_, surface_, context_) !=
EGL_TRUE) {
FML_LOG(ERROR) << "Could not make the context current";
LogLastEGLError();
return false;
}
return true;
}

bool AndroidEGLSurface::SwapBuffers() {
bool AndroidEGLSurface::SwapBuffers(fml::TimePoint target_time) {
TRACE_EVENT0("flutter", "AndroidContextGL::SwapBuffers");
return eglSwapBuffers(display_, surface_);
environment_->SetPresentationTime(surface_, target_time);
return eglSwapBuffers(environment_->Display(), surface_);
}

SkISize AndroidEGLSurface::GetSize() const {
EGLint width = 0;
EGLint height = 0;

if (!eglQuerySurface(display_, surface_, EGL_WIDTH, &width) ||
!eglQuerySurface(display_, surface_, EGL_HEIGHT, &height)) {
if (!eglQuerySurface(environment_->Display(), surface_, EGL_WIDTH, &width) ||
!eglQuerySurface(environment_->Display(), surface_, EGL_HEIGHT,
&height)) {
FML_LOG(ERROR) << "Unable to query EGL surface size";
LogLastEGLError();
return SkISize::Make(0, 0);
Expand Down Expand Up @@ -229,27 +234,24 @@ std::unique_ptr<AndroidEGLSurface> AndroidContextGL::CreateOnscreenSurface(
if (window->IsFakeWindow()) {
return CreatePbufferSurface();
} else {
EGLDisplay display = environment_->Display();

const EGLint attribs[] = {EGL_NONE};

EGLSurface surface = eglCreateWindowSurface(
display, config_,
environment_->Display(), config_,
reinterpret_cast<EGLNativeWindowType>(window->handle()), attribs);
return std::make_unique<AndroidEGLSurface>(surface, display, context_);
return std::make_unique<AndroidEGLSurface>(surface, environment_, context_);
}
}

std::unique_ptr<AndroidEGLSurface> AndroidContextGL::CreateOffscreenSurface()
const {
// We only ever create pbuffer surfaces for background resource loading
// contexts. We never bind the pbuffer to anything.
EGLDisplay display = environment_->Display();

const EGLint attribs[] = {EGL_WIDTH, 1, EGL_HEIGHT, 1, EGL_NONE};

EGLSurface surface = eglCreatePbufferSurface(display, config_, attribs);
return std::make_unique<AndroidEGLSurface>(surface, display,
EGLSurface surface =
eglCreatePbufferSurface(environment_->Display(), config_, attribs);
return std::make_unique<AndroidEGLSurface>(surface, environment_,
resource_context_);
}

Expand All @@ -260,7 +262,7 @@ std::unique_ptr<AndroidEGLSurface> AndroidContextGL::CreatePbufferSurface()
const EGLint attribs[] = {EGL_WIDTH, 1, EGL_HEIGHT, 1, EGL_NONE};

EGLSurface surface = eglCreatePbufferSurface(display, config_, attribs);
return std::make_unique<AndroidEGLSurface>(surface, display, context_);
return std::make_unique<AndroidEGLSurface>(surface, environment_, context_);
}

fml::RefPtr<AndroidEnvironmentGL> AndroidContextGL::Environment() const {
Expand Down
14 changes: 11 additions & 3 deletions shell/platform/android/android_context_gl.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@
#ifndef FLUTTER_SHELL_PLATFORM_ANDROID_ANDROID_CONTEXT_GL_H_
#define FLUTTER_SHELL_PLATFORM_ANDROID_ANDROID_CONTEXT_GL_H_

#include <EGL/egl.h>
#define EGL_EGLEXT_PROTOTYPES
#include <EGL/eglext.h>

#include "flutter/fml/macros.h"
#include "flutter/fml/memory/ref_counted.h"
#include "flutter/fml/memory/ref_ptr.h"
Expand All @@ -25,7 +29,9 @@ namespace flutter {
///
class AndroidEGLSurface {
public:
AndroidEGLSurface(EGLSurface surface, EGLDisplay display, EGLContext context);
AndroidEGLSurface(EGLSurface surface,
fml::RefPtr<AndroidEnvironmentGL> environment,
EGLContext context);
~AndroidEGLSurface();

//----------------------------------------------------------------------------
Expand All @@ -47,9 +53,11 @@ class AndroidEGLSurface {
/// @brief This only applies to on-screen surfaces such as those created
/// by `AndroidContextGL::CreateOnscreenSurface`.
///
/// @param target_time The vsync target time for the buffer.
///
/// @return Whether the EGL surface color buffer was swapped.
///
bool SwapBuffers();
bool SwapBuffers(fml::TimePoint target_time);

//----------------------------------------------------------------------------
/// @return The size of an `EGLSurface`.
Expand All @@ -58,7 +66,7 @@ class AndroidEGLSurface {

private:
const EGLSurface surface_;
const EGLDisplay display_;
const fml::RefPtr<AndroidEnvironmentGL> environment_;
const EGLContext context_;
};

Expand Down
Loading