Documentation and other cleanup in dart:ui, plus a small performance improvement. (#38047)

* Remove nonsensical (and I think unnecessary?) comment.
* Document willChangeHint, isComplexHint
* Fix grammer in Rect docs
* Remove runtimeType.toString twice
* Add detail to ImageShader constructor docs.
* Document Vertices and drawVertices!
* Update checks in Vertices constructors (and add tests).
* Fix some typos.
* Minor other doc improvements.
* Fold in @jonahwilliams' performance improvement from #38041

Author: Hixie

flow/layers/display_list_raster_cache_item.cc

@@ -104,10 +104,6 @@ void DisplayListRasterCacheItem::PrerollFinalize(PrerollContext* context,
   }
   auto* raster_cache = context->raster_cache;
   SkRect bounds = display_list_->bounds().makeOffset(offset_.x(), offset_.y());
-  // We must to create an entry whenever if the react is intersect.
-  // if the rect is intersect we will get the entry access_count to confirm if
-  // it great than the threshold. Otherwise we only increase the entry
-  // access_count.
   bool visible = !context->state_stack.content_culled(bounds);
   int accesses = raster_cache->MarkSeen(key_id_, matrix, visible);
   if (!visible || accesses <= raster_cache->access_threshold()) {

lib/ui/compositing.dart

@@ -748,7 +748,25 @@ class SceneBuilder extends NativeFieldWrapperClass1 {
 
   /// Adds a [Picture] to the scene.
   ///
-  /// The picture is rasterized at the given offset.
+  /// The picture is rasterized at the given `offset`.
+  ///
+  /// The rendering _may_ be cached to reduce the cost of painting the picture
+  /// if it is reused in subsequent frames. Whether a picture is cached or not
+  /// depends on the backend implementation. When caching is considered, the
+  /// choice to cache or not cache is a heuristic based on how often the picture
+  /// is being painted and the cost of painting the picture. To disable this
+  /// caching, set `willChangeHint` to true. To force the caching to happen (in
+  /// backends that do caching), set `isComplexHint` to true. When both are set,
+  /// `willChangeHint` prevails.
+  ///
+  /// In general, setting these hints is not very useful. Backends that cache
+  /// pictures only do so for pictures that have been rendered three times
+  /// already; setting `willChangeHint` to true to avoid caching an animating
+  /// picture that changes every frame is therefore redundant, the picture
+  /// wouldn't have been cached anyway. Similarly, backends that cache pictures
+  /// are relatively aggressive about doing so, such that any image complicated
+  /// enough to warrant caching is probably already being cached even without
+  /// `isComplexHint` being set to true.
   void addPicture(
     Offset offset,
     Picture picture, {

lib/ui/geometry.dart

@@ -625,7 +625,7 @@ class Size extends OffsetBase {
 /// An immutable, 2D, axis-aligned, floating-point rectangle whose coordinates
 /// are relative to a given origin.
 ///
-/// A Rect can be created with one its constructors or from an [Offset] and a
+/// A Rect can be created with one of its constructors or from an [Offset] and a
 /// [Size] using the `&` operator:
 ///
 /// ```dart

lib/ui/painting.dart

@@ -25,8 +25,8 @@ abstract class Vertices {
   factory Vertices(
     VertexMode mode,
     List<Offset> positions, {
-    List<Offset>? textureCoordinates,
     List<Color>? colors,
+    List<Offset>? textureCoordinates,
     List<int>? indices,
   }) {
     return engine.renderer.createVertices(mode,
@@ -38,8 +38,8 @@ abstract class Vertices {
   factory Vertices.raw(
     VertexMode mode,
     Float32List positions, {
-    Float32List? textureCoordinates,
     Int32List? colors,
+    Float32List? textureCoordinates,
     Uint16List? indices,
   }) {
     return engine.renderer.createVerticesRaw(mode,

lib/web_ui/lib/canvas.dart

@@ -70,7 +70,7 @@ class SurfaceVertices implements ui.Vertices {
     if (assertionsEnabled) {
       return _disposed;
     }
-    throw StateError('Vertices.debugDisposed is only avialalbe when asserts are enabled.');
+    throw StateError('Vertices.debugDisposed is only available when asserts are enabled.');
   }
 }
 

lib/web_ui/lib/src/engine/html/render_vertices.dart

@@ -32,6 +32,7 @@ tests = [
   "lerp_test.dart",
   "locale_test.dart",
   "mask_filter_test.dart",
+  "painting_test.dart",
   "paragraph_builder_test.dart",
   "paragraph_test.dart",
   "path_test.dart",

testing/dart/BUILD.gn

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-import 'dart:typed_data' show ByteData, Float64List;
+import 'dart:typed_data';
 import 'dart:ui';
 
 import 'package:litetest/litetest.dart';

testing/dart/compositing_test.dart

@@ -0,0 +1,63 @@
+// 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.
+
+import 'dart:typed_data';
+import 'dart:ui';
+
+import 'package:litetest/litetest.dart';
+
+void main() {
+  test('Vertices checks', () {
+    try {
+      Vertices(
+        VertexMode.triangles,
+        const <Offset>[Offset.zero, Offset.zero, Offset.zero],
+        indices: Uint16List.fromList(const <int>[0, 2, 5]),
+      );
+      throw 'Vertices did not throw the expected error.';
+    } on ArgumentError catch (e) {
+      expect('$e', 'Invalid argument(s): "indices" values must be valid indices in the positions list (i.e. numbers in the range 0..2), but indices[2] is 5, which is too big.');
+    }
+    Vertices( // This one does not throw.
+      VertexMode.triangles,
+      const <Offset>[Offset.zero],
+    ).dispose();
+    Vertices( // This one should not throw.
+      VertexMode.triangles,
+      const <Offset>[Offset.zero, Offset.zero, Offset.zero],
+      indices: Uint16List.fromList(const <int>[0, 2, 1, 2, 0, 1, 2, 0]), // Uint16List implements List<int> so this is ok.
+    ).dispose();
+  });
+
+  test('Vertices.raw checks', () {
+    try {
+      Vertices.raw(
+        VertexMode.triangles,
+        Float32List.fromList(const <double>[0.0]),
+      );
+      throw 'Vertices.raw did not throw the expected error.';
+    } on ArgumentError catch (e) {
+      expect('$e', 'Invalid argument(s): "positions" must have an even number of entries (each coordinate is an x,y pair).');
+    }
+    try {
+      Vertices.raw(
+        VertexMode.triangles,
+        Float32List.fromList(const <double>[0.0, 0.0, 0.0, 0.0, 0.0, 0.0]),
+        indices: Uint16List.fromList(const <int>[0, 2, 5]),
+      );
+      throw 'Vertices.raw did not throw the expected error.';
+    } on ArgumentError catch (e) {
+      expect('$e', 'Invalid argument(s): "indices" values must be valid indices in the positions list (i.e. numbers in the range 0..2), but indices[2] is 5, which is too big.');
+    }
+    Vertices.raw( // This one does not throw.
+      VertexMode.triangles,
+      Float32List.fromList(const <double>[0.0, 0.0]),
+    ).dispose();
+    Vertices.raw( // This one should not throw.
+      VertexMode.triangles,
+      Float32List.fromList(const <double>[0.0, 0.0, 0.0, 0.0, 0.0, 0.0]),
+      indices: Uint16List.fromList(const <int>[0, 2, 1, 2, 0, 1, 2, 0]),
+    ).dispose();
+  });
+}

testing/dart/painting_test.dart

@@ -225,4 +225,19 @@ void main() {
     expect(newFirstMetric.getTangentForOffset(4.0)!.vector, const Offset(0.0, 1.0));
     expect(newFirstMetric.extractPath(4.0, 10.0).computeMetrics().first.length, 6.0);
   });
+
+  test('PathMetrics on a mutated path', () {
+    final Path path = Path()
+      ..lineTo(0, 30)
+      ..lineTo(40, 30)
+      ..moveTo(100, 0)
+      ..lineTo(100, 30)
+      ..lineTo(140, 30)
+      ..close();
+    final PathMetrics metrics = path.computeMetrics();
+    expect(metrics.toString(),
+      '(PathMetric(length: 70.0, isClosed: false, contourIndex: 0), '
+       'PathMetric(length: 120.0, isClosed: true, contourIndex: 1))',
+    );
+  });
 }