Skip to content

Conversation

@a-siva
Copy link
Contributor

@a-siva a-siva commented Sep 6, 2024

flutter/engine@c50eb8a...419fb8c

2024-09-06 98614782+auto-submit[bot]@users.noreply.github.com Reverts "[engine] always force platform channel responses to schedule a task. (#54975)" (flutter/engine#55000)
2024-09-06 skia-flutter-autoroll@skia.org Roll Skia from b6bab0fde426 to 6ad117bd2efe (2 revisions) (flutter/engine#54999)
2024-09-06 skia-flutter-autoroll@skia.org Roll Fuchsia Test Scripts from D9INMR2u4wcyiZ750... to 5dqcFlKzRjJb6V95W... (flutter/engine#54998)
2024-09-06 skia-flutter-autoroll@skia.org Roll Skia from a09312b70d37 to b6bab0fde426 (3 revisions) (flutter/engine#54997)
2024-09-06 skia-flutter-autoroll@skia.org Roll Skia from 368f209ccca5 to a09312b70d37 (1 revision) (flutter/engine#54995)
2024-09-06 skia-flutter-autoroll@skia.org Roll Skia from aec11ae18bb6 to 368f209ccca5 (3 revisions) (flutter/engine#54992)
2024-09-06 skia-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from xNv47d1TZmK9XgTxu... to PBeI0gGvgFdXV6hCg... (flutter/engine#54990)
2024-09-06 skia-flutter-autoroll@skia.org Roll Skia from 809f868ded1c to aec11ae18bb6 (22 revisions) (flutter/engine#54988)
2024-09-06 30870216+gaaclarke@users.noreply.github.com Removes the int storage from Color (flutter/engine#54714)
2024-09-06 chris@bracken.jp iOS,macOS: Add logging of duplicate codesign binaries (flutter/engine#54987)
2024-09-06 skia-flutter-autoroll@skia.org Roll Fuchsia Test Scripts from k4lKsecg0pdIp-U7c... to D9INMR2u4wcyiZ750... (flutter/engine#54984)
2024-09-05 a-siva@users.noreply.github.com Manual roll of Dart. (flutter/engine#54983)
2024-09-05 chris@bracken.jp iOS,macOS: add unsigned_binaries.txt (flutter/engine#54977)
2024-09-05 jason-simmons@users.noreply.github.com Manual Skia roll to 809f868ded1c (flutter/engine#54972)
2024-09-05 1961493+harryterkelsen@users.noreply.github.com [canvaskit] Fix incorrect calculation of ImageFilter paint bounds (flutter/engine#54980)
2024-09-05 jonahwilliams@google.com [engine] always force platform channel responses to schedule a task. (flutter/engine#54975)
2024-09-05 tugorez@users.noreply.github.com Fix unexpected ViewFocus events when Text Editing utilities change focus in the middle of a blur call. (flutter/engine#54965)

Also rolling transitive DEPS:
fuchsia/sdk/core/linux-amd64 from xNv47d1TZmK9 to PBeI0gGvgFdX

@flutter-dashboard
Copy link

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption, contact "@test-exemption-reviewer" in the #hackers channel in Discord (don't just cc them here, they won't see it!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. The test exemption team is a small volunteer group, so all reviewers should feel empowered to ask for tests, without delegating that responsibility entirely to the test exemption group.

@github-actions github-actions bot added tool Affects the "flutter" command-line tool. See also t: labels. engine flutter/engine related. See also e: labels. labels Sep 6, 2024
@christopherfujino
Copy link
Contributor

I'm working on fixing this PR

@github-actions github-actions bot added a: tests "flutter test", flutter_test, or one of our tests framework flutter/packages/flutter repository. See also f: labels. d: api docs Issues with https://api.flutter.dev/ d: examples Sample code and demos f: integration_test The flutter/packages/integration_test plugin labels Sep 6, 2024
'leak_tracker': '10.0.7', // https://github.com/flutter/devtools/issues/3951
'leak_tracker_testing': '3.0.1', // https://github.com/flutter/devtools/issues/3951
'leak_tracker_flutter_testing': '3.0.8', // https://github.com/flutter/devtools/issues/3951
'path_provider_android': '2.2.1', // https://github.com/flutter/flutter/issues/140796
Copy link
Contributor

Choose a reason for hiding this comment

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

FYI @goderbauer & @gmackall I re-pinned path_provider_android to unblock this roll :'(

@christopherfujino
Copy link
Contributor

A little bit of context, package:_macros recently bumped its version and package:analyzer was forced to raise its lower constraint on it: dart-lang/sdk@5285faa#diff-a8403f1fc69a35a5ee70b920d57c1d402b06e5d10929ec8daedfebbf8c34c42fR16

When this Dart SDK change was attempted in an engine -> framework autoroll, it failed pub solving in the flutter_tools: #154731 because flutter_tools did not yet have the newer version of analyzer.

I resolved the issue by manually running flutter update-packages --force-upgrade on a branch with the engine revision that had the newer version of _macros.

@christopherfujino
Copy link
Contributor

https://ci.chromium.org/ui/p/flutter/builders/try/Windows%20framework_tests_libraries/68219/infra

The following TestFailure was thrown running a test:
Expected: Color:<Color(0xff19e600)>
  Actual: Color:<Color(0xff1ae600)>

When the exception was thrown, this was the stack:
#4      main.<anonymous closure> (file:///C:/b/s/w/ir/x/w/flutter/packages/flutter/test/painting/decoration_image_lerp_test.dart:183:7)
<asynchronous suspension>
#5      testWidgets.<anonymous closure>.<anonymous closure> (package:flutter_test/src/widget_tester.dart:189:15)
<asynchronous suspension>
#6      TestWidgetsFlutterBinding._runTestBody (package:flutter_test/src/binding.dart:1032:5)
<asynchronous suspension>
<asynchronous suspension>
(elided one frame from package:stack_trace)

This was caught by the test expectation on the following line:
  file:///C:/b/s/w/ir/x/w/flutter/packages/flutter/test/painting/decoration_image_lerp_test.dart line 183
The test description was:
  ImageDecoration.lerp 1

@zanderso
Copy link
Member

zanderso commented Sep 6, 2024

https://ci.chromium.org/ui/p/flutter/builders/try/Windows%20framework_tests_libraries/68219/infra

The following TestFailure was thrown running a test:
Expected: Color:<Color(0xff19e600)>
  Actual: Color:<Color(0xff1ae600)>

When the exception was thrown, this was the stack:
#4      main.<anonymous closure> (file:///C:/b/s/w/ir/x/w/flutter/packages/flutter/test/painting/decoration_image_lerp_test.dart:183:7)
<asynchronous suspension>
#5      testWidgets.<anonymous closure>.<anonymous closure> (package:flutter_test/src/widget_tester.dart:189:15)
<asynchronous suspension>
#6      TestWidgetsFlutterBinding._runTestBody (package:flutter_test/src/binding.dart:1032:5)
<asynchronous suspension>
<asynchronous suspension>
(elided one frame from package:stack_trace)

This was caught by the test expectation on the following line:
  file:///C:/b/s/w/ir/x/w/flutter/packages/flutter/test/painting/decoration_image_lerp_test.dart line 183
The test description was:
  ImageDecoration.lerp 1

@gaaclarke Wondering if this could be due to flutter/engine#54714 ?

@a-siva
Copy link
Contributor Author

a-siva commented Sep 6, 2024

https://ci.chromium.org/ui/p/flutter/builders/try/Windows%20framework_tests_libraries/68219/infra

The following TestFailure was thrown running a test:
Expected: Color:<Color(0xff19e600)>
  Actual: Color:<Color(0xff1ae600)>

When the exception was thrown, this was the stack:
#4      main.<anonymous closure> (file:///C:/b/s/w/ir/x/w/flutter/packages/flutter/test/painting/decoration_image_lerp_test.dart:183:7)
<asynchronous suspension>
#5      testWidgets.<anonymous closure>.<anonymous closure> (package:flutter_test/src/widget_tester.dart:189:15)
<asynchronous suspension>
#6      TestWidgetsFlutterBinding._runTestBody (package:flutter_test/src/binding.dart:1032:5)
<asynchronous suspension>
<asynchronous suspension>
(elided one frame from package:stack_trace)

This was caught by the test expectation on the following line:
  file:///C:/b/s/w/ir/x/w/flutter/packages/flutter/test/painting/decoration_image_lerp_test.dart line 183
The test description was:
  ImageDecoration.lerp 1

this could be the golden file issue that @gaaclarke was talking about

@gaaclarke
Copy link
Member

this could be the golden file issue that @gaaclarke was talking about

It's not quite what I was expecting. I had to fix one of these types of failures to get the PR through presubmit checks. Here's how I addressed it: https://github.com/flutter/flutter/pull/154680/files

I'm not sure why the test runs here, but not in presubmit. I would be fine just updating that assertion or making it fuzzy like I did above as part of this roll.

@a-siva
Copy link
Contributor Author

a-siva commented Sep 6, 2024

this could be the golden file issue that @gaaclarke was talking about

It's not quite what I was expecting. I had to fix one of these types of failures to get the PR through presubmit checks. Here's how I addressed it: https://github.com/flutter/flutter/pull/154680/files

I'm not sure why the test runs here, but not in presubmit. I would be fine just updating that assertion or making it fuzzy like I did above as part of this roll.

@gaaclarke can you make your suggested change in this PR so we can rerun the tests and see if that works out.

@zanderso
Copy link
Member

zanderso commented Sep 6, 2024

Oops. I pushed a change that just naively updates the test expectation, but looking at the previous addition of a new matcher, I suspect that's not quite right.

@gaaclarke
Copy link
Member

I don't have permissions to push to this branch. Here's the patch:

commit 9095aa51144561d0d094c08f1a718bf30f6c1bc3
Author: Aaron Clarke <aaclarke@google.com>
Date:   Fri Sep 6 11:41:54 2024 -0700

    added fuzzy color matching

diff --git a/packages/flutter/test/painting/decoration_image_lerp_test.dart b/packages/flutter/test/painting/decoration_image_lerp_test.dart
index ab24e4fdc4..ad9fe6e064 100644
--- a/packages/flutter/test/painting/decoration_image_lerp_test.dart
+++ b/packages/flutter/test/painting/decoration_image_lerp_test.dart
@@ -15,6 +15,36 @@ import 'package:flutter/foundation.dart';
 import 'package:flutter/material.dart';
 import 'package:flutter_test/flutter_test.dart';
 
+// TODO(gaaclarke): Unify the different instances of _ColorMatcher.
+/// Positive result if the colors would be mapped to the same argb8888 color.
+class _ColorMatcher extends Matcher {
+  _ColorMatcher(this._target);
+
+  final ui.Color _target;
+
+  @override
+  Description describe(Description description) {
+    return description.add('matches "$_target"');
+  }
+
+  @override
+  bool matches(dynamic item, Map<dynamic, dynamic> matchState) {
+    if (item is ui.Color) {
+      return item.colorSpace == _target.colorSpace &&
+          (item.a - _target.a).abs() <= 0.004 &&
+          (item.r - _target.r).abs() <= 0.004 &&
+          (item.g - _target.g).abs() <= 0.004 &&
+          (item.b - _target.b).abs() <= 0.004;
+    } else {
+      return false;
+    }
+  }
+}
+
+Matcher _matchesColor(ui.Color color) {
+  return _ColorMatcher(color);
+}
+
 void main() {
   testWidgets('ImageDecoration.lerp 1', (WidgetTester tester) async {
     final MemoryImage green = MemoryImage(Uint8List.fromList(<int>[
@@ -178,17 +208,17 @@ void main() {
         return getPixel(x, y);
       }
       const Color lime = Color(0xFF00FF00);
-      expect(getBlockPixel(0), lime); // pure green
-      expect(getBlockPixel(1), lime); // 100% green 0% red
-      expect(getBlockPixel(2), const Color(0xFF1AE600));
-      expect(getBlockPixel(3), const Color(0xFF33CC00));
-      expect(getBlockPixel(4), const Color(0xFF808000)); // 50-50 mix green/red
-      expect(getBlockPixel(5), const Color(0xFFCD3200));
-      expect(getBlockPixel(6), const Color(0xFFE61900));
-      expect(getBlockPixel(7), const Color(0xFFFF0000)); // 0% green 100% red
-      expect(getBlockPixel(8), const Color(0xFFFF0000)); // pure red
+      expect(getBlockPixel(0), _matchesColor(lime)); // pure green
+      expect(getBlockPixel(1), _matchesColor(lime)); // 100% green 0% red
+      expect(getBlockPixel(2), _matchesColor(const Color(0xFF1AE600)));
+      expect(getBlockPixel(3), _matchesColor(const Color(0xFF33CC00)));
+      expect(getBlockPixel(4), _matchesColor(const Color(0xFF808000))); // 50-50 mix green/red
+      expect(getBlockPixel(5), _matchesColor(const Color(0xFFCD3200)));
+      expect(getBlockPixel(6), _matchesColor(const Color(0xFFE61900)));
+      expect(getBlockPixel(7), _matchesColor(const Color(0xFFFF0000))); // 0% green 100% red
+      expect(getBlockPixel(8), _matchesColor(const Color(0xFFFF0000))); // pure red
       for (int index = 9; index < 40; index += 1) {
-        expect(getBlockPixel(index), lime);
+        expect(getBlockPixel(index), _matchesColor(lime));
       }
     }

@gaaclarke
Copy link
Member

@zanderso how come you can push? This is what I got:

$ git push a-siva
error: Authentication error: Authentication required: You must have push access to verify locks
error: failed to push some refs to 'github.com:a-siva/flutter.git'

@zanderso
Copy link
Member

zanderso commented Sep 6, 2024

  1. Click on the link for the branch on @a-siva's fork above a-siva:tot.
  2. Navigate to the file to edit, and edit with the GitHub editor.
  3. Click the button to push to the branch.

@a-siva
Copy link
Contributor Author

a-siva commented Sep 6, 2024

I don't have permissions to push to this branch. Here's the patch:

commit 9095aa51144561d0d094c08f1a718bf30f6c1bc3
Author: Aaron Clarke <aaclarke@google.com>
Date:   Fri Sep 6 11:41:54 2024 -0700

    added fuzzy color matching

diff --git a/packages/flutter/test/painting/decoration_image_lerp_test.dart b/packages/flutter/test/painting/decoration_image_lerp_test.dart
index ab24e4fdc4..ad9fe6e064 100644
--- a/packages/flutter/test/painting/decoration_image_lerp_test.dart
+++ b/packages/flutter/test/painting/decoration_image_lerp_test.dart
@@ -15,6 +15,36 @@ import 'package:flutter/foundation.dart';
 import 'package:flutter/material.dart';
 import 'package:flutter_test/flutter_test.dart';
 
+// TODO(gaaclarke): Unify the different instances of _ColorMatcher.
+/// Positive result if the colors would be mapped to the same argb8888 color.
+class _ColorMatcher extends Matcher {
+  _ColorMatcher(this._target);
+
+  final ui.Color _target;
+
+  @override
+  Description describe(Description description) {
+    return description.add('matches "$_target"');
+  }
+
+  @override
+  bool matches(dynamic item, Map<dynamic, dynamic> matchState) {
+    if (item is ui.Color) {
+      return item.colorSpace == _target.colorSpace &&
+          (item.a - _target.a).abs() <= 0.004 &&
+          (item.r - _target.r).abs() <= 0.004 &&
+          (item.g - _target.g).abs() <= 0.004 &&
+          (item.b - _target.b).abs() <= 0.004;
+    } else {
+      return false;
+    }
+  }
+}
+
+Matcher _matchesColor(ui.Color color) {
+  return _ColorMatcher(color);
+}
+
 void main() {
   testWidgets('ImageDecoration.lerp 1', (WidgetTester tester) async {
     final MemoryImage green = MemoryImage(Uint8List.fromList(<int>[
@@ -178,17 +208,17 @@ void main() {
         return getPixel(x, y);
       }
       const Color lime = Color(0xFF00FF00);
-      expect(getBlockPixel(0), lime); // pure green
-      expect(getBlockPixel(1), lime); // 100% green 0% red
-      expect(getBlockPixel(2), const Color(0xFF1AE600));
-      expect(getBlockPixel(3), const Color(0xFF33CC00));
-      expect(getBlockPixel(4), const Color(0xFF808000)); // 50-50 mix green/red
-      expect(getBlockPixel(5), const Color(0xFFCD3200));
-      expect(getBlockPixel(6), const Color(0xFFE61900));
-      expect(getBlockPixel(7), const Color(0xFFFF0000)); // 0% green 100% red
-      expect(getBlockPixel(8), const Color(0xFFFF0000)); // pure red
+      expect(getBlockPixel(0), _matchesColor(lime)); // pure green
+      expect(getBlockPixel(1), _matchesColor(lime)); // 100% green 0% red
+      expect(getBlockPixel(2), _matchesColor(const Color(0xFF1AE600)));
+      expect(getBlockPixel(3), _matchesColor(const Color(0xFF33CC00)));
+      expect(getBlockPixel(4), _matchesColor(const Color(0xFF808000))); // 50-50 mix green/red
+      expect(getBlockPixel(5), _matchesColor(const Color(0xFFCD3200)));
+      expect(getBlockPixel(6), _matchesColor(const Color(0xFFE61900)));
+      expect(getBlockPixel(7), _matchesColor(const Color(0xFFFF0000))); // 0% green 100% red
+      expect(getBlockPixel(8), _matchesColor(const Color(0xFFFF0000))); // pure red
       for (int index = 9; index < 40; index += 1) {
-        expect(getBlockPixel(index), lime);
+        expect(getBlockPixel(index), _matchesColor(lime));
       }
     }

I will apply your patch and push it, not sure why you are getting the authentication error.

@a-siva a-siva requested a review from gaaclarke September 6, 2024 19:41
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 9, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 9, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 10, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 10, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 10, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 10, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 10, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 10, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 11, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 11, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 11, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 11, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 11, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 11, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 11, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 12, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 12, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 12, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 12, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 11, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 12, 2024
@a-siva a-siva deleted the tot branch March 25, 2025 22:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

a: tests "flutter test", flutter_test, or one of our tests d: api docs Issues with https://api.flutter.dev/ d: examples Sample code and demos engine flutter/engine related. See also e: labels. f: integration_test The flutter/packages/integration_test plugin framework flutter/packages/flutter repository. See also f: labels. tool Affects the "flutter" command-line tool. See also t: labels. will affect goldens Changes to golden files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants