feat: add disable_dirty_region_management option to fix flickering on iMX8MP (#334)#22
Conversation
… iMX8MP (sony#334) Some GPU drivers (e.g. iMX8MP / NXP Vivante GC7000L) mishandle EGL_KHR_partial_update, causing display flickering when dirty region management is active. The Flutter engine's embedder_surface_gl.cc also hardcodes supports_partial_repaint=true (issue #119601), making the build-time USE_OPENGL_DIRTY_REGION_MANAGEMENT=OFF insufficient. Add disable_dirty_region_management to FlutterDesktopViewProperties. When set to true: - EGL damage extensions (EGL_KHR_partial_update, EGL_EXT/KHR_swap_buffers_with_damage) are not loaded at ELinuxEGLSurface construction time, so eglSwapBuffers always performs a full swap. - PopulateExistingDamage returns full-screen damage immediately, so the Flutter engine schedules a full repaint every frame. - Frame damage history is not accumulated (no-op when disabled). Existing behavior is fully preserved for all users who do not set this flag (zero-initialised = false = management enabled). Signed-off-by: Akihiko Komada <aki1770@gmail.com>
| eglSetDamageRegionKHR_ = reinterpret_cast<PFNEGLSETDAMAGEREGIONKHRPROC>( | ||
| eglGetProcAddress("eglSetDamageRegionKHR")); | ||
| } | ||
| if (has_egl_extension(extensions, "EGL_KHR_partial_update")) { |
There was a problem hiding this comment.
GH formatting suggests there might be an indentation issue here?
There was a problem hiding this comment.
Good catch on the visual — it's intentional rather than a formatting error. The EGL_KHR_partial_update check is now nested inside a new if (!disable_dirty_region_management_) guard, so the deeper indentation reflects the new block structure. The outer if line may not be visible in the same diff hunk depending on context, which makes it look like a stray indent. Happy to add a blank line before the outer if to make the structure clearer in the diff if that helps.
|
Thank you! Appreciate you reviewing. Will wait on @martinetd for merge. |
|
AI-assisted — authored with Claude, reviewed by Komada. |
There was a problem hiding this comment.
Claude review below
Thanks for the work and for chasing sony#334, but I'd like to push back on this approach before it lands.
The PR description says USE_OPENGL_DIRTY_REGION_MANAGEMENT=OFF is "insufficient" because the engine hardcodes supports_partial_repaint=true. But that engine-internal flag only matters if the embedder actually invokes the partial-repaint EGL path — and in this fork it can't, by build configuration:
CMakeLists.txt:15-17:
# Disabled USE_DIRTY_REGION_MANAGEMENT due flicker issue.
#
option(USE_DIRTY_REGION_MANAGEMENT "Use Flutter dirty region management" OFF)The flag has been OFF by default in this fork specifically because of this flicker. When OFF, flutter_elinux_engine.cc:46-62 never wires config.open_gl.present_with_info / populate_existing_damage. The engine then calls the plain present callback, which routes to ELinuxEGLSurface::SwapBuffers() (the no-arg overload at line 100) — eglSwapBuffers only, no eglSetDamageRegionKHR, no eglSwapBuffersWithDamageEXT. The buggy iMX8MP code paths are unreachable.
So this PR adds a new public API field on FlutterDesktopViewProperties plumbed through 5 layers (71 lines, 8 files) to disable a feature that's already disabled by default. The only configuration where it would matter is one where someone has explicitly turned the build flag back ON — which is precisely the configuration affected users shouldn't be running.
A few questions before deciding what to do here:
- Are you actually building with
-DUSE_DIRTY_REGION_MANAGEMENT=ON? If so, what's driving that? - Are you seeing flicker with the default build (
OFF)? If yes, the symptom is interesting — it would mean something other than the partial-repaint path is involved, and we'd want to dig into that path rather than add a runtime flag for the partial-repaint path.
If neither of those is the case, I'd suggest closing this in favour of just documenting that the default build already handles sony#334. Happy to reopen the conversation if I'm missing the configuration you're operating in.
Code quality observation, separate from the design question: if this does get reworked, the natural place to centralize the toggle would be elinux_window.h rather than threading a new field through NativeWindow, since the build flag is already gated at the engine config layer.
Summary
Some GPU drivers (e.g. iMX8MP / NXP Vivante GC7000L) mishandle
EGL_KHR_partial_update, causing display flickering when dirty region management is active. The Flutter engine'sembedder_surface_gl.ccalso hardcodessupports_partial_repaint=true(issue flutter/flutter#119601), making the build-timeUSE_OPENGL_DIRTY_REGION_MANAGEMENT=OFFinsufficient.Fix
Add
disable_dirty_region_managementtoFlutterDesktopViewProperties. When set totrue:EGL_KHR_partial_update,EGL_EXT/KHR_swap_buffers_with_damage) are not loaded, soeglSwapBuffersalways performs a full swap.PopulateExistingDamagereturns full-screen damage immediately.Existing behavior is fully preserved for all users who do not set this flag (zero-initialised = false = management enabled).
8 files changed, +71/-38 lines
Closes sony#334
Signed-off-by: Akihiko Komada aki1770@gmail.com