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

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
6 checks passed
@auto-submit auto-submit bot deleted the revert_d283bc8d7e6025e52428aa3181065b3c18ecf119 branch September 27, 2024 17:09
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Sep 27, 2024
…155843)

flutter/engine@7c603de...f21f2b2

2024-09-27 [email protected] [docs] Fix broken links in docs/ (flutter/engine#55350)
2024-09-27 [email protected] Roll Skia from e77818421e91 to 7efc11f2ea9e (6 revisions) (flutter/engine#55489)
2024-09-27 [email protected] 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 [email protected] [web] Update builder json generator to reflect recent changes (flutter/engine#55307)
2024-09-27 [email protected] [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 [email protected],[email protected] 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
…lutter#155843)

flutter/engine@7c603de...f21f2b2

2024-09-27 [email protected] [docs] Fix broken links in docs/ (flutter/engine#55350)
2024-09-27 [email protected] Roll Skia from e77818421e91 to 7efc11f2ea9e (6 revisions) (flutter/engine#55489)
2024-09-27 [email protected] 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 [email protected] [web] Update builder json generator to reflect recent changes (flutter/engine#55307)
2024-09-27 [email protected] [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 [email protected],[email protected] 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.

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants