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

[Impeller] specialize the geometry for drawRect #36914

Merged
merged 1 commit into from
Oct 22, 2022

Conversation

jonahwilliams
Copy link
Member

Avoid polyline generation and tessellation when we are given a rect via canvas.drawRect. This improves the raster time of a frame made of thousands of rects to about 1/3 of the original time.

Before

image

After

image

Demo code

// 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 'package:flutter/material.dart';

void main() {
  runApp(
    const MaterialApp(
      home: Scaffold(
        body: Center(
          child: Benchmark(),
        )
      ),
    )
  );
}

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

  @override
  State<Benchmark> createState() => _BenchmarkState();
}

class _BenchmarkState extends State<Benchmark> {
  bool showQR = false;

  @override
  Widget build(BuildContext context) {
    if (!showQR) {
      return TextButton(
        key: const ValueKey<String>('Button'),
        onPressed: () {
          setState(() {
            showQR = true;
          });
        },
        child: const Text('Start Bench'),
      );
    }
    return CustomPaint(
      key: const ValueKey<String>('Painter'),
      painter: QRPainter(),
      size: const Size(800, 800),
    );
  }
}

// Draw a "QR" like-code with 400x400 squarees.
class QRPainter extends CustomPainter {
  @override
  void paint(Canvas canvas, Size size) {
    final double boxWidth = size.width / 400;
    final double boxHeight = size.height / 400;
    for (int i = 0; i < 400; i++) {
      for (int j = 0; j < 400; j++) {
        final Rect rect = Rect.fromLTWH(i * boxWidth, j * boxHeight, boxWidth, boxHeight);
          canvas.drawRect(rect, Paint()
            ..style = PaintingStyle.fill
            ..color = Colors.red
          );
      }
    }
  }

  @override
  bool shouldRepaint(covariant CustomPainter oldDelegate) {
    return false;
  }
}

@jonahwilliams
Copy link
Member Author

The difference in UI time is probably from local engine usage...

GeometryResult RectGeometry::GetPositionUVBuffer(const ContentContext& renderer,
const Entity& entity,
RenderPass& pass) {
// TODO(jonahwilliams): support texture coordinates in rect geometry.
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: file a bug and explain what this would block/where it would go wrong.

Copy link
Member Author

Choose a reason for hiding this comment

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

Its not really a useful API yet. I was actually thinking we should create a specific drawVertices subclass that adds these methods, since none of the other geometries will use it

Copy link
Contributor

@dnfield dnfield left a comment

Choose a reason for hiding this comment

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

I have mixed feelings about this.

  • This will help in cases where someone does canvas.drawRect, but not if they do canvas.drawPath with a path that has a bunch of (or a single) rect in it.
  • This might make it harder to do optimizations that would batch multiple canvas.drawFoo calls that are compatible into something that can be handled in a single draw call. I have a dream of making the Aiks canvas able to track the last entity it used and append to it when possible instead of always creating a new one. This means we'd need geometries to know how to append where possible. I think having this kind of specialized geometry won't affect that too much because rects are simple, but it may get harder if we start specializing other geometry types that are more complicated...
  • How common is canvas.drawRect compared to canvas.drawPath or canvas.drawRRect?
  • How does this compare to an approach like I'm describing above, where all the drawRect calls get batched into a single entity/geometry?

@jonahwilliams
Copy link
Member Author

  1. I think this is fine, its not fair to say that optimizing A is bad because it doesn't also optimize B
  2. On the contrary, I think this makes it easier. All geometries can be compatible with eachother, and you could imagine even baching at the geometry level.
  3. very common, the QR code, the timeline in wonders, et cetera. Notably, this is a place where impeller is noticably slower than Skia
  4. Batching rect calls makes them even slower, since you have to generate and tessellate a larger polygon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
autosubmit Merge PR when tree becomes green via auto submit App
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants