-
Notifications
You must be signed in to change notification settings - Fork 4
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
Investigate possible Flutter engine cause of jitter #45
Comments
Which particular example Dart file should I run to investigate this? |
There are multiples. But you can start with main_page_list_images_inspector |
Alright, I tried |
I did some testing and this looks to be related to shaders. Following Flutter's documentation on shader compilation jank seems to have mitigated the problem mostly. I am not sure if that is exactly the issue but it's a lead. But even with using a precompiled shader, I notice in the Flutter DevTools that there's still some jank rendering. |
The jitter problem isn't visible in the profiler, in my experience. That occasional jank frame isn't good, but that's a different problem. If you watch the UI as the panning animation runs, you'll see numerous jitter frames. And yet you won't see a bunch of back to back jank frames in the profiler. That's part of why this problem is difficult. Flutter doesn't seem to recognize that something isn't right. |
Interesting, so even with precompiled shaders it's jittery? I'll look more into it tomorrow by enabling the Skia tracing. Have you tried with running with Impeller? If not, I'll try tomorrow since I saw there seems to be others with this issue on Android and Impeller seems to be unaffected. |
I haven't tried this experience with Impeller. We tried our direct PDF renderer with Impeller and it was way worse than Skia. The Android version of Impeller is still considered unstable and experimental. But you're welcome to give it a try. You'll wanna keep a close eye on the screen as it pans. You might also want to download the Xodo app, which is a PDF renderer. You can pan around in there and feel the difference. |
Alright will do, I also came across this page which references a method called |
In the bitmap PDF renderer we're pushing texture layers, so there's nothing for us to save. That experience has the same jitter as this image based tiling experience. |
It appears the jitter is gone or reduced significantly under Impeller. Edit: looks like there's just a slight bit of jank under Impeller as well but I did notice GC is running every time it scrolls to the next page so that might be worth looking into. |
Is there a path forward where you could discover the relevant differences between Skia and Impeller, which lead to the elimination of the jitter? At least to the extent that we might point the Flutter team in that direction? Eventually Impeller will be the norm, and we'll work under the expectation of using Impeller, but that time hasn't arrived yet for Android. So we're interested in a path that would result in fixes to Flutter's integration with Skia such that this problem goes away. Do you think there's a path forward for you to help with that? |
The biggest change I've read for Impeller is that it compiles shaders before execution. I think if GC could not be executed during scrolling then it could lead to better performance. |
Those sound like two very different problems. Is the problem repeated shader compilation? Or is the problem repeated GCs? |
I can't trace whether it is the shaders affecting it more than GC. I did tracing and every time it starts doing a large paint, GC is running. GC is likely the large issue but shader recompilation can contribute somewhat. |
Can you check the If it's not already in |
Yeah, I don't know what the new GC API is. |
From this post: https://medium.com/flutter/whats-new-in-flutter-3-7-38cbea71133c Which links to this: dart-lang/sdk@c6a1eb1 |
Just checked and the new GC API is not being used at all. There isn't really any bindings to it from what I can see but adding the external references from here should provide access to the right methods so it'll just be a matter of calling those. |
Just tried that and those methods aren't available, Flutter is likely using a custom Dart build with specific changes so it's likely they didn't expose the methods. I also tried with Impeller in release mode, it's smooth enough that I can barely tell there's any jitter. Unfortunately, it is hard to determine whether or not the actual FPS is an exact 90FPS consistently. |
I'm looking for where we used that GC behavior. It's definitely there. It's just not easy to figure out. WRT FPS - there's a dev option setting on the tablet where you can display the current frame rate on the screen, however that won't show the jitter, either. The jitter is something that you have to see with your eyes. There's no other signal that we've found to confirm it's happening.
Ozzie said he tried with Impeller and the jitter is still there. When you run the vertical and horizontal animations, together, and it starts panning around the screen in an oval, you don't see recurring situations where you get a bunch of rapid, small visual stutters? |
Gotcha, I'm not too familiar with GC so I wasn't sure.
Yeah if I stare long enough at the screen, I can see some jitter but it's not a ton. It seems to be at certain frames. Debug and profile have the most jitter with Impeller, release has it more difficult for me to notice. |
If it's easier to reproduce and recognize with Skia, then let's go back to that. Impeller isn't really relevant to us, unless Impeller has no jitter, and you're able to learn something from Impeller. But we're building against the Skia version. That's where we're suffering the issue, and that's what customers will be stuck with. So we're trying to achieve one of the following:
Do you feel that you have possible paths to reach those goals, do you feel like it's completely unknown what to do, or where to look? |
Alright, I'll see what I can do to figure out how GC is affecting rendering through Skia. If it isn't GC then I'm not sure what else it could be since I haven't been able to get useful information from enabling Skia tracing. |
I don't know the structure of the engine at that level, but another thing you might try to look into is when/where/how textures and images are positioned for final rasterization. There's a chance that they might be placed with some kind of rounding error, which could be perceived as jitter. A low chance, but a chance that's what's going on. WRT GCs, if there are a bunch of GCs happening, it's probably worth understanding why they're happening. If we're panning around in a circle, we're not creating a bunch of new widgets, and we're not loading new images. What's getting recycled? |
Yeah, that's ultimately what I'm trying to understand with the GC. I see the memory usage rise from 69MB to 80MB and repeats for every frame. I'll look and see if there's some sort of way of doing verbose GC so I can figure where things are allocating and where things are being disposed of. |
Here's where we utilized the GC controls: https://github.com/Flutter-Bounty-Hunters/page_list_viewport/pull/14/files |
Also, when it comes to root causing, even if you do something like hack Dart or Flutter, itself, by raising the max memory threshold way up high, that's fine too. We need proof about where this problem is coming from.... |
Alright, I'll use that. I decided to add a break here and discovered this is called right before scrolling to the next page. Is the |
BTW, if you haven't already, you should download the Xodo PDF app and pan around in there. We've found that Xodo tends to be extremely smooth, at least in our experience. You might get a better feel for the jitter effect if you experience PDF panning without the jitter. |
Hmm alright, I'll try analyzing frames and compare it with Xodo. Slowing the video down to 1/4 speed, I can see that slight jitter you're talking about now. If you record a video using the same command on your device, is the jitter the same or worse than it is for me? |
Your guess is as good as mine. My first question would be how often does this situation happen in the trace? We know that we're seeing jitter that happens over and over, for many frames. Do you see an equivalent number of these trace results? If you do, then the question goes to the root cause. You've identified a long |
So as a disclaimer, I'm not able to super reliably reproduce the jittery issue and when I can reproduce it I'm not necessarily the best at visually detecting it. That said, I tried the following patch locally and it seems to help: diff --git a/shell/platform/android/platform_view_android.cc b/shell/platform/android/platform_view_android.cc
index b89c23a368..2ff8fc63ae 100644
--- a/shell/platform/android/platform_view_android.cc
+++ b/shell/platform/android/platform_view_android.cc
@@ -204,6 +204,12 @@ void PlatformViewAndroid::NotifyChanged(const SkISize& size) {
latch.Wait();
}
+PointerDataDispatcherMaker PlatformViewAndroid::GetDispatcherMaker() {
+ return [](DefaultPointerDataDispatcher::Delegate& delegate) {
+ return std::make_unique<SmoothPointerDataDispatcher>(delegate);
+ };
+}
+
void PlatformViewAndroid::DispatchPlatformMessage(JNIEnv* env,
std::string name,
jobject java_message_data,
diff --git a/shell/platform/android/platform_view_android.h b/shell/platform/android/platform_view_android.h
index 300f3ab9b6..59d3618990 100644
--- a/shell/platform/android/platform_view_android.h
+++ b/shell/platform/android/platform_view_android.h
@@ -117,6 +117,9 @@ class PlatformViewAndroid final : public PlatformView {
return platform_message_handler_;
}
+ // |PlatformView|
+ PointerDataDispatcherMaker GetDispatcherMaker() override;
+
private:
const std::shared_ptr<PlatformViewAndroidJNI> jni_facade_;
std::shared_ptr<AndroidContext> android_context_; @matthew-carroll are you able to apply that to an engine checkout and see if it fixes things for you? If so I could look into making it landable upstream. |
(that patch is meant to help with irregularities when the refresh rate on the screen and the touch input device are different and may get out of sync) |
@dnfield we can try the patch, but I'm not sure how it's relevant? We are currently reproducing the jitter with animations that are tied to a couple of buttons in the demo. There aren't any touch input events happening... |
Ah, sorry I had missed the buttons. Touch input could be something else on the device I'm looking at. |
For the buttons one, disabling the raster cache seems to help. Checking into that a bit more. |
Also, your trace is showing |
@dnfield I'm definitely on 3.10 stable, but @RossComputerGuy can you please confirm that you're on the latest Flutter stable version? If you're not, can you upgrade and then rerun your traces? |
I'm on Flutter master, should I be using a tagged version like 3.10? |
master should be fine |
Ok, it looks like Android internally is calling Right now I think we're sometimes running into a problem where we're double stuffing the buffer and failing to catch up. Each frame gets delayed. This is a known issue that's not got a really good solution unfortunately. |
Can you elaborate on that? Does this mean there's nothing we can do on our end to fix it? Does this mean that no one is going to address this on the Flutter side? Our client's goal has always been to deliver a premium reading experience. This bug makes that impossible. A reader that jitters every time the user touches the screen will never be perceived as premium. As a result, this seemingly minor issue is catastrophic for the client. So it's a very big deal if this problem is essentially a shoulder shrug from Flutter. |
I mean the thing I think I'm seeing is a known problem that no one has quite complained about loudly enough yet. There were two or three attempts at landing something that shoudl fix it, but none of them worked and they ended up causing other, bigger problems instead. We were able to fix this on iOS by using transactions. But I don't know that the same are available on Android (particularly in the GLES backend). I could try to ask around but it's not the first time I've tried to figure out what's going on with this particular problem. It's possible I'm missing something, that there's one or more other issues contributing to this (I do think the touch input is a problem on at least some android devices at this point for example). |
I can't seem to reproduce the jitter on iOS, which further seems to support my hypothesis. |
On a Samsung S6 tablet, I can see this example frequently exceeding budget on the raster thread. If I switch rendering to a SurfaceView instead of a TextureView, things get much better. There's still some occasional jitteriness but over all it looks much better in tracing and visually to me (specifically, don't do this: page_list_viewport/example/android/app/src/main/kotlin/com/example/example/MainActivity.kt Line 7 in cd74fce
|
(Basically, you should never use RenderMode.texture unless you absolutely must) |
Second big problem: The images you're drawing in the tiles are huge. As in, every single image your drawing is larger than the physical size of the screen. The framework has debug checking for this, but you're bypassing that by directly using This is causing tons of GPU work that is completely unnecessary, not to mention lots of memory usage. |
You can help address that by using a |
@RossComputerGuy can you try both of Dan's recommendations? Switch back to |
Yeah, I'll try those out. If its worth it, I want to get a trace dump in Perfetto. |
I just tried this out, I switched to surface rendering and removed the
This was in debug mode and I tried it with a prebuilt engine on tag |
Can you post a PR with those changes so that @kirkbyo and I can run the demo and see what happens? |
Pushed a PR |
I tried the PR that goes back to First, the experience is broken when the I added |
I'm not sure what the 72 is referring to, but the real kicker is to make sure that you're not decoding your images at full resolution - try to decode them as close as possible to the resolution you'll actually display them at. A little bigger is ok, but right now they're getting decoded as something like 5k resolution to be displayed on a 1080 screen. |
@dnfield - Makes sense. @RossComputerGuy can you update the PR so that this is accomplished? We still need full pinch to zoom capability, and we want to test the jitter with as many tiles on screen as possible. |
I've updated the PR, I made it a 5th of what it originally was and that works. |
We think that the cause of panning jitter might be due to behaviors within the Flutter engine. This ticket is to investigate possible causes and explore solutions.
The jitter seems to be more noticeable on higher refresh displays, such as the 90Hz Xiaomi Redmi tablet.
To recreate the jitter in a reproducible manner, use the floating action buttons at the lower right of the screen to animate panning up/down and left/right. You should see fairly frequent jitter, which looks a bit like rapid starting and stopping while panning.
The text was updated successfully, but these errors were encountered: