Skip to content

Commit

Permalink
Fix damage calculation when not providing populate_existing_damage fo…
Browse files Browse the repository at this point in the history
…r gl embedder (#45611)

# Description

This PR fixes the `gl_populate_existing_damage` in embedder.cc, which currently returns an empty rectangle when a full repaint is needed. This leads to the `frame_damage` being considered empty in rasterizer.cc, causing incorrect partial repaint within Flutter when `gl_populate_existing_damage` is not provided by the embedder.

# Related Issue

flutter/flutter#119601

# Tests

Add a new test. Also fixes damage calculation related tests in EmbedderTest by

* Use a new Dart embedder fixture entry point `render_gradient_retained` which retains the old layer to make sure layer tree diff happens.
* Add `latch.Wait()` after `SetGLPresentCallback` to make sure assertions have been executed.
* Make `existing_damage_rects` static since it should be valid after `populate_existing_damage` returns.
  • Loading branch information
ajihyf committed Sep 28, 2023
1 parent cc7c3c1 commit 789fbee
Show file tree
Hide file tree
Showing 5 changed files with 220 additions and 40 deletions.
2 changes: 0 additions & 2 deletions shell/gpu/gpu_surface_gl_delegate.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,6 @@ struct GLFrameInfo {
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 std::optional<SkIRect> existing_damage;
};
Expand Down
1 change: 0 additions & 1 deletion shell/platform/android/android_surface_gl_skia.cc
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,6 @@ 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(),
.existing_damage = onscreen_surface_->InitialDamage(),
};
}
Expand Down
22 changes: 7 additions & 15 deletions shell/platform/embedder/embedder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -363,38 +363,30 @@ InferOpenGLPlatformViewCreationCallback(
if (!populate_existing_damage) {
return flutter::GLFBOInfo{
.fbo_id = static_cast<uint32_t>(id),
.partial_repaint_enabled = false,
.existing_damage = SkIRect::MakeEmpty(),
.existing_damage = std::nullopt,
};
}

// Given the FBO's ID, get its existing damage.
FlutterDamage existing_damage;
populate_existing_damage(user_data, id, &existing_damage);

bool partial_repaint_enabled = true;
SkIRect existing_damage_rect;
std::optional<SkIRect> existing_damage_rect = std::nullopt;

// 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. 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.
FML_LOG(INFO) << "Damage with multiple rectangles not yet supported. "
"Repainting the whole frame.";
existing_damage_rect = SkIRect::MakeEmpty();
partial_repaint_enabled = false;
} else {
existing_damage_rect = FlutterRectToSkIRect(*(existing_damage.damage));
existing_damage_rect = SkIRect::MakeEmpty();
for (size_t i = 0; i < existing_damage.num_rects; i++) {
existing_damage_rect->join(
FlutterRectToSkIRect(existing_damage.damage[i]));
}
}

// Pass the information about this FBO to the rendering backend.
return flutter::GLFBOInfo{
.fbo_id = static_cast<uint32_t>(id),
.partial_repaint_enabled = partial_repaint_enabled,
.existing_damage = existing_damage_rect,
};
};
Expand Down
22 changes: 22 additions & 0 deletions shell/platform/embedder/fixtures/main.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1298,3 +1298,25 @@ void channel_listener_response() async {
});
signalNativeTest();
}

@pragma('vm:entry-point')
void render_gradient_retained() {
OffsetEngineLayer? offsetLayer; // Retain the offset layer.
PlatformDispatcher.instance.onBeginFrame = (Duration duration) {
Size size = Size(800.0, 600.0);

SceneBuilder builder = SceneBuilder();

offsetLayer = builder.pushOffset(0.0, 0.0, oldLayer: offsetLayer);

// display_list_layer will comparing the display_list
// no need to retain the picture
builder.addPicture(
Offset(0.0, 0.0), CreateGradientBox(size)); // gradient - flutter

builder.pop();

PlatformDispatcher.instance.views.first.render(builder.build());
};
PlatformDispatcher.instance.scheduleFrame();
}

0 comments on commit 789fbee

Please sign in to comment.