Merge branch 'master' into integration_test_for_debug_syms

Author: gmackall

dev/devicelab/bin/tasks/module_test_ios.dart

@@ -627,7 +627,6 @@ end
         swiftHostApp,
       );
 
-      final File swiftAnalyticsOutputFile = File(path.join(tempDir.path, 'analytics-swift.log'));
       final Directory swiftBuildDirectory = Directory(path.join(tempDir.path, 'build-swift'));
 
       await inDirectory(swiftHostApp, () async {
@@ -652,9 +651,7 @@ end
             'BUILD_DIR=${swiftBuildDirectory.path}',
             'COMPILER_INDEX_STORE_ENABLE=NO',
           ],
-          environment: <String, String>{
-            'FLUTTER_ANALYTICS_LOG_FILE': swiftAnalyticsOutputFile.path,
-          },
+          environment: <String, String>{'FLUTTER_SUPPRESS_ANALYTICS': 'true'},
         );
       });
 
@@ -665,16 +662,6 @@ end
         return TaskResult.failure('Failed to build existing Swift app .app');
       }
 
-      final String swiftAnalyticsOutput = swiftAnalyticsOutputFile.readAsStringSync();
-      if (!swiftAnalyticsOutput.contains('cd24: ios') ||
-          !swiftAnalyticsOutput.contains('cd25: true') ||
-          !swiftAnalyticsOutput.contains('viewName: assemble')) {
-        return TaskResult.failure(
-          'Building outer Swift app produced the following analytics: "$swiftAnalyticsOutput" '
-          'but not the expected strings: "cd24: ios", "cd25: true", "viewName: assemble"',
-        );
-      }
-
       return TaskResult.success(null);
     } catch (e) {
       return TaskResult.failure(e.toString());

dev/integration_tests/README.md

@@ -18,3 +18,7 @@ For example:
 ```sh
 flutter drive -t lib/keyboard_resize.dart --driver test_driver/keyboard_resize_test.dart
 ```
+
+## New tests require new CI runner
+Adding code to this directory will not automatically cause it to be run by any already existing ci tooling.
+This directory is intentinally a "choose your own adventure" piece of tooling.

dev/tools/bin/config/lockfile_exclusion.yaml

@@ -5,6 +5,8 @@
 # hello_world uses a Kotlin Gradle settings file (settings.gradle.kts). We don't have a template
 # Kotlin Gradle settings file (only a groovy one), so we currently can't generate it.
 - examples/hello_world/android
+# Also uses kts files.
+- dev/integration_tests/display_cutout_rotation/android
 
 # The following files must also be manually updated at the moment, because they don't use a
 # lib/main.dart (but they don't need exclusion, as we already skip that case).

engine/src/flutter/fml/logging.h

@@ -60,8 +60,27 @@ int GetVlogVerbosity();
 // kLogFatal and above is always true.
 bool ShouldCreateLogMessage(LogSeverity severity);
 
+constexpr bool ShouldCreateLogMessageConstexpr(LogSeverity severity,
+                                               bool true_arg) {
+  if (true_arg) {
+    return ShouldCreateLogMessage(severity);
+  }
+  return false;
+}
+
 [[noreturn]] void KillProcess();
 
+[[noreturn]] constexpr void KillProcessConstexpr(bool true_arg) {
+  if (true_arg) {
+    KillProcess();
+  }
+#if defined(_MSC_VER) && !defined(__clang__)
+  __assume(false);
+#else   // defined(_MSC_VER) && !defined(__clang__)
+  __builtin_unreachable();
+#endif  // defined(_MSC_VER) && !defined(__clang__)
+}
+
 }  // namespace fml
 
 #define FML_LOG_STREAM(severity) \
@@ -77,7 +96,7 @@ bool ShouldCreateLogMessage(LogSeverity severity);
             ::fml::LogMessage(::fml::kLogFatal, 0, 0, nullptr).stream()
 
 #define FML_LOG_IS_ON(severity) \
-  (::fml::ShouldCreateLogMessage(::fml::LOG_##severity))
+  (::fml::ShouldCreateLogMessageConstexpr(::fml::LOG_##severity, true))
 
 #define FML_LOG(severity) \
   FML_LAZY_STREAM(FML_LOG_STREAM(severity), FML_LOG_IS_ON(severity))
@@ -109,7 +128,7 @@ bool ShouldCreateLogMessage(LogSeverity severity);
 #define FML_UNREACHABLE()                          \
   {                                                \
     FML_LOG(ERROR) << "Reached unreachable code."; \
-    ::fml::KillProcess();                          \
+    ::fml::KillProcessConstexpr(true);             \
   }
 
 #endif  // FLUTTER_FML_LOGGING_H_

engine/src/flutter/impeller/display_list/aiks_dl_text_unittests.cc

@@ -31,6 +31,7 @@ struct TextRenderOptions {
   DlColor color = DlColor::kYellow();
   DlPoint position = DlPoint(100, 200);
   std::shared_ptr<DlMaskFilter> filter;
+  bool is_subpixel = false;
 };
 
 bool RenderTextInCanvasSkia(const std::shared_ptr<Context>& context,
@@ -57,6 +58,9 @@ bool RenderTextInCanvasSkia(const std::shared_ptr<Context>& context,
   }
   sk_sp<SkFontMgr> font_mgr = txt::GetDefaultFontManager();
   SkFont sk_font(font_mgr->makeFromData(mapping), options.font_size);
+  if (options.is_subpixel) {
+    sk_font.setSubpixel(true);
+  }
   auto blob = SkTextBlob::MakeFromString(text.c_str(), sk_font);
   if (!blob) {
     return false;
@@ -153,10 +157,12 @@ TEST_P(AiksTest, CanRenderTextFrameWithHalfScaling) {
 
 TEST_P(AiksTest, CanRenderTextFrameWithFractionScaling) {
   Scalar fine_scale = 0.f;
+  bool is_subpixel = false;
   auto callback = [&]() -> sk_sp<DisplayList> {
     if (AiksTest::ImGuiBegin("Controls", nullptr,
                              ImGuiWindowFlags_AlwaysAutoResize)) {
       ImGui::SliderFloat("Fine Scale", &fine_scale, -1, 1);
+      ImGui::Checkbox("subpixel", &is_subpixel);
       ImGui::End();
     }
 
@@ -168,7 +174,8 @@ TEST_P(AiksTest, CanRenderTextFrameWithFractionScaling) {
     builder.Scale(scale, scale);
     RenderTextInCanvasSkia(GetContext(), builder,
                            "the quick brown fox jumped over the lazy dog!.?",
-                           "Roboto-Regular.ttf");
+                           "Roboto-Regular.ttf",
+                           TextRenderOptions{.is_subpixel = is_subpixel});
     return builder.Build();
   };
 

engine/src/flutter/impeller/display_list/dl_golden_unittests.cc

@@ -366,6 +366,22 @@ int32_t CalculateMaxY(const impeller::testing::Screenshot* img) {
   }
   return max_y;
 }
+
+int32_t CalculateSpaceBetweenUI(const impeller::testing::Screenshot* img) {
+  const uint32_t* ptr = reinterpret_cast<const uint32_t*>(img->GetBytes());
+  ptr += img->GetWidth() * static_cast<int32_t>(img->GetHeight() / 2.0);
+  std::vector<size_t> boundaries;
+  uint32_t value = *ptr++;
+  for (size_t i = 1; i < img->GetWidth(); ++i) {
+    if (((*ptr & 0x00ffffff) != 0) != ((value & 0x00ffffff) != 0)) {
+      boundaries.push_back(i);
+    }
+    value = *ptr++;
+  }
+
+  assert(boundaries.size() == 6);
+  return boundaries[4] - boundaries[3];
+}
 }  // namespace
 
 TEST_P(DlGoldenTest, BaselineHE) {
@@ -399,5 +415,42 @@ TEST_P(DlGoldenTest, BaselineHE) {
   int32_t y_diff = std::abs(left_max_y - right_max_y);
   EXPECT_TRUE(y_diff <= 2) << "y diff: " << y_diff;
 }
+
+TEST_P(DlGoldenTest, MaintainsSpace) {
+  SetWindowSize(impeller::ISize(1024, 200));
+  impeller::Scalar font_size = 300;
+  auto callback = [&](const char* text,
+                      impeller::Scalar scale) -> sk_sp<DisplayList> {
+    DisplayListBuilder builder;
+    DlPaint paint;
+    paint.setColor(DlColor::ARGB(1, 0, 0, 0));
+    builder.DrawPaint(paint);
+    builder.Scale(scale, scale);
+    RenderTextInCanvasSkia(&builder, text, "Roboto-Regular.ttf",
+                           DlPoint::MakeXY(10, 300),
+                           TextRenderOptions{
+                               .font_size = font_size,
+                           });
+    return builder.Build();
+  };
+
+  std::optional<int32_t> last_space;
+  for (int i = 0; i <= 100; ++i) {
+    Scalar scale = 0.440 + i / 1000.0;
+    std::unique_ptr<impeller::testing::Screenshot> right =
+        MakeScreenshot(callback("ui", scale));
+    if (!right) {
+      GTEST_SKIP() << "making screenshots not supported.";
+    }
+
+    int32_t space = CalculateSpaceBetweenUI(right.get());
+    if (last_space.has_value()) {
+      int32_t diff = abs(space - *last_space);
+      EXPECT_TRUE(diff <= 1)
+          << "i:" << i << " space:" << space << " last_space:" << *last_space;
+    }
+    last_space = space;
+  }
+}
 }  // namespace testing
 }  // namespace flutter

engine/src/flutter/impeller/entity/contents/text_contents.cc

@@ -183,7 +183,6 @@ void TextContents::ComputeVertexData(
       Point screen_glyph_position =
           (screen_offset + unrounded_glyph_position + subpixel_adjustment)
               .Floor();
-
       for (const Point& point : unit_points) {
         Point position;
         if (is_translation_scale) {

engine/src/flutter/impeller/typographer/text_frame.cc

@@ -56,7 +56,7 @@ Scalar TextFrame::RoundScaledFontSize(Scalar scale) {
   // CTM, a glyph will fit in the atlas. If we clamp significantly, this may
   // reduce fidelity but is preferable to the alternative of failing to render.
   constexpr Scalar kMaximumTextScale = 48;
-  Scalar result = std::round(scale * 100) / 100;
+  Scalar result = std::round(scale * 200) / 200;
   return std::clamp(result, 0.0f, kMaximumTextScale);
 }
 

packages/flutter_goldens/lib/flutter_goldens.dart

@@ -320,7 +320,17 @@ class FlutterPostSubmitFileComparator extends FlutterGoldenFileComparator {
     golden = _addPrefix(golden);
     await update(golden, imageBytes);
     final File goldenFile = getGoldenFile(golden);
-    return skiaClient.imgtestAdd(golden.path, goldenFile);
+    try {
+      return await skiaClient.imgtestAdd(golden.path, goldenFile);
+    } on SkiaException catch (e) {
+      // Convert SkiaException -> TestFailure so that this class implements the
+      // contract of GoldenFileComparator, and matchesGoldenFile() converts the
+      // TestFailure into a standard reported test error (with a better stack
+      // trace, for example).
+      //
+      // https://github.com/flutter/flutter/issues/162621
+      throw TestFailure('$e');
+    }
   }
 
   /// Decides based on the current environment if goldens tests should be

packages/flutter_goldens/test/flutter_goldens_test.dart

@@ -853,6 +853,43 @@ void main() {
         );
         expect(fakeSkiaClient.initCalls, 0);
       });
+
+      test('reports a failure as a TestFailure', () async {
+        final List<String> log = <String>[];
+        final MemoryFileSystem fs = MemoryFileSystem();
+        final FakePlatform platform = FakePlatform(
+          environment: <String, String>{'FLUTTER_ROOT': _kFlutterRoot},
+          operatingSystem: 'macos',
+        );
+        fs.directory(_kFlutterRoot).createSync(recursive: true);
+        final Directory basedir = fs.directory('flutter/test/library/')
+          ..createSync(recursive: true);
+        final FlutterGoldenFileComparator comparator = FlutterPostSubmitFileComparator(
+          basedir.uri,
+          ThrowsOnImgTestAddSkiaClient(
+            message: 'Skia Gold received an unapproved image in post-submit',
+          ),
+          fs: fs,
+          platform: platform,
+          log: log.add,
+        );
+        await expectLater(
+          () async {
+            return comparator.compare(
+              Uint8List.fromList(_kTestPngBytes),
+              Uri.parse('flutter.golden_test.1.png'),
+            );
+          },
+          throwsA(
+            isA<TestFailure>().having(
+              (TestFailure error) => error.toString(),
+              'description',
+              contains('Skia Gold received an unapproved image in post-submit'),
+            ),
+          ),
+        );
+        expect(log, isEmpty);
+      });
     });
 
     group('Pre-Submit', () {
@@ -1210,6 +1247,21 @@ class FakeSkiaGoldClient extends Fake implements SkiaGoldClient {
   String cleanTestName(String fileName) => cleanTestNameValues[fileName] ?? '';
 }
 
+class ThrowsOnImgTestAddSkiaClient extends Fake implements SkiaGoldClient {
+  ThrowsOnImgTestAddSkiaClient({required this.message});
+  final String message;
+
+  @override
+  Future<void> imgtestInit() async {
+    // Assume this function works.
+  }
+
+  @override
+  Future<bool> imgtestAdd(String testName, File goldenFile) {
+    throw SkiaException(message);
+  }
+}
+
 class FakeLocalFileComparator extends Fake implements LocalFileComparator {
   @override
   late Uri basedir;