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
2 changes: 1 addition & 1 deletion flow/surface_frame.h
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ class SurfaceFrame {
// If existing damage is unspecified (nullopt), entire frame will be
// rasterized (no partial redraw). To signal that there is no existing
// damage use an empty SkIRect.
std::optional<SkIRect> existing_damage;
std::optional<SkIRect> existing_damage = std::nullopt;
};

SurfaceFrame(sk_sp<SkSurface> surface,
Expand Down
2 changes: 1 addition & 1 deletion shell/gpu/gpu_surface_gl_delegate.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ struct GLFBOInfo {
// 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;
const std::optional<SkIRect> existing_damage;
};

// Information passed during presentation of a frame.
Expand Down
8 changes: 3 additions & 5 deletions shell/gpu/gpu_surface_gl_skia.cc
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,6 @@ bool GPUSurfaceGLSkia::CreateOrUpdateSurfaces(const SkISize& size) {

onscreen_surface_ = std::move(onscreen_surface);
fbo_id_ = fbo_info.fbo_id;
supports_partial_repaint_ = fbo_info.partial_repaint_enabled;
existing_damage_ = fbo_info.existing_damage;

return true;
Expand Down Expand Up @@ -250,9 +249,9 @@ std::unique_ptr<SurfaceFrame> 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_;
if (!framebuffer_info.existing_damage.has_value()) {
framebuffer_info.existing_damage = existing_damage_;
}
Comment on lines +252 to +254
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks good to me! It should work given that FBOResetAfterPresent is not always called and it should also work if FBO was indeed reset and the Embedder passed information about the existing damage.

return std::make_unique<SurfaceFrame>(surface, std::move(framebuffer_info),
submit_callback,
std::move(context_switch));
Expand Down Expand Up @@ -303,7 +302,6 @@ bool GPUSurfaceGLSkia::PresentSurface(const SurfaceFrame& frame,

onscreen_surface_ = std::move(new_onscreen_surface);
fbo_id_ = fbo_info.fbo_id;
supports_partial_repaint_ = fbo_info.partial_repaint_enabled;
existing_damage_ = fbo_info.existing_damage;
}

Expand Down
8 changes: 4 additions & 4 deletions shell/gpu/gpu_surface_gl_skia.h
Original file line number Diff line number Diff line change
Expand Up @@ -67,17 +67,17 @@ class GPUSurfaceGLSkia : public Surface {
sk_sp<SkSurface> 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();
// The current FBO's existing damage, as tracked by the GPU surface, delegates
// still have an option of overriding this damage with their own in
// `GLContextFrameBufferInfo`.
std::optional<SkIRect> existing_damage_ = std::nullopt;
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
// hack to make avoid allocating resources for the root surface when an
// 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<GPUSurfaceGLSkia> weak_factory_;
Expand Down
2 changes: 2 additions & 0 deletions shell/platform/android/android_surface_gl_skia.cc
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,8 @@ GLFBOInfo AndroidSurfaceGLSkia::GLContextFBO(GLFrameInfo frame_info) const {
// The default window bound framebuffer on Android.
return GLFBOInfo{
.fbo_id = 0,
.partial_repaint_enabled = onscreen_surface_->SupportsPartialRepaint(),
Copy link
Contributor

Choose a reason for hiding this comment

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

The version in place before #35022 landed was this https://github.com/flutter/engine/pull/35022/files#:~:text=framebuffer%20on%20Android.-,return%200%3B,-return%20GLFBOInfo%7B. It did not pass any sort of information besides the fbo_id, which makes me think that this might not be the problem yet.

.existing_damage = onscreen_surface_->InitialDamage(),
};
}

Expand Down
11 changes: 11 additions & 0 deletions shell/platform/embedder/embedder_surface_gl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,17 @@ EmbedderSurfaceGL::GLProcResolver EmbedderSurfaceGL::GetGLProcResolver() const {
return gl_dispatch_table_.gl_proc_resolver;
}

// |GPUSurfaceGLDelegate|
SurfaceFrame::FramebufferInfo EmbedderSurfaceGL::GLContextFramebufferInfo()
const {
// Enable partial repaint by default on the embedders.
auto info = SurfaceFrame::FramebufferInfo{};
info.supports_readback = true;
info.supports_partial_repaint =
gl_dispatch_table_.gl_populate_existing_damage != nullptr;
return info;
}

// |EmbedderSurface|
std::unique_ptr<Surface> EmbedderSurfaceGL::CreateGPUSurface() {
const bool render_to_surface = !external_view_embedder_;
Expand Down
3 changes: 3 additions & 0 deletions shell/platform/embedder/embedder_surface_gl.h
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,9 @@ class EmbedderSurfaceGL final : public EmbedderSurface,
// |GPUSurfaceGLDelegate|
GLProcResolver GetGLProcResolver() const override;

// |GPUSurfaceGLDelegate|
SurfaceFrame::FramebufferInfo GLContextFramebufferInfo() const override;

FML_DISALLOW_COPY_AND_ASSIGN(EmbedderSurfaceGL);
};

Expand Down
20 changes: 20 additions & 0 deletions shell/platform/embedder/tests/embedder_unittests_gl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4065,6 +4065,26 @@ TEST_F(EmbedderTest, ExternalTextureGLRefreshedTooOften) {
EXPECT_TRUE(resolve_called);
}

TEST_F(EmbedderTest,
PresentInfoReceivesNoDamageWhenPopulateExistingDamageIsUndefined) {
auto& context = GetEmbedderContext(EmbedderTestContextType::kOpenGLContext);

EmbedderConfigBuilder builder(context);
builder.SetOpenGLRendererConfig(SkISize::Make(800, 600));
builder.SetDartEntrypoint("render_gradient");
builder.GetRendererConfig().open_gl.populate_existing_damage = nullptr;

auto engine = builder.LaunchEngine();
ASSERT_TRUE(engine.is_valid());

// No damage should be passed.
static_cast<EmbedderTestContextGL&>(context).SetGLPresentCallback(
[](FlutterPresentInfo present_info) {
ASSERT_EQ(present_info.frame_damage.damage, nullptr);
ASSERT_EQ(present_info.buffer_damage.damage, nullptr);
});
}

INSTANTIATE_TEST_SUITE_P(
EmbedderTestGlVk,
EmbedderTestMultiBackend,
Expand Down