Skip to content

Commit

Permalink
[Impeller] Make Entity move only, simplify construction of geometry/f…
Browse files Browse the repository at this point in the history
…ilter contents. (flutter#48596)

Redo of flutter#48585

Removes the Copy constructor of Entity, adds a clone method. Allows us to ensure that we move entity into the entity pass without using rvalue reference parameters

Fixes flutter/flutter#139569
  • Loading branch information
jonahwilliams authored Dec 5, 2023
1 parent e62543d commit 0924225
Show file tree
Hide file tree
Showing 12 changed files with 153 additions and 85 deletions.
12 changes: 7 additions & 5 deletions impeller/aiks/aiks_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2547,7 +2547,7 @@ TEST_P(AiksTest, OpacityPeepHoleApplicationTest) {
Entity entity;
entity.SetContents(SolidColorContents::Make(
PathBuilder{}.AddRect(rect).TakePath(), Color::Red()));
entity_pass->AddEntity(entity);
entity_pass->AddEntity(std::move(entity));
paint.color = Color::Red().WithAlpha(0.5);

delegate = std::make_shared<OpacityPeepholePassDelegate>(paint);
Expand Down Expand Up @@ -3239,20 +3239,22 @@ TEST_P(AiksTest, OpaqueEntitiesGetCoercedToSource) {
Picture picture = canvas.EndRecordingAsPicture();

// Extract the SolidColorSource.
Entity entity;
// Entity entity;
std::vector<Entity> entity;
std::shared_ptr<SolidColorContents> contents;
picture.pass->IterateAllEntities([&e = entity, &contents](Entity& entity) {
picture.pass->IterateAllEntities([e = &entity, &contents](Entity& entity) {
if (ScalarNearlyEqual(entity.GetTransform().GetScale().x, 1.618f)) {
e = entity;
contents =
std::static_pointer_cast<SolidColorContents>(entity.GetContents());
e->emplace_back(entity.Clone());
return false;
}
return true;
});

ASSERT_TRUE(entity.size() >= 1);
ASSERT_TRUE(contents->IsOpaque());
ASSERT_EQ(entity.GetBlendMode(), BlendMode::kSource);
ASSERT_EQ(entity[0].GetBlendMode(), BlendMode::kSource);
}

TEST_P(AiksTest, CanRenderDestructiveSaveLayer) {
Expand Down
133 changes: 104 additions & 29 deletions impeller/aiks/canvas.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
#include "impeller/aiks/paint_pass_delegate.h"
#include "impeller/entity/contents/atlas_contents.h"
#include "impeller/entity/contents/clip_contents.h"
#include "impeller/entity/contents/color_source_contents.h"
#include "impeller/entity/contents/filters/filter_contents.h"
#include "impeller/entity/contents/solid_rrect_blur_contents.h"
#include "impeller/entity/contents/text_contents.h"
#include "impeller/entity/contents/texture_contents.h"
Expand All @@ -23,6 +25,80 @@

namespace impeller {

namespace {

static std::shared_ptr<Contents> CreateContentsForGeometryWithFilters(
const Paint& paint,
std::shared_ptr<Geometry> geometry) {
std::shared_ptr<ColorSourceContents> contents =
paint.color_source.GetContents(paint);

// Attempt to apply the color filter on the CPU first.
// Note: This is not just an optimization; some color sources rely on
// CPU-applied color filters to behave properly.
bool needs_color_filter = paint.HasColorFilter();
if (needs_color_filter) {
auto color_filter = paint.GetColorFilter();
if (contents->ApplyColorFilter(color_filter->GetCPUColorFilterProc())) {
needs_color_filter = false;
}
}

contents->SetGeometry(std::move(geometry));
if (paint.mask_blur_descriptor.has_value()) {
// If there's a mask blur and we need to apply the color filter on the GPU,
// we need to be careful to only apply the color filter to the source
// colors. CreateMaskBlur is able to handle this case.
return paint.mask_blur_descriptor->CreateMaskBlur(
contents, needs_color_filter ? paint.GetColorFilter() : nullptr);
}

std::shared_ptr<Contents> contents_copy = std::move(contents);
// Image input types will directly set their color filter,
// if any. See `TiledTextureContents.SetColorFilter`.
if (needs_color_filter &&
paint.color_source.GetType() != ColorSource::Type::kImage) {
std::shared_ptr<ColorFilter> color_filter = paint.GetColorFilter();
contents_copy = color_filter->WrapWithGPUColorFilter(
FilterInput::Make(std::move(contents_copy)),
ColorFilterContents::AbsorbOpacity::kYes);
}

if (paint.image_filter) {
std::shared_ptr<FilterContents> filter = paint.image_filter->WrapInput(
FilterInput::Make(std::move(contents_copy)));
filter->SetRenderingMode(Entity::RenderingMode::kDirect);
return filter;
}

return contents_copy;
}

static std::shared_ptr<Contents> CreatePathContentsWithFilters(
const Paint& paint,
Path path = {}) {
std::shared_ptr<Geometry> geometry;
switch (paint.style) {
case Paint::Style::kFill:
geometry = Geometry::MakeFillPath(std::move(path));
break;
case Paint::Style::kStroke:
geometry = Geometry::MakeStrokePath(std::move(path), paint.stroke_width,
paint.stroke_miter, paint.stroke_cap,
paint.stroke_join);
break;
}

return CreateContentsForGeometryWithFilters(paint, std::move(geometry));
}

static std::shared_ptr<Contents> CreateCoverContentsWithFilters(
const Paint& paint) {
return CreateContentsForGeometryWithFilters(paint, Geometry::MakeCover());
}

} // namespace

Canvas::Canvas() {
Initialize(std::nullopt);
}
Expand Down Expand Up @@ -176,20 +252,19 @@ void Canvas::DrawPath(Path path, const Paint& paint) {
entity.SetTransform(GetCurrentTransform());
entity.SetClipDepth(GetClipDepth());
entity.SetBlendMode(paint.blend_mode);
entity.SetContents(
paint.WithFilters(paint.CreateContentsForEntity(std::move(path))));
entity.SetContents(CreatePathContentsWithFilters(paint, std::move(path)));

GetCurrentPass().AddEntity(entity);
GetCurrentPass().AddEntity(std::move(entity));
}

void Canvas::DrawPaint(const Paint& paint) {
Entity entity;
entity.SetTransform(GetCurrentTransform());
entity.SetClipDepth(GetClipDepth());
entity.SetBlendMode(paint.blend_mode);
entity.SetContents(paint.CreateContentsForEntity({}, true));
entity.SetContents(CreateCoverContentsWithFilters(paint));

GetCurrentPass().AddEntity(entity);
GetCurrentPass().AddEntity(std::move(entity));
}

bool Canvas::AttemptDrawBlurredRRect(const Rect& rect,
Expand Down Expand Up @@ -227,7 +302,7 @@ bool Canvas::AttemptDrawBlurredRRect(const Rect& rect,
entity.SetBlendMode(new_paint.blend_mode);
entity.SetContents(new_paint.WithFilters(std::move(contents)));

GetCurrentPass().AddEntity(entity);
GetCurrentPass().AddEntity(std::move(entity));

return true;
}
Expand All @@ -237,10 +312,10 @@ void Canvas::DrawLine(const Point& p0, const Point& p1, const Paint& paint) {
entity.SetTransform(GetCurrentTransform());
entity.SetClipDepth(GetClipDepth());
entity.SetBlendMode(paint.blend_mode);
entity.SetContents(paint.WithFilters(paint.CreateContentsForGeometry(
Geometry::MakeLine(p0, p1, paint.stroke_width, paint.stroke_cap))));
entity.SetContents(CreateContentsForGeometryWithFilters(
paint, Geometry::MakeLine(p0, p1, paint.stroke_width, paint.stroke_cap)));

GetCurrentPass().AddEntity(entity);
GetCurrentPass().AddEntity(std::move(entity));
}

void Canvas::DrawRect(Rect rect, const Paint& paint) {
Expand All @@ -257,10 +332,10 @@ void Canvas::DrawRect(Rect rect, const Paint& paint) {
entity.SetTransform(GetCurrentTransform());
entity.SetClipDepth(GetClipDepth());
entity.SetBlendMode(paint.blend_mode);
entity.SetContents(paint.WithFilters(
paint.CreateContentsForGeometry(Geometry::MakeRect(rect))));
entity.SetContents(
CreateContentsForGeometryWithFilters(paint, Geometry::MakeRect(rect)));

GetCurrentPass().AddEntity(entity);
GetCurrentPass().AddEntity(std::move(entity));
}

void Canvas::DrawRRect(Rect rect, Point corner_radii, const Paint& paint) {
Expand All @@ -278,10 +353,10 @@ void Canvas::DrawRRect(Rect rect, Point corner_radii, const Paint& paint) {
entity.SetTransform(GetCurrentTransform());
entity.SetClipDepth(GetClipDepth());
entity.SetBlendMode(paint.blend_mode);
entity.SetContents(paint.WithFilters(paint.CreateContentsForGeometry(
Geometry::MakeFillPath(std::move(path)))));
entity.SetContents(CreateContentsForGeometryWithFilters(
paint, Geometry::MakeFillPath(std::move(path))));

GetCurrentPass().AddEntity(entity);
GetCurrentPass().AddEntity(std::move(entity));
return;
}
DrawPath(std::move(path), paint);
Expand All @@ -304,9 +379,9 @@ void Canvas::DrawCircle(Point center, Scalar radius, const Paint& paint) {
? Geometry::MakeStrokedCircle(center, radius, paint.stroke_width)
: Geometry::MakeCircle(center, radius);
entity.SetContents(
paint.WithFilters(paint.CreateContentsForGeometry(geometry)));
CreateContentsForGeometryWithFilters(paint, std::move(geometry)));

GetCurrentPass().AddEntity(entity);
GetCurrentPass().AddEntity(std::move(entity));
}

void Canvas::ClipPath(Path path, Entity::ClipOperation clip_op) {
Expand Down Expand Up @@ -401,7 +476,7 @@ void Canvas::ClipGeometry(const std::shared_ptr<Geometry>& geometry,
entity.SetContents(std::move(contents));
entity.SetClipDepth(GetClipDepth());

GetCurrentPass().AddEntity(entity);
GetCurrentPass().AddEntity(std::move(entity));

++transform_stack_.back().clip_depth;
transform_stack_.back().contains_clips = true;
Expand Down Expand Up @@ -440,7 +515,7 @@ void Canvas::RestoreClip() {
entity.SetContents(std::make_shared<ClipRestoreContents>());
entity.SetClipDepth(GetClipDepth());

GetCurrentPass().AddEntity(entity);
GetCurrentPass().AddEntity(std::move(entity));
}

void Canvas::DrawPoints(std::vector<Point> points,
Expand All @@ -455,11 +530,12 @@ void Canvas::DrawPoints(std::vector<Point> points,
entity.SetTransform(GetCurrentTransform());
entity.SetClipDepth(GetClipDepth());
entity.SetBlendMode(paint.blend_mode);
entity.SetContents(paint.WithFilters(paint.CreateContentsForGeometry(
entity.SetContents(CreateContentsForGeometryWithFilters(
paint,
Geometry::MakePointField(std::move(points), radius,
/*round=*/point_style == PointStyle::kRound))));
/*round=*/point_style == PointStyle::kRound)));

GetCurrentPass().AddEntity(entity);
GetCurrentPass().AddEntity(std::move(entity));
}

void Canvas::DrawPicture(const Picture& picture) {
Expand Down Expand Up @@ -533,7 +609,7 @@ void Canvas::DrawImageRect(const std::shared_ptr<Image>& image,
entity.SetContents(paint.WithFilters(contents));
entity.SetTransform(GetCurrentTransform());

GetCurrentPass().AddEntity(entity);
GetCurrentPass().AddEntity(std::move(entity));
}

Picture Canvas::EndRecordingAsPicture() {
Expand Down Expand Up @@ -597,7 +673,7 @@ void Canvas::DrawTextFrame(const std::shared_ptr<TextFrame>& text_frame,
entity.SetContents(
paint.WithFilters(paint.WithMaskBlur(std::move(text_contents), true)));

GetCurrentPass().AddEntity(entity);
GetCurrentPass().AddEntity(std::move(entity));
}

static bool UseColorSourceContents(
Expand Down Expand Up @@ -635,9 +711,8 @@ void Canvas::DrawVertices(const std::shared_ptr<VerticesGeometry>& vertices,
// If there are no vertex color or texture coordinates. Or if there
// are vertex coordinates then only if the contents are an image.
if (UseColorSourceContents(vertices, paint)) {
auto contents = paint.CreateContentsForGeometry(vertices);
entity.SetContents(paint.WithFilters(std::move(contents)));
GetCurrentPass().AddEntity(entity);
entity.SetContents(CreateContentsForGeometryWithFilters(paint, vertices));
GetCurrentPass().AddEntity(std::move(entity));
return;
}

Expand Down Expand Up @@ -675,7 +750,7 @@ void Canvas::DrawVertices(const std::shared_ptr<VerticesGeometry>& vertices,
contents->SetSourceContents(std::move(src_contents));
entity.SetContents(paint.WithFilters(std::move(contents)));

GetCurrentPass().AddEntity(entity);
GetCurrentPass().AddEntity(std::move(entity));
}

void Canvas::DrawAtlas(const std::shared_ptr<Image>& atlas,
Expand Down Expand Up @@ -706,7 +781,7 @@ void Canvas::DrawAtlas(const std::shared_ptr<Image>& atlas,
entity.SetBlendMode(paint.blend_mode);
entity.SetContents(paint.WithFilters(contents));

GetCurrentPass().AddEntity(entity);
GetCurrentPass().AddEntity(std::move(entity));
}

} // namespace impeller
18 changes: 0 additions & 18 deletions impeller/aiks/paint.cc
Original file line number Diff line number Diff line change
Expand Up @@ -26,24 +26,6 @@ constexpr ColorMatrix kColorInversion = {
};
// clang-format on

std::shared_ptr<Contents> Paint::CreateContentsForEntity(Path path,
bool cover) const {
std::shared_ptr<Geometry> geometry;
switch (style) {
case Style::kFill:
geometry = cover ? Geometry::MakeCover()
: Geometry::MakeFillPath(std::move(path));
break;
case Style::kStroke:
geometry = cover ? Geometry::MakeCover()
: Geometry::MakeStrokePath(std::move(path), stroke_width,
stroke_miter, stroke_cap,
stroke_join);
break;
}
return CreateContentsForGeometry(geometry);
}

std::shared_ptr<Contents> Paint::CreateContentsForGeometry(
const std::shared_ptr<Geometry>& geometry) const {
auto contents = color_source.GetContents(*this);
Expand Down
3 changes: 0 additions & 3 deletions impeller/aiks/paint.h
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,6 @@ struct Paint {
std::shared_ptr<Contents> input,
const Matrix& effect_transform = Matrix()) const;

std::shared_ptr<Contents> CreateContentsForEntity(Path path = {},
bool cover = false) const;

std::shared_ptr<Contents> CreateContentsForGeometry(
const std::shared_ptr<Geometry>& geometry) const;

Expand Down
4 changes: 2 additions & 2 deletions impeller/entity/contents/filters/filter_contents.cc
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ std::optional<Rect> FilterContents::GetLocalCoverage(
}

std::optional<Rect> FilterContents::GetCoverage(const Entity& entity) const {
Entity entity_with_local_transform = entity;
Entity entity_with_local_transform = entity.Clone();
entity_with_local_transform.SetTransform(GetTransform(entity.GetTransform()));

return GetLocalCoverage(entity_with_local_transform);
Expand Down Expand Up @@ -269,7 +269,7 @@ std::optional<Entity> FilterContents::GetEntity(
const ContentContext& renderer,
const Entity& entity,
const std::optional<Rect>& coverage_hint) const {
Entity entity_with_local_transform = entity;
Entity entity_with_local_transform = entity.Clone();
entity_with_local_transform.SetTransform(GetTransform(entity.GetTransform()));

auto coverage = GetLocalCoverage(entity_with_local_transform);
Expand Down
3 changes: 2 additions & 1 deletion impeller/entity/contents/filters/filter_contents.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "impeller/core/formats.h"
#include "impeller/entity/contents/filters/inputs/filter_input.h"
#include "impeller/entity/entity.h"
#include "impeller/geometry/matrix.h"
#include "impeller/geometry/sigma.h"

namespace impeller {
Expand Down Expand Up @@ -230,7 +231,7 @@ class FilterContents : public Contents {
std::optional<Rect> GetLocalCoverage(const Entity& local_entity) const;

FilterInput::Vector inputs_;
Matrix effect_transform_;
Matrix effect_transform_ = Matrix();

FilterContents(const FilterContents&) = delete;

Expand Down
2 changes: 1 addition & 1 deletion impeller/entity/contents/filters/inputs/filter_input.cc
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ Matrix FilterInput::GetLocalTransform(const Entity& entity) const {
}

std::optional<Rect> FilterInput::GetLocalCoverage(const Entity& entity) const {
Entity local_entity = entity;
Entity local_entity = entity.Clone();
local_entity.SetTransform(GetLocalTransform(entity));
return GetCoverage(local_entity);
}
Expand Down
4 changes: 4 additions & 0 deletions impeller/entity/entity.cc
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,10 @@ Capture& Entity::GetCapture() const {
return capture_;
}

Entity Entity::Clone() const {
return Entity(*this);
}

void Entity::SetCapture(Capture capture) const {
capture_ = std::move(capture);
}
Expand Down
Loading

0 comments on commit 0924225

Please sign in to comment.