Skip to content
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.

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

Poor performance when subscribing to MouseRegion events #41194

Closed
LehaIvanov opened this issue Sep 24, 2019 · 19 comments · Fixed by #59883
Closed

Poor performance when subscribing to MouseRegion events #41194

LehaIvanov opened this issue Sep 24, 2019 · 19 comments · Fixed by #59883
Labels
a: mouse Issues related to using a mouse or mouse support c: performance Relates to speed or footprint issues (see "perf:" labels) f: gestures flutter/packages/flutter/gestures repository. framework flutter/packages/flutter repository. See also f: labels. P3 Issues that are less important to the Flutter project perf: speed Performance issues related to (mostly rendering) speed platform-web Web applications specifically

Comments

@LehaIvanov
Copy link
Contributor

Hello,

Currently we developing a Flutter Web application with the intent to move to production in a few months. Almost everything works great but we found a major performance issue when subscribing to mouse events for many widgets.

We use ListView to display/scroll grid-like data and scrolling performance is 60fps on our testing devices which is perfect for us. But when we attach mouse event listeners to our widgets/cells we get major performance regression. (we tried to show tooltips using Tooltip widget but the problem seems to be much wider).

We found that this issue somewhat relates to widgets having rounded borders and/or different borders specified for each side. The problem should be easily reproducible so we provide only minimal main.dart examples without videos.

Scrolling in both examples below produces less than 60fps, but if you comment onEnter: (_) => {} handler in MouseRegion performance will increase dramatically and will reach 60fps.

main.dart: Case 1: Border radius

import 'package:flutter/material.dart';

Future<void> main() async {
  runApp(MyApp());
}

@immutable
class MyApp extends StatelessWidget {
  @override
  Widget build(BuildContext context) {
    const int rowsCount = 60;
    const int columnsCount = 20;
    const double containerSize = 40;

    return MaterialApp(
      home: Scaffold(
          body: ListView.builder(
        itemCount: rowsCount,
        cacheExtent: rowsCount * containerSize,
        itemBuilder: (BuildContext context, _) => Row(
          children: List<Widget>.generate(
            columnsCount,
            (_) => MouseRegion(
              // Comment the line below to fix the performance problem
              onEnter: (_) => {},
              child: Container(
                decoration: BoxDecoration(
                  borderRadius: BorderRadius.circular(10),
                  border: Border.all(
                    width: 1,
                    color: Colors.lightBlue,
                  ),
                ),
                width: containerSize,
                height: containerSize,
              ),
            ),
          ),
        ),
      )),
    );
  }
}

main.dart: Case 1: Different side borders

import 'package:flutter/material.dart';

Future<void> main() async {
  runApp(MyApp());
}

@immutable
class MyApp extends StatelessWidget {
  Border _getBorder(int columnIndex, int rowIndex) {
    const BorderSide defaultBorderSide = BorderSide();

    return Border(
      left: columnIndex == 0 ? defaultBorderSide : BorderSide.none,
      top: rowIndex == 0 ? defaultBorderSide : BorderSide.none,
      right: defaultBorderSide,
      bottom: defaultBorderSide,
    );
  }

  @override
  Widget build(BuildContext context) {
    const int rowsCount = 60;
    const int columnsCount = 20;
    const double containerSize = 50;

    return MaterialApp(
      home: Scaffold(
          body: ListView.builder(
        itemCount: rowsCount,
        cacheExtent: rowsCount * containerSize,
        itemBuilder: (BuildContext context, int rowIndex) => Row(
          children: List<Widget>.generate(
            columnsCount,
            (int columnIndex) => MouseRegion(
              // Comment the line below to fix the performance problem
              onEnter: (_) => {},
              child: Container(
                decoration: BoxDecoration(
                  border: _getBorder(columnIndex, rowIndex),
                ),
                width: containerSize,
                height: containerSize,
              ),
            ),
          ),
        ),
      )),
    );
  }
}

We also found that the issue does not appear if we enable Skia based renderer so its source might be in Flutter Web engine, not in framework.

Please let me know if you need any additional information (videos, profile snapshot, etc) to diagnose the issue.

We wonder if there any existing issues/plans to improve Flutter Web performance in such scenarios, or is there any workaround for this?...

Doctor summary (to see all details, run flutter doctor -v):
[√] Flutter (Channel unknown, v1.10.6-pre.7, on Microsoft Windows [Version 10.0.17134.950], locale ru-RU)

 

[√] Android toolchain - develop for Android devices (Android SDK version 28.0.3)
[√] Chrome - develop for the web
[√] Visual Studio - develop for Windows (Visual Studio CommunityВ 2019 16.3.0)
[√] Android Studio (version 3.5)
[√] VS Code (version 1.38.1)
[√] Connected device (3 available)

 

• No issues found!
@LehaIvanov LehaIvanov added the c: performance Relates to speed or footprint issues (see "perf:" labels) label Sep 24, 2019
@jonahwilliams jonahwilliams added the platform-web Web applications specifically label Sep 24, 2019
@jonahwilliams
Copy link
Member

cc @ditman

@jonahwilliams
Copy link
Member

cc @gspencergoog

@ditman
Copy link
Member

ditman commented Sep 25, 2019

Woah, I'll try to profile this to see where the time is going, since I recently tweaked some rendering of DRRects. Thanks for the repro code!

@ditman ditman self-assigned this Sep 25, 2019
@gspencergoog
Copy link
Contributor

The current implementation generates a new frame for every mouse move, but that would only produce a regression when the mouse is moving, not otherwise. There's a fix for that in #41014

@LehaIvanov
Copy link
Contributor Author

@gspencergoog, today we just switched our Flutter installation to this commit in #41014 and the issue is still there, we didn't notice any improvements in our cases...

@gspencergoog
Copy link
Contributor

OK, that's as I suspected. More likely it has to do with the rendering.

@ditman
Copy link
Member

ditman commented Sep 30, 2019

I did some profiling, and the issue seems to be related to a spike on DOM operations, and not necessarily to how we draw drrects into canvas (phew). I'll keep digging.

@LehaIvanov you call this a 'regression': has this ever worked well for you, and then it started being slow after some change, or is it just that performance gets worse when adding the onEnters?

@LehaIvanov
Copy link
Contributor Author

@ditman thank you for your help with this issue.

I used "regression" term by mistake: this is not a regression - I had never seen this working well.
Actually, I mean "performance degradation" and yes - the performance just gets worse when we add mouse events.

We also did some investigations and we almost pretty sure that this issue not related to rounded rect painting: it seems to be related to engine/compositing code and/or DOM operations.

Please let me know if you need any further help with this. We plan to go to production with Flutter Web in a few months and this is currently the only blocker we have. So I will definitely do everything
that will help to land the fix sooner :)

@yjbanov yjbanov added this to the October 2019 milestone Oct 3, 2019
@ditman
Copy link
Member

ditman commented Oct 10, 2019

@LehaIvanov So after a bunch of investigation, it seems that the issue is that active MouseRegions create additional PictureLayers that "break" the optimizations that the relayout boundaries of the Row widget provide (we end up with "too many layers", which in web translates to "too much DOM churn").

I have a pushed a fix to my flutter fork (which you can maybe try on your app?), but I need to seek approval from the owners of the MouseRegions feature (@gspencergoog ?), because I might be breaking something that I'm not even aware of.

Apologies for the delayed response, it took me a while to figure a root cause for this one :)

@jonahwilliams
Copy link
Member

The MouseRegion creates AnnotatedRegion layers in the layer tree, which I assume are passed to the web engine as is? These don't have any effect on rendering, so it might be possible to filter these out, so to speak, when talking to the web engine

@LehaIvanov
Copy link
Contributor Author

@ditman, thank you very much for your efforts working on this issue. Today we tested your fork with our cases and everything works great!

Actually, we are trying to render many elements on screen with tooltips. We found another performance issue (not related to this one) with tooltips / global routes. We plan to fill an issue and create a PR that fixes it ourselves in a few days.

So we are looking forward to see your changes in Flutter master soon :)

@ditman
Copy link
Member

ditman commented Oct 10, 2019

@LehaIvanov thanks, we're looking forward to see what you put in production, if it's publicly available!

@jonahwilliams I think that's more or less what I did. The "problem" seems to be that instead of having a tree like this:

Row
  PaintLayer

(Which is the case when the MouseRegion is not active) we end up with this:

Row
  MouseRegion
    PaintLayer
  MouseRegion
    PaintLayer
  ...

So the relayout boundary of the Row, which allows it to bunch a group of paints together into a larger PaintLayer is split into smaller ones (children of the MouseRegion), which in turn causes a very large number of DOM nodes to be created and moved around.

This is not a big problem in other platforms, where canvas allocation is not as bad (in fact the OP says that with the Skia backend, this is fine), but in normal web with canvas, touching the DOM this much is quite expensive and performance drops.

I didn't find a (fast) way to do post-processing here, since layer creation is somewhat tied to DOM nodes. We may be able to brainstorm some ideas for a more proper fix for this, rather than "don't break up the rendering when pushing layers that really don't paint anything", but that felt like a much larger scope (to the point that I'm not entirely sure how large it'd be :P)

@dkwingsmt dkwingsmt self-assigned this Oct 10, 2019
@dkwingsmt dkwingsmt modified the milestones: October 2019, November 2019 Oct 10, 2019
@dkwingsmt dkwingsmt removed their assignment Oct 10, 2019
@dkwingsmt
Copy link
Contributor

I'd like explore the possibility of removing the need of annotated region layers and letting the parent layer or the layer tree take care of the annotation. However I'll have to do it after my current project (mouse cursor).

@ditman
Copy link
Member

ditman commented Oct 11, 2019

Thanks for taking this over @dkwingsmt!

I'm going to dis-assign myself, since I'm not going to work on this directly. Feel free to ping me if I can be of any assistance (I'll keep my branch up just for reference, at least for a while)!

@ditman ditman removed their assignment Oct 11, 2019
@Hixie Hixie modified the milestones: November 2019, Goals Nov 27, 2019
@Hixie Hixie added a: desktop Running on desktop and removed a: desktop Running on desktop labels Nov 27, 2019
@Hixie Hixie changed the title Performance regression when subscribing to MouseRegion events Poor performance when subscribing to MouseRegion events Nov 27, 2019
@hossainiir
Copy link

Hi, is there any update for this issue?

@dkwingsmt
Copy link
Contributor

@hossainiir I do have a plan for this and I’m working on it now. It is due to some design issues about hover detection and we want to fix it in the most elegant way.

@hossainiir
Copy link

@dkwingsmt Thanks, In my app I have about 100 controls that have MouseRegion, this issue causes my app to have lot lags.

@agyimr
Copy link

agyimr commented Feb 3, 2020

I also created a little pet project web application, deployed it and the performance is extremely slow... I mean I am unable to scroll the site on mobile.

I hope it gets fixed because I have great plans for flutter web :)

@mariamhas mariamhas added this to Not Started in Web Performance via automation Feb 19, 2020
@kf6gpe kf6gpe added the P2 Important issues not at the top of the work list label May 29, 2020
@yjbanov yjbanov added P3 Issues that are less important to the Flutter project and removed P2 Important issues not at the top of the work list labels Jun 16, 2020
@iapicca iapicca added a: mouse Issues related to using a mouse or mouse support f: gestures flutter/packages/flutter/gestures repository. framework flutter/packages/flutter repository. See also f: labels. labels Jul 3, 2020
Web Performance automation moved this from Not Started to Done Jul 7, 2020
@liyuqian liyuqian added the perf: speed Performance issues related to (mostly rendering) speed label Jul 9, 2020
@github-actions
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 13, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
a: mouse Issues related to using a mouse or mouse support c: performance Relates to speed or footprint issues (see "perf:" labels) f: gestures flutter/packages/flutter/gestures repository. framework flutter/packages/flutter repository. See also f: labels. P3 Issues that are less important to the Flutter project perf: speed Performance issues related to (mostly rendering) speed platform-web Web applications specifically
Projects
Development

Successfully merging a pull request may close this issue.