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] Reduce render-pass overhead when using wide gamut (iOS) #131567

Closed
knopp opened this issue Jul 30, 2023 · 11 comments · Fixed by flutter/engine#47808
Closed

[Impeller] Reduce render-pass overhead when using wide gamut (iOS) #131567

knopp opened this issue Jul 30, 2023 · 11 comments · Fixed by flutter/engine#47808
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 team-engine Owned by Engine team triaged-engine Triaged by Engine team

Comments

@knopp
Copy link
Member

knopp commented Jul 30, 2023

Consider the following example:

code
import 'dart:ui';
import 'package:flutter/material.dart';

void main() {
  runApp(const MainApp());
}

const kSigma = 1.0;
const kNumberOfBlurs = 6;

class _Blur extends StatelessWidget {
  const _Blur();

  @override
  Widget build(BuildContext context) {
    return ClipRRect(
      child: BackdropFilter(
        blendMode: BlendMode.srcIn,
        filter: ImageFilter.blur(
          sigmaX: kSigma,
          sigmaY: kSigma,
        ),
        child: Container(
          color: Colors.red.withAlpha(30),
          child: const Text('Blur'),
        ),
      ),
    );
  }
}

class MainApp extends StatelessWidget {
  const MainApp({super.key});

  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      home: Scaffold(
        body: Stack(
          children: [
            ListView.builder(
                itemBuilder: (context, index) {
                  return Container(
                    padding: const EdgeInsets.all(10),
                    child: Text('Item $index'),
                  );
                },
                itemCount: 1000),
            for (var i = 0; i < kNumberOfBlurs; ++i) ...[
              Positioned(
                left: 0,
                right: 0,
                top: i * 150,
                height: 60,
                child: const _Blur(),
              ),
            ]
          ],
        ),
      ),
    );
  }
}
screenshot

Screen Shot 2023-07-30 at 09 49 18

It has 6 blur backdrops with sigma=1 (to focus on render pass overhead and not blur filter performance).

This is the performance on A15 (iPhone 13 Pro) with wide gamut disabled:
Screenshot 2023-07-29 at 16 40 56

and wide gamut enabled:
Screenshot 2023-07-29 at 16 42 13

I tried couple of things to figure out why the regression is so large:

I noticed that there is quite a lot of texture allocation per frame. Caching textures improved the frame time by around 1-1.5ms.

Biggest bottleneck by far seems to be the blits for MSAA backdrop. They take around 80% of frame time.

Disabling MSAA backdrop (reverting back to LoadAction::kLoad + StoreAction::kStoreAndMultisampleResolve/kMultisampleResolve) seems to improve the performance considerably:
Screenshot 2023-07-29 at 22 42 22

cc @gaaclarke, @bdero, @jonahwilliams

@gaaclarke
Copy link
Member

gaaclarke commented Jul 31, 2023

It has 6 blur backdrops with sigma=1 (to focus on render pass overhead and not blur filter performance).

The amount of work required to calculate the blurs doesn't depend on the sigma value. Check out IPGaussian in gaussian.glsl.

edit: Maybe the dart sigma value is used to calculate the blur radius?

@knopp
Copy link
Member Author

knopp commented Jul 31, 2023

It has 6 blur backdrops with sigma=1 (to focus on render pass overhead and not blur filter performance).

The amount of work required to calculate the blurs doesn't depend on the sigma value. Check out IPGaussian in gaussian.glsl.

The sigma (as specified in ImageFilter.blur) is used to calculate blur radius, which affects the number of texture samples inside gaussian_blur.glsl. The number of of image samples seems to be the biggest bottleneck currently (see #131573). What am I missing here?

@gaaclarke
Copy link
Member

Thanks, that makes more sense. I didn't know we don't have a separate knob for radius at the dart level.

@dam-ease dam-ease added engine flutter/engine repository. See also e: labels. team-engine Owned by Engine team labels Aug 1, 2023
@chinmaygarde
Copy link
Member

This is related to making all blurs faster and just happens to show up more urgently when wide-gamut is enabled. That work is tracked in #131580. Closing as duplicate.

@chinmaygarde chinmaygarde closed this as not planned Won't fix, can't repro, duplicate, stale Aug 7, 2023
@knopp
Copy link
Member Author

knopp commented Aug 7, 2023

@chinmaygarde, the MSAA backdrop blit being significantly slower than LoadAction::kLoad + StoreAction::kStoreAndMultisampleResolve is an issue regardless of blur. MSAA blackdrop was introduced as an optimisation, but that's not always the case, shouldn't this be tracked?

@chinmaygarde
Copy link
Member

Ah, my bad. That makes sense. Can we reword this to describe the action-item instead of the symptom?

@chinmaygarde chinmaygarde reopened this Aug 7, 2023
@chinmaygarde chinmaygarde added the P2 Important issues not at the top of the work list label Aug 14, 2023
@chinmaygarde chinmaygarde added P1 High-priority issues at the top of the work list and removed P2 Important issues not at the top of the work list labels Aug 14, 2023
@jonahwilliams
Copy link
Member

@knopp I'm not getting similar numbers to you, can you post your full trace for this application?

@jonahwilliams jonahwilliams removed their assignment Aug 14, 2023
@jonahwilliams jonahwilliams added P2 Important issues not at the top of the work list and removed P1 High-priority issues at the top of the work list labels Aug 14, 2023
@chinmaygarde chinmaygarde added the triaged-engine Triaged by Engine team label Aug 14, 2023
@knopp
Copy link
Member Author

knopp commented Aug 14, 2023

Attached are traces for current main with wide gamut enabled and disabled. Tomorrow I'll try to publish a branch with MSAA backdrop disabled and appropriate. This is on iPhone 13 Pro.

Traces.zip

@knopp
Copy link
Member Author

knopp commented Aug 14, 2023

@jonahwilliams, I was playing with this and it seems initially I missed that for MSAA backdrop removal to perform better, it is also necessary to reuse textures. Without reusing textures removing MSAA backdrop performs much worse.

no-msaa-backdrop-reuse-textures dart_devtools_2023-08-15_01_11_41.977.json.zip

Here is my branch I tested this with: https://github.com/knopp/engine/commits/remove_msaa_backdrop
It's very hacky, but it can render the example sub 8ms on iPhone 13 pro, compared to ~12ms on main.

@jonahwilliams
Copy link
Member

FWIW we've started recylcing render target textures after flutter/engine#44527

Very simple herustics, but seem to get a good hit rate locally.

auto-submit bot pushed a commit to flutter/engine that referenced this issue Nov 8, 2023
As discovered by @knopp in flutter/flutter#131567 (comment), this is actually reducing performance substantially when there are multiple blurs. In the case of flutter/flutter#132735 , removing this capbility improves GPU performance from 400ms per frame to ~100 ms per frame.

Fixes flutter/flutter#131567 (comment)

-----

  | Macrobench | Example App
-- | -- | --
TOT | 250 | 450
W/Out OnScreen | 203-187 | 125-109
W/Out Onscreen and Resolve | 203 | 125
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 Nov 22, 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 team-engine Owned by Engine team triaged-engine Triaged by Engine team
Projects
No open projects
Archived in project
5 participants