Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

[Impeller] Faster path for circle geometry, rename compute path geometry. #47845

Closed
jonahwilliams wants to merge 9 commits into from
Closed

[Impeller] Faster path for circle geometry, rename compute path geometry. #47845

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
4ada719
[Impeller] faster path for drawCircle.
Nov 9, 2023
4fa541e
++
Nov 9, 2023
28eed8a
++
Nov 9, 2023
4ecec73
++
Nov 9, 2023
674a1d0
++
Nov 9, 2023
c696eb9
++
Nov 9, 2023
b57a6da
Revert "++"
Nov 10, 2023
f4a83ca
Revert "++"
Nov 10, 2023
ff999e3
Revert "++"
Nov 10, 2023
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
8 changes: 4 additions & 4 deletions ci/licenses_golden/licenses_flutter
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3118,14 +3118,14 @@ ORIGIN: ../../../flutter/impeller/entity/entity_pass_target.cc + ../../../flutte
ORIGIN: ../../../flutter/impeller/entity/entity_pass_target.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/impeller/entity/entity_playground.cc + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/impeller/entity/entity_playground.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/impeller/entity/geometry/circle_geometry.cc + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/impeller/entity/geometry/circle_geometry.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/impeller/entity/geometry/cover_geometry.cc + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/impeller/entity/geometry/cover_geometry.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/impeller/entity/geometry/fill_path_geometry.cc + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/impeller/entity/geometry/fill_path_geometry.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/impeller/entity/geometry/geometry.cc + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/impeller/entity/geometry/geometry.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/impeller/entity/geometry/point_field_geometry.cc + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/impeller/entity/geometry/point_field_geometry.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/impeller/entity/geometry/rect_geometry.cc + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/impeller/entity/geometry/rect_geometry.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/impeller/entity/geometry/stroke_path_geometry.cc + ../../../flutter/LICENSE
Expand Down Expand Up @@ -5875,14 +5875,14 @@ FILE: ../../../flutter/impeller/entity/entity_pass_target.cc
FILE: ../../../flutter/impeller/entity/entity_pass_target.h
FILE: ../../../flutter/impeller/entity/entity_playground.cc
FILE: ../../../flutter/impeller/entity/entity_playground.h
FILE: ../../../flutter/impeller/entity/geometry/circle_geometry.cc
FILE: ../../../flutter/impeller/entity/geometry/circle_geometry.h
FILE: ../../../flutter/impeller/entity/geometry/cover_geometry.cc
FILE: ../../../flutter/impeller/entity/geometry/cover_geometry.h
FILE: ../../../flutter/impeller/entity/geometry/fill_path_geometry.cc
FILE: ../../../flutter/impeller/entity/geometry/fill_path_geometry.h
FILE: ../../../flutter/impeller/entity/geometry/geometry.cc
FILE: ../../../flutter/impeller/entity/geometry/geometry.h
FILE: ../../../flutter/impeller/entity/geometry/point_field_geometry.cc
FILE: ../../../flutter/impeller/entity/geometry/point_field_geometry.h
FILE: ../../../flutter/impeller/entity/geometry/rect_geometry.cc
FILE: ../../../flutter/impeller/entity/geometry/rect_geometry.h
FILE: ../../../flutter/impeller/entity/geometry/stroke_path_geometry.cc
Expand Down
16 changes: 14 additions & 2 deletions impeller/aiks/canvas.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,18 @@ void Canvas::DrawCircle(Point center, Scalar radius, const Paint& paint) {
paint)) {
return;
}
if (paint.style == Paint::Style::kFill) {
Entity entity;
entity.SetTransformation(GetCurrentTransformation());
entity.SetClipDepth(GetClipDepth());
entity.SetBlendMode(paint.blend_mode);
entity.SetContents(paint.WithFilters(paint.CreateContentsForGeometry(
Geometry::MakeCircle({center}, radius, true))));

GetCurrentPass().AddEntity(entity);
return;
}

auto circle_path =
PathBuilder{}
.AddCircle(center, radius)
Expand Down Expand Up @@ -434,8 +446,8 @@ void Canvas::DrawPoints(std::vector<Point> points,
entity.SetClipDepth(GetClipDepth());
entity.SetBlendMode(paint.blend_mode);
entity.SetContents(paint.WithFilters(paint.CreateContentsForGeometry(
Geometry::MakePointField(std::move(points), radius,
/*round=*/point_style == PointStyle::kRound))));
Geometry::MakeCircle(std::move(points), radius,
/*round=*/point_style == PointStyle::kRound))));

GetCurrentPass().AddEntity(entity);
}
Expand Down
4 changes: 2 additions & 2 deletions impeller/entity/BUILD.gn
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,14 +188,14 @@ impeller_component("entity") {
"entity_pass_delegate.h",
"entity_pass_target.cc",
"entity_pass_target.h",
"geometry/circle_geometry.cc",
"geometry/circle_geometry.h",
"geometry/cover_geometry.cc",
"geometry/cover_geometry.h",
"geometry/fill_path_geometry.cc",
"geometry/fill_path_geometry.h",
"geometry/geometry.cc",
"geometry/geometry.h",
"geometry/point_field_geometry.cc",
"geometry/point_field_geometry.h",
"geometry/rect_geometry.cc",
"geometry/rect_geometry.h",
"geometry/stroke_path_geometry.cc",
Expand Down
34 changes: 17 additions & 17 deletions impeller/entity/entity_unittests.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@
#include "impeller/entity/entity_pass.h"
#include "impeller/entity/entity_pass_delegate.h"
#include "impeller/entity/entity_playground.h"
#include "impeller/entity/geometry/circle_geometry.h"
#include "impeller/entity/geometry/geometry.h"
#include "impeller/entity/geometry/point_field_geometry.h"
#include "impeller/entity/geometry/stroke_path_geometry.h"
#include "impeller/geometry/color.h"
#include "impeller/geometry/geometry_asserts.h"
Expand Down Expand Up @@ -2390,27 +2390,27 @@ TEST_P(EntityTest, TessellateConvex) {
}
}

TEST_P(EntityTest, PointFieldGeometryDivisions) {
TEST_P(EntityTest, CircleGeometryDivisions) {
// Square always gives 4 divisions.
ASSERT_EQ(PointFieldGeometry::ComputeCircleDivisions(24.0, false), 4u);
ASSERT_EQ(PointFieldGeometry::ComputeCircleDivisions(2.0, false), 4u);
ASSERT_EQ(PointFieldGeometry::ComputeCircleDivisions(200.0, false), 4u);

ASSERT_EQ(PointFieldGeometry::ComputeCircleDivisions(0.5, true), 4u);
ASSERT_EQ(PointFieldGeometry::ComputeCircleDivisions(1.5, true), 8u);
ASSERT_EQ(PointFieldGeometry::ComputeCircleDivisions(5.5, true), 24u);
ASSERT_EQ(PointFieldGeometry::ComputeCircleDivisions(12.5, true), 34u);
ASSERT_EQ(PointFieldGeometry::ComputeCircleDivisions(22.3, true), 22u);
ASSERT_EQ(PointFieldGeometry::ComputeCircleDivisions(40.5, true), 40u);
ASSERT_EQ(PointFieldGeometry::ComputeCircleDivisions(100.0, true), 100u);
ASSERT_EQ(CircleGeometry::ComputeCircleDivisions(24.0, false), 4u);
ASSERT_EQ(CircleGeometry::ComputeCircleDivisions(2.0, false), 4u);
ASSERT_EQ(CircleGeometry::ComputeCircleDivisions(200.0, false), 4u);

ASSERT_EQ(CircleGeometry::ComputeCircleDivisions(0.5, true), 4u);
ASSERT_EQ(CircleGeometry::ComputeCircleDivisions(1.5, true), 8u);
ASSERT_EQ(CircleGeometry::ComputeCircleDivisions(5.5, true), 24u);
ASSERT_EQ(CircleGeometry::ComputeCircleDivisions(12.5, true), 34u);
ASSERT_EQ(CircleGeometry::ComputeCircleDivisions(22.3, true), 22u);
ASSERT_EQ(CircleGeometry::ComputeCircleDivisions(40.5, true), 40u);
ASSERT_EQ(CircleGeometry::ComputeCircleDivisions(100.0, true), 100u);
// Caps at 140.
ASSERT_EQ(PointFieldGeometry::ComputeCircleDivisions(1000.0, true), 140u);
ASSERT_EQ(PointFieldGeometry::ComputeCircleDivisions(20000.0, true), 140u);
ASSERT_EQ(CircleGeometry::ComputeCircleDivisions(1000.0, true), 140u);
ASSERT_EQ(CircleGeometry::ComputeCircleDivisions(20000.0, true), 140u);
}

TEST_P(EntityTest, PointFieldGeometryCoverage) {
TEST_P(EntityTest, CircleGeometryCoverage) {
std::vector<Point> points = {{10, 20}, {100, 200}};
auto geometry = Geometry::MakePointField(points, 5.0, false);
auto geometry = Geometry::MakeCircle(points, 5.0, false);
ASSERT_EQ(*geometry->GetCoverage(Matrix()), Rect::MakeLTRB(5, 15, 105, 205));
ASSERT_EQ(*geometry->GetCoverage(Matrix::MakeTranslation({30, 0, 0})),
Rect::MakeLTRB(35, 15, 135, 205));
Expand Down
119 changes: 91 additions & 28 deletions ...r/entity/geometry/point_field_geometry.cc → impeller/entity/geometry/circle_geometry.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,54 +2,54 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "impeller/entity/geometry/point_field_geometry.h"
#include "impeller/entity/geometry/circle_geometry.h"

#include "impeller/renderer/command_buffer.h"
#include "impeller/renderer/compute_command.h"

namespace impeller {

PointFieldGeometry::PointFieldGeometry(std::vector<Point> points,
Scalar radius,
bool round)
// The minimum number of points before compute is used to fill out the geometry.
// This number was arbitrarily chosen.
static constexpr size_t kMinComputeSize = 32;

CircleGeometry::CircleGeometry(std::vector<Point> points,
Scalar radius,
bool round)
: points_(std::move(points)), radius_(radius), round_(round) {}

PointFieldGeometry::~PointFieldGeometry() = default;
CircleGeometry::~CircleGeometry() = default;

GeometryResult PointFieldGeometry::GetPositionBuffer(
const ContentContext& renderer,
const Entity& entity,
RenderPass& pass) {
if (renderer.GetDeviceCapabilities().SupportsCompute()) {
GeometryResult CircleGeometry::GetPositionBuffer(const ContentContext& renderer,
const Entity& entity,
RenderPass& pass) {
if (points_.size() > kMinComputeSize &&
renderer.GetDeviceCapabilities().SupportsCompute()) {
return GetPositionBufferGPU(renderer, entity, pass);
}
auto vtx_builder = GetPositionBufferCPU(renderer, entity, pass);
if (!vtx_builder.has_value()) {
return {};
}

auto& host_buffer = pass.GetTransientsBuffer();
return {
.type = PrimitiveType::kTriangle,
.vertex_buffer = vtx_builder->CreateVertexBuffer(host_buffer),
.vertex_buffer = GetPositionBufferCPU(renderer, entity, pass),
.transform = Matrix::MakeOrthographic(pass.GetRenderTargetSize()) *
entity.GetTransformation(),
.prevent_overdraw = false,
};
}

GeometryResult PointFieldGeometry::GetPositionUVBuffer(
GeometryResult CircleGeometry::GetPositionUVBuffer(
Rect texture_coverage,
Matrix effect_transform,
const ContentContext& renderer,
const Entity& entity,
RenderPass& pass) {
if (renderer.GetDeviceCapabilities().SupportsCompute()) {
if (points_.size() > kMinComputeSize &&
renderer.GetDeviceCapabilities().SupportsCompute()) {
return GetPositionBufferGPU(renderer, entity, pass, texture_coverage,
effect_transform);
}

auto vtx_builder = GetPositionBufferCPU(renderer, entity, pass);
auto vtx_builder = GetPositionUVBufferCPU(renderer, entity, pass);
if (!vtx_builder.has_value()) {
return {};
}
Expand All @@ -66,10 +66,74 @@ GeometryResult PointFieldGeometry::GetPositionUVBuffer(
};
}

VertexBuffer CircleGeometry::GetPositionBufferCPU(
const ContentContext& renderer,
const Entity& entity,
RenderPass& pass) {
if (radius_ < 0.0) {
return {};
}
auto determinant = entity.GetTransformation().GetDeterminant();
if (determinant == 0) {
return {};
}

Scalar min_size = 1.0f / sqrt(std::abs(determinant));
Copy link
Contributor

@flar flar Nov 10, 2023

Choose a reason for hiding this comment

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

min_size is appropriate for strokes (i.e. when this is created from DrawPoints), but not when we are filling an actual circle (i.e. when created from DrawCircle).

Scalar radius = std::max(radius_, min_size);

auto vertices_per_geom = ComputeCircleDivisions(
entity.GetTransformation().GetMaxBasisLength() * radius, round_);
auto points_per_circle = 3 + (vertices_per_geom - 3) * 3;
auto total = points_per_circle * points_.size();
auto radian_start = round_ ? 0.0f : 0.785398f;
auto radian_step = k2Pi / vertices_per_geom;

auto& host_buffer = pass.GetTransientsBuffer();

/// Precompute all relative points and angles for a fixed geometry size.
auto elapsed_angle = radian_start;
std::vector<Point> angle_table(vertices_per_geom);
for (auto i = 0u; i < vertices_per_geom; i++) {
angle_table[i] = Point(cos(elapsed_angle), sin(elapsed_angle)) * radius;
Copy link
Member

@bdero bdero Nov 9, 2023

Choose a reason for hiding this comment

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

Instead of generating a table for the whole circle, you could just generate points for the 0 to PI/2 range and reuse the table for all 4 quadrants.

elapsed_angle += radian_step;
}

auto vertex_buffer = host_buffer.Emplace(
total * sizeof(Point), alignof(Point), [&](uint8_t* buffer) {
Point* points = reinterpret_cast<Point*>(buffer);
auto offset = 0u;
for (auto i = 0u; i < points_.size(); i++) {
auto center = points_[i];

auto origin = center + angle_table[0];
points[offset++] = origin;

auto pt1 = center + angle_table[1];
points[offset++] = pt1;

auto pt2 = center + angle_table[2];
points[offset++] = pt2;

for (auto j = 0u; j < vertices_per_geom - 3; j++) {
points[offset++] = origin;
points[offset++] = pt2;
pt2 = center + angle_table[j + 3];
points[offset++] = pt2;
Copy link
Member

@bdero bdero Nov 9, 2023

Choose a reason for hiding this comment

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

You could reduce the verts by 1/3 by making this a triangle strip

Copy link
Contributor Author

@jonahwilliams jonahwilliams Nov 10, 2023

Choose a reason for hiding this comment

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

Ahh, I had a brain fart. The reason this wasn't working is that this code is also used for draw points, so I'd also need to insert padding to break the strip. I'd rather leave this as is for now.

}
}
});

return VertexBuffer{
.vertex_buffer = vertex_buffer,
.vertex_count = total,
.index_type = IndexType::kNone,
};
}

std::optional<VertexBufferBuilder<SolidFillVertexShader::PerVertexData>>
PointFieldGeometry::GetPositionBufferCPU(const ContentContext& renderer,
const Entity& entity,
RenderPass& pass) {
CircleGeometry::GetPositionUVBufferCPU(const ContentContext& renderer,
const Entity& entity,
RenderPass& pass) {
if (radius_ < 0.0) {
return std::nullopt;
}
Expand Down Expand Up @@ -122,7 +186,7 @@ PointFieldGeometry::GetPositionBufferCPU(const ContentContext& renderer,
return vtx_builder;
}

GeometryResult PointFieldGeometry::GetPositionBufferGPU(
GeometryResult CircleGeometry::GetPositionBufferGPU(
const ContentContext& renderer,
const Entity& entity,
RenderPass& pass,
Expand Down Expand Up @@ -241,8 +305,8 @@ GeometryResult PointFieldGeometry::GetPositionBufferGPU(
/// @brief Compute the number of vertices to divide each circle into.
///
/// @return the number of vertices.
size_t PointFieldGeometry::ComputeCircleDivisions(Scalar scaled_radius,
bool round) {
size_t CircleGeometry::ComputeCircleDivisions(Scalar scaled_radius,
bool round) {
if (!round) {
return 4;
}
Expand All @@ -265,13 +329,12 @@ size_t PointFieldGeometry::ComputeCircleDivisions(Scalar scaled_radius,
}

// |Geometry|
GeometryVertexType PointFieldGeometry::GetVertexType() const {
GeometryVertexType CircleGeometry::GetVertexType() const {
return GeometryVertexType::kPosition;
}

// |Geometry|
std::optional<Rect> PointFieldGeometry::GetCoverage(
const Matrix& transform) const {
std::optional<Rect> CircleGeometry::GetCoverage(const Matrix& transform) const {
if (points_.size() > 0) {
// Doesn't use MakePointBounds as this isn't resilient to points that
// all lie along the same axis.
Expand Down
20 changes: 12 additions & 8 deletions ...er/entity/geometry/point_field_geometry.h → impeller/entity/geometry/circle_geometry.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,11 @@

namespace impeller {

class PointFieldGeometry : public Geometry {
class CircleGeometry : public Geometry {
public:
PointFieldGeometry(std::vector<Point> points, Scalar radius, bool round);
CircleGeometry(std::vector<Point> points, Scalar radius, bool round);

~PointFieldGeometry();
~CircleGeometry();

static size_t ComputeCircleDivisions(Scalar scaled_radius, bool round);

Expand Down Expand Up @@ -42,18 +42,22 @@ class PointFieldGeometry : public Geometry {
std::optional<Rect> texture_coverage = std::nullopt,
std::optional<Matrix> effect_transform = std::nullopt);

VertexBuffer GetPositionBufferCPU(const ContentContext& renderer,
const Entity& entity,
RenderPass& pass);

std::optional<VertexBufferBuilder<SolidFillVertexShader::PerVertexData>>
GetPositionBufferCPU(const ContentContext& renderer,
const Entity& entity,
RenderPass& pass);
GetPositionUVBufferCPU(const ContentContext& renderer,
const Entity& entity,
RenderPass& pass);

std::vector<Point> points_;
Scalar radius_;
bool round_;

PointFieldGeometry(const PointFieldGeometry&) = delete;
CircleGeometry(const CircleGeometry&) = delete;

PointFieldGeometry& operator=(const PointFieldGeometry&) = delete;
CircleGeometry& operator=(const CircleGeometry&) = delete;
};

} // namespace impeller
10 changes: 5 additions & 5 deletions impeller/entity/geometry/geometry.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@

#include <optional>

#include "impeller/entity/geometry/circle_geometry.h"
#include "impeller/entity/geometry/cover_geometry.h"
#include "impeller/entity/geometry/fill_path_geometry.h"
#include "impeller/entity/geometry/point_field_geometry.h"
#include "impeller/entity/geometry/rect_geometry.h"
#include "impeller/entity/geometry/stroke_path_geometry.h"
#include "impeller/geometry/rect.h"
Expand Down Expand Up @@ -117,10 +117,10 @@ std::unique_ptr<Geometry> Geometry::MakeFillPath(
return std::make_unique<FillPathGeometry>(path, inner_rect);
}

std::unique_ptr<Geometry> Geometry::MakePointField(std::vector<Point> points,
Scalar radius,
bool round) {
return std::make_unique<PointFieldGeometry>(std::move(points), radius, round);
std::unique_ptr<Geometry> Geometry::MakeCircle(std::vector<Point> points,
Scalar radius,
bool round) {
return std::make_unique<CircleGeometry>(std::move(points), radius, round);
}

std::unique_ptr<Geometry> Geometry::MakeStrokePath(const Path& path,
Expand Down
Loading