Reland "[Windows] Move to FlutterCompositor for rendering" #49262

loic-sharma · 2023-12-19T21:32:32Z

Reland

#48849 was reverted as it incorrectly expected to receive always 1 layer. However, the engine will present 0 layers on an "empty" app. This pull request is split into two commits:

df604a1 is the original pull request, unchanged
c30b369 adds the ability to "clear" the view if the engine presents 0 layers

Original pull request description

This migrates the Windows embedder to FlutterCompositor so that the engine renders off-screen to a framebuffer instead of directly onto the window's surface. This will allow us to support platform views and multiple views on Windows.

Addresses flutter/flutter#128904

Pre-launch Checklist

I read the Contributor Guide and followed the process outlined there for submitting PRs.
I read the Tree Hygiene wiki page, which explains my responsibilities.
I read and followed the Flutter Style Guide and the C++, Objective-C, Java style guides.
I listed at least one issue that this PR fixes in the description above.
I added new tests to check the change I am making or feature I am adding, or the PR is test-exempt. See testing the engine for instructions on writing and running engine tests.
I updated/added relevant documentation (doc comments with ///).
I signed the CLA.
All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

This reverts commit 923f9e2.

loic-sharma · 2023-12-19T22:34:52Z

shell/platform/windows/compositor_opengl.cc

+  return true;
+}
+
+bool CompositorOpenGL::ClearSurface() {


This clears previously presented layers if the engine presents with 0 layers

loic-sharma · 2023-12-19T22:35:16Z

shell/platform/windows/compositor_software.cc

+
+  // Clear the view if there are no layers to present.
+  if (layers_count == 0) {
+    return engine_->view()->ClearSoftwareBitmap();


This clears previously presented layers if the engine presents with 0 layers

dkwingsmt · 2023-12-20T00:39:49Z

This looks good to me, but I'd like people more specialized in these code to approve it.

shell/platform/windows/compositor.h

cbracken

Thanks for breaking this up into two pieces. Looks great! Missed the doc nitpick on the first one but other than that this looks fantastic; I've got no useful review comments other than lgtm!

yaakovschectman

LGTM, looking forward to it landing

shell/platform/windows/compositor_opengl.cc

chinmaygarde · 2023-12-20T19:20:46Z

shell/platform/windows/compositor_opengl.cc

+  gl_->BindFramebuffer(GL_READ_FRAMEBUFFER, source_id);
+  gl_->BindFramebuffer(GL_DRAW_FRAMEBUFFER, kWindowFrameBufferId);
+
+  gl_->BlitFramebuffer(0,                    // srcX0


What version of GL are you using as the base? BlitFramebuffer won't be available on ES2 and you'll have to resort to render to texture instead.

From my understanding, ANGLE supports GLES 3.0 if Direct 3D 11 is available, which it is on Windows 7 and higher. I've tested this on a single Windows 7 machine.

I'm definitely concerned by this though. I plan to ask the Windows community members to test their apps on the master channel once this is rolled into the framework.

…lutter/engine#49262)

…140519) Roll Flutter Engine from c70f0a495ace to 1b1b2a12a597 (32 revisions) flutter/engine@c70f0a4...1b1b2a1 2023-12-21 skia-flutter-autoroll@skia.org Roll Dart SDK from 4a10533a4dc8 to bcf68d22f0fa (1 revision) (flutter/engine#49329) 2023-12-21 skia-flutter-autoroll@skia.org Roll Dart SDK from 1d0c3ecd1349 to 4a10533a4dc8 (1 revision) (flutter/engine#49326) 2023-12-21 flar@google.com Ensure sorted rects in ui.Canvas for legacy compatibility (flutter/engine#49309) 2023-12-21 skia-flutter-autoroll@skia.org Roll Dart SDK from 63a452b45026 to 1d0c3ecd1349 (1 revision) (flutter/engine#49318) 2023-12-21 dnfield@google.com [Impeller] Make IPLR files multi-platform (flutter/engine#49253) 2023-12-21 ditman@gmail.com [web] Defer injection of platform views until needed. (flutter/engine#48960) 2023-12-21 skia-flutter-autoroll@skia.org Roll Dart SDK from 1aef027ec953 to 63a452b45026 (2 revisions) (flutter/engine#49311) 2023-12-21 skia-flutter-autoroll@skia.org Roll Skia from 29917d8c97ca to 4b16117e94b2 (4 revisions) (flutter/engine#49310) 2023-12-21 737941+loic-sharma@users.noreply.github.com Reland "[Windows] Move to FlutterCompositor for rendering" (flutter/engine#49262) 2023-12-21 30870216+gaaclarke@users.noreply.github.com Reland `[Impeller] new blur: refactored math and fixed expanded padding size` (flutter/engine#49302) 2023-12-20 dkwingsmt@users.noreply.github.com Multiview pipeline Pt. 1: Skip illegal render calls (flutter/engine#49266) 2023-12-20 barpac02@gmail.com SemanticsUpdateBuilder: make all args non-null (flutter/engine#49148) 2023-12-20 30870216+gaaclarke@users.noreply.github.com [Impeller] fixed Rect::Contains (flutter/engine#49294) 2023-12-20 skia-flutter-autoroll@skia.org Roll Dart SDK from 97c3b7e1885a to 1aef027ec953 (1 revision) (flutter/engine#49295) 2023-12-20 30870216+gaaclarke@users.noreply.github.com Revert "[Impeller] new blur: refactored math and fixed expanded padding size" (flutter/engine#49298) 2023-12-20 30870216+gaaclarke@users.noreply.github.com [Impeller] new blur: refactored math and fixed expanded padding size (flutter/engine#49206) 2023-12-20 dkwingsmt@users.noreply.github.com Multi-view pointer event (flutter/engine#46213) 2023-12-20 1961493+harryterkelsen@users.noreply.github.com [web:multiview] Only call `Renderer.clearFragmentProgramCache` on hot restart (flutter/engine#48758) 2023-12-20 skia-flutter-autoroll@skia.org Roll Skia from 9cb1bb1164ea to 29917d8c97ca (1 revision) (flutter/engine#49289) 2023-12-20 bdero@google.com [Impeller] Add interactive Blur+Clip AiksTest. (flutter/engine#49283) 2023-12-20 sergiy.dubovik@supercell.com [macos] FlutterKeyboardManager memory leak fix (flutter/engine#48824) 2023-12-20 zanderso@users.noreply.github.com Don't guard Windows arm64 Dart SDK download on the release candidate flag (flutter/engine#49244) 2023-12-20 15619084+vashworth@users.noreply.github.com Fix testAppExtensionLaunching for Xcode 15/iOS 17 (flutter/engine#49242) 2023-12-20 skia-flutter-autoroll@skia.org Roll Skia from 8060d6b36066 to 9cb1bb1164ea (2 revisions) (flutter/engine#49288) 2023-12-20 skia-flutter-autoroll@skia.org Roll Dart SDK from ed415d966d8a to 97c3b7e1885a (1 revision) (flutter/engine#49287) 2023-12-20 skia-flutter-autoroll@skia.org Roll Skia from d0f09ad481f7 to 8060d6b36066 (1 revision) (flutter/engine#49285) 2023-12-20 kevinjchisholm@gmail.com [release] Update release config (flutter/engine#49254) 2023-12-20 skia-flutter-autoroll@skia.org Roll Dart SDK from 1732c4c92ccd to ed415d966d8a (1 revision) (flutter/engine#49274) 2023-12-20 skia-flutter-autoroll@skia.org Roll Dart SDK from 4c59838945d9 to 1732c4c92ccd (1 revision) (flutter/engine#49269) 2023-12-20 goderbauer@google.com Sync lints with flutter/flutter (flutter/engine#49192) 2023-12-19 1961493+harryterkelsen@users.noreply.github.com [web] Enforce onDrawFrame/onBeginFrame render rule (flutter/engine#49214) 2023-12-19 barpac02@gmail.com [Docs] Add more info about running tests on iOS (flutter/engine#48859) 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 jimgraham@google.com,rmistry@google.com,zra@google.com 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 ...

…lutter#140519) Roll Flutter Engine from c70f0a495ace to 1b1b2a12a597 (32 revisions) flutter/engine@c70f0a4...1b1b2a1 2023-12-21 skia-flutter-autoroll@skia.org Roll Dart SDK from 4a10533a4dc8 to bcf68d22f0fa (1 revision) (flutter/engine#49329) 2023-12-21 skia-flutter-autoroll@skia.org Roll Dart SDK from 1d0c3ecd1349 to 4a10533a4dc8 (1 revision) (flutter/engine#49326) 2023-12-21 flar@google.com Ensure sorted rects in ui.Canvas for legacy compatibility (flutter/engine#49309) 2023-12-21 skia-flutter-autoroll@skia.org Roll Dart SDK from 63a452b45026 to 1d0c3ecd1349 (1 revision) (flutter/engine#49318) 2023-12-21 dnfield@google.com [Impeller] Make IPLR files multi-platform (flutter/engine#49253) 2023-12-21 ditman@gmail.com [web] Defer injection of platform views until needed. (flutter/engine#48960) 2023-12-21 skia-flutter-autoroll@skia.org Roll Dart SDK from 1aef027ec953 to 63a452b45026 (2 revisions) (flutter/engine#49311) 2023-12-21 skia-flutter-autoroll@skia.org Roll Skia from 29917d8c97ca to 4b16117e94b2 (4 revisions) (flutter/engine#49310) 2023-12-21 737941+loic-sharma@users.noreply.github.com Reland "[Windows] Move to FlutterCompositor for rendering" (flutter/engine#49262) 2023-12-21 30870216+gaaclarke@users.noreply.github.com Reland `[Impeller] new blur: refactored math and fixed expanded padding size` (flutter/engine#49302) 2023-12-20 dkwingsmt@users.noreply.github.com Multiview pipeline Pt. 1: Skip illegal render calls (flutter/engine#49266) 2023-12-20 barpac02@gmail.com SemanticsUpdateBuilder: make all args non-null (flutter/engine#49148) 2023-12-20 30870216+gaaclarke@users.noreply.github.com [Impeller] fixed Rect::Contains (flutter/engine#49294) 2023-12-20 skia-flutter-autoroll@skia.org Roll Dart SDK from 97c3b7e1885a to 1aef027ec953 (1 revision) (flutter/engine#49295) 2023-12-20 30870216+gaaclarke@users.noreply.github.com Revert "[Impeller] new blur: refactored math and fixed expanded padding size" (flutter/engine#49298) 2023-12-20 30870216+gaaclarke@users.noreply.github.com [Impeller] new blur: refactored math and fixed expanded padding size (flutter/engine#49206) 2023-12-20 dkwingsmt@users.noreply.github.com Multi-view pointer event (flutter/engine#46213) 2023-12-20 1961493+harryterkelsen@users.noreply.github.com [web:multiview] Only call `Renderer.clearFragmentProgramCache` on hot restart (flutter/engine#48758) 2023-12-20 skia-flutter-autoroll@skia.org Roll Skia from 9cb1bb1164ea to 29917d8c97ca (1 revision) (flutter/engine#49289) 2023-12-20 bdero@google.com [Impeller] Add interactive Blur+Clip AiksTest. (flutter/engine#49283) 2023-12-20 sergiy.dubovik@supercell.com [macos] FlutterKeyboardManager memory leak fix (flutter/engine#48824) 2023-12-20 zanderso@users.noreply.github.com Don't guard Windows arm64 Dart SDK download on the release candidate flag (flutter/engine#49244) 2023-12-20 15619084+vashworth@users.noreply.github.com Fix testAppExtensionLaunching for Xcode 15/iOS 17 (flutter/engine#49242) 2023-12-20 skia-flutter-autoroll@skia.org Roll Skia from 8060d6b36066 to 9cb1bb1164ea (2 revisions) (flutter/engine#49288) 2023-12-20 skia-flutter-autoroll@skia.org Roll Dart SDK from ed415d966d8a to 97c3b7e1885a (1 revision) (flutter/engine#49287) 2023-12-20 skia-flutter-autoroll@skia.org Roll Skia from d0f09ad481f7 to 8060d6b36066 (1 revision) (flutter/engine#49285) 2023-12-20 kevinjchisholm@gmail.com [release] Update release config (flutter/engine#49254) 2023-12-20 skia-flutter-autoroll@skia.org Roll Dart SDK from 1732c4c92ccd to ed415d966d8a (1 revision) (flutter/engine#49274) 2023-12-20 skia-flutter-autoroll@skia.org Roll Dart SDK from 4c59838945d9 to 1732c4c92ccd (1 revision) (flutter/engine#49269) 2023-12-20 goderbauer@google.com Sync lints with flutter/flutter (flutter/engine#49192) 2023-12-19 1961493+harryterkelsen@users.noreply.github.com [web] Enforce onDrawFrame/onBeginFrame render rule (flutter/engine#49214) 2023-12-19 barpac02@gmail.com [Docs] Add more info about running tests on iOS (flutter/engine#48859) 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 jimgraham@google.com,rmistry@google.com,zra@google.com 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 ...

…lutter#49262)" This reverts commit 00d7d23.

@OverRide

…49461) This reverts #49262 (00d7d23) as it regressed [`material_floating_search_bar`](https://pub.dev/packages/material_floating_search_bar_2)'s animation. This revert was created manually due to merge conflicts. Issue tracking bug: flutter/flutter#140828 Part of flutter/flutter#128904 <details> <summary>Minimal repro of the broken animation...</summary> Here's what the animation is supposed to look like: ![good](https://publish-01.obsidian.md/access/b48ac8ca270cd9dac18c4a64d11b1c02/assets/2023-12-28-compositor_animation_regression_good.gif) Here's what the animation actually looks like: ![bad](https://publish-01.obsidian.md/access/b48ac8ca270cd9dac18c4a64d11b1c02/assets/2023-12-28-compositor_animation_regression_bad.gif) Here is a minimal repro of the broken animation: ```dart // The Windows compositor changes regresses the animation in // the `material_floating_search_bar_2` package: // // https://pub.dev/packages/material_floating_search_bar_2/versions/0.5.0 // // Below is a minimal repro of the broken animation. This has two pieces: // // 1. The background fades to a grey color // 2. A box is "revealed" using a custom clipper // // On framework commit b417fb8 this animation is broken on Windows. // On framework commit 9c2a756 this animation works as expected on Windows. // // Good gif: https://publish-01.obsidian.md/access/b48ac8ca270cd9dac18c4a64d11b1c02/assets/2023-12-28-compositor_animation_regression_good.gif // Bad gif: https://publish-01.obsidian.md/access/b48ac8ca270cd9dac18c4a64d11b1c02/assets/2023-12-28-compositor_animation_regression_bad.gif import 'dart:math'; import 'dart:ui'; import 'package:flutter/material.dart'; void main() { runApp(const MyApp()); } class MyApp extends StatelessWidget { const MyApp({super.key}); @OverRide Widget build(BuildContext context) { // Not using `MaterialApp` is necessary to reproduce: return Container( color: Colors.white, child: const Directionality( textDirection: TextDirection.ltr, child: FloatingSearchBar(), ), ); // Switching to `MaterialApp` fixes the issue: // return const MaterialApp( // home: Scaffold( // body: FloatingSearchBar(), // ), // ); } } class FloatingSearchBar extends StatefulWidget { const FloatingSearchBar({super.key}); @OverRide FloatingSearchBarState createState() => FloatingSearchBarState(); } class FloatingSearchBarState extends State<FloatingSearchBar> with SingleTickerProviderStateMixin { late final AnimationController _controller = AnimationController( vsync: this, duration: const Duration(seconds: 2), ); @OverRide void dispose() { _controller.dispose(); super.dispose(); } void _animate() { if (_controller.isDismissed || _controller.status == AnimationStatus.reverse) { _controller.forward(); } else { _controller.reverse(); } } @OverRide Widget build(BuildContext context) { return AnimatedBuilder( animation: _controller, builder: (BuildContext context, _) { return Stack( children: <Widget>[ if (!_controller.isDismissed) FadeTransition( opacity: _controller, child: const SizedBox.expand( child: DecoratedBox( decoration: BoxDecoration(color: Colors.black26), ), ), ), _buildSearchBar(), ], ); }, ); } Widget _buildSearchBar() { return Column( crossAxisAlignment: CrossAxisAlignment.stretch, children: <Widget>[ // This is where the search text input would go... GestureDetector( onTap: () => _animate(), child: Text( switch (_controller.status) { AnimationStatus.forward || AnimationStatus.completed => 'Click to close', AnimationStatus.reverse || AnimationStatus.dismissed => 'Click to open', }, style: const TextStyle(color: Colors.black), ), ), // Below are where the search results would be. Clicking on the search // input above reveals the results below. // Removing this fixes the background's fade transition. ClipOval( clipper: _CircularRevealClipper( fraction: _controller.value, ), child: DecoratedBox( decoration: BoxDecoration( color: Colors.white, // Removing this line fixes the background's fade transition. borderRadius: BorderRadius.circular(16.0), ), child: const Padding( padding: EdgeInsets.all(64.0), child: Text( 'Hello world', style: TextStyle(color: Colors.black), ), ), ), ), ], ); } } class _CircularRevealClipper extends CustomClipper<Rect> { const _CircularRevealClipper({required this.fraction}); final double fraction; @OverRide Rect getClip(Size size) { final double halfWidth = size.width * 0.5; final double maxRadius = sqrt(halfWidth * halfWidth + size.height * size.height); final double radius = lerpDouble(0.0, maxRadius, fraction) ?? 0; return Rect.fromCircle( center: Offset(halfWidth, 0), radius: radius, ); } @OverRide bool shouldReclip(CustomClipper<Rect> oldClipper) { if (oldClipper is _CircularRevealClipper) { return oldClipper.fraction != fraction; } return true; } } ``` </details> [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style

## Original pull request description This migrates the Windows embedder to `FlutterCompositor` so that the engine renders off-screen to a framebuffer instead of directly onto the window's surface. This will allow us to support platform views and multiple views on Windows. Addresses flutter/flutter#128904 ## Reland (again) #49262 was reverted as it regressed [`package:material_floating_search_bar_2`](https://pub.dev/packages/material_floating_search_bar_2/versions/0.5.0). See: flutter/flutter#140828 This pull request is split into the following commits: 1. d337378 is the previous reland pull request, unchanged 2. e866af0 disables the scissor test before blitting the framebuffer, allowing us to "force" copy the framebuffer's contents by ignoring scissoring values [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style

loic-sharma added 2 commits December 18, 2023 10:33

[Windows] Move to FlutterCompositor for rendering

df604a1

This reverts commit 923f9e2.

Clear the view if presenting no layers

c30b369

github-actions bot added affects: desktop platform-linux platform-windows e: impeller labels Dec 19, 2023

Fix comment

e84e19d

loic-sharma requested review from cbracken, dkwingsmt and yaakovschectman December 19, 2023 22:33

loic-sharma marked this pull request as ready for review December 19, 2023 22:33

loic-sharma commented Dec 19, 2023

View reviewed changes

cbracken reviewed Dec 20, 2023

View reviewed changes

shell/platform/windows/compositor.h Outdated Show resolved Hide resolved

cbracken approved these changes Dec 20, 2023

View reviewed changes

yaakovschectman approved these changes Dec 20, 2023

View reviewed changes

Feedback

6077e6d

loic-sharma force-pushed the windows_compositor_2 branch from 9fafc4f to 6077e6d Compare December 20, 2023 18:39

chinmaygarde reviewed Dec 20, 2023

View reviewed changes

shell/platform/windows/compositor_opengl.cc Show resolved Hide resolved

chinmaygarde reviewed Dec 20, 2023

View reviewed changes

shell/platform/windows/compositor_opengl.cc Outdated Show resolved Hide resolved

chinmaygarde reviewed Dec 20, 2023

View reviewed changes

Feedback

9e4b1ba

chinmaygarde approved these changes Dec 20, 2023

View reviewed changes

loic-sharma and others added 2 commits December 20, 2023 14:41

Merge branch 'main' into windows_compositor_2

264385e

Format

917d940

loic-sharma added the autosubmit Merge PR when tree becomes green via auto submit App label Dec 20, 2023

auto-submit bot merged commit 00d7d23 into flutter:main Dec 21, 2023

engine-flutter-autoroll mentioned this pull request Dec 21, 2023

Roll Flutter Engine from c70f0a495ace to 1b1b2a12a597 (32 revisions) flutter/flutter#140519

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Dec 21, 2023

00d7d23fb Reland "[Windows] Move to FlutterCompositor for rendering" (f…

e36c513

…lutter/engine#49262)

loic-sharma added a commit to loic-sharma/flutter-engine that referenced this pull request Jan 2, 2024

Revert "Reland "[Windows] Move to FlutterCompositor for rendering" (f…

a3b62c7

…lutter#49262)" This reverts commit 00d7d23.

loic-sharma mentioned this pull request Jan 2, 2024

Revert "Reland "[Windows] Move to FlutterCompositor for rendering" #49461

Merged

8 tasks

loic-sharma added a commit to loic-sharma/flutter-engine that referenced this pull request Jan 2, 2024

Revert "Reland "[Windows] Move to FlutterCompositor for rendering" (f…

ee0e814

…lutter#49262)" This reverts commit 00d7d23.

loic-sharma mentioned this pull request Jan 2, 2024

[Windows] Compositor rendering regresses apps with faded backgrounds flutter/flutter#140828

Closed

3 tasks

loic-sharma mentioned this pull request Jan 11, 2024

Reland "[Windows] Move to FlutterCompositor for rendering" #49726

Merged

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reland "[Windows] Move to FlutterCompositor for rendering" #49262

Reland "[Windows] Move to FlutterCompositor for rendering" #49262

loic-sharma commented Dec 19, 2023 •

edited

Loading

loic-sharma Dec 19, 2023

loic-sharma Dec 19, 2023

dkwingsmt commented Dec 20, 2023

cbracken left a comment

yaakovschectman left a comment

chinmaygarde Dec 20, 2023

loic-sharma Dec 20, 2023 •

edited

Loading

Reland "[Windows] Move to FlutterCompositor for rendering" #49262

Reland "[Windows] Move to FlutterCompositor for rendering" #49262

Conversation

loic-sharma commented Dec 19, 2023 • edited Loading

Reland

Original pull request description

Pre-launch Checklist

loic-sharma Dec 19, 2023

Choose a reason for hiding this comment

loic-sharma Dec 19, 2023

Choose a reason for hiding this comment

dkwingsmt commented Dec 20, 2023

cbracken left a comment

Choose a reason for hiding this comment

yaakovschectman left a comment

Choose a reason for hiding this comment

chinmaygarde Dec 20, 2023

Choose a reason for hiding this comment

loic-sharma Dec 20, 2023 • edited Loading

Choose a reason for hiding this comment

loic-sharma commented Dec 19, 2023 •

edited

Loading

loic-sharma Dec 20, 2023 •

edited

Loading