Skip to content

Commit

Permalink
[CP] PR #54514 and #54537 onto the release branch (#54556)
Browse files Browse the repository at this point in the history
Context: b/332811967
  • Loading branch information
chingjun authored Aug 15, 2024
1 parent 2f577d8 commit 61b9f5b
Show file tree
Hide file tree
Showing 12 changed files with 70 additions and 114 deletions.
16 changes: 5 additions & 11 deletions flow/diff_context.cc
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ void DiffContext::BeginSubtree() {
bool had_integral_transform = state_.integral_transform;
state_.rect_index = rects_->size();
state_.has_filter_bounds_adjustment = false;
state_.has_volatile_layer = false;
state_.has_texture = false;
state_.integral_transform = false;

if (had_integral_transform) {
Expand Down Expand Up @@ -203,20 +203,14 @@ void DiffContext::AddLayerBounds(const SkRect& rect) {
}
}

void DiffContext::RepaintEntireFrame() {
auto frame_rect = SkRect::MakeIWH(frame_size_.width(), frame_size_.height());
rects_->push_back(frame_rect);
AddDamage(frame_rect);
}

void DiffContext::MarkSubtreeHasVolatileLayer() {
void DiffContext::MarkSubtreeHasTextureLayer() {
// Set the has_texture flag on current state and all parent states. That
// way we'll know that we can't skip diff for retained layers because
// they contain a TextureLayer.
for (auto& state : state_stack_) {
state.has_volatile_layer = true;
state.has_texture = true;
}
state_.has_volatile_layer = true;
state_.has_texture = true;
}

void DiffContext::AddExistingPaintRegion(const PaintRegion& region) {
Expand Down Expand Up @@ -244,7 +238,7 @@ PaintRegion DiffContext::CurrentSubtreeRegion() const {
readbacks_.begin(), readbacks_.end(),
[&](const Readback& r) { return r.position >= state_.rect_index; });
return PaintRegion(rects_, state_.rect_index, rects_->size(), has_readback,
state_.has_volatile_layer);
state_.has_texture);
}

void DiffContext::AddDamage(const PaintRegion& damage) {
Expand Down
12 changes: 4 additions & 8 deletions flow/diff_context.h
Original file line number Diff line number Diff line change
Expand Up @@ -107,18 +107,14 @@ class DiffContext {

bool IsSubtreeDirty() const { return state_.dirty; }

// Marks that current subtree contains a volatile layer. A volatile layer will
// do diffing even if it is a retained subtree. Necessary for TextureLayer
// and PlatformViewLayer.
void MarkSubtreeHasVolatileLayer();
// Marks that current subtree contains a TextureLayer. This is needed to
// ensure that we'll Diff the TextureLayer even if inside retained layer.
void MarkSubtreeHasTextureLayer();

// Add layer bounds to current paint region; rect is in "local" (layer)
// coordinates.
void AddLayerBounds(const SkRect& rect);

// Marks entire frame as dirty.
void RepaintEntireFrame();

// Add entire paint region of retained layer for current subtree. This can
// only be used in subtrees that are not dirty, otherwise ancestor transforms
// or clips may result in different paint region.
Expand Down Expand Up @@ -235,7 +231,7 @@ class DiffContext {
bool has_filter_bounds_adjustment = false;

// Whether there is a texture layer in this subtree.
bool has_volatile_layer = false;
bool has_texture = false;
};

void MakeTransformIntegral(DisplayListMatrixClipState& matrix_clip);
Expand Down
2 changes: 1 addition & 1 deletion flow/layers/container_layer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ void ContainerLayer::DiffChildren(DiffContext* context,
auto prev_layer = prev_layers[i_prev];
auto paint_region = context->GetOldLayerPaintRegion(prev_layer.get());
if (layer == prev_layer && !paint_region.has_readback() &&
!paint_region.has_volatile_layer()) {
!paint_region.has_texture()) {
// for retained layers, stop processing the subtree and add existing
// region; We know current subtree is not dirty (every ancestor up to
// here matches) so the retained subtree will render identically to
Expand Down
11 changes: 0 additions & 11 deletions flow/layers/platform_view_layer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,17 +13,6 @@ PlatformViewLayer::PlatformViewLayer(const SkPoint& offset,
int64_t view_id)
: offset_(offset), size_(size), view_id_(view_id) {}

void PlatformViewLayer::Diff(DiffContext* context, const Layer* old_layer) {
DiffContext::AutoSubtreeRestore subtree(context);
// Ensure that Diff is called again even if this layer is in retained subtree.
context->MarkSubtreeHasVolatileLayer();
// Partial repaint is disabled when platform view is present. This will also
// set whole frame as the layer paint region, which will ensure full repaint
// when the layer is removed.
context->RepaintEntireFrame();
context->SetLayerPaintRegion(this, context->CurrentSubtreeRegion());
}

void PlatformViewLayer::Preroll(PrerollContext* context) {
set_paint_bounds(SkRect::MakeXYWH(offset_.x(), offset_.y(), size_.width(),
size_.height()));
Expand Down
1 change: 0 additions & 1 deletion flow/layers/platform_view_layer.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ class PlatformViewLayer : public Layer {
public:
PlatformViewLayer(const SkPoint& offset, const SkSize& size, int64_t view_id);

void Diff(DiffContext* context, const Layer* old_layer) override;
void Preroll(PrerollContext* context) override;
void Paint(PaintContext& context) const override;

Expand Down
41 changes: 0 additions & 41 deletions flow/layers/platform_view_layer_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
#include "flutter/flow/layers/platform_view_layer.h"
#include "flutter/flow/layers/transform_layer.h"

#include "flutter/flow/testing/diff_context_test.h"
#include "flutter/flow/testing/layer_test.h"
#include "flutter/flow/testing/mock_embedder.h"
#include "flutter/flow/testing/mock_layer.h"
Expand Down Expand Up @@ -140,45 +139,5 @@ TEST_F(PlatformViewLayerTest, StateTransfer) {
transform_layer1->Paint(paint_ctx);
}

using PlatformViewLayerDiffTest = DiffContextTest;

TEST_F(PlatformViewLayerDiffTest, PlatformViewRetainedLayer) {
MockLayerTree tree1(SkISize::Make(800, 600));
auto container = std::make_shared<ContainerLayer>();
tree1.root()->Add(container);
auto layer = std::make_shared<PlatformViewLayer>(SkPoint::Make(100, 100),
SkSize::Make(100, 100), 0);
container->Add(layer);

MockLayerTree tree2(SkISize::Make(800, 600));
tree2.root()->Add(container); // retained layer

auto damage = DiffLayerTree(tree1, MockLayerTree(SkISize::Make(800, 600)));
EXPECT_EQ(damage.frame_damage, SkIRect::MakeLTRB(0, 0, 800, 600));

damage = DiffLayerTree(tree2, tree1);
EXPECT_EQ(damage.frame_damage, SkIRect::MakeLTRB(0, 0, 800, 600));
}

TEST_F(PlatformViewLayerDiffTest, FullRepaintAfterRemovingLayer) {
MockLayerTree tree1(SkISize::Make(800, 600));
auto container = std::make_shared<ContainerLayer>();
tree1.root()->Add(container);
auto layer = std::make_shared<PlatformViewLayer>(SkPoint::Make(100, 100),
SkSize::Make(100, 100), 0);
container->Add(layer);

auto damage = DiffLayerTree(tree1, MockLayerTree(SkISize::Make(800, 600)));
EXPECT_EQ(damage.frame_damage, SkIRect::MakeLTRB(0, 0, 800, 600));

// Second layer tree with the PlatformViewLayer removed.
MockLayerTree tree2(SkISize::Make(800, 600));
auto container2 = std::make_shared<ContainerLayer>();
tree2.root()->Add(container2);

damage = DiffLayerTree(tree2, tree1);
EXPECT_EQ(damage.frame_damage, SkIRect::MakeLTRB(0, 0, 800, 600));
}

} // namespace testing
} // namespace flutter
2 changes: 1 addition & 1 deletion flow/layers/texture_layer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ void TextureLayer::Diff(DiffContext* context, const Layer* old_layer) {
// TextureLayer is inside retained layer.
// See ContainerLayer::DiffChildren
// https://github.com/flutter/flutter/issues/92925
context->MarkSubtreeHasVolatileLayer();
context->MarkSubtreeHasTextureLayer();
context->AddLayerBounds(SkRect::MakeXYWH(offset_.x(), offset_.y(),
size_.width(), size_.height()));
context->SetLayerPaintRegion(this, context->CurrentSubtreeRegion());
Expand Down
12 changes: 6 additions & 6 deletions flow/paint_region.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,12 @@ class PaintRegion {
size_t from,
size_t to,
bool has_readback,
bool has_volatile_layer)
bool has_texture)
: rects_(std::move(rects)),
from_(from),
to_(to),
has_readback_(has_readback),
has_volatile_layer_(has_volatile_layer) {}
has_texture_(has_texture) {}

std::vector<SkRect>::const_iterator begin() const {
FML_DCHECK(is_valid());
Expand All @@ -56,16 +56,16 @@ class PaintRegion {
// that performs readback
bool has_readback() const { return has_readback_; }

// Returns whether there is a TextureLayer or PlatformViewLayer in subtree
// represented by this region.
bool has_volatile_layer() const { return has_volatile_layer_; }
// Returns whether there is a TextureLayer in subtree represented by this
// region.
bool has_texture() const { return has_texture_; }

private:
std::shared_ptr<std::vector<SkRect>> rects_;
size_t from_ = 0;
size_t to_ = 0;
bool has_readback_ = false;
bool has_volatile_layer_ = false;
bool has_texture_ = false;
};

} // namespace flutter
Expand Down
10 changes: 9 additions & 1 deletion shell/common/rasterizer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -743,9 +743,17 @@ DrawSurfaceStatus Rasterizer::DrawToSurfaceUnsafe(
// when leaf layer tracing is enabled we wish to repaint the whole frame
// for accurate performance metrics.
if (frame->framebuffer_info().supports_partial_repaint) {
// Disable partial repaint if external_view_embedder_ SubmitFlutterView 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 force_full_repaint =
external_view_embedder_ &&
(!raster_thread_merger_ || raster_thread_merger_->IsMerged());

damage = std::make_unique<FrameDamage>();
auto existing_damage = frame->framebuffer_info().existing_damage;
if (existing_damage.has_value()) {
if (existing_damage.has_value() && !force_full_repaint) {
damage->SetPreviousLayerTree(GetLastLayerTree(view_id));
damage->AddAdditionalDamage(existing_damage.value());
damage->SetClipAlignment(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,14 +84,16 @@ class PlatformViewsController {
///
/// Called from the raster thread.
PostPrerollResult PostPrerollAction(
const fml::RefPtr<fml::RasterThreadMerger>& raster_thread_merger);
const fml::RefPtr<fml::RasterThreadMerger>& raster_thread_merger,
bool impeller_enabled);

/// @brief Mark the end of a compositor frame.
///
/// May determine changes are required to the thread merging state.
/// Called from the raster thread.
void EndFrame(bool should_resubmit_frame,
const fml::RefPtr<fml::RasterThreadMerger>& raster_thread_merger);
const fml::RefPtr<fml::RasterThreadMerger>& raster_thread_merger,
bool impeller_enabled);

/// @brief Returns the Canvas for the overlay slice for the given platform view.
///
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,11 @@

namespace {

#ifdef FML_OS_IOS_SIMULATOR
// The number of frames the rasterizer task runner will continue
// to run on the platform thread after no platform view is rendered.
//
// Note: this is an arbitrary number.
static const int kDefaultMergedLeaseDuration = 10;
#endif // FML_OS_IOS_SIMULATOR

static constexpr NSUInteger kFlutterClippingMaskViewPoolCapacity = 5;

Expand Down Expand Up @@ -291,43 +289,52 @@ bool ClipRRectContainsPlatformViewBoundingRect(const SkRRect& clip_rrect,
}

PostPrerollResult PlatformViewsController::PostPrerollAction(
const fml::RefPtr<fml::RasterThreadMerger>& raster_thread_merger) {
const fml::RefPtr<fml::RasterThreadMerger>& raster_thread_merger,
bool impeller_enabled) {
// TODO(jonahwilliams): remove this once Software backend is removed for iOS Sim.
#ifdef FML_OS_IOS_SIMULATOR
if (composition_order_.empty()) {
return PostPrerollResult::kSuccess;
}
if (!raster_thread_merger->IsMerged()) {
// The raster thread merger may be disabled if the rasterizer is being
// created or teared down.
//
// In such cases, the current frame is dropped, and a new frame is attempted
// with the same layer tree.
//
// Eventually, the frame is submitted once this method returns `kSuccess`.
// At that point, the raster tasks are handled on the platform thread.
CancelFrame();
return PostPrerollResult::kSkipAndRetryFrame;
}
// If the post preroll action is successful, we will display platform views in the current frame.
// In order to sync the rendering of the platform views (quartz) with skia's rendering,
// We need to begin an explicit CATransaction. This transaction needs to be submitted
// after the current frame is submitted.
raster_thread_merger->ExtendLeaseTo(kDefaultMergedLeaseDuration);
return PostPrerollResult::kSuccess;
const bool merge_threads = true;
#else
return PostPrerollResult::kSuccess;
const bool merge_threads = !impeller_enabled;
#endif // FML_OS_IOS_SIMULATOR

if (merge_threads) {
if (composition_order_.empty()) {
return PostPrerollResult::kSuccess;
}
if (!raster_thread_merger->IsMerged()) {
// The raster thread merger may be disabled if the rasterizer is being
// created or teared down.
//
// In such cases, the current frame is dropped, and a new frame is attempted
// with the same layer tree.
//
// Eventually, the frame is submitted once this method returns `kSuccess`.
// At that point, the raster tasks are handled on the platform thread.
CancelFrame();
return PostPrerollResult::kSkipAndRetryFrame;
}
// If the post preroll action is successful, we will display platform views in the current
// frame. In order to sync the rendering of the platform views (quartz) with skia's rendering,
// We need to begin an explicit CATransaction. This transaction needs to be submitted
// after the current frame is submitted.
raster_thread_merger->ExtendLeaseTo(kDefaultMergedLeaseDuration);
}
return PostPrerollResult::kSuccess;
}

void PlatformViewsController::EndFrame(
bool should_resubmit_frame,
const fml::RefPtr<fml::RasterThreadMerger>& raster_thread_merger) {
const fml::RefPtr<fml::RasterThreadMerger>& raster_thread_merger,
bool impeller_enabled) {
#if FML_OS_IOS_SIMULATOR
if (should_resubmit_frame) {
bool run_check = true;
#else
bool run_check = !impeller_enabled;
#endif // FML_OS_IOS_SIMULATOR
if (run_check && should_resubmit_frame) {
raster_thread_merger->MergeWithLease(kDefaultMergedLeaseDuration);
}
#endif // FML_OS_IOS_SIMULATOR
}

void PlatformViewsController::PushFilterToVisitedPlatformViews(
Expand Down
8 changes: 5 additions & 3 deletions shell/platform/darwin/ios/ios_external_view_embedder.mm
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,8 @@
const fml::RefPtr<fml::RasterThreadMerger>& raster_thread_merger) {
TRACE_EVENT0("flutter", "IOSExternalViewEmbedder::PostPrerollAction");
FML_CHECK(platform_views_controller_);
PostPrerollResult result = platform_views_controller_->PostPrerollAction(raster_thread_merger);
PostPrerollResult result = platform_views_controller_->PostPrerollAction(
raster_thread_merger, ios_context_->GetBackend() != IOSRenderingBackend::kSkia);
return result;
}

Expand Down Expand Up @@ -91,7 +92,8 @@
bool should_resubmit_frame,
const fml::RefPtr<fml::RasterThreadMerger>& raster_thread_merger) {
TRACE_EVENT0("flutter", "IOSExternalViewEmbedder::EndFrame");
platform_views_controller_->EndFrame(should_resubmit_frame, raster_thread_merger);
platform_views_controller_->EndFrame(should_resubmit_frame, raster_thread_merger,
ios_context_->GetBackend() != IOSRenderingBackend::kSkia);
}

// |ExternalViewEmbedder|
Expand All @@ -100,7 +102,7 @@
#if FML_OS_IOS_SIMULATOR
return true;
#else
return false;
return ios_context_->GetBackend() == IOSRenderingBackend::kSkia;
#endif // FML_OS_IOS_SIMULATOR
}

Expand Down

0 comments on commit 61b9f5b

Please sign in to comment.