Skip to content

Making TextPainter rounding hack disabled by default - #132094

Merged
auto-submit[bot] merged 3 commits into
flutter:masterfrom
LongCatIsLooong:text-painter-fix-tests
Aug 10, 2023
Merged

Making TextPainter rounding hack disabled by default#132094
auto-submit[bot] merged 3 commits into
flutter:masterfrom
LongCatIsLooong:text-painter-fix-tests

Conversation

@LongCatIsLooong

@LongCatIsLooong LongCatIsLooong commented Aug 7, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

Migrate tests in flutter/flutter. Once the tests here and in *_customer_testing are migrated, the default value of the migration flag will be changed from false to true, making the rounding hack disabled by default.

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
  • I signed the CLA.
  • I listed at least one issue that this PR fixes in the description above.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is test-exempt.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@github-actions github-actions Bot added a: text input Entering text in a text field or keyboard related problems framework flutter/packages/flutter repository. See also f: labels. p: material_ui material_ui package in flutter/packages d: api docs Issues with https://api.flutter.dev/ d: examples Sample code and demos a: internationalization Supporting other languages or locales. (aka i18n) p: cupertino_ui cupertino_ui package in flutter/packages labels Aug 7, 2023
@LongCatIsLooong
LongCatIsLooong force-pushed the text-painter-fix-tests branch 3 times, most recently from a8da0fe to 21f0ea8 Compare August 8, 2023 18:57
@github-actions github-actions Bot added the engine flutter/engine related. See also e: labels. label Aug 8, 2023
@LongCatIsLooong
LongCatIsLooong force-pushed the text-painter-fix-tests branch from 71583e4 to 18802bf Compare August 9, 2023 03:01
@LongCatIsLooong LongCatIsLooong changed the title Try making TextPainter rounding hack disabled by default Making TextPainter rounding hack disabled by default Aug 9, 2023
@LongCatIsLooong
LongCatIsLooong marked this pull request as ready for review August 9, 2023 04:57
@LongCatIsLooong LongCatIsLooong removed a: text input Entering text in a text field or keyboard related problems engine flutter/engine related. See also e: labels. a: internationalization Supporting other languages or locales. (aka i18n) d: api docs Issues with https://api.flutter.dev/ labels Aug 9, 2023
@LongCatIsLooong LongCatIsLooong mentioned this pull request Aug 9, 2023
Merged
8 tasks
justinmc
justinmc approved these changes Aug 9, 2023
View reviewed changes

@justinmc justinmc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM 👍. Looks like a lot of cruft going away because of this rounding thing. A super editor test is failing but I guess it has to be migrated too.

Comment thread packages/flutter/lib/src/painting/text_painter.dart Outdated
Comment on lines 517 to 520

@justinmc justinmc Aug 9, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can you explain this again, are you planning to remove the assert wrapping?

@LongCatIsLooong LongCatIsLooong Aug 9, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah it will be removed once flutter-team-archive/engine#44544 lands.

Comment thread packages/flutter/test/cupertino/dialog_test.dart Outdated
Comment on lines 350 to 351

@justinmc justinmc Aug 9, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can you remove the TODO above here? I see that you mentioned the 99933 issue in some other tests.

@github-actions github-actions Bot added a: text input Entering text in a text field or keyboard related problems a: internationalization Supporting other languages or locales. (aka i18n) d: api docs Issues with https://api.flutter.dev/ labels Aug 9, 2023
@LongCatIsLooong
LongCatIsLooong force-pushed the text-painter-fix-tests branch from 17ffa22 to 802ca4c Compare August 9, 2023 20:52
@LongCatIsLooong
LongCatIsLooong force-pushed the text-painter-fix-tests branch from 802ca4c to e38a64f Compare August 9, 2023 21:22
@LongCatIsLooong LongCatIsLooong added the autosubmit Merge PR when tree becomes green via auto submit App label Aug 9, 2023
@LongCatIsLooong
LongCatIsLooong deleted the text-painter-fix-tests branch August 10, 2023 00:31
auto-submit Bot pushed a commit to flutter-team-archive/engine that referenced this pull request Aug 10, 2023
@LongCatIsLooong
Disable text rounding hack by default (#44544)
7b6be5c 
This depends on flutter/flutter#132094 and customer_testing migration.

I'll announce this change and add a g3 fix after this lands.

[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Aug 10, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 10, 2023
@engine-flutter-autoroll
3f831b6 Making TextPainter rounding hack disabled by default (flutter…
f16c2b5 
…/flutter#132094)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Aug 10, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 10, 2023
@engine-flutter-autoroll
3f831b6 Making TextPainter rounding hack disabled by default (flutter…
015dc38 
…/flutter#132094)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Aug 10, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 10, 2023
@engine-flutter-autoroll
3f831b6 Making TextPainter rounding hack disabled by default (flutter…
43347f7 
…/flutter#132094)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Aug 10, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 10, 2023
@engine-flutter-autoroll
3f831b6 Making TextPainter rounding hack disabled by default (flutter…
3ccae10 
…/flutter#132094)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Aug 10, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 10, 2023
@engine-flutter-autoroll
3f831b6 Making TextPainter rounding hack disabled by default (flutter…
a1f4ff4 
…/flutter#132094)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Aug 10, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 10, 2023
@engine-flutter-autoroll
3f831b6 Making TextPainter rounding hack disabled by default (flutter…
6eea098 
…/flutter#132094)
@LongCatIsLooong LongCatIsLooong mentioned this pull request Aug 10, 2023
Merged
8 tasks
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Aug 10, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 10, 2023
@engine-flutter-autoroll
3f831b6 Making TextPainter rounding hack disabled by default (flutter…
be06749 
…/flutter#132094)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Aug 10, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 10, 2023
@engine-flutter-autoroll
3f831b6 Making TextPainter rounding hack disabled by default (flutter…
c9d464a 
…/flutter#132094)
auto-submit Bot pushed a commit that referenced this pull request Aug 10, 2023
@LongCatIsLooong
TextPainter migration cleanup (#132317)
e1fd2ac 
Undo temporary changes made in #132094
@HansMuller HansMuller mentioned this pull request Aug 11, 2023
Closed
gaaclarke pushed a commit to gaaclarke/engine that referenced this pull request Aug 30, 2023
@LongCatIsLooong @gaaclarke
Disable text rounding hack by default (flutter-team-archive#44544)
d349902 
This depends on flutter/flutter#132094 and customer_testing migration.

I'll announce this change and add a g3 fix after this lands.

[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 15, 2023
@engine-flutter-autoroll
3f831b6 Making TextPainter rounding hack disabled by default (flutter…
2b15581 
…/flutter#132094)
zhongdaiqi pushed a commit to zhongdaiqi/flutter_flutter that referenced this pull request Oct 9, 2026
@LongCatIsLooong
Disable text rounding hack by default (flutter-team-archive/engine#44544
e156901 
)

This depends on flutter/flutter#132094 and customer_testing migration.

I'll announce this change and add a g3 fix after this lands.

[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@justinmc justinmc justinmc approved these changes
+1 more reviewer
@HansMuller HansMuller HansMuller approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

a: internationalization Supporting other languages or locales. (aka i18n) a: text input Entering text in a text field or keyboard related problems autosubmit Merge PR when tree becomes green via auto submit App d: api docs Issues with https://api.flutter.dev/ d: examples Sample code and demos framework flutter/packages/flutter repository. See also f: labels. p: cupertino_ui cupertino_ui package in flutter/packages p: material_ui material_ui package in flutter/packages will affect goldens Changes to golden files

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@LongCatIsLooong @justinmc @HansMuller