[android] set presentation time via eglPresentationTimeANDROID #33881

iskakaushik · 2022-06-07T19:11:47Z

Attempt to reland: #29727

TODO:

This isn't right in the cases where the frame has been resubmitted.
This hasn't been wired for impeller yet.

flutter-dashboard · 2022-06-07T19:11:49Z

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on Discord!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

Attempt to reland: flutter#29727 TODO: 1. This isn't right in the cases where the frame has been resubmitted. 2. This hasn't been wired for impeller yet.

iskakaushik · 2022-06-08T14:45:56Z

On a test application, this was seen to reduce the total time spent in dequeueBuffer from ~550ms to ~150ms.

cc: @dnfield

dnfield

It'd be great to have some kind of test for this, especially if we have to special case resubmit frames.

dnfield · 2022-06-08T14:48:56Z

shell/common/rasterizer.cc

@@ -604,6 +604,9 @@ RasterStatus Rasterizer::DrawToSurfaceUnsafe(
    }

    SurfaceFrame::SubmitInfo submit_info;
+    // TODO (kaushikiska): this can be in the past and might need to get snapped
+    // to future as this frame could have been resubmitted.
+    submit_info.presentation_time = frame_timings_recorder.GetVsyncTargetTime();


Bug?

Should we avoid setting this if it's a resubmit?

filed an issue. for now i'll set this only if it is > now(), I think it should be the case for majority of frames. Once the issue is addressed we can snap this to the next vsync target time,.

dnfield · 2022-06-08T14:49:38Z

shell/gpu/gpu_surface_gl_impeller.cc

@@ -63,6 +63,8 @@ std::unique_ptr<SurfaceFrame> GPUSurfaceGLImpeller::AcquireFrame(
      GLPresentInfo present_info = {
          .fbo_id = 0,
          .damage = std::nullopt,
+          // TODO (kaushikiska): wire-up presentation time to impeller backend.


dnfield · 2022-06-08T14:50:58Z

shell/platform/android/android_egl_surface.cc

@@ -60,6 +60,17 @@ void LogLastEGLError() {
  FML_LOG(ERROR) << "Unknown EGL Error";
 }

+namespace {
+
+static bool HasExtension(const char* extensions, const char* name) {


We have this method in like 3 or 4 places now. Or maybe we will with the swiftshader patch. We should consider moving it somewhere common

@dnfield i think this is out of scope for this PR. Will refactor this later if that is ok.

iskakaushik · 2022-06-08T15:55:48Z

It'd be great to have some kind of test for this, especially if we have to special case resubmit frames.

Done! Added tests for cases where the frame is submitted in the future vs in past.

dnfield · 2022-06-08T16:46:13Z

shell/common/rasterizer_unittests.cc

+                           thread_host.raster_thread->GetTaskRunner(),
+                           thread_host.ui_thread->GetTaskRunner(),
+                           thread_host.io_thread->GetTaskRunner());
+  MockDelegate delegate;


nit: use NiceMock<MockDelegate> (using ::testing::NiceMock) so this doesn't spam output about unused calls, here and elsewhere.

nice! flutter/flutter#105631, i think we want to use it everywhere in this file.

I'm updating the rest of the file in #33890 which is currently stuck on Fuchsia weirdness.

dnfield · 2022-06-08T16:47:22Z

shell/platform/android/android_egl_surface.cc

+  if (HasExtension(extensions, "EGL_ANDROID_presentation_time")) {
+    presentation_time_proc_ =
+        reinterpret_cast<PFNEGLPRESENTATIONTIMEANDROIDPROC>(
+            eglGetProcAddress("eglPresentationTimeANDROID"));


For future us: this was the big part of what was missing last time.

When I tested this locally, I hacked in a defintiion of this method and saw improvements. I then refactored things to actually do the proc lookup and put the extension name instead of the method name here, which resulted in no improvements (and slight regressions!).

dnfield

LGTM with nit

dnfield · 2022-06-08T16:48:10Z

/cc @ds84182 @MxSoul FYI

flutter/engine#33881)

…er#33881) Attempt to reland: flutter#29727 with some fixes.

zanderso · 2022-08-02T15:18:42Z

Could this change have been responsible for the improvement to 99%-ile raster times in backdrop_filter_perf?
https://flutter-flutter-perf.skia.org/e/?begin=1653514394&end=1659451447&keys=X29da4c759a0ed692aa4ac443fa2cfbb3&requestType=0&xbaroffset=30147

flutter-dashboard bot added platform-android needs tests labels Jun 7, 2022

[android] set presentation time via eglPresentationTimeANDROID

19c63af

Attempt to reland: flutter#29727 TODO: 1. This isn't right in the cases where the frame has been resubmitted. 2. This hasn't been wired for impeller yet.

iskakaushik force-pushed the egl-presentation branch from eb7178e to 19c63af Compare June 7, 2022 21:22

dnfield reviewed Jun 8, 2022

View reviewed changes

iskakaushik added 2 commits June 8, 2022 11:02

add issues for TODOs

7908bdf

add a test to ensure presentation time passed by rasterizer

a846db7

dnfield reviewed Jun 8, 2022

View reviewed changes

dnfield approved these changes Jun 8, 2022

View reviewed changes

iskakaushik merged commit 10ff302 into flutter:main Jun 8, 2022

This was referenced Jun 8, 2022

Use eglPresentationTimeANDROID flutter/flutter#93352

Closed

dequeueBuffer may be unnecessarily blocking CPU progress on the raster thread flutter/flutter#93346

Closed

engine-flutter-autoroll mentioned this pull request Jun 8, 2022

Roll Flutter Engine from 38d950a44245 to 10ff30257d76 (5 revisions) flutter/flutter#105637

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 8, 2022

10ff302 [android] set presentation time via eglPresentationTimeANDROID (

360af1b

flutter/engine#33881)

engine-flutter-autoroll mentioned this pull request Jun 8, 2022

Roll Flutter Engine from 38d950a44245 to 350733877419 (6 revisions) flutter/flutter#105640

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 8, 2022

10ff302 [android] set presentation time via eglPresentationTimeANDROID (

e714551

flutter/engine#33881)

engine-flutter-autoroll mentioned this pull request Jun 8, 2022

Roll Flutter Engine from 38d950a44245 to 38b217294d7e (8 revisions) flutter/flutter#105647

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 8, 2022

10ff302 [android] set presentation time via eglPresentationTimeANDROID (

a977d23

flutter/engine#33881)

engine-flutter-autoroll mentioned this pull request Jun 8, 2022

Roll Flutter Engine from 38d950a44245 to d088e572f328 (10 revisions) flutter/flutter#105650

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 8, 2022

10ff302 [android] set presentation time via eglPresentationTimeANDROID (

17e5239

flutter/engine#33881)

iskakaushik mentioned this pull request Jun 9, 2022

Snap presentation time to next vsync target time if a frame was resubmitted. flutter/flutter#105596

Open

houhuayong pushed a commit to houhuayong/engine that referenced this pull request Jun 21, 2022

[android] set presentation time via eglPresentationTimeANDROID (flutt…

019adba

…er#33881) Attempt to reland: flutter#29727 with some fixes.

jonahwilliams mentioned this pull request Sep 28, 2022

Android scroll physics deceleration feels stuttery on ToT, 3.3 compared to 3.0. flutter/flutter#112503

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[android] set presentation time via eglPresentationTimeANDROID #33881

[android] set presentation time via eglPresentationTimeANDROID #33881

iskakaushik commented Jun 7, 2022

flutter-dashboard bot commented Jun 7, 2022

iskakaushik commented Jun 8, 2022

dnfield left a comment

dnfield Jun 8, 2022

iskakaushik Jun 8, 2022

dnfield Jun 8, 2022

iskakaushik Jun 8, 2022

dnfield Jun 8, 2022

iskakaushik Jun 8, 2022

dnfield Jun 8, 2022

iskakaushik commented Jun 8, 2022

dnfield Jun 8, 2022

iskakaushik Jun 8, 2022

dnfield Jun 8, 2022

dnfield Jun 8, 2022

dnfield left a comment

dnfield commented Jun 8, 2022

zanderso commented Aug 2, 2022 •

edited

Loading

[android] set presentation time via eglPresentationTimeANDROID #33881

[android] set presentation time via eglPresentationTimeANDROID #33881

Conversation

iskakaushik commented Jun 7, 2022

flutter-dashboard bot commented Jun 7, 2022

iskakaushik commented Jun 8, 2022

dnfield left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

iskakaushik commented Jun 8, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dnfield left a comment

Choose a reason for hiding this comment

dnfield commented Jun 8, 2022

zanderso commented Aug 2, 2022 • edited Loading

zanderso commented Aug 2, 2022 •

edited

Loading