diff --git a/flow/instrumentation.cc b/flow/instrumentation.cc index ea85d06e1cce9..541b8635b6aa7 100644 --- a/flow/instrumentation.cc +++ b/flow/instrumentation.cc @@ -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" @@ -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; } @@ -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); @@ -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++) { @@ -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) { @@ -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) { @@ -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(); @@ -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(current_bytes - min_bytes) / + static_cast(max_bytes - min_bytes); + path.lineTo( + x + ((static_cast(i) / static_cast(kMaxSamples)) * + width), + y + ((1.0 - ratio) * height)); } path.rLineTo(100, 0); @@ -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); @@ -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 { diff --git a/flow/instrumentation.h b/flow/instrumentation.h index a4697830cf972..49ca887844cec 100644 --- a/flow/instrumentation.h +++ b/flow/instrumentation.h @@ -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(); @@ -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; diff --git a/flow/layers/layer_tree.cc b/flow/layers/layer_tree.cc index c8778630fc1b4..0f598ae6f153f 100644 --- a/flow/layers/layer_tree.cc +++ b/flow/layers/layer_tree.cc @@ -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" @@ -117,7 +116,7 @@ void LayerTree::Paint(CompositorContext::ScopedFrame& frame, } Layer::PaintContext context = { - (SkCanvas*)&internal_nodes_canvas, + static_cast(&internal_nodes_canvas), frame.canvas(), frame.gr_context(), frame.view_embedder(), @@ -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 LayerTree::Flatten(const SkRect& bounds) { @@ -171,7 +171,7 @@ sk_sp LayerTree::Flatten(const SkRect& bounds) { internal_nodes_canvas.addCanvas(canvas); Layer::PaintContext paint_context = { - (SkCanvas*)&internal_nodes_canvas, + static_cast(&internal_nodes_canvas), canvas, // canvas nullptr, nullptr, diff --git a/flow/layers/performance_overlay_layer.cc b/flow/layers/performance_overlay_layer.cc index 3aac5a54d9a72..05ade5e21af73 100644 --- a/flow/layers/performance_overlay_layer.cc +++ b/flow/layers/performance_overlay_layer.cc @@ -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 #include @@ -14,7 +13,7 @@ namespace flutter { namespace { -void VisualizeStopWatch(SkCanvas& canvas, +void VisualizeStopWatch(SkCanvas* canvas, const Stopwatch& stopwatch, SkScalar x, SkScalar y, @@ -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); } } @@ -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; @@ -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_); diff --git a/flow/matrix_decomposition.cc b/flow/matrix_decomposition.cc index 3d3cb9e969cd9..2207fa35fb878 100644 --- a/flow/matrix_decomposition.cc +++ b/flow/matrix_decomposition.cc @@ -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" @@ -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) { @@ -71,14 +70,14 @@ MatrixDecomposition::MatrixDecomposition(SkM44 matrix) : valid_(false) { scale_.x = row[0].length(); - SkV3Normalize(row[0]); + SkV3Normalize(&row[0]); 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; @@ -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; diff --git a/flow/matrix_decomposition_unittests.cc b/flow/matrix_decomposition_unittests.cc index f3c6a46c1f985..dc95033e43898 100644 --- a/flow/matrix_decomposition_unittests.cc +++ b/flow/matrix_decomposition_unittests.cc @@ -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" @@ -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); @@ -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; @@ -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); diff --git a/flow/paint_utils.cc b/flow/paint_utils.cc index b19cd02cfe216..38fc17979a0c3 100644 --- a/flow/paint_utils.cc +++ b/flow/paint_utils.cc @@ -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" diff --git a/flow/raster_cache.cc b/flow/raster_cache.cc index c9fcad4a34b49..1e39e3701a679 100644 --- a/flow/raster_cache.cc +++ b/flow/raster_cache.cc @@ -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" @@ -160,10 +159,11 @@ std::unique_ptr 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( + &internal_nodes_canvas), + /* leaf_nodes_canvas= */ canvas, + /* gr_context= */ context->gr_context, + /* view_embedder= */ nullptr, context->raster_time, context->ui_time, context->texture_registry, @@ -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; } diff --git a/flow/raster_cache.h b/flow/raster_cache.h index d71b4e2ff9aed..138ce673c58d7 100644 --- a/flow/raster_cache.h +++ b/flow/raster_cache.h @@ -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; virtual SkISize image_dimensions() const { return image_ ? image_->dimensions() : SkISize::Make(0, 0); diff --git a/flow/rtree_unittests.cc b/flow/rtree_unittests.cc index d5c8466c1ca7d..1a1498efc8af3 100644 --- a/flow/rtree_unittests.cc +++ b/flow/rtree_unittests.cc @@ -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 "rtree.h" @@ -12,7 +11,7 @@ namespace flutter { namespace testing { -TEST(RTree, searchNonOverlappingDrawnRects_NoIntersection) { +TEST(RTree, searchNonOverlappingDrawnRectsNoIntersection) { auto rtree_factory = RTreeFactory(); auto recorder = std::make_unique(); auto recording_canvas = @@ -32,7 +31,7 @@ TEST(RTree, searchNonOverlappingDrawnRects_NoIntersection) { ASSERT_TRUE(hits.empty()); } -TEST(RTree, searchNonOverlappingDrawnRects_SingleRectIntersection) { +TEST(RTree, searchNonOverlappingDrawnRectsSingleRectIntersection) { auto rtree_factory = RTreeFactory(); auto recorder = std::make_unique(); auto recording_canvas = @@ -54,7 +53,7 @@ TEST(RTree, searchNonOverlappingDrawnRects_SingleRectIntersection) { ASSERT_EQ(*hits.begin(), SkRect::MakeLTRB(120, 120, 160, 160)); } -TEST(RTree, searchNonOverlappingDrawnRects_IgnoresNonDrawingRecords) { +TEST(RTree, searchNonOverlappingDrawnRectsIgnoresNonDrawingRecords) { auto rtree_factory = RTreeFactory(); auto recorder = std::make_unique(); auto recording_canvas = @@ -82,7 +81,7 @@ TEST(RTree, searchNonOverlappingDrawnRects_IgnoresNonDrawingRecords) { ASSERT_EQ(*hits.begin(), SkRect::MakeLTRB(120, 120, 180, 180)); } -TEST(RTree, searchNonOverlappingDrawnRects_MultipleRectIntersection) { +TEST(RTree, searchNonOverlappingDrawnRectsMultipleRectIntersection) { auto rtree_factory = RTreeFactory(); auto recorder = std::make_unique(); auto recording_canvas = @@ -113,7 +112,7 @@ TEST(RTree, searchNonOverlappingDrawnRects_MultipleRectIntersection) { ASSERT_EQ(*std::next(hits.begin(), 1), SkRect::MakeLTRB(300, 100, 400, 200)); } -TEST(RTree, searchNonOverlappingDrawnRects_JoinRectsWhenIntersectedCase1) { +TEST(RTree, searchNonOverlappingDrawnRectsJoinRectsWhenIntersectedCase1) { auto rtree_factory = RTreeFactory(); auto recorder = std::make_unique(); auto recording_canvas = @@ -147,7 +146,7 @@ TEST(RTree, searchNonOverlappingDrawnRects_JoinRectsWhenIntersectedCase1) { ASSERT_EQ(*hits.begin(), SkRect::MakeLTRB(100, 100, 175, 175)); } -TEST(RTree, searchNonOverlappingDrawnRects_JoinRectsWhenIntersectedCase2) { +TEST(RTree, searchNonOverlappingDrawnRectsJoinRectsWhenIntersectedCase2) { auto rtree_factory = RTreeFactory(); auto recorder = std::make_unique(); auto recording_canvas = @@ -188,7 +187,7 @@ TEST(RTree, searchNonOverlappingDrawnRects_JoinRectsWhenIntersectedCase2) { ASSERT_EQ(*hits.begin(), SkRect::MakeLTRB(50, 50, 500, 250)); } -TEST(RTree, searchNonOverlappingDrawnRects_JoinRectsWhenIntersectedCase3) { +TEST(RTree, searchNonOverlappingDrawnRectsJoinRectsWhenIntersectedCase3) { auto rtree_factory = RTreeFactory(); auto recorder = std::make_unique(); auto recording_canvas = diff --git a/fml/logging.cc b/fml/logging.cc index 8c4796db0a504..d4273b9381da7 100644 --- a/fml/logging.cc +++ b/fml/logging.cc @@ -39,9 +39,8 @@ const char* StripPath(const char* path) { auto* p = strrchr(path, '/'); if (p) { return p + 1; - } else { - return path; } + return path; } } // namespace diff --git a/fml/memory/ref_counted_unittest.cc b/fml/memory/ref_counted_unittest.cc index 959117c488e38..4cefb8b5ff5f9 100644 --- a/fml/memory/ref_counted_unittest.cc +++ b/fml/memory/ref_counted_unittest.cc @@ -227,6 +227,8 @@ TEST(RefCountedTest, NullAssignmentToNull) { // No-op null assignment using move constructor. r1 = std::move(r2); EXPECT_TRUE(r1.get() == nullptr); + // The clang linter flags the method called on the moved-from reference, but + // this is testing the move implementation, so it is marked NOLINT. EXPECT_TRUE(r2.get() == nullptr); // NOLINT(clang-analyzer-cplusplus.Move) EXPECT_FALSE(r1); EXPECT_FALSE(r2); @@ -272,6 +274,8 @@ TEST(RefCountedTest, NonNullAssignmentToNull) { RefPtr r2; // Move assignment (to null ref pointer). r2 = std::move(r1); + // The clang linter flags the method called on the moved-from reference, but + // this is testing the move implementation, so it is marked NOLINT. EXPECT_TRUE(r1.get() == nullptr); // NOLINT(clang-analyzer-cplusplus.Move) EXPECT_EQ(created, r2.get()); EXPECT_FALSE(r1); @@ -336,6 +340,8 @@ TEST(RefCountedTest, NullAssignmentToNonNull) { // Null assignment using move constructor. r1 = std::move(r2); EXPECT_TRUE(r1.get() == nullptr); + // The clang linter flags the method called on the moved-from reference, but + // this is testing the move implementation, so it is marked NOLINT. EXPECT_TRUE(r2.get() == nullptr); // NOLINT(clang-analyzer-cplusplus.Move) EXPECT_FALSE(r1); EXPECT_FALSE(r2); @@ -389,6 +395,8 @@ TEST(RefCountedTest, NonNullAssignmentToNonNull) { RefPtr r2(MakeRefCounted(nullptr, &was_destroyed2)); // Move assignment (to non-null ref pointer). r2 = std::move(r1); + // The clang linter flags the method called on the moved-from reference, but + // this is testing the move implementation, so it is marked NOLINT. EXPECT_TRUE(r1.get() == nullptr); // NOLINT(clang-analyzer-cplusplus.Move) EXPECT_FALSE(r2.get() == nullptr); EXPECT_FALSE(r1); @@ -436,7 +444,13 @@ TEST(RefCountedTest, SelfAssignment) { { MyClass* created = nullptr; was_destroyed = false; - RefPtr r(MakeRefCounted(&created, &was_destroyed)); + // This line is marked NOLINT because the clang linter does not reason about + // the value of the reference count. In particular, the self-assignment + // below is handled in the copy constructor by a refcount increment then + // decrement. The linter sees only that the decrement might destroy the + // object. + RefPtr r(MakeRefCounted( // NOLINT + &created, &was_destroyed)); // Copy. ALLOW_SELF_ASSIGN_OVERLOADED(r = r); EXPECT_EQ(created, r.get()); diff --git a/fml/memory/weak_ptr_unittest.cc b/fml/memory/weak_ptr_unittest.cc index a1db3ae1d1988..e055ad9095409 100644 --- a/fml/memory/weak_ptr_unittest.cc +++ b/fml/memory/weak_ptr_unittest.cc @@ -38,6 +38,8 @@ TEST(WeakPtrTest, MoveConstruction) { WeakPtrFactory factory(&data); WeakPtr ptr = factory.GetWeakPtr(); WeakPtr ptr2(std::move(ptr)); + // The clang linter flags the method called on the moved-from reference, but + // this is testing the move implementation, so it is marked NOLINT. EXPECT_EQ(nullptr, ptr.get()); // NOLINT EXPECT_EQ(&data, ptr2.get()); } @@ -60,6 +62,8 @@ TEST(WeakPtrTest, MoveAssignment) { WeakPtr ptr2; EXPECT_EQ(nullptr, ptr2.get()); ptr2 = std::move(ptr); + // The clang linter flags the method called on the moved-from reference, but + // this is testing the move implementation, so it is marked NOLINT. EXPECT_EQ(nullptr, ptr.get()); // NOLINT EXPECT_EQ(&data, ptr2.get()); }