Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Impeller] Improve performance of CupertinoPicker with opacity peephole #40101

Merged
merged 13 commits into from Mar 15, 2023
11 changes: 9 additions & 2 deletions impeller/aiks/canvas.cc
Expand Up @@ -363,8 +363,15 @@ void Canvas::SaveLayer(
Save(true, paint.blend_mode, backdrop_filter);

auto& new_layer_pass = GetCurrentPass();
new_layer_pass.SetDelegate(
std::make_unique<PaintPassDelegate>(paint, bounds));

// Only apply opacity peephole on default blending.
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this deserve a TODO/bug to extend this to other blend modes?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think we can in general.

if (paint.blend_mode == BlendMode::kSourceOver) {
new_layer_pass.SetDelegate(
std::make_unique<OpacityPeepholePassDelegate>(paint, bounds));
} else {
new_layer_pass.SetDelegate(
std::make_unique<PaintPassDelegate>(paint, bounds));
}

if (bounds.has_value() && !backdrop_filter.has_value()) {
// Render target switches due to a save layer can be elided. In such cases
Expand Down
92 changes: 91 additions & 1 deletion impeller/aiks/paint_pass_delegate.cc
Expand Up @@ -6,10 +6,14 @@

#include "impeller/entity/contents/contents.h"
#include "impeller/entity/contents/texture_contents.h"
#include "impeller/entity/entity_pass.h"
#include "impeller/geometry/path_builder.h"

namespace impeller {

/// PaintPassDelegate
/// ----------------------------------------------

PaintPassDelegate::PaintPassDelegate(Paint paint, std::optional<Rect> coverage)
: paint_(std::move(paint)), coverage_(coverage) {}

Expand All @@ -27,7 +31,7 @@ bool PaintPassDelegate::CanElide() {
}

// |EntityPassDelgate|
bool PaintPassDelegate::CanCollapseIntoParentPass() {
bool PaintPassDelegate::CanCollapseIntoParentPass(EntityPass* entity_pass) {
return false;
}

Expand All @@ -44,4 +48,90 @@ std::shared_ptr<Contents> PaintPassDelegate::CreateContentsForSubpassTarget(
effect_transform);
}

/// OpacityPeepholePassDelegate
/// ----------------------------------------------

OpacityPeepholePassDelegate::OpacityPeepholePassDelegate(
Paint paint,
std::optional<Rect> coverage)
: paint_(std::move(paint)), coverage_(coverage) {}

// |EntityPassDelgate|
OpacityPeepholePassDelegate::~OpacityPeepholePassDelegate() = default;

// |EntityPassDelgate|
std::optional<Rect> OpacityPeepholePassDelegate::GetCoverageRect() {
return coverage_;
}

// |EntityPassDelgate|
bool OpacityPeepholePassDelegate::CanElide() {
return paint_.blend_mode == BlendMode::kDestination;
}

// |EntityPassDelgate|
bool OpacityPeepholePassDelegate::CanCollapseIntoParentPass(
EntityPass* entity_pass) {
// Note: determing whether any coverage intersects has quadradic complexity in
// the number of rectangles, and depending on whether or not we cache at
// different levels of the entity tree may end up cubic. In the interest of
// proving whether or not this optimization is valuable, we only consider very
// simple peephole optimizations here - where there is a single drawing
// command wrapped in save layer. This would indicate something like an
// Opacity or FadeTransition wrapping a very simple widget, like in the
// CupertinoPicker.
if (entity_pass->GetEntityCount() > 3) {
Copy link
Member Author

Choose a reason for hiding this comment

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

One concern I have with working on this optimization now: subpasses are extra expensive right now on the metal backend due to the whole waitForScheduled, and also potentially due to lack of multithreaded encoding.

We might tune these thresholds too high and end up trading a bunch of raster time for decreasing returns of GPU utilization.

Copy link
Member

@bdero bdero Mar 8, 2023

Choose a reason for hiding this comment

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

Yes! We're terribly underutilizing the GPU here. Sibling EntityPasses are fundamentally independent and can be evaluated in parallel on the CPU (not a huge impact in this case) and executed in parallel on the GPU (absolutely enormous impact in this case). Once we have a proper synchronization solution, we can remove all of the host<->device syncs that are blocking the CPU and saturate the GPU with parallelizable work.

Copy link
Member

@bdero bdero Mar 8, 2023

Choose a reason for hiding this comment

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

Although I think we added texture atlas sharing between EntityPasses at some point. That (and any other resource sharing between EntityPasses) needs to be reverted, of course.

Copy link
Member Author

Choose a reason for hiding this comment

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

Can we get around that by making glyph atlas construction happen eagerly? Otherwise, glyph atlas construction and upload takes a substantial amount of time.

Copy link
Member Author

Choose a reason for hiding this comment

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

or, more specifically, can we push construction of the glyph atlas including all text frames into a different thread and then use whatever resource tracking mechanism we create to ensure the subsequent commands that depend on this are ordered correctly?

Copy link
Member

Choose a reason for hiding this comment

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

Can we get around that by making glyph atlas construction happen eagerly

Yeah I think that's probably the best option -- just render the whole thing at once and upload everything just before the EntityPass run. Then the uploaded atlases become read only for the frame and don't restrict parallelism.

// Single paint command with a save layer would be:
// 1. clip
// 2. draw command
// 3. restore.
return false;
}
bool all_can_accept = true;
std::vector<Rect> all_coverages;
auto had_subpass = entity_pass->IterateUntilSubpass(
[&all_coverages, &all_can_accept](Entity& entity) {
auto contents = entity.GetContents();
if (!contents->CanAcceptOpacity(entity)) {
all_can_accept = false;
return false;
}
auto maybe_coverage = contents->GetCoverage(entity);
if (maybe_coverage.has_value()) {
auto coverage = maybe_coverage.value();
for (const auto& cv : all_coverages) {
if (cv.IntersectsWithRect(coverage)) {
all_can_accept = false;
return false;
}
}
all_coverages.push_back(coverage);
}
return true;
});
if (had_subpass || !all_can_accept) {
return false;
}
auto alpha = paint_.color.alpha;
entity_pass->IterateUntilSubpass([&alpha](Entity& entity) {
entity.GetContents()->InheritOpacity(alpha);
return true;
});
return true;
}

// |EntityPassDelgate|
std::shared_ptr<Contents>
OpacityPeepholePassDelegate::CreateContentsForSubpassTarget(
std::shared_ptr<Texture> target,
const Matrix& effect_transform) {
auto contents = TextureContents::MakeRect(Rect::MakeSize(target->GetSize()));
contents->SetTexture(target);
contents->SetSourceRect(Rect::MakeSize(target->GetSize()));
contents->SetOpacity(paint_.color.alpha);
contents->SetDeferApplyingOpacity(true);
return paint_.WithFiltersForSubpassTarget(std::move(contents),
effect_transform);
}

} // namespace impeller
37 changes: 36 additions & 1 deletion impeller/aiks/paint_pass_delegate.h
Expand Up @@ -12,6 +12,8 @@

namespace impeller {

class EntityPass;

class PaintPassDelegate final : public EntityPassDelegate {
public:
PaintPassDelegate(Paint paint, std::optional<Rect> coverage);
Expand All @@ -26,7 +28,7 @@ class PaintPassDelegate final : public EntityPassDelegate {
bool CanElide() override;

// |EntityPassDelgate|
bool CanCollapseIntoParentPass() override;
bool CanCollapseIntoParentPass(EntityPass* entity_pass) override;

// |EntityPassDelgate|
std::shared_ptr<Contents> CreateContentsForSubpassTarget(
Expand All @@ -40,4 +42,37 @@ class PaintPassDelegate final : public EntityPassDelegate {
FML_DISALLOW_COPY_AND_ASSIGN(PaintPassDelegate);
};

/// A delegate that attempts to forward opacity from a save layer to
/// child contents.
///
/// Currently this has a hardcoded limit of 3 entities in a pass, and
/// cannot forward to child subpass delegates.
class OpacityPeepholePassDelegate final : public EntityPassDelegate {
public:
OpacityPeepholePassDelegate(Paint paint, std::optional<Rect> coverage);

// |EntityPassDelgate|
~OpacityPeepholePassDelegate() override;

// |EntityPassDelegate|
std::optional<Rect> GetCoverageRect() override;

// |EntityPassDelgate|
bool CanElide() override;

// |EntityPassDelgate|
bool CanCollapseIntoParentPass(EntityPass* entity_pass) override;

// |EntityPassDelgate|
std::shared_ptr<Contents> CreateContentsForSubpassTarget(
std::shared_ptr<Texture> target,
const Matrix& effect_transform) override;

private:
const Paint paint_;
const std::optional<Rect> coverage_;

FML_DISALLOW_COPY_AND_ASSIGN(OpacityPeepholePassDelegate);
};

} // namespace impeller
12 changes: 12 additions & 0 deletions impeller/entity/contents/clip_contents.cc
Expand Up @@ -70,6 +70,12 @@ bool ClipContents::ShouldRender(
return true;
}

bool ClipContents::CanAcceptOpacity(const Entity& entity) const {
return true;
}

void ClipContents::InheritOpacity(Scalar opacity) {}

bool ClipContents::Render(const ContentContext& renderer,
const Entity& entity,
RenderPass& pass) const {
Expand Down Expand Up @@ -166,6 +172,12 @@ bool ClipRestoreContents::ShouldRender(
return true;
}

bool ClipRestoreContents::CanAcceptOpacity(const Entity& entity) const {
return true;
}

void ClipRestoreContents::InheritOpacity(Scalar opacity) {}

bool ClipRestoreContents::Render(const ContentContext& renderer,
const Entity& entity,
RenderPass& pass) const {
Expand Down
11 changes: 11 additions & 0 deletions impeller/entity/contents/clip_contents.h
Expand Up @@ -41,6 +41,11 @@ class ClipContents final : public Contents {
bool Render(const ContentContext& renderer,
const Entity& entity,
RenderPass& pass) const override;
// |Contents|
bool CanAcceptOpacity(const Entity& entity) const override;

// |Contents|
void InheritOpacity(Scalar opacity) override;

private:
std::unique_ptr<Geometry> geometry_;
Expand Down Expand Up @@ -78,6 +83,12 @@ class ClipRestoreContents final : public Contents {
const Entity& entity,
RenderPass& pass) const override;

// |Contents|
bool CanAcceptOpacity(const Entity& entity) const override;

// |Contents|
void InheritOpacity(Scalar opacity) override;

private:
std::optional<Rect> restore_coverage_;

Expand Down
8 changes: 8 additions & 0 deletions impeller/entity/contents/color_source_contents.cc
Expand Up @@ -42,6 +42,14 @@ std::optional<Rect> ColorSourceContents::GetCoverage(
return geometry_->GetCoverage(entity.GetTransformation());
};

bool ColorSourceContents::CanAcceptOpacity(const Entity& entity) const {
return true;
}

void ColorSourceContents::InheritOpacity(Scalar opacity) {
SetAlpha(GetAlpha() * opacity);
}

bool ColorSourceContents::ShouldRender(
const Entity& entity,
const std::optional<Rect>& stencil_coverage) const {
Expand Down
10 changes: 8 additions & 2 deletions impeller/entity/contents/color_source_contents.h
Expand Up @@ -33,11 +33,17 @@ class ColorSourceContents : public Contents {
bool ShouldRender(const Entity& entity,
const std::optional<Rect>& stencil_coverage) const override;

protected:
const std::shared_ptr<Geometry>& GetGeometry() const;
// | Contents|
bool CanAcceptOpacity(const Entity& entity) const override;

// | Contents|
void InheritOpacity(Scalar opacity) override;

Scalar GetAlpha() const;

protected:
const std::shared_ptr<Geometry>& GetGeometry() const;

private:
std::shared_ptr<Geometry> geometry_;
Matrix inverse_matrix_;
Expand Down
8 changes: 8 additions & 0 deletions impeller/entity/contents/contents.cc
Expand Up @@ -110,6 +110,14 @@ std::optional<Snapshot> Contents::RenderToSnapshot(
return snapshot;
}

bool Contents::CanAcceptOpacity(const Entity& entity) const {
return false;
}

void Contents::InheritOpacity(Scalar opacity) {
FML_UNREACHABLE();
}

bool Contents::ShouldRender(const Entity& entity,
const std::optional<Rect>& stencil_coverage) const {
if (!stencil_coverage.has_value()) {
Expand Down
13 changes: 13 additions & 0 deletions impeller/entity/contents/contents.h
Expand Up @@ -83,6 +83,19 @@ class Contents {

void SetColorSourceSize(Size size) { color_source_size_ = size; }

/// @brief Whether or not this contents can accept the opacity peephole
/// optimization.
///
/// By default all contents return false. Contents are responsible
/// for determining whether or not their own geometries intersect in
/// a way that makes accepting opacity impossible. It is always safe
/// to return false, especially if computing overlap would be
/// computationally expensive.
virtual bool CanAcceptOpacity(const Entity& entity) const;
Copy link
Member

Choose a reason for hiding this comment

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

Can this just be a single call. Say "virtual bool InheritOpacity(...)". The return value can indicate if the operation succeeded.

Copy link
Member Author

Choose a reason for hiding this comment

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

We have to make sure the entities in a pass aren't overlapping too, so we can't combine them. basically there would need to be two levels of checks:

  1. For each entity in the pass, it needs to be able to accept opacity and not overlap other entities.
  2. Within an entities contents, its geometry needs to be non-overlapping.

Copy link
Member

Choose a reason for hiding this comment

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

sgtm


/// @brief Inherit the provided opacity.
virtual void InheritOpacity(Scalar opacity);

protected:

private:
Expand Down
11 changes: 11 additions & 0 deletions impeller/entity/contents/solid_color_contents.cc
Expand Up @@ -28,6 +28,17 @@ void SolidColorContents::SetGeometry(std::shared_ptr<Geometry> geometry) {
geometry_ = std::move(geometry);
}

// | Contents|
bool SolidColorContents::CanAcceptOpacity(const Entity& entity) const {
return true;
}

// | Contents|
void SolidColorContents::InheritOpacity(Scalar opacity) {
auto color = color_;
color_ = color.WithAlpha(color.alpha * opacity);
}

std::optional<Rect> SolidColorContents::GetCoverage(
const Entity& entity) const {
if (color_.IsTransparent()) {
Expand Down
6 changes: 6 additions & 0 deletions impeller/entity/contents/solid_color_contents.h
Expand Up @@ -35,6 +35,12 @@ class SolidColorContents final : public Contents {

const Color& GetColor() const;

// | Contents|
bool CanAcceptOpacity(const Entity& entity) const override;

// | Contents|
void InheritOpacity(Scalar opacity) override;

// |Contents|
std::optional<Rect> GetCoverage(const Entity& entity) const override;

Expand Down
19 changes: 16 additions & 3 deletions impeller/entity/contents/text_contents.cc
Expand Up @@ -50,6 +50,19 @@ void TextContents::SetColor(Color color) {
color_ = color;
}

Color TextContents::GetColor() const {
return color_;
}

bool TextContents::CanAcceptOpacity(const Entity& entity) const {
return !frame_.MaybeHasOverlapping();
}

void TextContents::InheritOpacity(Scalar opacity) {
auto color = color_;
color_ = color.WithAlpha(color.alpha * opacity);
}

void TextContents::SetInverseMatrix(Matrix matrix) {
inverse_matrix_ = matrix;
}
Expand Down Expand Up @@ -122,9 +135,9 @@ static bool CommonRender(
// interpolated vertex information is also used in the fragment shader to
// sample from the glyph atlas.

const std::array<Point, 4> unit_points = {Point{0, 0}, Point{1, 0},
Point{0, 1}, Point{1, 1}};
const std::array<uint32_t, 6> indices = {0, 1, 2, 1, 2, 3};
constexpr std::array<Point, 4> unit_points = {Point{0, 0}, Point{1, 0},
Point{0, 1}, Point{1, 1}};
constexpr std::array<uint32_t, 6> indices = {0, 1, 2, 1, 2, 3};

VertexBufferBuilder<typename VS::PerVertexData> vertex_builder;

Expand Down
6 changes: 6 additions & 0 deletions impeller/entity/contents/text_contents.h
Expand Up @@ -32,6 +32,12 @@ class TextContents final : public Contents {

void SetColor(Color color);

Color GetColor() const;

bool CanAcceptOpacity(const Entity& entity) const override;

void InheritOpacity(Scalar opacity) override;

void SetInverseMatrix(Matrix matrix);

// |Contents|
Expand Down