Commit 2e31bd80 authored by klausw's avatar klausw Committed by Commit bot

Revert of Improve transition between opaque and translucent compositor views....

Revert 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 5fe29f76 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}
parent dd3c8c6e
......@@ -343,7 +343,7 @@ public abstract class ChromeActivity extends AsyncInitializationActivity
// Set up the animation placeholder to be the SurfaceView. This disables the
// SurfaceView's 'hole' clipping during animations that are notified to the window.
mWindowAndroid.setAnimationPlaceholderView(mCompositorViewHolder.getCompositorView());
mWindowAndroid.setAnimationPlaceholderView(mCompositorViewHolder.getSurfaceView());
// Inform the WindowAndroid of the keyboard accessory view.
mWindowAndroid.setKeyboardAccessoryView((ViewGroup) findViewById(R.id.keyboard_accessory));
......
......@@ -20,6 +20,7 @@ import android.util.AttributeSet;
import android.util.Pair;
import android.view.DragEvent;
import android.view.MotionEvent;
import android.view.SurfaceView;
import android.view.View;
import android.view.ViewGroup;
import android.view.accessibility.AccessibilityEvent;
......@@ -417,9 +418,9 @@ public class CompositorViewHolder extends FrameLayout
}
/**
* @return The SurfaceView proxy used by the Compositor.
* @return The SurfaceView used by the Compositor.
*/
public View getCompositorView() {
public SurfaceView getSurfaceView() {
return mCompositorView;
}
......@@ -975,7 +976,7 @@ public class CompositorViewHolder extends FrameLayout
mTabModelSelector = null;
mLayerTitleCache.setTabModelSelector(null);
setTab(null);
getCompositorView().setVisibility(View.INVISIBLE);
getSurfaceView().setVisibility(View.INVISIBLE);
return selector;
}
......@@ -985,7 +986,7 @@ public class CompositorViewHolder extends FrameLayout
* @param tabModelSelector
*/
public void onExitVR(TabModelSelector tabModelSelector) {
getCompositorView().setVisibility(View.VISIBLE);
getSurfaceView().setVisibility(View.VISIBLE);
attachToTabModelSelector(tabModelSelector);
}
......
......@@ -126,7 +126,6 @@ chrome_java_sources = [
"java/src/org/chromium/chrome/browser/childaccounts/ChildAccountFeedbackReporter.java",
"java/src/org/chromium/chrome/browser/childaccounts/ChildAccountService.java",
"java/src/org/chromium/chrome/browser/childaccounts/ExternalFeedbackReporter.java",
"java/src/org/chromium/chrome/browser/compositor/CompositorSurfaceManager.java",
"java/src/org/chromium/chrome/browser/compositor/CompositorView.java",
"java/src/org/chromium/chrome/browser/compositor/CompositorViewHolder.java",
"java/src/org/chromium/chrome/browser/compositor/Invalidator.java",
......@@ -1561,7 +1560,6 @@ chrome_junit_test_java_sources = [
"junit/src/org/chromium/chrome/browser/ShortcutHelperTest.java",
"junit/src/org/chromium/chrome/browser/SSLClientCertificateRequestTest.java",
"junit/src/org/chromium/chrome/browser/compositor/overlays/strip/StripLayoutHelperTest.java",
"junit/src/org/chromium/chrome/browser/compositor/CompositorSurfaceManagerTest.java",
"junit/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchSelectionControllerTest.java",
"junit/src/org/chromium/chrome/browser/cookies/CanonicalCookieTest.java",
"junit/src/org/chromium/chrome/browser/crash/LogcatExtractionRunnableUnitTest.java",
......
......@@ -175,12 +175,6 @@ gpu::gles2::ContextCreationAttribHelper GetCompositorContextAttributes(
// specified
// (IOW check that a <= 0 && rgb > 0 && rgb <= 565) then alpha should be
// -1.
// TODO(liberato): This condition is memorized in ComositorView.java, to
// avoid using two surfaces temporarily during alpha <-> no alpha
// transitions. If these mismatch, then we risk a power regression if the
// SurfaceView is not marked as eOpaque (FORMAT_OPAQUE), and we have an
// EGL surface with an alpha channel. SurfaceFlinger needs at least one of
// those hints to optimize out alpha blending.
attributes.alpha_size = 0;
attributes.red_size = 5;
attributes.green_size = 6;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment