Commit 9dff500a authored by Khushal's avatar Khushal Committed by Chromium LUCI CQ

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

This reverts commit 158b87d2.

Reason for revert: Fix for crbug.com/1148995 which caused a crash in the old overlay path has landed.

Original change's description:
> 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: Khushal <khushalsagar@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#827533}

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: I3f40ae5ee1142d061aa2e4ae18d22b9019a28878
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2564943Reviewed-by: default avatarKhushal <khushalsagar@chromium.org>
Commit-Queue: Khushal <khushalsagar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#831900}
parent 1bf6fe2f
...@@ -368,9 +368,6 @@ void CompositorImpl::SetRootLayer(scoped_refptr<cc::Layer> root_layer) { ...@@ -368,9 +368,6 @@ 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,11 +978,6 @@ public class WindowAndroid implements AndroidPermissionDelegate, DisplayAndroidO ...@@ -978,11 +978,6 @@ 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 abstract class DisplayAndroid { public class DisplayAndroid {
/** /**
* DisplayAndroidObserver interface for changes to this Display. * DisplayAndroidObserver interface for changes to this Display.
*/ */
...@@ -254,8 +254,6 @@ public abstract class DisplayAndroid { ...@@ -254,8 +254,6 @@ public abstract 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,16 +23,12 @@ import java.util.List; ...@@ -23,16 +23,12 @@ 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(
...@@ -127,7 +123,6 @@ import java.util.List; ...@@ -127,7 +123,6 @@ 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")
...@@ -169,9 +164,4 @@ import java.util.List; ...@@ -169,9 +164,4 @@ 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,9 +57,4 @@ public class VirtualDisplayAndroid extends DisplayAndroid { ...@@ -57,9 +57,4 @@ public class VirtualDisplayAndroid extends DisplayAndroid {
public void destroy() { public void destroy() {
getManager().removeVirtualDisplay(this); getManager().removeVirtualDisplay(this);
} }
@Override
public boolean applyDisableSurfaceControlWorkaround() {
return false;
}
} }
...@@ -243,12 +243,6 @@ void WindowAndroid::Force60HzRefreshRateIfNeeded() { ...@@ -243,12 +243,6 @@ 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,8 +106,6 @@ class UI_ANDROID_EXPORT WindowAndroid : public ViewAndroid { ...@@ -106,8 +106,6 @@ 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;
......
...@@ -316,7 +316,14 @@ void OnTransactionCompletedOnAnyThread(void* context, ...@@ -316,7 +316,14 @@ void OnTransactionCompletedOnAnyThread(void* context,
// static // static
bool SurfaceControl::IsSupported() { bool SurfaceControl::IsSupported() {
if (!base::android::BuildInfo::GetInstance()->is_at_least_q()) const auto* build_info = base::android::BuildInfo::GetInstance();
// 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