Skip to content

[stable] Removes assumption that basis scalar and rounded_scalar match #165428

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
auto-submit merged 3 commits into from
Mar 28, 2025
Merged

[stable] Removes assumption that basis scalar and rounded_scalar match #165428

auto-submit merged 3 commits into from
Mar 28, 2025

Conversation

@gaaclarke
Copy link
Member

@gaaclarke gaaclarke commented Mar 18, 2025

Impacted Users

Impeller, all platforms

Impact Description

Text that is scaled over 48x will render at the incorrect size and position

Workaround

Change the font size instead of using a scale, but this isn't always clear from the framework usage.

Risk

If it is wrong, text will be rendered incorrectly.

Test Coverage

A golden test was written but they aren't automatically executed on release branches.

Validation Steps

#165130 has reproduction code

gaaclarke and others added 2 commits March 18, 2025 13:16
@gaaclarke
Removes assumption that basis scalar and rounded_scalar match (flutte…
64ead24 
…r#165166)

fixes flutter#165130

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

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

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview
[Tree Hygiene]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md
[test-exempt]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests
[Flutter Style Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md
[Features we expect every widget to implement]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes
[Discord]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md
[Data Driven Fixes]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
@gaaclarke
updated changelog
d5d316f
@gaaclarke gaaclarke requested a review from jonahwilliams March 18, 2025 20:24
@github-actions github-actions bot added a: text input Entering text in a text field or keyboard related problems engine flutter/engine related. See also e: labels. e: impeller Impeller rendering backend issues and features requests labels Mar 18, 2025
@gaaclarke gaaclarke added cp: review Cherry-picks in the review queue and removed a: text input Entering text in a text field or keyboard related problems engine flutter/engine related. See also e: labels. e: impeller Impeller rendering backend issues and features requests labels Mar 18, 2025
jonahwilliams
jonahwilliams approved these changes Mar 18, 2025
View reviewed changes
Copy link
Contributor

@jonahwilliams jonahwilliams left a comment

Choose a reason for hiding this comment

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

LGTM

@jonahwilliams jonahwilliams added the cp: stable cherry pick this pull request to stable release candidate branch label Mar 20, 2025
justinmc
justinmc approved these changes Mar 26, 2025
View reviewed changes
Copy link
Contributor

@justinmc justinmc left a comment

Choose a reason for hiding this comment

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

LGTM 👍 . @gaaclarke Are the tests supposed to be failing here?

@jonahwilliams
Copy link
Contributor

jonahwilliams commented Mar 26, 2025

We're not sure why the tests are failing:

error: {
  code: 5
  message: builder not found: "Linux_android_emu_34 android_defines_test"
}
error: {
  code: 5
  message: builder not found: "Linux_android_emu_34 external_textures_integration_test"
}

seems like an issue where main/stable disagree about the set of builders? I think we've resolved that before by cherry picking recipie changes into stable.

@justinmc justinmc mentioned this pull request Mar 26, 2025
Merged
auto-submit bot pushed a commit that referenced this pull request Mar 27, 2025
@justinmc
Cherry pick to fix linux tests (#166021)
3548963 
The `Linux tool_integration_tests*` have been failing on this release branch, blocking cherry picks #165430 and #165428. This PR attempts to cherry pick fixes that will make the tests go green.

This PR contains cherry-picks of three PRs:

  * #164935
  * #165620
  * #165661

Example of the failures I was seeing: https://github.com/flutter/flutter/pull/165428/checks?check_run_id=38997055797.

```
error: {
  code: 5
  message: builder not found: "Linux tool_integration_tests_1_6"
}
```
@github-actions github-actions bot added a: text input Entering text in a text field or keyboard related problems engine flutter/engine related. See also e: labels. e: impeller Impeller rendering backend issues and features requests labels Mar 27, 2025
@justinmc justinmc added the autosubmit Merge PR when tree becomes green via auto submit App label Mar 28, 2025
@auto-submit auto-submit bot merged commit e272b5c into flutter:flutter-3.29-candidate.0 Mar 28, 2025
129 checks passed
@justinmc
Copy link
Contributor

justinmc commented Mar 28, 2025

The failures here were successfully fixed by #166021. I'm hoping that the same PR will also fix the other cherry pick #165430.

@reidbaker reidbaker mentioned this pull request Apr 4, 2025
Merged
9 tasks
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 14, 2025
@engine-flutter-autoroll
e272b5cde [stable] Removes assumption that basis scalar and rounded_s…
38bf195 
…calar match (flutter/flutter#165428)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 15, 2025
@engine-flutter-autoroll
e272b5cde [stable] Removes assumption that basis scalar and rounded_s…
40e5920 
…calar match (flutter/flutter#165428)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 15, 2025
@engine-flutter-autoroll
e272b5cde [stable] Removes assumption that basis scalar and rounded_s…
d5c0469 
…calar match (flutter/flutter#165428)
Fintasys pushed a commit to Fintasys/flutter that referenced this pull request May 14, 2025
@justinmc @Fintasys
Cherry pick to fix linux tests (flutter#166021)
b06705b 
The `Linux tool_integration_tests*` have been failing on this release branch, blocking cherry picks flutter#165430 and flutter#165428. This PR attempts to cherry pick fixes that will make the tests go green.

This PR contains cherry-picks of three PRs:

  * flutter#164935
  * flutter#165620
  * flutter#165661

Example of the failures I was seeing: https://github.com/flutter/flutter/pull/165428/checks?check_run_id=38997055797.

```
error: {
  code: 5
  message: builder not found: "Linux tool_integration_tests_1_6"
}
```
Fintasys pushed a commit to Fintasys/flutter that referenced this pull request May 14, 2025
@gaaclarke @Fintasys
[stable] Removes assumption that basis scalar and rounded_scalar match (
9e78446 
flutter#165428)

## Impacted Users
Impeller, all platforms

## Impact Description
Text that is scaled over 48x will render at the incorrect size and position

## Workaround
Change the font size instead of using a scale, but this isn't always clear from the framework usage.

## Risk
If it is wrong, text will be rendered incorrectly.

## Test Coverage
A golden test was written but they aren't automatically executed on release branches.

## Validation Steps
flutter#165130 has reproduction code
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

@jonahwilliams jonahwilliams jonahwilliams approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

a: text input Entering text in a text field or keyboard related problems autosubmit Merge PR when tree becomes green via auto submit App cp: review Cherry-picks in the review queue cp: stable cherry pick this pull request to stable release candidate branch e: impeller Impeller rendering backend issues and features requests engine flutter/engine related. See also e: labels.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@gaaclarke @jonahwilliams @justinmc