Commit 158b87d2 authored by Khushal's avatar Khushal Committed by Commit Bot

Revert "gpu: Disable SurfaceControl on Samsung devices running Q."

This reverts commit c8c22806.

Reason for revert: crbug.com/1144660

Original change's description:
> gpu: Disable SurfaceControl on Samsung devices running Q.
>
> A platform bug can result in transaction acks getting dropped which
> stalls all rendering. A fix for this will be released in an R update
> soon so this changes disables using this API in Q.
>
> This also reverts an earlier workaround which was added for samsung dex
> mode since its not necessary after this change.
>
> TBR=skyostil@chromium.org
>
> Bug: 1131081
> Change-Id: I8d053f160fd8ad5bf6ce47d5e6e295c322bace69
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2491401
> Commit-Queue: Khushal <khushalsagar@chromium.org>
> Reviewed-by: Bo <boliu@chromium.org>
> Reviewed-by: Tommy Nyquist <nyquist@chromium.org>
> Auto-Submit: Khushal <khushalsagar@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#820511}

TBR=nyquist@chromium.org,boliu@chromium.org,khushalsagar@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 1131081
Change-Id: I55a3a9e8329b062ffbd506650fd7aca483172908
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2538417
Commit-Queue: Khushal <khushalsagar@chromium.org>
Reviewed-by: default avatarKhushal <khushalsagar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#827533}
parent b7fa55af
...@@ -367,6 +367,9 @@ void CompositorImpl::SetRootLayer(scoped_refptr<cc::Layer> root_layer) { ...@@ -367,6 +367,9 @@ void CompositorImpl::SetRootLayer(scoped_refptr<cc::Layer> root_layer) {
void CompositorImpl::SetSurface(const base::android::JavaRef<jobject>& surface, void CompositorImpl::SetSurface(const base::android::JavaRef<jobject>& surface,
bool can_be_used_with_surface_control) { bool can_be_used_with_surface_control) {
can_be_used_with_surface_control &=
!root_window_->ApplyDisableSurfaceControlWorkaround();
JNIEnv* env = base::android::AttachCurrentThread(); JNIEnv* env = base::android::AttachCurrentThread();
gpu::GpuSurfaceTracker* tracker = gpu::GpuSurfaceTracker::Get(); gpu::GpuSurfaceTracker* tracker = gpu::GpuSurfaceTracker::Get();
......
...@@ -978,6 +978,11 @@ public class WindowAndroid implements AndroidPermissionDelegate, DisplayAndroidO ...@@ -978,6 +978,11 @@ public class WindowAndroid implements AndroidPermissionDelegate, DisplayAndroidO
window.setAttributes(params); window.setAttributes(params);
} }
@CalledByNative
private boolean applyDisableSurfaceControlWorkaround() {
return mDisplayAndroid.applyDisableSurfaceControlWorkaround();
}
@SuppressLint("NewApi") @SuppressLint("NewApi")
// mSupportedRefreshRateModes should only be set if Display.Mode is available. // mSupportedRefreshRateModes should only be set if Display.Mode is available.
@TargetApi(Build.VERSION_CODES.M) @TargetApi(Build.VERSION_CODES.M)
......
...@@ -20,7 +20,7 @@ import java.util.WeakHashMap; ...@@ -20,7 +20,7 @@ import java.util.WeakHashMap;
* anywhere, as long as the corresponding WindowAndroids are destroyed. The observers are * anywhere, as long as the corresponding WindowAndroids are destroyed. The observers are
* held weakly so to not lead to leaks. * held weakly so to not lead to leaks.
*/ */
public class DisplayAndroid { public abstract class DisplayAndroid {
/** /**
* DisplayAndroidObserver interface for changes to this Display. * DisplayAndroidObserver interface for changes to this Display.
*/ */
...@@ -254,6 +254,8 @@ public class DisplayAndroid { ...@@ -254,6 +254,8 @@ public class DisplayAndroid {
update(null, null, null, null, null, null, isDisplayServerWideColorGamut, null, null, null); update(null, null, null, null, null, null, isDisplayServerWideColorGamut, null, null, null);
} }
public abstract boolean applyDisableSurfaceControlWorkaround();
/** /**
* Update the display to the provided parameters. Null values leave the parameter unchanged. * Update the display to the provided parameters. Null values leave the parameter unchanged.
*/ */
......
...@@ -23,12 +23,16 @@ import java.util.List; ...@@ -23,12 +23,16 @@ import java.util.List;
*/ */
/* package */ class PhysicalDisplayAndroid extends DisplayAndroid { /* package */ class PhysicalDisplayAndroid extends DisplayAndroid {
private static final String TAG = "DisplayAndroid"; private static final String TAG = "DisplayAndroid";
private static final String SAMSUNG_DEX_DISPLAY = "Desktop";
// When this object exists, a positive value means that the forced DIP scale is set and // When this object exists, a positive value means that the forced DIP scale is set and
// the zero means it is not. The non existing object (i.e. null reference) means that // the zero means it is not. The non existing object (i.e. null reference) means that
// the existence and value of the forced DIP scale has not yet been determined. // the existence and value of the forced DIP scale has not yet been determined.
private static Float sForcedDIPScale; private static Float sForcedDIPScale;
// This is a workaround for crbug.com/1042581.
private final boolean mDisableSurfaceControlWorkaround;
private static boolean hasForcedDIPScale() { private static boolean hasForcedDIPScale() {
if (sForcedDIPScale == null) { if (sForcedDIPScale == null) {
String forcedScaleAsString = CommandLine.getInstance().getSwitchValue( String forcedScaleAsString = CommandLine.getInstance().getSwitchValue(
...@@ -123,6 +127,7 @@ import java.util.List; ...@@ -123,6 +127,7 @@ import java.util.List;
/* package */ PhysicalDisplayAndroid(Display display) { /* package */ PhysicalDisplayAndroid(Display display) {
super(display.getDisplayId()); super(display.getDisplayId());
mDisableSurfaceControlWorkaround = display.getName().equals(SAMSUNG_DEX_DISPLAY);
} }
@SuppressWarnings("deprecation") @SuppressWarnings("deprecation")
...@@ -164,4 +169,9 @@ import java.util.List; ...@@ -164,4 +169,9 @@ import java.util.List;
bitsPerComponent(pixelFormatId), display.getRotation(), isWideColorGamut, null, bitsPerComponent(pixelFormatId), display.getRotation(), isWideColorGamut, null,
display.getRefreshRate(), currentMode, supportedModes); display.getRefreshRate(), currentMode, supportedModes);
} }
@Override
public boolean applyDisableSurfaceControlWorkaround() {
return mDisableSurfaceControlWorkaround;
}
} }
...@@ -57,4 +57,9 @@ public class VirtualDisplayAndroid extends DisplayAndroid { ...@@ -57,4 +57,9 @@ public class VirtualDisplayAndroid extends DisplayAndroid {
public void destroy() { public void destroy() {
getManager().removeVirtualDisplay(this); getManager().removeVirtualDisplay(this);
} }
@Override
public boolean applyDisableSurfaceControlWorkaround() {
return false;
}
} }
...@@ -243,6 +243,12 @@ void WindowAndroid::Force60HzRefreshRateIfNeeded() { ...@@ -243,6 +243,12 @@ void WindowAndroid::Force60HzRefreshRateIfNeeded() {
Java_WindowAndroid_setPreferredRefreshRate(env, GetJavaObject(), 60.f); Java_WindowAndroid_setPreferredRefreshRate(env, GetJavaObject(), 60.f);
} }
bool WindowAndroid::ApplyDisableSurfaceControlWorkaround() {
JNIEnv* env = AttachCurrentThread();
return Java_WindowAndroid_applyDisableSurfaceControlWorkaround(
env, GetJavaObject());
}
bool WindowAndroid::HasPermission(const std::string& permission) { bool WindowAndroid::HasPermission(const std::string& permission) {
JNIEnv* env = AttachCurrentThread(); JNIEnv* env = AttachCurrentThread();
return Java_WindowAndroid_hasPermission( return Java_WindowAndroid_hasPermission(
......
...@@ -106,6 +106,8 @@ class UI_ANDROID_EXPORT WindowAndroid : public ViewAndroid { ...@@ -106,6 +106,8 @@ class UI_ANDROID_EXPORT WindowAndroid : public ViewAndroid {
void SetForce60HzRefreshRate(); void SetForce60HzRefreshRate();
bool ApplyDisableSurfaceControlWorkaround();
class TestHooks { class TestHooks {
public: public:
virtual ~TestHooks() = default; virtual ~TestHooks() = default;
......
...@@ -305,14 +305,7 @@ void OnTransactionCompletedOnAnyThread(void* context, ...@@ -305,14 +305,7 @@ void OnTransactionCompletedOnAnyThread(void* context,
// static // static
bool SurfaceControl::IsSupported() { bool SurfaceControl::IsSupported() {
const auto* build_info = base::android::BuildInfo::GetInstance(); if (!base::android::BuildInfo::GetInstance()->is_at_least_q())
// Disabled on Samsung devices due to a platform bug fixed in R.
int min_sdk_version = base::android::SDK_VERSION_Q;
if (base::EqualsCaseInsensitiveASCII(build_info->manufacturer(), "samsung"))
min_sdk_version = base::android::SDK_VERSION_R;
if (build_info->sdk_int() < min_sdk_version)
return false; return false;
CHECK(SurfaceControlMethods::Get().supported); CHECK(SurfaceControlMethods::Get().supported);
......
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