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

macos platformview UI blinking #138936

Closed
2 tasks done
renanyoy opened this issue Nov 23, 2023 · 16 comments · Fixed by flutter/engine#43301 or flutter/engine#52082
Closed
2 tasks done

macos platformview UI blinking #138936

renanyoy opened this issue Nov 23, 2023 · 16 comments · Fixed by flutter/engine#43301 or flutter/engine#52082
Assignees
Labels
a: platform-views Embedding Android/iOS views in Flutter apps c: rendering UI glitches reported at the engine/skia rendering level engine flutter/engine repository. See also e: labels. found in release: 3.16 Found to occur in 3.16 found in release: 3.17 Found to occur in 3.17 has reproducible steps The issue has been confirmed reproducible and is ready to work on platform-mac Building on or for macOS specifically r: fixed Issue is closed as already fixed in a newer version team-desktop Owned by Desktop platforms team

Comments

@renanyoy
Copy link

Is there an existing issue for this?

Steps to reproduce

  • build the attached example (works in xcode)
  • run it
  • plays a little with windows resize and the scroll
  • at some moment the UI will start blinking

Expected results

  • no UI blinking

Actual results

  • UI blinking

I think it comes from ClipRRect() around the platform view, without it all seems working well

Code sample

Code sample

source code

Screenshots or Video

Screenshots / Video demonstration Screenshot 2023-11-23 at 08 40 28 Screenshot 2023-11-23 at 08 40 33

Logs

Logs

too many logs, quarantine..

Flutter Doctor output

Doctor output
[✓] Flutter (Channel main, 3.17.0-3.0.pre.20, on macOS 14.1.1 23B81 darwin-arm64, locale en-FR)
    • Flutter version 3.17.0-3.0.pre.20 on channel main at /Users/renanyoy/code/sdk/flutter
    • Upstream repository https://github.com/flutter/flutter.git
    • Framework revision 6007b91a37 (2 weeks ago), 2023-11-07 02:43:32 -0500
    • Engine revision 7f4d56a7f4
    • Dart version 3.3.0 (build 3.3.0-102.0.dev)
    • DevTools version 2.30.0-dev.1

[✓] Android toolchain - develop for Android devices (Android SDK version 30.0.3)
    • Android SDK at /Users/renanyoy/Library/Android/sdk
    • Platform android-33, build-tools 30.0.3
    • Java binary at: /Applications/Android Studio.app/Contents/jbr/Contents/Home/bin/java
    • Java version OpenJDK Runtime Environment (build 17.0.6+0-17.0.6b802.4-9586694)
    • All Android licenses accepted.

[✓] Xcode - develop for iOS and macOS (Xcode 15.0.1)
    • Xcode at /Applications/Xcode.app/Contents/Developer
    • Build 15A507
    • CocoaPods version 1.13.0

[✓] Chrome - develop for the web
    • Chrome at /Applications/Google Chrome.app/Contents/MacOS/Google Chrome

[✓] Android Studio (version 2022.2)
    • Android Studio at /Applications/Android Studio.app/Contents
    • Flutter plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/9212-flutter
    • Dart plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/6351-dart
    • Java version OpenJDK Runtime Environment (build 17.0.6+0-17.0.6b802.4-9586694)

[✓] VS Code (version 1.84.2)
    • VS Code at /Applications/Visual Studio Code.app/Contents
    • Flutter extension version 3.76.0

[✓] Connected device (3 available)
    • IPhone10.renan (mobile) • 00008020-000640992209002E • ios            • iOS 17.1.1 21B91
    • macOS (desktop)         • macos                     • darwin-arm64   • macOS 14.1.1 23B81 darwin-arm64
    • Chrome (web)            • chrome                    • web-javascript • Google Chrome 119.0.6045.159

[✓] Network resources
    • All expected network resources are available.

• No issues found!
@danagbemava-nc danagbemava-nc added the in triage Presently being triaged by the triage team label Nov 23, 2023
@danagbemava-nc
Copy link
Member

Reproducible using the code sample provided above.

Screen.Recording.2023-11-23.at.10.16.41.mov
flutter doctor -v
[✓] Flutter (Channel stable, 3.16.0, on macOS 14.1.1 23B81 darwin-arm64, locale en-GB)
    • Flutter version 3.16.0 on channel stable at /Users/nexus/dev/sdks/flutter
    • Upstream repository https://github.com/flutter/flutter.git
    • Framework revision db7ef5bf9f (8 days ago), 2023-11-15 11:25:44 -0800
    • Engine revision 74d16627b9
    • Dart version 3.2.0
    • DevTools version 2.28.2

[✓] Android toolchain - develop for Android devices (Android SDK version 34.0.0-rc1)
    • Android SDK at /Users/nexus/Library/Android/sdk
    • Platform android-34, build-tools 34.0.0-rc1
    • Java binary at: /Users/nexus/Applications/Android Studio.app/Contents/jbr/Contents/Home/bin/java
    • Java version OpenJDK Runtime Environment (build 17.0.6+0-17.0.6b829.9-10027231)
    • All Android licenses accepted.

[✓] Xcode - develop for iOS and macOS (Xcode 15.0)
    • Xcode at /Applications/Xcode-15.0.0-Release.Candidate.app/Contents/Developer
    • Build 15A240d
    • CocoaPods version 1.13.0

[✓] Chrome - develop for the web
    • Chrome at /Applications/Google Chrome.app/Contents/MacOS/Google Chrome

[✓] Android Studio (version 2022.3)
    • Android Studio at /Users/nexus/Applications/Android Studio.app/Contents
    • Flutter plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/9212-flutter
    • Dart plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/6351-dart
    • Java version OpenJDK Runtime Environment (build 17.0.6+0-17.0.6b829.9-10027231)

[✓] IntelliJ IDEA Ultimate Edition (version 2023.2.5)
    • IntelliJ at /Users/nexus/Applications/IntelliJ IDEA Ultimate.app
    • Flutter plugin version 76.3.4
    • Dart plugin version 232.10072.19

[✓] VS Code (version 1.84.2)
    • VS Code at /Applications/Visual Studio Code.app/Contents
    • Flutter extension version 3.76.0

[✓] Connected device (5 available)
    • Pixel 7 (mobile)     • 28291FDH2001SA            • android-arm64  • Android 14 (API 34)
    • Nexus (mobile)       • 00008020-001875E83A38002E • ios            • iOS 17.1.1 21B91
    • Dean’s iPad (mobile) • 00008103-000825C811E3401E • ios            • iOS 17.1 21B74
    • macOS (desktop)      • macos                     • darwin-arm64   • macOS 14.1.1 23B81 darwin-arm64
    • Chrome (web)         • chrome                    • web-javascript • Google Chrome 119.0.6045.159

[✓] Network resources
    • All expected network resources are available.

• No issues found!
[!] Flutter (Channel master, 3.17.0-15.0.pre.13, on macOS 14.1.1 23B81 darwin-arm64, locale en-GB)
    • Flutter version 3.17.0-15.0.pre.13 on channel master at /Users/nexus/dev/sdks/flutters
    ! Warning: `flutter` on your path resolves to /Users/nexus/dev/sdks/flutter/bin/flutter, which is not inside your current Flutter SDK checkout at /Users/nexus/dev/sdks/flutters. Consider adding /Users/nexus/dev/sdks/flutters/bin to the front of your path.
    ! Warning: `dart` on your path resolves to /Users/nexus/dev/sdks/flutter/bin/dart, which is not inside your current Flutter SDK checkout at /Users/nexus/dev/sdks/flutters. Consider adding /Users/nexus/dev/sdks/flutters/bin to the front of your path.
    • Upstream repository https://github.com/flutter/flutter.git
    • Framework revision 106667eb4b (12 hours ago), 2023-11-22 22:15:57 +0000
    • Engine revision f331e0afaf
    • Dart version 3.3.0 (build 3.3.0-152.0.dev)
    • DevTools version 2.30.0-dev.4
    • If those were intentional, you can disregard the above warnings; however it is recommended to use "git" directly to perform update checks and upgrades.

[✓] Android toolchain - develop for Android devices (Android SDK version 34.0.0-rc1)
    • Android SDK at /Users/nexus/Library/Android/sdk
    • Platform android-34, build-tools 34.0.0-rc1
    • Java binary at: /Users/nexus/Applications/Android Studio.app/Contents/jbr/Contents/Home/bin/java
    • Java version OpenJDK Runtime Environment (build 17.0.6+0-17.0.6b829.9-10027231)
    • All Android licenses accepted.

[✓] Xcode - develop for iOS and macOS (Xcode 15.0)
    • Xcode at /Applications/Xcode-15.0.0-Release.Candidate.app/Contents/Developer
    • Build 15A240d
    • CocoaPods version 1.13.0

[✓] Chrome - develop for the web
    • Chrome at /Applications/Google Chrome.app/Contents/MacOS/Google Chrome

[✓] Android Studio (version 2022.3)
    • Android Studio at /Users/nexus/Applications/Android Studio.app/Contents
    • Flutter plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/9212-flutter
    • Dart plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/6351-dart
    • Java version OpenJDK Runtime Environment (build 17.0.6+0-17.0.6b829.9-10027231)

[✓] IntelliJ IDEA Ultimate Edition (version 2023.2.5)
    • IntelliJ at /Users/nexus/Applications/IntelliJ IDEA Ultimate.app
    • Flutter plugin version 76.3.4
    • Dart plugin version 232.10072.19

[✓] VS Code (version 1.84.2)
    • VS Code at /Applications/Visual Studio Code.app/Contents
    • Flutter extension version 3.76.0

[✓] Connected device (5 available)
    • Pixel 7 (mobile)     • 28291FDH2001SA            • android-arm64  • Android 14 (API 34)
    • Nexus (mobile)       • 00008020-001875E83A38002E • ios            • iOS 17.1.1 21B91
    • Dean’s iPad (mobile) • 00008103-000825C811E3401E • ios            • iOS 17.1 21B74
    • macOS (desktop)      • macos                     • darwin-arm64   • macOS 14.1.1 23B81 darwin-arm64
    • Chrome (web)         • chrome                    • web-javascript • Google Chrome 119.0.6045.159

[✓] Network resources
    • All expected network resources are available.

! Doctor found issues in 1 category.

@danagbemava-nc danagbemava-nc added engine flutter/engine repository. See also e: labels. platform-mac Building on or for macOS specifically a: platform-views Embedding Android/iOS views in Flutter apps c: rendering UI glitches reported at the engine/skia rendering level has reproducible steps The issue has been confirmed reproducible and is ready to work on team-desktop Owned by Desktop platforms team found in release: 3.16 Found to occur in 3.16 found in release: 3.17 Found to occur in 3.17 and removed in triage Presently being triaged by the triage team labels Nov 23, 2023
@knopp knopp self-assigned this Nov 23, 2023
@knopp
Copy link
Member

knopp commented Nov 23, 2023

EDIT: This is not the case (see the last comment). There is currently a full window surface for each of the platform view (because of the Flutter outline).

This seems to happen because these MTKViews are thrashing the main thread, sometimes to the point where core animation does not manage to update the server with new flutter surface. This is not something our compositor on macOS expects to happen. We only have two surfaces that we're flipping in order to not allocate more memory than needed, but in pathological cases such as this one we end up drawing into front buffer.

When obtaining backbuffer surface we don't check if is used by compositor, because generally it's not a problem if compositor still holds it at the beginning of frame for a few milliseconds. It does become a problem if compositor is using it for another 2-3 frames like in this example.

I'm not entirely sure what the best solution here is. We may need to handle situation when jank is not caused by Flutter.

@knopp
Copy link
Member

knopp commented Nov 23, 2023

Btw, have you considered using external textures for this? Using this many CAMetalLayers on platform thread is likely not going to result in good performance, it's just too heavy, but you should be able to render most of the workload to external texture and render those in flutter.

@renanyoy
Copy link
Author

renanyoy commented Nov 24, 2023

@knopp it's just a stressing example to be sure it happens quickly. In my app, I use max 10 small 160x90 (x screen scale) at a time on screen. also I pause all rendering as soon as the window enter live resizing... but it still happens sometime..

if there is a way to share MTLTexture with flutter, and display them using the flutter renderer, would be faster for sure.. but I don't know how do it..

@knopp
Copy link
Member

knopp commented Nov 24, 2023

Look at FlutterTextureRegistry (it's same on macOS and iOS). The texture object that you register needs to provide a CVPixelBufferRef. You can create CVPixelBufferRef from IOSurface through CVPixelBufferCreateWithIOSurface. The same IOSurface can be backing a metal texture. This will not be entirely trivial and will need some housekeeping, but if you get it right it should be much cheaper than using metal views. Plus you can do all rendering on background thread.

I'm suggesting this because even when the issue here gets fixed, you'll still be dropping frames with your current approach (dropping CA frames not caused by Flutter is the underlying reason for the flicker).

@renanyoy
Copy link
Author

renanyoy commented Nov 24, 2023

@knopp thank you, I will look at it. For my app all the real rendering is done in background, so it's not too important if the UI drop some frame sometime.. but it still better if not and if I can spare some cpu cycles..

@knopp
Copy link
Member

knopp commented Nov 24, 2023

This is possibly relevant: #49757
CADisplayLink / CVDisplayLink on main thread should reflect dropped frames, which should prevent raster thread from producing more frames than the compositor can display.

@knopp
Copy link
Member

knopp commented Nov 24, 2023

Actually, I just noticed, that because every of these platform views has a flutter outline, there are 19 total surfaces on Flutter side. That is way, way too much, and perhaps even bigger reason why we're missing core animation deadline.

Good news here is, I have a PR in progress just for this: flutter/engine#43301. I tested it, and it reduces the number of surfaces to 1 (2 when scrolling because of overlay). It seems to completely get rid of the issue.

harryterkelsen pushed a commit to harryterkelsen/engine that referenced this issue Nov 27, 2023
…utter#43301)

Fixes flutter/flutter#129710
Fixes flutter/flutter#138936

Implements https://flutter.dev/go/optimized-platform-view-layers

## Example

This scene would normally require 5 surfaces, but with the PR it comes
down to 2 (when drawing over platform views) and 1 when all drawing is
outside of platform views).


https://github.com/flutter/flutter/assets/96958/091da832-bfbc-44a2-8da5-d55d84024c96

*If you had to change anything in the [flutter/tests] repo, include a
link to the migration guide as per the [breaking change policy].*

## 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
@danagbemava-nc danagbemava-nc added the r: fixed Issue is closed as already fixed in a newer version label Nov 28, 2023
@renanyoy
Copy link
Author

renanyoy commented Dec 2, 2023

@knopp I think I got the fix on master branch, seems better.. but it's still happening sometime when using lots of camera inputs.. they must hang a lil the main thread when they are starting.. I think would be nice to have a frame counter or something like that to sync the compositor thread and the renderer thread in flutter.. in case something hang... I'll try to avoid the camera start on main thread still ;)

@knopp
Copy link
Member

knopp commented Dec 2, 2023

I think a main thread display link would help here. It is on my TODO list. In any case, if you can provide another example that breaks, it would be quite helpful.

@knopp
Copy link
Member

knopp commented Dec 6, 2023

Reopening because with opacity set on large number of platform views, there is still flicker because we're pushing more frames than the compositor can flip. This still needs to be addressed.

@knopp knopp reopened this Dec 6, 2023
@danagbemava-nc danagbemava-nc removed the r: fixed Issue is closed as already fixed in a newer version label Dec 7, 2023
@knopp
Copy link
Member

knopp commented Dec 19, 2023

There are two problems here:

  • With opacity = 1 all those CAMetalLayer will hog the platform thread preventing render server to take over front surface. After which we keep writing to the back buffer, which is still used by the render server. For this we should try to detect the situation and possibly switch to multiple buffers. Also I'd strongly suggest drawing to this many metal layers on platform thread.
  • With opacity < 1, raster threads gets blocked for several frame periods waiting in [commandBuffer waitUntilScheduled], after which it returns several frames in a burst. We could possibly try skipping rasterization of these frames, but the whole thing seems a bit pathological, I even had whole computer freezing when trying to profile this. Long story short, CAMetaLayer really doesn't seem to behave nicely when parent layer is not opaque (clip might also matter).

@renanyoy
Copy link
Author

renanyoy commented Jan 14, 2024

@knopp I confirm, I got multiple computer freezes lately when minimizing and reverting back the app window.
Could you point me the source code files that manage this part ? Just to understand better.. thank you!

@renanyoy
Copy link
Author

renanyoy commented Feb 23, 2024

@knopp , little update, since one of the last sonoma update, apple has fixed the freezing.. it should be easier to debug this problem now..
I'm on Sonoma 14.3.1 (23D60)

@knopp
Copy link
Member

knopp commented Apr 11, 2024

I'm fairly confident that we're being too optimistic with single back buffer here. I really wanted it to work, but there's no way to know for sure that the compositor has picked up front surface. And when that happens (and compositor misses the frame), we end up drawing to the previous front surface while it is still in use.

I think we're going to need similar logic to retrieve surface as FlutterMetalLayer is using.

knopp added a commit to flutter/engine that referenced this issue Apr 17, 2024
…use (#52082)

Fixes flutter/flutter#138936

Currently the engine hold a single backbuffer per flutter view layer,
which gets swapped with front buffer. This however is too optimistic, as
the front buffer in some cases might not get immediately picked up by
the compositor. When that happens, during next frame the cache may
return a backbuffer that is in fact still being used by the compositor.
Rendering to such surface will result in artifacts seen in the video.
This seems more likely to happen with busy platform thread. It is also
more likely to happen with the presence of "heavy" platform views such
as `WKWebView`.

IOSurface is able to report when it is being used by the compositor
(`IOSurfaceIsInUse`). This PR ensures that the backing store cache never
returns surface that is being used by compositor. This may result in
transiently keeping more surfaces than before, as the compositor
sometimes seems to holds on to the surface longer than it is displayed,
but that's preferable outcome to visual glitches.

This PR adds `age` field to `FlutterSurface`. When returning buffers to
the surface cache (during compositor commit), the age of each surface
currently present in cache increases by one, while the newly returned
surfaces have their age set to 0.

When returning surfaces from cache, a surface with lowest age that is
not in use by compositor is returned.

Surfaces with age that reaches 30 are evicted from the pool. Reaching
this age means one of two things:
- When surface is still in use at age 30 it means the compositor is
holding on to it much longer than we expect. In this case just forget
about the surface, we can't really do anything about it.
- When surface is not in use at age 30, it means it hasn't been removed
from cache for 29 subsequent frames. That means the cache is holding more
surfaces than needed.

Removing all surfaces from cache after idle period remains unchanged.

Before: 


https://github.com/flutter/engine/assets/96958/ba2bfc43-525e-4a88-b37c-61842994d3bc

After:


https://github.com/flutter/engine/assets/96958/8b5d393e-3031-46b1-b3f0-cb7f63f8d960


*If you had to change anything in the [flutter/tests] repo, include a
link to the migration guide as per the [breaking change policy].*

## 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 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
[test-exempt]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#tests
[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
@danagbemava-nc danagbemava-nc added the r: fixed Issue is closed as already fixed in a newer version label Apr 17, 2024
Copy link

github-actions bot commented May 1, 2024

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 May 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
a: platform-views Embedding Android/iOS views in Flutter apps c: rendering UI glitches reported at the engine/skia rendering level engine flutter/engine repository. See also e: labels. found in release: 3.16 Found to occur in 3.16 found in release: 3.17 Found to occur in 3.17 has reproducible steps The issue has been confirmed reproducible and is ready to work on platform-mac Building on or for macOS specifically r: fixed Issue is closed as already fixed in a newer version team-desktop Owned by Desktop platforms team
Projects
None yet
3 participants