Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Conversation

@harryterkelsen
Copy link
Contributor

@harryterkelsen harryterkelsen commented Apr 15, 2024

Cherry pick of #49572 to fix flutter/flutter#145563

Cherry-pick request issue: flutter/flutter#146787

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 and the C++, Objective-C, Java style guides.
  • I listed at least one issue that this PR fixes in the description above.
  • I added new tests to check the change I am making or feature I am adding, or the PR is test-exempt. See testing the engine for instructions on writing and running engine tests.
  • I updated/added relevant documentation (doc comments with ///).
  • I signed the CLA.
  • All existing and new tests are passing.

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

flutter#49572)

This PR refactors rasterization to support two different modes, single
GL context mode backed by a single OffscreenCanvas (Chrome), and
multi-context mode which uses many GL contexts and canvases.

## Pre-launch Checklist

- [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] and the [C++,
Objective-C, Java style guides].
- [x] I listed at least one issue that this PR fixes in the description
above.
- [x] I added new tests to check the change I am making or feature I am
adding, or the PR is [test-exempt]. See [testing the engine] for
instructions on writing and running engine tests.
- [x] I updated/added relevant documentation (doc comments with `///`).
- [x] I signed the [CLA].
- [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/wiki/Tree-hygiene#overview
[Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene
[test-exempt]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#tests
[Flutter Style Guide]:
https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo
[C++, Objective-C, Java style guides]:
https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
[testing the engine]:
https://github.com/flutter/flutter/wiki/Testing-the-engine
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes
[Discord]: https://github.com/flutter/flutter/wiki/Chat
Copy link
Contributor

@yjbanov yjbanov left a comment

Choose a reason for hiding this comment

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

lgtm

@godofredoc
Copy link
Contributor

@harryterkelsen @yjbanov this CP seems to be the same as #51730 which was closed because of concerns landing the complex change as a hotfix to stable. Can you please sync up with @zanderso before continuing with CP?

@yjbanov
Copy link
Contributor

yjbanov commented Apr 16, 2024

I think we should accept this hot fix. It fixes a critical performance issue (some apps are nearly unusable in Firefox and, to a lesser extent, in Safari). It is less risky now compared to the time the cherry pick was requested. This change has been tested with many of our customers since it landed in January.

@godofredoc godofredoc added the autosubmit Merge PR when tree becomes green via auto submit App label Apr 16, 2024
@auto-submit auto-submit bot merged commit a0b2d83 into flutter:flutter-3.19-candidate.1 Apr 16, 2024
@itsjustkevin
Copy link
Contributor

@yjbanov this seems to be working fine on beta, but just in case, could you monitor the change as it lands in stable to ensure the regressions are no longer there?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

autosubmit Merge PR when tree becomes green via auto submit App e: impeller platform-android platform-web Code specifically for the web engine

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Severe performance regression on Firefox in v. 3.19

4 participants