DescriptionRevert of Improve transition between opaque and translucent compositor views. (patchset #24 id:460001 of https://codereview.chromium.org/2201483002/ )
Reason for revert:
This change unfortunately breaks WebVR and probably VrShell, I confirmed via
bisect that 5fe29f761d6791d746bd7e058d6f3d79227cc117 is the last working change
right before this. It's plausible since it touches code such as detachForVR and
related code, seems like they don't agree on which surfaces should be visible.
Original issue's description:
> Improve transition between opaque and translucent compositor views.
>
> On Android, when we'd like to use a SurfaceView for video playback,
> we must enable the alpha channel on the CompositorView. Otherwise,
> we cannot draw a hole to see the video's SurfaceView. We can't keep
> transparency enabled all the time since it can cause power
> regressions on some hardware.
>
> However, because SurfaceView destroys and recreates the surface when
> changing the pixel format, there is a visible flash during the
> transition. This CL removes that.
>
> It causes the CompositorView to have two SurfaceView child views,
> rather than extending SurfaceView directly. When a request is made
> to switch the output format, it makes the change to the background
> SurfaceView, and swaps it to the front. It also keeps the outgoing
> SurfaceView around until the new one is in use by the compositor.
>
> There are a few interesting bits:
>
> - SurfaceViews are invisible until the first buffer is posted. So,
> we can continue to see the outgoing view until the compositor is
> ready to use the new one.
> - All buffers are released by the outgoing SurfaceView when its
> visibility is set to Gone. 'dumpsys SurfaceFlinger' shows that the
> same amount of gralloc memory is used in steady state.
> - No power regression was observed on a Nexus 5.
> - Unfortunately, new SurfaceViews are z-ordered behind existing ones,
> which makes it critical that we guess when to delete the outgoing
> (topmost) SurfaceView. We currently time one frame, and then hide
> it. Empirically, this works fine.
> - This might seem like a more natural extension to
> CompositorViewHolder, but the CompositorView currently owns the
> native compositor. Since CompositorViewHolder is already fairly
> complicated, it wasn't clear that it would be a good idea to
> refactor that into CVH.
>
> Another approach is to use EGL to change the buffer format to include
> an alpha channel, or not. This avoids the power regression, since
> opaque surfaces and buffers without alpha channels are treated the
> same by SurfaceFlinger. However, it causes problems for virtualized
> contexts. In particular, the off-screen contexts will have an alpha
> channel at all times, so they cannot be shared with the compositor
> context without one. For some hardware (e.g., QC) that requires the
> use of virtualized GL contexts rather than share groups, this may
> have a stability regression. So, we don't try it.
>
> Please see https://goo.gl/aAmQzR for the design doc.
>
> BUG=629130
>
> Review-Url: https://codereview.chromium.org/2201483002
> Cr-Commit-Position: refs/heads/master@{#456142}
> Committed: https://chromium.googlesource.com/chromium/src/+/1e87636c63028418b779dadeaadbe4e5f487c76e
TBR=aelias@chromium.org,dtrainor@chromium.org,mdjones@chromium.org,tedchoc@chromium.org,watk@chromium.org,liberato@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=629130
Review-Url: https://codereview.chromium.org/2742133002
Cr-Commit-Position: refs/heads/master@{#456245}
Committed: https://chromium.googlesource.com/chromium/src/+/2e31bd807912a2540769e990c9f053d209bf383b
Patch Set 1 #
Messages
Total messages: 8 (4 generated)
|