Garbage is displayed momentarily when exiting fullscreen video on Android |
||||||||||
Issue descriptionWhen exiting fullscreen (on a video element) there's visible garbage displayed on some devices. This was on a N6. But it's also visible in debug builds as a blue background (The color that GlRenderer::ClearFramebuffer clears to). See attached screenrecording. I think the root cause is our lack of synchronization between LTHI::set_has_transparent_background() in the browser compositor with frames from the renderer that have a transparent background. When we enter fullscreen: 1) blink gives the page a transparent background and calls RenderWidgetCompositor::setHasTransparentBackground() which calls LTH::set_has_transparent_background() 2) we create a ContentVideoView which has the side effect of calling CompositorView::SetOverlayVideoMode(), which calls LTHI::set_has_transparent_background(). 3) LTHI sets RenderPass::has_transparent_background based on this property 4) GLRenderer::ClearFrameBuffer checks the render pass background to decide whether to actually clear the framebuffer or not. If it's false, the framebuffer isn't cleared. (https://cs.chromium.org/chromium/src/cc/output/gl_renderer.cc?sq=package:chromium&dr=CSs&rcl=1469546500&l=437) My theory is that when exiting fullscreen we render some frames that have a transparent bg after the LTHI background is reset to opaque. So we don't clear the framebuffer for a few frames and the transition looks bad. It seems to me that the bg transparency should propagate with the compositor frame, and the browser compositor should set its own bg transparency based on that. That way we don't need to use the existence of ContentVideoView to signal it and everything is synchonized.
,
Aug 3 2016
,
Aug 18 2016
Can we get this fixed for M54? If this is consistently reproducible I think it's more impactful than a P3, esp. given the other internal reports. Feel free to deprioritize if you disagree.
,
Aug 18 2016
,
Sep 1 2016
We shouldn't be showing garbage even if GlRenderer has not cleared.
,
Sep 1 2016
From what I remember, hardcoding GLRenderer to always clear fixed the issue.
,
Sep 1 2016
I'm inclined to punt to 55 because it seems like a nonregression and to be device-specific (I can't repro on my Qualcomm Galaxy S7). Victor, is there someone on your team with the cycles to work on this?
,
Sep 1 2016
In my opinion this is a minor UI glitch and doesn't warrant P1.
,
Sep 1 2016
I believe we don't force-clear the onscreen surfaces at command buffer level, because this would have a performance cost on some devices, but only the browser (trusted) creates those, so there is no security aspect to it. So it's the browser compositor / UI's responsibility to fully cover the screen (or to clear).
,
Sep 1 2016
On tilers there's often a higher performance cost to not clear. I'll find someone to own this.
,
Sep 1 2016
Because Swap discards (and hence why garbage shows up), it has the same performance effect as clearing on tilers.
,
Sep 1 2016
Reassigning to jbauman@ after offline discussion.
,
Sep 1 2016
Yeah, I think we should be doing SetContentsOpaque on the DelegatedFrameHostAndroid's layer based on has_transparent_background from the root render pass of the child surface. We also need to figure out what background color the browser's compositor should have. It's currently set to white, which looks a bit odd in these transitions.
,
Sep 2 2016
Issue 643635 has been merged into this issue.
,
Sep 8 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/862bef99a8b729f501689b0658f029464ff84135 commit 862bef99a8b729f501689b0658f029464ff84135 Author: jbauman <jbauman@chromium.org> Date: Thu Sep 08 00:32:47 2016 Set content opacity of DFHAndroid surfacelayer based on renderer frame contents. The browser updates the overlay video mode flag before the renderer can send a new frame. This means that when switching from fullscreen to non-fullscreen the browser was expecting the renderer the fill the frame completely and wasn't drawing the background, but it was accidentally leaving part of the frame corrupted and uncleared. BUG= 631866 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel Review-Url: https://codereview.chromium.org/2302023003 Cr-Commit-Position: refs/heads/master@{#417131} [modify] https://crrev.com/862bef99a8b729f501689b0658f029464ff84135/content/browser/media/android/browser_media_player_manager.cc [modify] https://crrev.com/862bef99a8b729f501689b0658f029464ff84135/content/browser/renderer_host/compositor_impl_android.cc [modify] https://crrev.com/862bef99a8b729f501689b0658f029464ff84135/content/browser/renderer_host/render_widget_host_view_android.cc [modify] https://crrev.com/862bef99a8b729f501689b0658f029464ff84135/content/browser/renderer_host/render_widget_host_view_android.h [modify] https://crrev.com/862bef99a8b729f501689b0658f029464ff84135/ui/android/delegated_frame_host_android.cc [modify] https://crrev.com/862bef99a8b729f501689b0658f029464ff84135/ui/android/delegated_frame_host_android.h
,
Sep 13 2016
,
May 1 2017
|
||||||||||
►
Sign in to add a comment |
||||||||||
Comment 1 by w...@chromium.org
, Aug 1 2016