sony / flutter-embedded-linux

Embedded Linux embedding for Flutter
BSD 3-Clause "New" or "Revised" License
1.2k stars 127 forks source link

Flickering occurs since flutter-elinux 3.7.6 #334

Open makotosato-at opened 1 year ago

makotosato-at commented 1 year ago

Hello.

I am using an i.MX8MP board and weston. Using flutter-elinux 3.7.6 and #333, I get flickering in my application. (It works fine in version 3.3.10.)

I attached a video:

https://user-images.githubusercontent.com/51475851/223674341-bd80be82-8718-4ecb-8525-6bc66d9d7208.mp4

Window size is 1920x1080. In the application, movie@30fps(color bars) and scrolling text are displayed. Is there any solution?

It doesn't solve anything, but I tried the following and it worked fine:

--- a/src/flutter/shell/platform/linux_embedded/surface/elinux_egl_surface.cc
+++ b/src/flutter/shell/platform/linux_embedded/surface/elinux_egl_surface.cc
@@ -189,6 +189,7 @@ void ELinuxEGLSurface::PopulateExistingDamage(const intptr_t fbo_id,
       }
     }
   }
+  existing_damage->damage[0] = {0, 0, 1920, 1080};
 }

 // Auxiliary function used to transform a FlutterRect into the format that is
HidenoriMatsubayashi commented 1 year ago

Can you share the test source files you used?

makotosato-at commented 1 year ago

Thanks for your reply.

I attached a test source. flickering.zip

0xkelvin commented 1 year ago

thanks for raising this, we facing same issues, so would like to follow up

HidenoriMatsubayashi commented 1 year ago

Is this issue reproducible on Linux desktop PC? Does it happen on only i.mx8m + weston?

makotosato-at commented 1 year ago

For now, only imx8mp + weston.

HidenoriMatsubayashi commented 1 year ago

unfortunately, I cannot reproduce this because I don't have imx8mp board.

Can you please try compiler options below?

makotosato-at commented 1 year ago

Thank you. I tried it, but it did not work.

HidenoriMatsubayashi commented 1 year ago

(It works fine in version 3.3.10.)

Hmm, version 3.7.0 also doesn't work, right? If so, it might depend on flutter/engine.

HidenoriMatsubayashi commented 1 year ago

I found similar issue https://github.com/flutter/flutter/issues/100522

makotosato-at commented 1 year ago

Thanks for the information. I will watch this issue.

calvinchengx commented 1 year ago

This link to 100522 is impeller related.

Root cause of flickering bug is described here - https://github.com/flutter/flutter/issues/121740

And this line of code is the problem - https://github.com/flutter/engine/blob/4a90fbcd69011f8dd593840a0e901d2cede5398b/flow/layers/display_list_raster_cache_item.cc#L112

There was an attempt to fix this bug here - https://github.com/flutter/engine/pull/39690 - but it created the above chicken-and-egg issue 121740.

The root cause is that picture layers used to declare themselves as able to inherit opacity when they became eligible to cache. The problem is that "eligible to cache" doesn't mean that they will be successful. In particular, we only allow 3 pictures to cache per frame so if there were more than 3 children than all of them might say they can inherit opacity, but only 3 would be telling the truth. The fix requires the children to wait until they successfully cache themselves before they declare that they can inherit opacity. The problem is that if the parent is caching the children then it will tell the children not to cache themselves. This policy prevents double-caching where the parent is caching the children who are also caching themselves. Unfortunately, since the children can never cache themselves, they also never declare that they can inherit opacity. Thus, a chicken and egg situation.

The fix will require some careful decisions about the eligibility and potential eligibility of the children to cache themselves and thus inherit opacity.

barribarrier commented 1 year ago

Same problem. My app flickers for a few seconds on launch and then stabilizes itself (no more flicker)

x64 linux Sway: 1.7-1 Flutter-elinux: 3.10.1

JakeSays commented 1 year ago

I'm seeing this same thing, flutter version 3.10.05 and using the X11, wayland, and gbm embedders on both a raspberry pi 3a+, and a radxa zero.

HidenoriMatsubayashi commented 1 year ago

@makotosato-at I suppose this issue will not occur on flutter version 3.10.6 (latest stable version). Can you still observe it? If no, let's close this issue.

0xkelvin commented 1 year ago

hi @HidenoriMatsubayashi it is still happens on 3.10.6

HidenoriMatsubayashi commented 1 year ago

Hello, Kelvin. Okay, thanks.

JakeSays commented 1 year ago

I was able to resolve the issue by enabling damage tracking. I was building without it initially. There is currently a bug in flutter that breaks the non-damage tracking code path.

makotosato-at commented 1 year ago

Hello. @HidenoriMatsubayashi It still occurs.

@JakeSays How do I enable damage tracking?

JakeSays commented 1 year ago

@makotosato-at there's a cmake option USE_DIRTY_REGION_MANAGEMENT that enables it, but it looks like it's on by default. I don't use cmake for building elinux_flutter and had the feature disabled in my build system.

For reference here is the flutter bug: https://github.com/flutter/flutter/issues/119601

makotosato-at commented 1 year ago

@JakeSays Thanks. But in my environment, it occurs even if USE_DIRTY_REGION_MANAGEMENT=OFF.

HidenoriMatsubayashi commented 1 year ago

I think this is not the embedder's issue and it depends on flutter/engine (+ H/W). Many developers for mobile have reported similar flicker issues (https://github.com/search?q=repo%3Aflutter%2Fflutter+flicker&type=issues). So, we need to watch flutter/engine updates.

makotosato-at commented 1 year ago

Hello. I don't know why. But, in my environment, the following patch seems to work fine.

diff --git a/src/flutter/shell/platform/linux_embedded/surface/elinux_egl_surface.cc b/src/flutter/shell/platform/linux_embedded/surface/elinux_egl_surface.cc
index 07fede8..f4e198a 100644
--- a/src/flutter/shell/platform/linux_embedded/surface/elinux_egl_surface.cc
+++ b/src/flutter/shell/platform/linux_embedded/surface/elinux_egl_surface.cc
@@ -168,29 +168,6 @@ void ELinuxEGLSurface::PopulateExistingDamage(const intptr_t fbo_id,
       0, 0, static_cast<double>(width_px_), static_cast<double>(height_px_)};
   existing_damage->damage = existing_damage_map_[fbo_id];

-  if (age > 1) {
-    --age;
-    // join up to (age - 1) last rects from damage history
-    for (auto i = damage_history_.rbegin();
-         i != damage_history_.rend() && age > 0; ++i, --age) {
-      if (i == damage_history_.rbegin()) {
-        if (i != damage_history_.rend()) {
-          existing_damage->damage[0] = {i->left, i->top, i->right, i->bottom};
-        }
-      } else {
-        // Auxiliary function to union the damage regions comprised by two
-        // FlutterRect element. It saves the result of this join in the rect
-        // variable.
-        FlutterRect* rect = &(existing_damage->damage[0]);
-        const FlutterRect additional_rect = *i;
-
-        rect->left = std::min(rect->left, additional_rect.left);
-        rect->top = std::min(rect->top, additional_rect.top);
-        rect->right = std::max(rect->right, additional_rect.right);
-        rect->bottom = std::max(rect->bottom, additional_rect.bottom);
-      }
-    }
-  }
 }

 // Auxiliary function used to transform a FlutterRect into the format that is
HidenoriMatsubayashi commented 1 year ago

Good catch!

But, those code are basically copied from engine/examples/glfw_drm/FlutterEmbedderGLFW.cc. What values do eglQuerySurface(display_, surface_, EGL_BUFFER_AGE_EXT, &age) and age return? Also, is GL_EXT_buffer_age extension supported in your board?

  // Given the FBO age, create existing damage region by joining all frame
  // damages since FBO was last used
  EGLint age = 0;
  if (eglQuerySurface(display_, surface_, EGL_BUFFER_AGE_EXT, &age) !=
          EGL_TRUE ||
      age == 0) {
    age = 4;  // Virtually no driver should have a swapchain length > 4.
  }
makotosato-at commented 1 year ago

What values do eglQuerySurface(display, surface, EGL_BUFFER_AGE_EXT, &age) and age return? Also, is GL_EXT_buffer_age extension supported in your board?

eglQuerySurface(display_, surface_, EGL_BUFFER_AGE_EXT, &age) returns EGL_TRUE and age is 3. My board supports EGL_EXT_buffer_age.

HidenoriMatsubayashi commented 1 year ago

Okay, but current code looks good to me. Your patch/https://github.com/sony/flutter-embedded-linux/issues/334#issuecomment-1704893622 equals full repaint or disable DIRTY_REGION_MANAGEMENT, I think.

HidenoriMatsubayashi commented 1 year ago

https://github.com/flutter/flutter/issues/97482#issuecomment-1034102176 might be related this flicker issue. We might need to consider 32x32 alignment. And also, I found https://github.com/flutter/flutter/issues/123353.

makotosato-at commented 1 year ago

Thank you for the information. I will do some more research.

HidenoriMatsubayashi commented 1 year ago

Maybe partial repaint shouldn't be used with GLES. Anyway, once @makotosato-at's investigation is done, we can decide next action. I think adding a runtime command option to enable/disable DIRTY_REGION_MANAGEMENT is one of our options.

makotosato-at commented 1 year ago

In my environment, it occurs even if USE_DIRTY_REGION_MANAGEMENT=OFF.

This is probably due to the following code: https://github.com/flutter/engine/blob/main/shell/platform/embedder/embedder_surface_gl.cc#L91 info.supports_partial_repaint is always true.

HidenoriMatsubayashi commented 1 year ago

No, I mean although we use dirty region management APIs of flutter/engine, it always full repaints with an additional command-line option and its new logic when the option is disabled.

  existing_damage_map_[fbo_id][0] = FlutterRect{
      0, 0, static_cast<double>(width_px_), static_cast<double>(height_px_)};
  existing_damage->damage = existing_damage_map_[fbo_id];

In my environment, it occurs even if USE_DIRTY_REGION_MANAGEMENT=OFF.

We have to use them to avoid a critical issue in flutter/engine. See https://github.com/flutter/flutter/issues/119601

makotosato-at commented 1 year ago

No, I mean although we use dirty region management APIs of flutter/engine, it always full repaints with an additional command-line option and its new logic when the option is disabled.

I understand.

We have to use them to avoid a critical issue in flutter/engine. See https://github.com/flutter/flutter/issues/119601

Thank you. I missed that.

HidenoriMatsubayashi commented 1 year ago

@makotosato-at

I was also reproduce a similar rendering issue on Raspberry Pi3 B+ (https://github.com/sony/flutter-elinux/issues/222). And also, it looks like config.open_gl.fbo_reset_after_present = true; is related to the flicker.

engine/shell/gpu/gpu_surface_gl_skia.cc:

  if (delegate_->GLContextFBOResetAfterPresent()) {
    auto current_size =
        SkISize::Make(onscreen_surface_->width(), onscreen_surface_->height());

    GLFrameInfo frame_info = {static_cast<uint32_t>(current_size.width()),
                              static_cast<uint32_t>(current_size.height())};

    // The FBO has changed, ask the delegate for the new FBO and do a surface
    // re-wrap.
    const GLFBOInfo fbo_info = delegate_->GLContextFBO(frame_info);
    auto new_onscreen_surface =
        WrapOnscreenSurface(context_.get(),  // GL context
                            current_size,    // root surface size
                            fbo_info.fbo_id  // window FBO ID
        );

    if (!new_onscreen_surface) {
      return false;
    }

    onscreen_surface_ = std::move(new_onscreen_surface);
    fbo_id_ = fbo_info.fbo_id;
    existing_damage_ = fbo_info.existing_damage;
  }

I'll disable it temporarily for now.

HidenoriMatsubayashi commented 1 year ago

@makotosato-at if you have a change to verify https://github.com/sony/flutter-embedded-linux/pull/385, that would be great.

makotosato-at commented 1 year ago

Hello.

In my environment, this issue does not improve if fbo_reset_after_present = false.

I think, if the following PR is merged, this issue will be improved. https://github.com/flutter/engine/pull/45611

HidenoriMatsubayashi commented 1 year ago

Okay, thanks. I'll revert https://github.com/sony/flutter-embedded-linux/pull/385 later. Let's wait to be merged https://github.com/flutter/engine/pull/45611.

HidenoriMatsubayashi commented 1 year ago

Even if https://github.com/flutter/engine/pull/45611 is merged into master branch, it takes 2-3 months to be propagated to the stable channel. So, I'll temporarily merge @makotosato-at 's change (https://github.com/sony/flutter-embedded-linux/issues/334#issuecomment-1704893622) into this repo as a workaround.

HidenoriMatsubayashi commented 11 months ago

https://github.com/sony/flutter-embedded-linux/pull/388 has been applied in flutter-elinux v3.13.9. Once I confirm https://github.com/flutter/engine/pull/45611 is propagated to the stable channel, I'll revert https://github.com/sony/flutter-embedded-linux/pull/388.

HidenoriMatsubayashi commented 10 months ago

Once I confirm https://github.com/flutter/engine/pull/45611 is propagated to the stable channel, I'll revert https://github.com/sony/flutter-embedded-linux/pull/388.

The change has been included in flutter 3.16.0. I created https://github.com/sony/flutter-embedded-linux/pull/401

HidenoriMatsubayashi commented 10 months ago

@makotosato-at Would it be possible for you to confirm if the issue has been fixed in flutter-3.16.1?

makotosato-at commented 10 months ago

Hello. It works fine! (Set to USE_DIRTY_REGION_MANAGEMENT=OFF)

HidenoriMatsubayashi commented 10 months ago

(Set to USE_DIRTY_REGION_MANAGEMENT=OFF)

Hm? What do you mean?

makotosato-at commented 10 months ago

I built flutter-embedded-linux with the following changes to CMakeLists.txt. option(USE_DIRTY_REGION_MANAGEMENT "Use Flutter dirty region management" OFF) Then, using the resulting library, it worked fine.

HidenoriMatsubayashi commented 10 months ago

Sorry for the confusion. Can you please use flutter-elinux 3.16.1 as is?

HidenoriMatsubayashi commented 10 months ago

If that works fine, we can close this issue.

makotosato-at commented 10 months ago

Sorry. I tried 3.16.1 as is, but it did not work...

HidenoriMatsubayashi commented 10 months ago

Really? flutter 3.16.1 has https://github.com/flutter/engine/pull/45611 though...

makotosato-at commented 10 months ago

I think that improvements have been made. Before flutter/engine#45611, it did not work, regardless of whether USE_DIRTY_REGION_MANAGEMENT was ON or OFF. With flutter/engine#45611, it now works when USE_DIRTY_REGION_MANAGEMENT is OFF.

HidenoriMatsubayashi commented 10 months ago

I see...So, it sounds we need to create a change to disable USE_DIRTY_REGION_MANAGEMENT for now.

HidenoriMatsubayashi commented 9 months ago

For now I made https://github.com/sony/flutter-embedded-linux/pull/406 as a workaround.