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.

Sign up for GitHub

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

[engine] changes to DlVertices::Builder and Stopwatch visualizer. #57031

Merged
merged 3 commits into from
Dec 11, 2024
Merged

[engine] changes to DlVertices::Builder and Stopwatch visualizer. #57031

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
48806e0
[engine] changes to DlVertices::Builder and Stopwatch visualizer.
jonahwilliams Dec 6, 2024
ca1cf8c
Merge branch 'main' of github.com:flutter/engine into vertices_and_vi…
jonahwilliams Dec 11, 2024
c87a3cb
jim feedback.
jonahwilliams Dec 11, 2024
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
58 changes: 35 additions & 23 deletions display_list/dl_vertices.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,8 @@ std::shared_ptr<DlVertices> DlVertices::Make(
const SkPoint texture_coordinates[],
const DlColor colors[],
int index_count,
const uint16_t indices[]) {
const uint16_t indices[],
const DlRect* bounds) {
if (!vertices || vertex_count <= 0) {
vertex_count = 0;
texture_coordinates = nullptr;
Expand Down Expand Up @@ -75,6 +76,9 @@ std::shared_ptr<DlVertices> DlVertices::Make(
if (indices) {
builder.store_indices(indices);
}
if (bounds != nullptr) {
builder.store_bounds(*bounds);
}

return builder.build();
}
Expand Down Expand Up @@ -223,53 +227,53 @@ static void store_points(char* dst, int offset, const float* src, int count) {
}

void DlVertices::Builder::store_vertices(const SkPoint vertices[]) {
FML_CHECK(is_valid());
FML_CHECK(needs_vertices_);
FML_DCHECK(is_valid());
FML_DCHECK(needs_vertices_);
char* pod = reinterpret_cast<char*>(vertices_.get());
size_t bytes = vertices_->vertex_count_ * sizeof(vertices[0]);
memcpy(pod + vertices_->vertices_offset_, vertices, bytes);
needs_vertices_ = false;
}

void DlVertices::Builder::store_vertices(const float vertices[]) {
FML_CHECK(is_valid());
FML_CHECK(needs_vertices_);
FML_DCHECK(is_valid());
FML_DCHECK(needs_vertices_);
char* pod = reinterpret_cast<char*>(vertices_.get());
store_points(pod, vertices_->vertices_offset_, vertices,
vertices_->vertex_count_);
needs_vertices_ = false;
}

void DlVertices::Builder::store_texture_coordinates(const SkPoint coords[]) {
FML_CHECK(is_valid());
FML_CHECK(needs_texture_coords_);
FML_DCHECK(is_valid());
FML_DCHECK(needs_texture_coords_);
char* pod = reinterpret_cast<char*>(vertices_.get());
size_t bytes = vertices_->vertex_count_ * sizeof(coords[0]);
memcpy(pod + vertices_->texture_coordinates_offset_, coords, bytes);
needs_texture_coords_ = false;
}

void DlVertices::Builder::store_texture_coordinates(const float coords[]) {
FML_CHECK(is_valid());
FML_CHECK(needs_texture_coords_);
FML_DCHECK(is_valid());
FML_DCHECK(needs_texture_coords_);
char* pod = reinterpret_cast<char*>(vertices_.get());
store_points(pod, vertices_->texture_coordinates_offset_, coords,
vertices_->vertex_count_);
needs_texture_coords_ = false;
}

void DlVertices::Builder::store_colors(const DlColor colors[]) {
FML_CHECK(is_valid());
FML_CHECK(needs_colors_);
FML_DCHECK(is_valid());
FML_DCHECK(needs_colors_);
char* pod = reinterpret_cast<char*>(vertices_.get());
size_t bytes = vertices_->vertex_count_ * sizeof(colors[0]);
memcpy(pod + vertices_->colors_offset_, colors, bytes);
needs_colors_ = false;
}

void DlVertices::Builder::store_colors(const uint32_t colors[]) {
FML_CHECK(is_valid());
FML_CHECK(needs_colors_);
FML_DCHECK(is_valid());
FML_DCHECK(needs_colors_);
char* pod = reinterpret_cast<char*>(vertices_.get());
DlColor* dlcolors_ptr =
reinterpret_cast<DlColor*>(pod + vertices_->colors_offset_);
Expand All @@ -280,29 +284,37 @@ void DlVertices::Builder::store_colors(const uint32_t colors[]) {
}

void DlVertices::Builder::store_indices(const uint16_t indices[]) {
FML_CHECK(is_valid());
FML_CHECK(needs_indices_);
FML_DCHECK(is_valid());
FML_DCHECK(needs_indices_);
char* pod = reinterpret_cast<char*>(vertices_.get());
size_t bytes = vertices_->index_count_ * sizeof(indices[0]);
memcpy(pod + vertices_->indices_offset_, indices, bytes);
needs_indices_ = false;
}

void DlVertices::Builder::store_bounds(DlRect bounds) {
vertices_->bounds_ = SkRect::MakeLTRB(bounds.GetLeft(), bounds.GetTop(),
bounds.GetRight(), bounds.GetBottom());
needs_bounds_ = false;
}

std::shared_ptr<DlVertices> DlVertices::Builder::build() {
FML_CHECK(is_valid());
FML_DCHECK(is_valid());
if (vertices_->vertex_count() <= 0) {
// We set this to true in the constructor to make sure that they
// call store_vertices() only once, but if there are no vertices
// then we will not object to them never having stored any vertices
needs_vertices_ = false;
}
FML_CHECK(!needs_vertices_);
FML_CHECK(!needs_texture_coords_);
FML_CHECK(!needs_colors_);
FML_CHECK(!needs_indices_);

vertices_->bounds_ =
compute_bounds(vertices_->vertices(), vertices_->vertex_count_);
FML_DCHECK(!needs_vertices_);
FML_DCHECK(!needs_texture_coords_);
FML_DCHECK(!needs_colors_);
FML_DCHECK(!needs_indices_);

if (needs_bounds_) {
vertices_->bounds_ =
compute_bounds(vertices_->vertices(), vertices_->vertex_count_);
}

return std::move(vertices_);
}
Expand Down
7 changes: 6 additions & 1 deletion display_list/dl_vertices.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,9 @@ class DlVertices {
/// promised by (index_count > 0).
void store_indices(const uint16_t indices[]);

/// @brief Overwrite the internal bounds with a precomputed bounding rect.
void store_bounds(DlRect bounds);

/// @brief Finalizes and the constructed DlVertices object.
///
/// fails if any of the optional data promised in the constructor is
Expand All @@ -167,6 +170,7 @@ class DlVertices {
bool needs_texture_coords_;
bool needs_colors_;
bool needs_indices_;
bool needs_bounds_ = true;
};

//--------------------------------------------------------------------------
Expand All @@ -183,7 +187,8 @@ class DlVertices {
const SkPoint texture_coordinates[],
const DlColor colors[],
int index_count = 0,
const uint16_t indices[] = nullptr);
const uint16_t indices[] = nullptr,
const DlRect* bounds = nullptr);

/// Returns the size of the object including all of the inlined data.
size_t size() const;
Expand Down
55 changes: 32 additions & 23 deletions flow/layers/performance_overlay_layer.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,26 +34,25 @@ void VisualizeStopWatch(DlCanvas* canvas,
bool show_graph,
bool show_labels,
const std::string& label_prefix,
const std::string& font_path) {
std::vector<DlPoint>& point_storage,
std::vector<DlColor>& color_storage,
const SkFont& font) {
const int label_x = 8; // distance from x
const int label_y = -10; // distance from y+height

if (show_graph) {
SkRect visualization_rect = SkRect::MakeXYWH(x, y, width, height);
std::unique_ptr<StopwatchVisualizer> visualizer;

DlRect visualization_rect = DlRect::MakeXYWH(x, y, width, height);
if (impeller_enabled) {
visualizer = std::make_unique<DlStopwatchVisualizer>(stopwatch);
DlStopwatchVisualizer(stopwatch, point_storage, color_storage)
.Visualize(canvas, visualization_rect);
} else {
visualizer = std::make_unique<SkStopwatchVisualizer>(stopwatch);
SkStopwatchVisualizer(stopwatch).Visualize(canvas, visualization_rect);
}

visualizer->Visualize(canvas, visualization_rect);
}

if (show_labels) {
auto text = PerformanceOverlayLayer::MakeStatisticsText(
stopwatch, label_prefix, font_path);
auto text = PerformanceOverlayLayer::MakeStatisticsText(stopwatch, font,
label_prefix);
// Historically SK_ColorGRAY (== 0xFF888888) was used here
DlPaint paint(DlColor(0xFF888888));
#ifdef IMPELLER_SUPPORTS_RENDERING
Expand All @@ -69,24 +68,28 @@ void VisualizeStopWatch(DlCanvas* canvas,

} // namespace

sk_sp<SkTextBlob> PerformanceOverlayLayer::MakeStatisticsText(
const Stopwatch& stopwatch,
const std::string& label_prefix,
const std::string& font_path) {
SkFont font;
// static
SkFont PerformanceOverlayLayer::MakeStatisticsFont(std::string_view font_path) {
sk_sp<SkFontMgr> font_mgr = txt::GetDefaultFontManager();
if (font_path == "") {
if (sk_sp<SkTypeface> face = font_mgr->matchFamilyStyle(nullptr, {})) {
font = SkFont(face, 15);
return SkFont(face, 15);
} else {
// In Skia's Android fontmgr, matchFamilyStyle can return null instead
// of falling back to a default typeface. If that's the case, we can use
// legacyMakeTypeface, which *does* use that default typeface.
font = SkFont(font_mgr->legacyMakeTypeface(nullptr, {}), 15);
return SkFont(font_mgr->legacyMakeTypeface(nullptr, {}), 15);
}
} else {
font = SkFont(font_mgr->makeFromFile(font_path.c_str()), 15);
return SkFont(font_mgr->makeFromFile(font_path.data()), 15);
}
}

// static
sk_sp<SkTextBlob> PerformanceOverlayLayer::MakeStatisticsText(
const Stopwatch& stopwatch,
const SkFont& font,
std::string_view label_prefix) {
// Make sure there's not an empty typeface returned, or we won't see any text.
FML_DCHECK(font.getTypeface()->countGlyphs() > 0);

Expand Down Expand Up @@ -134,16 +137,22 @@ void PerformanceOverlayLayer::Paint(PaintContext& context) const {
SkScalar width = paint_bounds().width() - (padding * 2);
SkScalar height = paint_bounds().height() / 2;
auto mutator = context.state_stack.save();
// Cached storage for vertex output.
std::vector<DlPoint> vertices_storage;
std::vector<DlColor> color_storage;
Copy link
Contributor

Choose a reason for hiding this comment

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

Weren't these supposed to be cached somewhere? It looks like they are recreated on every frame?

Copy link
Contributor

Choose a reason for hiding this comment

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

Or is this just re-use between the 2 graphs?

Copy link
Member Author

Choose a reason for hiding this comment

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

Just between the two graphs. originally I was much more ambitious but it turns out that we rebuild the layer each frame so there isn't a good place to put the storage.

SkFont font = MakeStatisticsFont(font_path_);

VisualizeStopWatch(
context.canvas, context.impeller_enabled, context.raster_time, x, y,
width, height - padding, options_ & kVisualizeRasterizerStatistics,
options_ & kDisplayRasterizerStatistics, "Raster", font_path_);
VisualizeStopWatch(context.canvas, context.impeller_enabled,
context.raster_time, x, y, width, height - padding,
options_ & kVisualizeRasterizerStatistics,
options_ & kDisplayRasterizerStatistics, "Raster",
vertices_storage, color_storage, font);

VisualizeStopWatch(context.canvas, context.impeller_enabled, context.ui_time,
x, y + height, width, height - padding,
options_ & kVisualizeEngineStatistics,
options_ & kDisplayEngineStatistics, "UI", font_path_);
options_ & kDisplayEngineStatistics, "UI",
vertices_storage, color_storage, font);
}

} // namespace flutter
6 changes: 4 additions & 2 deletions flow/layers/performance_overlay_layer.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,11 @@ const int kVisualizeEngineStatistics = 1 << 3;

class PerformanceOverlayLayer : public Layer {
public:
static SkFont MakeStatisticsFont(std::string_view font_path);

static sk_sp<SkTextBlob> MakeStatisticsText(const Stopwatch& stopwatch,
const std::string& label_prefix,
const std::string& font_path);
const SkFont& font,
std::string_view label_prefix);

bool IsReplacing(DiffContext* context, const Layer* layer) const override {
return layer->as_performance_overlay_layer() != nullptr;
Expand Down
3 changes: 2 additions & 1 deletion flow/layers/performance_overlay_layer_unittests.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,7 @@ TEST_F(PerformanceOverlayLayerTest, SimpleRasterizerStatistics) {
const SkRect layer_bounds = SkRect::MakeLTRB(0.0f, 0.0f, 64.0f, 64.0f);
const uint64_t overlay_opts = kDisplayRasterizerStatistics;
auto layer = std::make_shared<PerformanceOverlayLayer>(overlay_opts);
auto font = PerformanceOverlayLayer::MakeStatisticsFont("");

// TODO(): Note calling code has to call set_paint_bounds right now. Make
// this a constructor parameter and move the set_paint_bounds into Preroll
Expand All @@ -210,7 +211,7 @@ TEST_F(PerformanceOverlayLayerTest, SimpleRasterizerStatistics) {

layer->Paint(display_list_paint_context());
auto overlay_text = PerformanceOverlayLayer::MakeStatisticsText(
display_list_paint_context().raster_time, "Raster", "");
display_list_paint_context().raster_time, font, "Raster");
auto overlay_text_data = overlay_text->serialize(SkSerialProcs{});
// Historically SK_ColorGRAY (== 0xFF888888) was used here
DlPaint text_paint(DlColor(0xFF888888));
Expand Down
2 changes: 1 addition & 1 deletion flow/stopwatch.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ class StopwatchVisualizer {
///
/// @param canvas The canvas to draw on.
/// @param[in] rect The rectangle to draw in.
virtual void Visualize(DlCanvas* canvas, const SkRect& rect) const = 0;
virtual void Visualize(DlCanvas* canvas, const DlRect& rect) const = 0;

FML_DISALLOW_COPY_AND_ASSIGN(StopwatchVisualizer);

Expand Down
Loading
Loading