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

Reverts "[Impeller] hash even less stuff per frame. (#55092)" #55491

Merged
auto-submit merged 1 commit into from
Sep 27, 2024
Merged

Reverts "[Impeller] hash even less stuff per frame. (#55092)" #55491

auto-submit merged 1 commit into from
Sep 27, 2024

Conversation

@auto-submit
Copy link
Contributor

@auto-submit auto-submit bot commented Sep 27, 2024

Reverts: #55092

Initiated by: jonahwilliams

Reason for reverting: framework golden failures.

Original PR Author: jonahwilliams

Reviewed By: {chinmaygarde, jtmcdole}

This change reverts the following previous change:
Follow up to #55060

Currently we have multiple stages of hashing while font rendering, which is relatively expensive for the actualy required workload. First, we hash the contents of all text frames to compute the unique set of glyphs per frame. Then we diff this hash set against the hashmap of glyphs within the atlas. Finally we hash and lookup the final rendered bounds for each glyph.

We can simplify this to 2. hash lookups for glyphs not yet in the atlas and 1. hash lookup for glyphs that are in the atlas. This is done by combing the step where we uniquely compute glyphs per frame with the diff against the current atlas. When this lookup is performed, we also store the glyph position (if found) in the text_frame itself - which allows text contents to skip the last hash, as long as the glyph has already been rendered.

Before

flutter_03

After

flutter_04

Using this handy dandy test app:

import 'package:flutter/material.dart';

void main() {
  runApp(MyApp());
}

class MyApp extends StatelessWidget {

  Widget build(context) {
    return MaterialApp(
      home: Scaffold(
        appBar: AppBar(
          title: Text('Platform View'),
        ),
        body: SafeArea(child: Stack(children: [
          SizedBox(
            width: 380,
            height: 380,
            child: LinearProgressIndicator(),
          ),
          Stack(
            children: List<Widget>.generate(1000, (index) {
              // The problem already happens with a small amount of widgets.
              // Using an excessive amount of widgets is just to make the problem more evident.
              return Text("Lots of Texts represent a Widget with complex components.");
            }),
          ),

          Align(
            alignment: Alignment.bottomCenter,
            child:
            TextButton(
              child: Text("Button"),
              onPressed: () {
                print("Tap ${DateTime.now()}");
              },
            ),
          ),
        ],
        ),
        ),
      ),
    );
  }
}

@auto-submit auto-submit bot added the revert of Bot Only: Tracking label for bot. Tracks new revert of pull requests. label Sep 27, 2024
@auto-submit auto-submit bot merged commit 852dd3a into main Sep 27, 2024
@auto-submit auto-submit bot deleted the revert_d283bc8d7e6025e52428aa3181065b3c18ecf119 branch September 27, 2024 17:09
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Sep 27, 2024
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 27, 2024
@engine-flutter-autoroll
852dd3a Reverts "[Impeller] hash even less stuff per frame. (flutter#…
f3abdfb 
…55092)" (flutter/engine#55491)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Sep 27, 2024
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 27, 2024
@engine-flutter-autoroll
852dd3a Reverts "[Impeller] hash even less stuff per frame. (flutter#…
4173825 
…55092)" (flutter/engine#55491)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Sep 27, 2024
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 27, 2024
@engine-flutter-autoroll
852dd3a Reverts "[Impeller] hash even less stuff per frame. (flutter#…
a04fa8d 
…55092)" (flutter/engine#55491)
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Sep 27, 2024
@engine-flutter-autoroll
Roll Flutter Engine from 7c603de to f21f2b2 (6 revisions) (#155843)
2dbd73d 
flutter/engine@7c603de...f21f2b2

2024-09-27 1083941774@qq.com [docs] Fix broken links in docs/ (flutter/engine#55350)
2024-09-27 skia-flutter-autoroll@skia.org Roll Skia from e77818421e91 to 7efc11f2ea9e (6 revisions) (flutter/engine#55489)
2024-09-27 jason-simmons@users.noreply.github.com Listen for uncaught exceptions during loading of a web test suite in Chrome (flutter/engine#55166)
2024-09-27 98614782+auto-submit[bot]@users.noreply.github.com Reverts "[Impeller] hash even less stuff per frame. (#55092)" (flutter/engine#55491)
2024-09-27 mdebbar@google.com [web] Update builder json generator to reflect recent changes (flutter/engine#55307)
2024-09-27 jonahwilliams@google.com [Impeller] hash even less stuff per frame. (flutter/engine#55092)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-engine-flutter-autoroll
Please CC bdero@google.com,zra@google.com on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
thejitenpatel pushed a commit to thejitenpatel/flutter that referenced this pull request Oct 1, 2024
@engine-flutter-autoroll @thejitenpatel
Roll Flutter Engine from 7c603de to f21f2b2 (6 revisions) (flutter#15…
1cfce20 
…5843)

flutter/engine@7c603de...f21f2b2

2024-09-27 1083941774@qq.com [docs] Fix broken links in docs/ (flutter/engine#55350)
2024-09-27 skia-flutter-autoroll@skia.org Roll Skia from e77818421e91 to 7efc11f2ea9e (6 revisions) (flutter/engine#55489)
2024-09-27 jason-simmons@users.noreply.github.com Listen for uncaught exceptions during loading of a web test suite in Chrome (flutter/engine#55166)
2024-09-27 98614782+auto-submit[bot]@users.noreply.github.com Reverts "[Impeller] hash even less stuff per frame. (flutter#55092)" (flutter/engine#55491)
2024-09-27 mdebbar@google.com [web] Update builder json generator to reflect recent changes (flutter/engine#55307)
2024-09-27 jonahwilliams@google.com [Impeller] hash even less stuff per frame. (flutter/engine#55092)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-engine-flutter-autoroll
Please CC bdero@google.com,zra@google.com on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@fluttergithubbot fluttergithubbot fluttergithubbot approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@jonahwilliams jonahwilliams

Labels

e: impeller platform-ios revert of Bot Only: Tracking label for bot. Tracks new revert of pull requests.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@fluttergithubbot @jonahwilliams