Skip to content

[Impeller] Enabling RTree regressed canvas drawing with a large number of operations #126202

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

Closed
jonahwilliams opened this issue May 6, 2023 · 1 comment · Fixed by flutter/engine#42399
Labels
e: impeller Impeller rendering backend issues and features requests engine flutter/engine repository. See also e: labels. P2 Important issues not at the top of the work list

Comments

@jonahwilliams
Copy link
Member

Consider an application that does a large amount of Canvas drawing. After flutter/engine#41606 , the computation of the Rtree can easily dwarf other costs in the raster thread, especially if the culling is unsuccessful.

We should be smarter about this, potentially by avoiding reaching into individual pictures and using those bounds as opposed to the individual ops.

Here is an application that will stress this code by drawing a large number of small objects. I was investigating this as a motivating case for a shape buffer to avoid circle tesellation.

// Copyright 2014 The Flutter Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

import 'dart:async';
import 'dart:math' as math;

import 'package:flutter/material.dart';
void main() {
  runApp(const Example());
}

class Example extends StatefulWidget {
  const Example({super.key});

  @override
  State<Example> createState() => _ExampleState();
}

class _ExampleState extends State<Example> {
  late Timer timer;

  @override
  void initState() {
    super.initState();
    timer = Timer.periodic(const Duration(milliseconds: 4), (t) {
      setState(() {

      });
    });
  }

  @override
  void dispose() {
    timer.cancel();
    super.dispose();
  }

  @override
  Widget build(BuildContext context) {
    return Center(child:CustomPaint(painter: RandomCirclesPainter(
    color: Colors.red,
    radius: 0.5,
    count: 5000
  ), size: const Size(400, 400),));
  }
}

class RandomCirclesPainter extends CustomPainter {
  RandomCirclesPainter({
    required this.count,
    required this.radius,
    required this.color,
  });

  final int count;
  final double radius;
  final Color color;

  @override
  void paint(Canvas canvas, Size size) {
    // Create a paint object with the specified color.
    Paint paint = Paint()..color = color;

    // Generate a random list of offsets for the circles.
    List<Offset> offsets = List.generate(
      count,
      (_) => Offset(
        math.Random().nextDouble() * size.width,
        math.Random().nextDouble() * size.height,
      ),
    );

    // Draw the circles at the specified offsets.
    for (Offset offset in offsets) {
      canvas.drawCircle(
        offset,
        radius,
        paint,
      );
    }
  }

  @override
  bool shouldRepaint(RandomCirclesPainter oldDelegate) {
    return true;
  }
}

image

@jonahwilliams jonahwilliams added engine flutter/engine repository. See also e: labels. e: impeller Impeller rendering backend issues and features requests labels May 6, 2023
@chinmaygarde chinmaygarde added the P2 Important issues not at the top of the work list label May 8, 2023
@github-project-automation github-project-automation bot moved this to 🤔 Needs Triage in Impeller May 8, 2023
@chinmaygarde chinmaygarde moved this from 🤔 Needs Triage to ⚡ Performance in Impeller May 8, 2023
knopp added a commit to flutter/engine that referenced this issue Jun 5, 2023
Fixes flutter/flutter#116070
Fixes flutter/flutter#126202

Introduces `DlRegion` class which implements subset of `SkRegion`
required to get non-overlapping rectangles from region.

The implementation is different and faster than `SkRegion` for this
particular use-case (`display_list_region_benchmarks`):

Edit: Updated benchmark to latest revision and natively (initial run
went through rosetta)
```
----------------------------------------------------------------------------
Benchmark                                  Time             CPU   Iterations
----------------------------------------------------------------------------
BM_RegionBenchmarkDlRegion/Tiny          616 us          616 us          908
BM_RegionBenchmarkSkRegion/Tiny        70559 us        70557 us           10
BM_RegionBenchmarkDlRegion/Small        1315 us         1314 us          537
BM_RegionBenchmarkSkRegion/Small      121736 us       121717 us            6
BM_RegionBenchmarkDlRegion/Medium       1079 us         1079 us          650
BM_RegionBenchmarkSkRegion/Medium      22039 us        22035 us           32
BM_RegionBenchmarkDlRegion/Large         399 us          399 us         1763
BM_RegionBenchmarkSkRegion/Large        1510 us         1510 us          466
```

## 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 Hixie said 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
[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
@github-project-automation github-project-automation bot moved this from ⚡ Performance to ✅ Done in Impeller Jun 5, 2023
@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 Jun 19, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
e: impeller Impeller rendering backend issues and features requests engine flutter/engine repository. See also e: labels. P2 Important issues not at the top of the work list
Projects
No open projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants