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

Enable linting in flow/, and fml/ #20134

Merged
merged 1 commit into from
Aug 1, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
32 changes: 18 additions & 14 deletions flow/instrumentation.cc
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
// Copyright 2013 The Flutter Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
// FLUTTER_NOLINT

#include "flutter/flow/instrumentation.h"

Expand Down Expand Up @@ -52,16 +51,18 @@ double Stopwatch::UnitFrameInterval(double raster_time_ms) const {
double Stopwatch::UnitHeight(double raster_time_ms,
double max_unit_interval) const {
double unitHeight = UnitFrameInterval(raster_time_ms) / max_unit_interval;
if (unitHeight > 1.0)
if (unitHeight > 1.0) {
unitHeight = 1.0;
}
return unitHeight;
}

fml::TimeDelta Stopwatch::MaxDelta() const {
fml::TimeDelta max_delta;
for (size_t i = 0; i < kMaxSamples; i++) {
if (laps_[i] > max_delta)
if (laps_[i] > max_delta) {
max_delta = laps_[i];
}
}
return max_delta;
}
Expand Down Expand Up @@ -135,7 +136,7 @@ void Stopwatch::InitVisualizeSurface(const SkRect& rect) const {
cache_canvas->drawPath(path, paint);
}

void Stopwatch::Visualize(SkCanvas& canvas, const SkRect& rect) const {
void Stopwatch::Visualize(SkCanvas* canvas, const SkRect& rect) const {
// Initialize visualize cache if it has not yet been initialized.
InitVisualizeSurface(rect);

Expand Down Expand Up @@ -191,8 +192,9 @@ void Stopwatch::Visualize(SkCanvas& canvas, const SkRect& rect) const {

// Limit the number of markers displayed. After a certain point, the graph
// becomes crowded
if (frame_marker_count > kMaxFrameMarkers)
if (frame_marker_count > kMaxFrameMarkers) {
frame_marker_count = 1;
}

for (size_t frame_index = 0; frame_index < frame_marker_count;
frame_index++) {
Expand Down Expand Up @@ -224,7 +226,7 @@ void Stopwatch::Visualize(SkCanvas& canvas, const SkRect& rect) const {

// Draw the cached surface onto the output canvas.
paint.reset();
visualize_cache_surface_->draw(&canvas, rect.x(), rect.y(), &paint);
visualize_cache_surface_->draw(canvas, rect.x(), rect.y(), &paint);
}

CounterValues::CounterValues() : current_sample_(kMaxSamples - 1) {
Expand All @@ -238,7 +240,7 @@ void CounterValues::Add(int64_t value) {
values_[current_sample_] = value;
}

void CounterValues::Visualize(SkCanvas& canvas, const SkRect& rect) const {
void CounterValues::Visualize(SkCanvas* canvas, const SkRect& rect) const {
size_t max_bytes = GetMaxValue();

if (max_bytes == 0) {
Expand All @@ -252,7 +254,7 @@ void CounterValues::Visualize(SkCanvas& canvas, const SkRect& rect) const {

// Paint the background.
paint.setColor(0x99FFFFFF);
canvas.drawRect(rect, paint);
canvas->drawRect(rect, paint);

// Establish the graph position.
const SkScalar x = rect.x();
Expand All @@ -268,10 +270,12 @@ void CounterValues::Visualize(SkCanvas& canvas, const SkRect& rect) const {

for (size_t i = 0; i < kMaxSamples; ++i) {
int64_t current_bytes = values_[i];
double ratio =
(double)(current_bytes - min_bytes) / (max_bytes - min_bytes);
path.lineTo(x + (((double)(i) / (double)kMaxSamples) * width),
y + ((1.0 - ratio) * height));
double ratio = static_cast<double>(current_bytes - min_bytes) /
static_cast<double>(max_bytes - min_bytes);
path.lineTo(
x + ((static_cast<double>(i) / static_cast<double>(kMaxSamples)) *
width),
y + ((1.0 - ratio) * height));
}

path.rLineTo(100, 0);
Expand All @@ -280,7 +284,7 @@ void CounterValues::Visualize(SkCanvas& canvas, const SkRect& rect) const {

// Draw the graph.
paint.setColor(0xAA0000FF);
canvas.drawPath(path, paint);
canvas->drawPath(path, paint);

// Paint the vertical marker for the current frame.
const double sample_unit_width = (1.0 / kMaxSamples);
Expand All @@ -294,7 +298,7 @@ void CounterValues::Visualize(SkCanvas& canvas, const SkRect& rect) const {
const auto marker_rect = SkRect::MakeLTRB(
sample_x, y,
sample_x + width * sample_unit_width + sample_margin_width * 2, bottom);
canvas.drawRect(marker_rect, paint);
canvas->drawRect(marker_rect, paint);
}

int64_t CounterValues::GetCurrentValue() const {
Expand Down
4 changes: 2 additions & 2 deletions flow/instrumentation.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ class Stopwatch {

void InitVisualizeSurface(const SkRect& rect) const;

void Visualize(SkCanvas& canvas, const SkRect& rect) const;
void Visualize(SkCanvas* canvas, const SkRect& rect) const;

void Start();

Expand Down Expand Up @@ -81,7 +81,7 @@ class CounterValues {

void Add(int64_t value);

void Visualize(SkCanvas& canvas, const SkRect& rect) const;
void Visualize(SkCanvas* canvas, const SkRect& rect) const;

int64_t GetCurrentValue() const;

Expand Down
8 changes: 4 additions & 4 deletions flow/layers/layer_tree.cc
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
// Copyright 2013 The Flutter Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
// FLUTTER_NOLINT

#include "flutter/flow/layers/layer_tree.h"

Expand Down Expand Up @@ -117,7 +116,7 @@ void LayerTree::Paint(CompositorContext::ScopedFrame& frame,
}

Layer::PaintContext context = {
(SkCanvas*)&internal_nodes_canvas,
static_cast<SkCanvas*>(&internal_nodes_canvas),
frame.canvas(),
frame.gr_context(),
frame.view_embedder(),
Expand All @@ -129,8 +128,9 @@ void LayerTree::Paint(CompositorContext::ScopedFrame& frame,
frame_physical_depth_,
frame_device_pixel_ratio_};

if (root_layer_->needs_painting())
if (root_layer_->needs_painting()) {
root_layer_->Paint(context);
}
}

sk_sp<SkPicture> LayerTree::Flatten(const SkRect& bounds) {
Expand Down Expand Up @@ -171,7 +171,7 @@ sk_sp<SkPicture> LayerTree::Flatten(const SkRect& bounds) {
internal_nodes_canvas.addCanvas(canvas);

Layer::PaintContext paint_context = {
(SkCanvas*)&internal_nodes_canvas,
static_cast<SkCanvas*>(&internal_nodes_canvas),
canvas, // canvas
nullptr,
nullptr,
Expand Down
12 changes: 6 additions & 6 deletions flow/layers/performance_overlay_layer.cc
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
// Copyright 2013 The Flutter Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
// FLUTTER_NOLINT

#include <iomanip>
#include <iostream>
Expand All @@ -14,7 +13,7 @@
namespace flutter {
namespace {

void VisualizeStopWatch(SkCanvas& canvas,
void VisualizeStopWatch(SkCanvas* canvas,
const Stopwatch& stopwatch,
SkScalar x,
SkScalar y,
Expand All @@ -37,7 +36,7 @@ void VisualizeStopWatch(SkCanvas& canvas,
stopwatch, label_prefix, font_path);
SkPaint paint;
paint.setColor(SK_ColorGRAY);
canvas.drawTextBlob(text, x + label_x, y + height + label_y, paint);
canvas->drawTextBlob(text, x + label_x, y + height + label_y, paint);
}
}

Expand Down Expand Up @@ -77,8 +76,9 @@ PerformanceOverlayLayer::PerformanceOverlayLayer(uint64_t options,
void PerformanceOverlayLayer::Paint(PaintContext& context) const {
const int padding = 8;

if (!options_)
if (!options_) {
return;
}

TRACE_EVENT0("flutter", "PerformanceOverlayLayer::Paint");
SkScalar x = paint_bounds().x() + padding;
Expand All @@ -88,11 +88,11 @@ void PerformanceOverlayLayer::Paint(PaintContext& context) const {
SkAutoCanvasRestore save(context.leaf_nodes_canvas, true);

VisualizeStopWatch(
*context.leaf_nodes_canvas, context.raster_time, x, y, width,
context.leaf_nodes_canvas, context.raster_time, x, y, width,
height - padding, options_ & kVisualizeRasterizerStatistics,
options_ & kDisplayRasterizerStatistics, "Raster", font_path_);

VisualizeStopWatch(*context.leaf_nodes_canvas, context.ui_time, x, y + height,
VisualizeStopWatch(context.leaf_nodes_canvas, context.ui_time, x, y + height,
width, height - padding,
options_ & kVisualizeEngineStatistics,
options_ & kDisplayEngineStatistics, "UI", font_path_);
Expand Down
17 changes: 8 additions & 9 deletions flow/matrix_decomposition.cc
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
// Copyright 2013 The Flutter Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
// FLUTTER_NOLINT

#include "flutter/flow/matrix_decomposition.h"

Expand All @@ -18,12 +17,12 @@ MatrixDecomposition::MatrixDecomposition(const SkMatrix& matrix)
: MatrixDecomposition(SkM44{matrix}) {}

// Use custom normalize to avoid skia precision loss/normalize() privatization.
static inline void SkV3Normalize(SkV3& v) {
double mag = sqrt(v.x * v.x + v.y * v.y + v.z * v.z);
static inline void SkV3Normalize(SkV3* v) {
double mag = sqrt(v->x * v->x + v->y * v->y + v->z * v->z);
double scale = 1.0 / mag;
v.x *= scale;
v.y *= scale;
v.z *= scale;
v->x *= scale;
v->y *= scale;
v->z *= scale;
}

MatrixDecomposition::MatrixDecomposition(SkM44 matrix) : valid_(false) {
Expand Down Expand Up @@ -71,14 +70,14 @@ MatrixDecomposition::MatrixDecomposition(SkM44 matrix) : valid_(false) {

scale_.x = row[0].length();

SkV3Normalize(row[0]);
SkV3Normalize(&row[0]);
Copy link
Member

Choose a reason for hiding this comment

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

This is so much better.


shear_.x = row[0].dot(row[1]);
row[1] = SkV3Combine(row[1], 1.0, row[0], -shear_.x);

scale_.y = row[1].length();

SkV3Normalize(row[1]);
SkV3Normalize(&row[1]);

shear_.x /= scale_.y;

Expand All @@ -89,7 +88,7 @@ MatrixDecomposition::MatrixDecomposition(SkM44 matrix) : valid_(false) {

scale_.z = row[2].length();

SkV3Normalize(row[2]);
SkV3Normalize(&row[2]);

shear_.y /= scale_.z;
shear_.z /= scale_.z;
Expand Down
13 changes: 7 additions & 6 deletions flow/matrix_decomposition_unittests.cc
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
// Copyright 2013 The Flutter Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
// FLUTTER_NOLINT

#include "flutter/fml/build_config.h"

Expand Down Expand Up @@ -98,7 +97,8 @@ TEST(MatrixDecomposition, Combination) {

TEST(MatrixDecomposition, ScaleFloatError) {
constexpr float scale_increment = 0.00001f;
for (float scale = 0.0001f; scale < 2.0f; scale += scale_increment) {
float scale = 0.0001f;
while (scale < 2.0f) {
SkM44 matrix;
matrix.setScale(scale, scale, 1.0f);

Expand All @@ -111,11 +111,12 @@ TEST(MatrixDecomposition, ScaleFloatError) {
ASSERT_FLOAT_EQ(0, decomposition3.rotation().x);
ASSERT_FLOAT_EQ(0, decomposition3.rotation().y);
ASSERT_FLOAT_EQ(0, decomposition3.rotation().z);
scale += scale_increment;
}

SkM44 matrix;
const auto scale = 1.7734375f;
matrix.setScale(scale, scale, 1.f);
const auto scale1 = 1.7734375f;
matrix.setScale(scale1, scale1, 1.f);

// Bug upper bound (empirical)
const auto scale2 = 1.773437559603f;
Expand All @@ -136,8 +137,8 @@ TEST(MatrixDecomposition, ScaleFloatError) {
flutter::MatrixDecomposition decomposition3(matrix3);
ASSERT_TRUE(decomposition3.IsValid());

ASSERT_FLOAT_EQ(scale, decomposition.scale().x);
ASSERT_FLOAT_EQ(scale, decomposition.scale().y);
ASSERT_FLOAT_EQ(scale1, decomposition.scale().x);
ASSERT_FLOAT_EQ(scale1, decomposition.scale().y);
ASSERT_FLOAT_EQ(1.f, decomposition.scale().z);
ASSERT_FLOAT_EQ(0, decomposition.rotation().x);
ASSERT_FLOAT_EQ(0, decomposition.rotation().y);
Expand Down
1 change: 0 additions & 1 deletion flow/paint_utils.cc
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
// Copyright 2013 The Flutter Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
// FLUTTER_NOLINT

#include "flutter/flow/paint_utils.h"

Expand Down
12 changes: 6 additions & 6 deletions flow/raster_cache.cc
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
// Copyright 2013 The Flutter Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
// FLUTTER_NOLINT

#include "flutter/flow/raster_cache.h"

Expand Down Expand Up @@ -160,10 +159,11 @@ std::unique_ptr<RasterCacheResult> RasterCache::RasterizeLayer(
canvas_size.height());
internal_nodes_canvas.addCanvas(canvas);
Layer::PaintContext paintContext = {
(SkCanvas*)&internal_nodes_canvas, // internal_nodes_canvas
canvas, // leaf_nodes_canvas
context->gr_context, // gr_context
nullptr, // view_embedder
/* internal_nodes_canvas= */ static_cast<SkCanvas*>(
&internal_nodes_canvas),
/* leaf_nodes_canvas= */ canvas,
/* gr_context= */ context->gr_context,
/* view_embedder= */ nullptr,
context->raster_time,
context->ui_time,
context->texture_registry,
Expand Down Expand Up @@ -233,7 +233,7 @@ bool RasterCache::Draw(const SkPicture& picture, SkCanvas& canvas) const {
entry.used_this_frame = true;

if (entry.image) {
entry.image->draw(canvas);
entry.image->draw(canvas, nullptr);
return true;
}

Expand Down
2 changes: 1 addition & 1 deletion flow/raster_cache.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ class RasterCacheResult {

virtual ~RasterCacheResult() = default;

virtual void draw(SkCanvas& canvas, const SkPaint* paint = nullptr) const;
virtual void draw(SkCanvas& canvas, const SkPaint* paint) const;
Copy link
Member

Choose a reason for hiding this comment

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

You can also overload draw with a non-virtual method that just takes in a "canvas" if you want the user code to look the same.

Copy link
Member Author

Choose a reason for hiding this comment

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

There were only a couple of call sites, so I left it as is.


virtual SkISize image_dimensions() const {
return image_ ? image_->dimensions() : SkISize::Make(0, 0);
Expand Down
Loading