[Impeller] dont unnecessarily copy point data out of display list. (flutter/engine#56492)

Display list now stores impeller::Point objects, so have the PointFieldGeometry reference these points directly. As the dispatching/recording is immediate, there is no need to copy to secondary storage.
This commit is contained in:
Jonah Williams 2024-11-11 13:07:11 -08:00 committed by GitHub
parent 0489b6bbab
commit c93a57dfd0
8 changed files with 49 additions and 48 deletions

View File

@ -638,7 +638,8 @@ void Canvas::ClipGeometry(const Geometry& geometry,
clip_depth);
}
void Canvas::DrawPoints(std::vector<Point> points,
void Canvas::DrawPoints(const Point points[],
uint32_t count,
Scalar radius,
const Paint& paint,
PointStyle point_style) {
@ -650,7 +651,7 @@ void Canvas::DrawPoints(std::vector<Point> points,
entity.SetTransform(GetCurrentTransform());
entity.SetBlendMode(paint.blend_mode);
PointFieldGeometry geom(std::move(points), radius,
PointFieldGeometry geom(points, count, radius,
/*round=*/point_style == PointStyle::kRound);
AddRenderEntityWithFiltersToCurrentPass(entity, &geom, paint);
}

View File

@ -200,7 +200,8 @@ class Canvas {
void DrawCircle(const Point& center, Scalar radius, const Paint& paint);
void DrawPoints(std::vector<Point> points,
void DrawPoints(const Point points[],
uint32_t count,
Scalar radius,
const Paint& paint,
PointStyle point_style);

View File

@ -27,7 +27,6 @@
#include "impeller/entity/entity.h"
#include "impeller/entity/geometry/ellipse_geometry.h"
#include "impeller/entity/geometry/fill_path_geometry.h"
#include "impeller/entity/geometry/geometry.h"
#include "impeller/entity/geometry/rect_geometry.h"
#include "impeller/entity/geometry/round_rect_geometry.h"
#include "impeller/geometry/color.h"
@ -663,14 +662,14 @@ void DlDispatcherBase::drawPoints(PointMode mode,
switch (mode) {
case flutter::DlCanvas::PointMode::kPoints: {
// Cap::kButt is also treated as a square.
auto point_style = paint.stroke_cap == Cap::kRound ? PointStyle::kRound
PointStyle point_style = paint.stroke_cap == Cap::kRound
? PointStyle::kRound
: PointStyle::kSquare;
auto radius = paint.stroke_width;
Scalar radius = paint.stroke_width;
if (radius > 0) {
radius /= 2.0;
}
GetCanvas().DrawPoints(skia_conversions::ToPoints(points, count), radius,
paint, point_style);
GetCanvas().DrawPoints(points, count, radius, paint, point_style);
} break;
case flutter::DlCanvas::PointMode::kLines:
for (uint32_t i = 1; i < count; i += 2) {

View File

@ -35,6 +35,7 @@
#include "impeller/entity/entity.h"
#include "impeller/entity/entity_playground.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/entity/geometry/superellipse_geometry.h"
#include "impeller/geometry/color.h"
@ -2095,9 +2096,9 @@ TEST_P(EntityTest, TiledTextureContentsIsOpaque) {
TEST_P(EntityTest, PointFieldGeometryCoverage) {
std::vector<Point> points = {{10, 20}, {100, 200}};
auto geometry = Geometry::MakePointField(points, 5.0, false);
ASSERT_EQ(*geometry->GetCoverage(Matrix()), Rect::MakeLTRB(5, 15, 105, 205));
ASSERT_EQ(*geometry->GetCoverage(Matrix::MakeTranslation({30, 0, 0})),
PointFieldGeometry geometry(points.data(), 2, 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));
}

View File

@ -13,7 +13,6 @@
#include "impeller/entity/geometry/ellipse_geometry.h"
#include "impeller/entity/geometry/fill_path_geometry.h"
#include "impeller/entity/geometry/line_geometry.h"
#include "impeller/entity/geometry/point_field_geometry.h"
#include "impeller/entity/geometry/rect_geometry.h"
#include "impeller/entity/geometry/round_rect_geometry.h"
#include "impeller/entity/geometry/stroke_path_geometry.h"
@ -63,12 +62,6 @@ 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::MakeStrokePath(const Path& path,
Scalar stroke_width,
Scalar miter_limit,

View File

@ -83,10 +83,6 @@ class Geometry {
static std::unique_ptr<Geometry> MakeRoundRect(const Rect& rect,
const Size& radii);
static std::unique_ptr<Geometry> MakePointField(std::vector<Point> points,
Scalar radius,
bool round);
virtual GeometryResult GetPositionBuffer(const ContentContext& renderer,
const Entity& entity,
RenderPass& pass) const = 0;

View File

@ -12,10 +12,14 @@
namespace impeller {
PointFieldGeometry::PointFieldGeometry(std::vector<Point> points,
PointFieldGeometry::PointFieldGeometry(const Point* points,
size_t point_count,
Scalar radius,
bool round)
: points_(std::move(points)), radius_(radius), round_(round) {}
: point_count_(point_count),
radius_(radius),
round_(round),
points_(points) {}
PointFieldGeometry::~PointFieldGeometry() = default;
@ -23,7 +27,7 @@ GeometryResult PointFieldGeometry::GetPositionBuffer(
const ContentContext& renderer,
const Entity& entity,
RenderPass& pass) const {
if (radius_ < 0.0 || points_.empty()) {
if (radius_ < 0.0 || point_count_ == 0) {
return {};
}
const Matrix& transform = entity.GetTransform();
@ -52,7 +56,7 @@ GeometryResult PointFieldGeometry::GetPositionBuffer(
});
FML_DCHECK(circle_vertices.size() == generator.GetVertexCount());
vertex_count = (circle_vertices.size() + 2) * points_.size() - 2;
vertex_count = (circle_vertices.size() + 2) * point_count_ - 2;
buffer_view = host_buffer.Emplace(
vertex_count * sizeof(Point), alignof(Point), [&](uint8_t* data) {
Point* output = reinterpret_cast<Point*>(data);
@ -66,7 +70,7 @@ GeometryResult PointFieldGeometry::GetPositionBuffer(
// the strip. This could be optimized out if we switched to using
// primitive restart.
Point last_point = circle_vertices.back() + center;
for (size_t i = 1; i < points_.size(); i++) {
for (size_t i = 1; i < point_count_; i++) {
Point center = points_[i];
output[offset++] = last_point;
output[offset++] = Point(center + circle_vertices[0]);
@ -77,7 +81,7 @@ GeometryResult PointFieldGeometry::GetPositionBuffer(
}
});
} else {
vertex_count = 6 * points_.size() - 2;
vertex_count = 6 * point_count_ - 2;
buffer_view = host_buffer.Emplace(
vertex_count * sizeof(Point), alignof(Point), [&](uint8_t* data) {
Point* output = reinterpret_cast<Point*>(data);
@ -97,7 +101,7 @@ GeometryResult PointFieldGeometry::GetPositionBuffer(
// For all subequent points, insert a degenerate triangle to break
// the strip. This could be optimized out if we switched to using
// primitive restart.
for (size_t i = 1; i < points_.size(); i++) {
for (size_t i = 1; i < point_count_; i++) {
Point point = points_[i];
Point first = Point(point.x - radius, point.y - radius);
@ -129,22 +133,21 @@ GeometryResult PointFieldGeometry::GetPositionBuffer(
// |Geometry|
std::optional<Rect> PointFieldGeometry::GetCoverage(
const Matrix& transform) const {
if (points_.size() > 0) {
if (point_count_ > 0) {
// Doesn't use MakePointBounds as this isn't resilient to points that
// all lie along the same axis.
auto first = points_.begin();
auto last = points_.end();
auto left = first->x;
auto top = first->y;
auto right = first->x;
auto bottom = first->y;
for (auto it = first + 1; it < last; ++it) {
left = std::min(left, it->x);
top = std::min(top, it->y);
right = std::max(right, it->x);
bottom = std::max(bottom, it->y);
Scalar left = points_[0].x;
Scalar top = points_[0].y;
Scalar right = points_[0].x;
Scalar bottom = points_[0].y;
for (auto i = 1u; i < point_count_; i++) {
const Point point = points_[i];
left = std::min(left, point.x);
top = std::min(top, point.y);
right = std::max(right, point.x);
bottom = std::max(bottom, point.y);
}
auto coverage = Rect::MakeLTRB(left - radius_, top - radius_,
Rect coverage = Rect::MakeLTRB(left - radius_, top - radius_,
right + radius_, bottom + radius_);
return coverage.TransformBounds(transform);
}

View File

@ -10,24 +10,31 @@
namespace impeller {
/// @brief A geometry class specialized for Canvas::DrawPoints.
///
/// Does not hold ownership of the allocated point data, which is expected to be
/// maintained via the display list structure.
class PointFieldGeometry final : public Geometry {
public:
PointFieldGeometry(std::vector<Point> points, Scalar radius, bool round);
PointFieldGeometry(const Point* points,
size_t point_count,
Scalar radius,
bool round);
~PointFieldGeometry() override;
// |Geometry|
std::optional<Rect> GetCoverage(const Matrix& transform) const override;
private:
// |Geometry|
GeometryResult GetPositionBuffer(const ContentContext& renderer,
const Entity& entity,
RenderPass& pass) const override;
// |Geometry|
std::optional<Rect> GetCoverage(const Matrix& transform) const override;
std::vector<Point> points_;
size_t point_count_;
Scalar radius_;
bool round_;
const Point* points_;
PointFieldGeometry(const PointFieldGeometry&) = delete;