From c86397fddfd743ae0f7896982039539d60d9ebef Mon Sep 17 00:00:00 2001 From: John Reck Date: Thu, 26 Jan 2023 12:02:42 -0500 Subject: [PATCH] Add Display#getHdrSdrRatio Bug: 242764315 Test: Logs in SilkFX; verified onDisplayChanged is not firing when this situation happens Change-Id: I26a081e07fb4bb2d021c47db4258c0ce0c4f6f8f --- .../hardware/display/DisplayManager.java | 16 ++- .../display/DisplayManagerGlobal.java | 72 +++++++--- core/java/android/view/Display.java | 129 ++++++++++++++++++ core/java/android/view/DisplayInfo.java | 21 ++- .../server/display/DisplayDeviceConfig.java | 4 +- .../server/display/DisplayDeviceInfo.java | 18 +++ .../display/DisplayDeviceRepository.java | 21 ++- .../server/display/DisplayManagerService.java | 10 ++ .../server/display/LocalDisplayAdapter.java | 32 ++++- .../server/display/LogicalDisplay.java | 1 + .../server/display/LogicalDisplayMapper.java | 38 ++++-- .../display/LocalDisplayAdapterTest.java | 56 ++++++-- .../test/silkfx/common/ColorModeControls.kt | 35 +++++ 13 files changed, 398 insertions(+), 55 deletions(-) diff --git a/core/java/android/hardware/display/DisplayManager.java b/core/java/android/hardware/display/DisplayManager.java index d49cc4438dd44..65e5802de5aa3 100644 --- a/core/java/android/hardware/display/DisplayManager.java +++ b/core/java/android/hardware/display/DisplayManager.java @@ -540,7 +540,8 @@ public final class DisplayManager { EVENT_FLAG_DISPLAY_ADDED, EVENT_FLAG_DISPLAY_CHANGED, EVENT_FLAG_DISPLAY_REMOVED, - EVENT_FLAG_DISPLAY_BRIGHTNESS + EVENT_FLAG_DISPLAY_BRIGHTNESS, + EVENT_FLAG_HDR_SDR_RATIO_CHANGED }) @Retention(RetentionPolicy.SOURCE) public @interface EventsMask {} @@ -583,6 +584,19 @@ public final class DisplayManager { */ public static final long EVENT_FLAG_DISPLAY_BRIGHTNESS = 1L << 3; + /** + * Event flag to register for a display's hdr/sdr ratio changes. This notification is sent + * through the {@link DisplayListener#onDisplayChanged} callback method. New hdr/sdr + * values can be retrieved via {@link Display#getHdrSdrRatio()}. + * + * Requires that {@link Display#isHdrSdrRatioAvailable()} is true. + * + * @see #registerDisplayListener(DisplayListener, Handler, long) + * + * @hide + */ + public static final long EVENT_FLAG_HDR_SDR_RATIO_CHANGED = 1L << 4; + /** @hide */ public DisplayManager(Context context) { mContext = context; diff --git a/core/java/android/hardware/display/DisplayManagerGlobal.java b/core/java/android/hardware/display/DisplayManagerGlobal.java index d9db177b4c18f..f269feb2afd12 100644 --- a/core/java/android/hardware/display/DisplayManagerGlobal.java +++ b/core/java/android/hardware/display/DisplayManagerGlobal.java @@ -38,6 +38,7 @@ import android.hardware.graphics.common.DisplayDecorationSupport; import android.media.projection.IMediaProjection; import android.media.projection.MediaProjection; import android.os.Handler; +import android.os.HandlerExecutor; import android.os.IBinder; import android.os.Looper; import android.os.Message; @@ -56,11 +57,12 @@ import com.android.internal.annotations.VisibleForTesting; import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; -import java.util.ArrayList; import java.util.Collections; import java.util.List; import java.util.Objects; +import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.Executor; +import java.util.concurrent.atomic.AtomicLong; /** * Manager communication with the display manager service on behalf of @@ -82,11 +84,12 @@ public final class DisplayManagerGlobal { // orientation change before the display info cache has actually been invalidated. private static final boolean USE_CACHE = false; - @IntDef(prefix = {"SWITCHING_TYPE_"}, value = { + @IntDef(prefix = {"EVENT_DISPLAY_"}, flag = true, value = { EVENT_DISPLAY_ADDED, EVENT_DISPLAY_CHANGED, EVENT_DISPLAY_REMOVED, - EVENT_DISPLAY_BRIGHTNESS_CHANGED + EVENT_DISPLAY_BRIGHTNESS_CHANGED, + EVENT_DISPLAY_HDR_SDR_RATIO_CHANGED, }) @Retention(RetentionPolicy.SOURCE) public @interface DisplayEvent {} @@ -95,6 +98,7 @@ public final class DisplayManagerGlobal { public static final int EVENT_DISPLAY_CHANGED = 2; public static final int EVENT_DISPLAY_REMOVED = 3; public static final int EVENT_DISPLAY_BRIGHTNESS_CHANGED = 4; + public static final int EVENT_DISPLAY_HDR_SDR_RATIO_CHANGED = 5; @UnsupportedAppUsage private static DisplayManagerGlobal sInstance; @@ -109,7 +113,8 @@ public final class DisplayManagerGlobal { private DisplayManagerCallback mCallback; private @EventsMask long mRegisteredEventsMask = 0; - private final ArrayList mDisplayListeners = new ArrayList<>(); + private final CopyOnWriteArrayList mDisplayListeners = + new CopyOnWriteArrayList<>(); private final SparseArray mDisplayInfoCache = new SparseArray<>(); private final ColorSpace mWideColorSpace; @@ -315,6 +320,19 @@ public final class DisplayManagerGlobal { */ public void registerDisplayListener(@NonNull DisplayListener listener, @Nullable Handler handler, @EventsMask long eventsMask) { + Looper looper = getLooperForHandler(handler); + Handler springBoard = new Handler(looper); + registerDisplayListener(listener, new HandlerExecutor(springBoard), eventsMask); + } + + /** + * Register a listener for display-related changes. + * + * @param listener The listener that will be called when display changes occur. + * @param executor Executor for the thread that will be receiving the callbacks. Cannot be null. + */ + public void registerDisplayListener(@NonNull DisplayListener listener, + @NonNull Executor executor, @EventsMask long eventsMask) { if (listener == null) { throw new IllegalArgumentException("listener must not be null"); } @@ -326,8 +344,7 @@ public final class DisplayManagerGlobal { synchronized (mLock) { int index = findDisplayListenerLocked(listener); if (index < 0) { - Looper looper = getLooperForHandler(handler); - mDisplayListeners.add(new DisplayListenerDelegate(listener, looper, eventsMask)); + mDisplayListeners.add(new DisplayListenerDelegate(listener, executor, eventsMask)); registerCallbackIfNeededLocked(); } else { mDisplayListeners.get(index).setEventsMask(eventsMask); @@ -408,6 +425,7 @@ public final class DisplayManagerGlobal { } private void handleDisplayEvent(int displayId, @DisplayEvent int event) { + final DisplayInfo info; synchronized (mLock) { if (USE_CACHE) { mDisplayInfoCache.remove(displayId); @@ -417,11 +435,7 @@ public final class DisplayManagerGlobal { } } - final int numListeners = mDisplayListeners.size(); - DisplayInfo info = getDisplayInfo(displayId); - for (int i = 0; i < numListeners; i++) { - mDisplayListeners.get(i).sendDisplayEvent(displayId, event, info); - } + info = getDisplayInfoLocked(displayId); if (event == EVENT_DISPLAY_CHANGED && mDispatchNativeCallbacks) { // Choreographer only supports a single display, so only dispatch refresh rate // changes for the default display. @@ -438,6 +452,11 @@ public final class DisplayManagerGlobal { } } } + // Accepting an Executor means the listener may be synchronously invoked, so we must + // not be holding mLock when we do so + for (DisplayListenerDelegate listener : mDisplayListeners) { + listener.sendDisplayEvent(displayId, event, info); + } } public void startWifiDisplayScan() { @@ -1075,34 +1094,42 @@ public final class DisplayManagerGlobal { } } - private static final class DisplayListenerDelegate extends Handler { + private static final class DisplayListenerDelegate { public final DisplayListener mListener; public volatile long mEventsMask; private final DisplayInfo mDisplayInfo = new DisplayInfo(); + private final Executor mExecutor; + private AtomicLong mGenerationId = new AtomicLong(1); - DisplayListenerDelegate(DisplayListener listener, @NonNull Looper looper, + DisplayListenerDelegate(DisplayListener listener, @NonNull Executor executor, @EventsMask long eventsMask) { - super(looper, null, true /*async*/); + mExecutor = executor; mListener = listener; mEventsMask = eventsMask; } public void sendDisplayEvent(int displayId, @DisplayEvent int event, DisplayInfo info) { - Message msg = obtainMessage(event, displayId, 0, info); - sendMessage(msg); + long generationId = mGenerationId.get(); + Message msg = Message.obtain(null, event, displayId, 0, info); + mExecutor.execute(() -> { + // If the generation id's don't match we were canceled but still need to recycle() + if (generationId == mGenerationId.get()) { + handleMessage(msg); + } + msg.recycle(); + }); } public void clearEvents() { - removeCallbacksAndMessages(null); + mGenerationId.incrementAndGet(); } public void setEventsMask(@EventsMask long newEventsMask) { mEventsMask = newEventsMask; } - @Override - public void handleMessage(Message msg) { + private void handleMessage(Message msg) { if (DEBUG) { Trace.beginSection( "DisplayListenerDelegate(" + eventToString(msg.what) @@ -1134,6 +1161,11 @@ public final class DisplayManagerGlobal { mListener.onDisplayRemoved(msg.arg1); } break; + case EVENT_DISPLAY_HDR_SDR_RATIO_CHANGED: + if ((mEventsMask & DisplayManager.EVENT_FLAG_HDR_SDR_RATIO_CHANGED) != 0) { + mListener.onDisplayChanged(msg.arg1); + } + break; } if (DEBUG) { Trace.endSection(); @@ -1255,6 +1287,8 @@ public final class DisplayManagerGlobal { return "REMOVED"; case EVENT_DISPLAY_BRIGHTNESS_CHANGED: return "BRIGHTNESS_CHANGED"; + case EVENT_DISPLAY_HDR_SDR_RATIO_CHANGED: + return "HDR_SDR_RATIO_CHANGED"; } return "UNKNOWN"; } diff --git a/core/java/android/view/Display.java b/core/java/android/view/Display.java index 6b1499feb59ca..1563fc0d5075e 100644 --- a/core/java/android/view/Display.java +++ b/core/java/android/view/Display.java @@ -56,6 +56,8 @@ import java.lang.annotation.RetentionPolicy; import java.util.ArrayList; import java.util.Arrays; import java.util.List; +import java.util.concurrent.Executor; +import java.util.function.Consumer; /** * Provides information about the size and density of a logical display. @@ -111,6 +113,8 @@ public final class Display { private int mCachedAppWidthCompat; private int mCachedAppHeightCompat; + private ArrayList mHdrSdrRatioListeners = new ArrayList<>(); + /** * The default Display id, which is the id of the primary display assuming there is one. */ @@ -1291,6 +1295,102 @@ public final class Display { } } + /** + * @return Whether the display supports reporting an hdr/sdr ratio. If this is false, + * {@link #getHdrSdrRatio()} will always be 1.0f + * @hide + * TODO: make public + */ + public boolean isHdrSdrRatioAvailable() { + synchronized (mLock) { + updateDisplayInfoLocked(); + return !Float.isNaN(mDisplayInfo.hdrSdrRatio); + } + } + + /** + * @return The current hdr/sdr ratio expressed as the ratio of targetHdrPeakBrightnessInNits / + * targetSdrWhitePointInNits. If {@link #isHdrSdrRatioAvailable()} is false, this + * always returns 1.0f. + * + * @hide + * TODO: make public + */ + public float getHdrSdrRatio() { + synchronized (mLock) { + updateDisplayInfoLocked(); + return Float.isNaN(mDisplayInfo.hdrSdrRatio) + ? 1.0f : mDisplayInfo.hdrSdrRatio; + } + } + + private int findHdrSdrRatioListenerLocked(Consumer listener) { + for (int i = 0; i < mHdrSdrRatioListeners.size(); i++) { + final HdrSdrRatioListenerWrapper wrapper = mHdrSdrRatioListeners.get(i); + if (wrapper.mListener == listener) { + return i; + } + } + return -1; + } + + /** + * Registers a listener that will be invoked whenever the display's hdr/sdr ratio has changed. + * After receiving the callback on the specified Executor, call {@link #getHdrSdrRatio()} to + * get the updated value. + * If {@link #isHdrSdrRatioAvailable()} is false, then an IllegalStateException will be thrown + * + * @see #unregisterHdrSdrRatioChangedListener(Consumer) + * @param executor The executor to invoke the listener on + * @param listener The listener to invoke when the HDR/SDR ratio changes + * @throws IllegalStateException if {@link #isHdrSdrRatioAvailable()} is false + * @hide + * TODO: Make public + */ + public void registerHdrSdrRatioChangedListener(@NonNull Executor executor, + @NonNull Consumer listener) { + if (!isHdrSdrRatioAvailable()) { + throw new IllegalStateException("HDR/SDR ratio changed not available"); + } + HdrSdrRatioListenerWrapper toRegister = null; + synchronized (mLock) { + if (findHdrSdrRatioListenerLocked(listener) == -1) { + toRegister = new HdrSdrRatioListenerWrapper(listener); + mHdrSdrRatioListeners.add(toRegister); + } // else already listening, don't do anything + } + if (toRegister != null) { + // Although we only care about the HDR/SDR ratio changing, that can also come in the + // form of the larger DISPLAY_CHANGED event + mGlobal.registerDisplayListener(toRegister, executor, + DisplayManager.EVENT_FLAG_HDR_SDR_RATIO_CHANGED + | DisplayManagerGlobal.EVENT_DISPLAY_CHANGED); + } + + } + + /** + * @param listener The previously + * {@link #registerHdrSdrRatioChangedListener(Executor, Consumer) registered} + * hdr/sdr ratio listener to remove. + * + * @see #registerHdrSdrRatioChangedListener(Executor, Consumer) + * @hide + * TODO: Make public + */ + public void unregisterHdrSdrRatioChangedListener(Consumer listener) { + HdrSdrRatioListenerWrapper toRemove = null; + synchronized (mLock) { + int index = findHdrSdrRatioListenerLocked(listener); + if (index != -1) { + toRemove = mHdrSdrRatioListeners.remove(index); + } + } + if (toRemove != null) { + mGlobal.unregisterDisplayListener(toRemove); + } + } + /** * Sets the default {@link Display.Mode} to use for the display. The display mode includes * preference for resolution and refresh rate. @@ -2528,4 +2628,33 @@ public final class Display { } } } + + private class HdrSdrRatioListenerWrapper implements DisplayManager.DisplayListener { + Consumer mListener; + float mLastReportedRatio = 1.f; + + private HdrSdrRatioListenerWrapper(Consumer listener) { + mListener = listener; + } + + @Override + public void onDisplayAdded(int displayId) { + // don't care + } + + @Override + public void onDisplayRemoved(int displayId) { + // don't care + } + + @Override + public void onDisplayChanged(int displayId) { + if (displayId == getDisplayId()) { + float newRatio = getHdrSdrRatio(); + if (newRatio != mLastReportedRatio) { + mListener.accept(Display.this); + } + } + } + } } diff --git a/core/java/android/view/DisplayInfo.java b/core/java/android/view/DisplayInfo.java index 3a02c481c6406..03689188a9b31 100644 --- a/core/java/android/view/DisplayInfo.java +++ b/core/java/android/view/DisplayInfo.java @@ -39,6 +39,8 @@ import android.util.ArraySet; import android.util.DisplayMetrics; import android.util.proto.ProtoOutputStream; +import com.android.internal.display.BrightnessSynchronizer; + import java.util.Arrays; import java.util.Objects; @@ -340,6 +342,13 @@ public final class DisplayInfo implements Parcelable { @Nullable public SurfaceControl.RefreshRateRange layoutLimitedRefreshRate; + /** + * The current hdr/sdr ratio for the display. If the display doesn't support hdr/sdr ratio + * queries then this is NaN + */ + public float hdrSdrRatio = Float.NaN; + + public static final @android.annotation.NonNull Creator CREATOR = new Creator() { @Override public DisplayInfo createFromParcel(Parcel source) { @@ -415,7 +424,8 @@ public final class DisplayInfo implements Parcelable { && Objects.equals(roundedCorners, other.roundedCorners) && installOrientation == other.installOrientation && Objects.equals(displayShape, other.displayShape) - && Objects.equals(layoutLimitedRefreshRate, other.layoutLimitedRefreshRate); + && Objects.equals(layoutLimitedRefreshRate, other.layoutLimitedRefreshRate) + && BrightnessSynchronizer.floatEquals(hdrSdrRatio, other.hdrSdrRatio); } @Override @@ -471,6 +481,7 @@ public final class DisplayInfo implements Parcelable { installOrientation = other.installOrientation; displayShape = other.displayShape; layoutLimitedRefreshRate = other.layoutLimitedRefreshRate; + hdrSdrRatio = other.hdrSdrRatio; } public void readFromParcel(Parcel source) { @@ -532,6 +543,7 @@ public final class DisplayInfo implements Parcelable { installOrientation = source.readInt(); displayShape = source.readTypedObject(DisplayShape.CREATOR); layoutLimitedRefreshRate = source.readTypedObject(SurfaceControl.RefreshRateRange.CREATOR); + hdrSdrRatio = source.readFloat(); } @Override @@ -591,6 +603,7 @@ public final class DisplayInfo implements Parcelable { dest.writeInt(installOrientation); dest.writeTypedObject(displayShape, flags); dest.writeTypedObject(layoutLimitedRefreshRate, flags); + dest.writeFloat(hdrSdrRatio); } @Override @@ -852,6 +865,12 @@ public final class DisplayInfo implements Parcelable { sb.append(Surface.rotationToString(installOrientation)); sb.append(", layoutLimitedRefreshRate "); sb.append(layoutLimitedRefreshRate); + sb.append(", hdrSdrRatio "); + if (Float.isNaN(hdrSdrRatio)) { + sb.append("not_available"); + } else { + sb.append(hdrSdrRatio); + } sb.append("}"); return sb.toString(); } diff --git a/services/core/java/com/android/server/display/DisplayDeviceConfig.java b/services/core/java/com/android/server/display/DisplayDeviceConfig.java index a107f33b52441..2e17b5c4572de 100644 --- a/services/core/java/com/android/server/display/DisplayDeviceConfig.java +++ b/services/core/java/com/android/server/display/DisplayDeviceConfig.java @@ -447,7 +447,7 @@ public class DisplayDeviceConfig { // so -2 is used instead private static final float INVALID_BRIGHTNESS_IN_CONFIG = -2f; - private static final float NITS_INVALID = -1; + static final float NITS_INVALID = -1; // Length of the ambient light horizon used to calculate the long term estimate of ambient // light. @@ -828,7 +828,7 @@ public class DisplayDeviceConfig { /** * Calculates the nits value for the specified backlight value if a mapping exists. * - * @return The mapped nits or 0 if no mapping exits. + * @return The mapped nits or {@link #NITS_INVALID} if no mapping exits. */ public float getNitsFromBacklight(float backlight) { if (mBacklightToNitsSpline == null) { diff --git a/services/core/java/com/android/server/display/DisplayDeviceInfo.java b/services/core/java/com/android/server/display/DisplayDeviceInfo.java index b7b7031d2c4d9..213ee646fc34f 100644 --- a/services/core/java/com/android/server/display/DisplayDeviceInfo.java +++ b/services/core/java/com/android/server/display/DisplayDeviceInfo.java @@ -225,6 +225,16 @@ final class DisplayDeviceInfo { */ public static final int DIFF_COLOR_MODE = 1 << 2; + /** + * Diff result: The hdr/sdr ratio differs + */ + public static final int DIFF_HDR_SDR_RATIO = 1 << 3; + + /** + * Diff result: Catch-all for "everything changed" + */ + public static final int DIFF_EVERYTHING = 0XFFFFFFFF; + /** * Gets the name of the display device, which may be derived from EDID or * other sources. The name may be localized and displayed to the user. @@ -414,6 +424,9 @@ final class DisplayDeviceInfo { public float brightnessMaximum; public float brightnessDefault; + // NaN means unsupported + public float hdrSdrRatio = Float.NaN; + /** * Install orientation of display panel relative to its natural orientation. */ @@ -449,6 +462,9 @@ final class DisplayDeviceInfo { if (colorMode != other.colorMode) { diff |= DIFF_COLOR_MODE; } + if (!BrightnessSynchronizer.floatEquals(hdrSdrRatio, other.hdrSdrRatio)) { + diff |= DIFF_HDR_SDR_RATIO; + } if (!Objects.equals(name, other.name) || !Objects.equals(uniqueId, other.uniqueId) || width != other.width @@ -527,6 +543,7 @@ final class DisplayDeviceInfo { brightnessMinimum = other.brightnessMinimum; brightnessMaximum = other.brightnessMaximum; brightnessDefault = other.brightnessDefault; + hdrSdrRatio = other.hdrSdrRatio; roundedCorners = other.roundedCorners; installOrientation = other.installOrientation; displayShape = other.displayShape; @@ -575,6 +592,7 @@ final class DisplayDeviceInfo { sb.append(", brightnessMinimum ").append(brightnessMinimum); sb.append(", brightnessMaximum ").append(brightnessMaximum); sb.append(", brightnessDefault ").append(brightnessDefault); + sb.append(", hdrSdrRatio ").append(hdrSdrRatio); if (roundedCorners != null) { sb.append(", roundedCorners ").append(roundedCorners); } diff --git a/services/core/java/com/android/server/display/DisplayDeviceRepository.java b/services/core/java/com/android/server/display/DisplayDeviceRepository.java index 33a63a9907c2b..ea52a3d4e45c6 100644 --- a/services/core/java/com/android/server/display/DisplayDeviceRepository.java +++ b/services/core/java/com/android/server/display/DisplayDeviceRepository.java @@ -42,7 +42,6 @@ class DisplayDeviceRepository implements DisplayAdapter.Listener { private static final Boolean DEBUG = false; public static final int DISPLAY_DEVICE_EVENT_ADDED = 1; - public static final int DISPLAY_DEVICE_EVENT_CHANGED = 2; public static final int DISPLAY_DEVICE_EVENT_REMOVED = 3; /** @@ -83,15 +82,15 @@ class DisplayDeviceRepository implements DisplayAdapter.Listener { Trace.beginAsyncSection(tag, 0); } switch (event) { - case DISPLAY_DEVICE_EVENT_ADDED: + case DisplayAdapter.DISPLAY_DEVICE_EVENT_ADDED: handleDisplayDeviceAdded(device); break; - case DISPLAY_DEVICE_EVENT_CHANGED: + case DisplayAdapter.DISPLAY_DEVICE_EVENT_CHANGED: handleDisplayDeviceChanged(device); break; - case DISPLAY_DEVICE_EVENT_REMOVED: + case DisplayAdapter.DISPLAY_DEVICE_EVENT_REMOVED: handleDisplayDeviceRemoved(device); break; } @@ -174,7 +173,7 @@ class DisplayDeviceRepository implements DisplayAdapter.Listener { if (diff == DisplayDeviceInfo.DIFF_STATE) { Slog.i(TAG, "Display device changed state: \"" + info.name + "\", " + Display.stateToString(info.state)); - } else if (diff != 0) { + } else if (diff != DisplayDeviceInfo.DIFF_HDR_SDR_RATIO) { Slog.i(TAG, "Display device changed: " + info); } @@ -188,7 +187,7 @@ class DisplayDeviceRepository implements DisplayAdapter.Listener { device.mDebugLastLoggedDeviceInfo = info; device.applyPendingDisplayDeviceInfoChangesLocked(); - sendEventLocked(device, DISPLAY_DEVICE_EVENT_CHANGED); + sendChangedEventLocked(device, diff); if (DEBUG) { Trace.traceEnd(Trace.TRACE_TAG_POWER); } @@ -216,12 +215,22 @@ class DisplayDeviceRepository implements DisplayAdapter.Listener { } } + @GuardedBy("mSyncRoot") + private void sendChangedEventLocked(DisplayDevice device, int diff) { + final int size = mListeners.size(); + for (int i = 0; i < size; i++) { + mListeners.get(i).onDisplayDeviceChangedLocked(device, diff); + } + } + /** * Listens to {@link DisplayDevice} events from {@link DisplayDeviceRepository}. */ public interface Listener { void onDisplayDeviceEventLocked(DisplayDevice device, int event); + void onDisplayDeviceChangedLocked(DisplayDevice device, int diff); + // TODO: multi-display - Try to remove the need for requestTraversal...it feels like // a shoe-horned method for a shoe-horned feature. void onTraversalRequested(); diff --git a/services/core/java/com/android/server/display/DisplayManagerService.java b/services/core/java/com/android/server/display/DisplayManagerService.java index 237e78bbb58a2..6e16f9fc413fc 100644 --- a/services/core/java/com/android/server/display/DisplayManagerService.java +++ b/services/core/java/com/android/server/display/DisplayManagerService.java @@ -1745,6 +1745,10 @@ public final class DisplayManagerService extends SystemService { mHandler.sendEmptyMessage(MSG_LOAD_BRIGHTNESS_CONFIGURATIONS); } + private void handleLogicalDisplayHdrSdrRatioChangedLocked(@NonNull LogicalDisplay display) { + sendDisplayEventLocked(display, DisplayManagerGlobal.EVENT_DISPLAY_HDR_SDR_RATIO_CHANGED); + } + private void notifyDefaultDisplayDeviceUpdated(LogicalDisplay display) { mDisplayModeDirector.defaultDisplayDeviceUpdated(display.getPrimaryDisplayDeviceLocked() .mDisplayDeviceConfig); @@ -2983,6 +2987,10 @@ public final class DisplayManagerService extends SystemService { case LogicalDisplayMapper.LOGICAL_DISPLAY_EVENT_DEVICE_STATE_TRANSITION: handleLogicalDisplayDeviceStateTransitionLocked(display); break; + + case LogicalDisplayMapper.LOGICAL_DISPLAY_EVENT_HDR_SDR_RATIO_CHANGED: + handleLogicalDisplayHdrSdrRatioChangedLocked(display); + break; } } @@ -3052,6 +3060,8 @@ public final class DisplayManagerService extends SystemService { return (mask & DisplayManager.EVENT_FLAG_DISPLAY_BRIGHTNESS) != 0; case DisplayManagerGlobal.EVENT_DISPLAY_REMOVED: return (mask & DisplayManager.EVENT_FLAG_DISPLAY_REMOVED) != 0; + case DisplayManagerGlobal.EVENT_DISPLAY_HDR_SDR_RATIO_CHANGED: + return (mask & DisplayManager.EVENT_FLAG_HDR_SDR_RATIO_CHANGED) != 0; default: // This should never happen. Slog.e(TAG, "Unknown display event " + event); diff --git a/services/core/java/com/android/server/display/LocalDisplayAdapter.java b/services/core/java/com/android/server/display/LocalDisplayAdapter.java index be5980bdfffd7..8f52c97c1f877 100644 --- a/services/core/java/com/android/server/display/LocalDisplayAdapter.java +++ b/services/core/java/com/android/server/display/LocalDisplayAdapter.java @@ -204,6 +204,7 @@ final class LocalDisplayAdapter extends DisplayAdapter { // This is only set in the runnable returned from requestDisplayStateLocked. private float mBrightnessState = PowerManager.BRIGHTNESS_INVALID_FLOAT; private float mSdrBrightnessState = PowerManager.BRIGHTNESS_INVALID_FLOAT; + private float mCurrentHdrSdrRatio = Float.NaN; private int mDefaultModeId = INVALID_MODE_ID; private int mSystemPreferredModeId = INVALID_MODE_ID; private int mDefaultModeGroup; @@ -729,6 +730,7 @@ final class LocalDisplayAdapter extends DisplayAdapter { mInfo.brightnessMinimum = PowerManager.BRIGHTNESS_MIN; mInfo.brightnessMaximum = PowerManager.BRIGHTNESS_MAX; mInfo.brightnessDefault = getDisplayDeviceConfig().getBrightnessDefault(); + mInfo.hdrSdrRatio = mCurrentHdrSdrRatio; } return mInfo; } @@ -840,12 +842,10 @@ final class LocalDisplayAdapter extends DisplayAdapter { private void setCommittedState(int state) { // After the display state is set, let's update the committed state. - getHandler().post(() -> { - synchronized (getSyncRoot()) { - mCommittedState = state; - updateDeviceInfoLocked(); - } - }); + synchronized (getSyncRoot()) { + mCommittedState = state; + updateDeviceInfoLocked(); + } } private void setDisplayBrightness(float brightnessState, @@ -881,6 +881,9 @@ final class LocalDisplayAdapter extends DisplayAdapter { "SdrScreenBrightness", BrightnessSynchronizer.brightnessFloatToInt( sdrBrightnessState)); + + handleHdrSdrNitsChanged(nits, sdrNits); + } finally { Trace.traceEnd(Trace.TRACE_TAG_POWER); } @@ -897,6 +900,23 @@ final class LocalDisplayAdapter extends DisplayAdapter { private float backlightToNits(float backlight) { return getDisplayDeviceConfig().getNitsFromBacklight(backlight); } + + void handleHdrSdrNitsChanged(float displayNits, float sdrNits) { + final float newHdrSdrRatio; + if (displayNits != DisplayDeviceConfig.NITS_INVALID + && sdrNits != DisplayDeviceConfig.NITS_INVALID) { + newHdrSdrRatio = displayNits / sdrNits; + } else { + newHdrSdrRatio = Float.NaN; + } + if (!BrightnessSynchronizer.floatEquals( + mCurrentHdrSdrRatio, newHdrSdrRatio)) { + synchronized (getSyncRoot()) { + mCurrentHdrSdrRatio = newHdrSdrRatio; + updateDeviceInfoLocked(); + } + } + } }; } return null; diff --git a/services/core/java/com/android/server/display/LogicalDisplay.java b/services/core/java/com/android/server/display/LogicalDisplay.java index 1086c5565227e..64eb95b0f421f 100644 --- a/services/core/java/com/android/server/display/LogicalDisplay.java +++ b/services/core/java/com/android/server/display/LogicalDisplay.java @@ -448,6 +448,7 @@ final class LogicalDisplay { mBaseDisplayInfo.brightnessMinimum = deviceInfo.brightnessMinimum; mBaseDisplayInfo.brightnessMaximum = deviceInfo.brightnessMaximum; mBaseDisplayInfo.brightnessDefault = deviceInfo.brightnessDefault; + mBaseDisplayInfo.hdrSdrRatio = deviceInfo.hdrSdrRatio; mBaseDisplayInfo.roundedCorners = deviceInfo.roundedCorners; mBaseDisplayInfo.installOrientation = deviceInfo.installOrientation; mBaseDisplayInfo.displayShape = deviceInfo.displayShape; diff --git a/services/core/java/com/android/server/display/LogicalDisplayMapper.java b/services/core/java/com/android/server/display/LogicalDisplayMapper.java index c69586279a498..fad8a565c25a6 100644 --- a/services/core/java/com/android/server/display/LogicalDisplayMapper.java +++ b/services/core/java/com/android/server/display/LogicalDisplayMapper.java @@ -71,6 +71,7 @@ class LogicalDisplayMapper implements DisplayDeviceRepository.Listener { public static final int LOGICAL_DISPLAY_EVENT_SWAPPED = 4; public static final int LOGICAL_DISPLAY_EVENT_FRAME_RATE_OVERRIDES_CHANGED = 5; public static final int LOGICAL_DISPLAY_EVENT_DEVICE_STATE_TRANSITION = 6; + public static final int LOGICAL_DISPLAY_EVENT_HDR_SDR_RATIO_CHANGED = 7; public static final int DISPLAY_GROUP_EVENT_ADDED = 1; public static final int DISPLAY_GROUP_EVENT_CHANGED = 2; @@ -221,14 +222,6 @@ class LogicalDisplayMapper implements DisplayDeviceRepository.Listener { handleDisplayDeviceAddedLocked(device); break; - case DisplayDeviceRepository.DISPLAY_DEVICE_EVENT_CHANGED: - if (DEBUG) { - Slog.d(TAG, "Display device changed: " + device.getDisplayDeviceInfoLocked()); - } - finishStateTransitionLocked(false /*force*/); - updateLogicalDisplaysLocked(); - break; - case DisplayDeviceRepository.DISPLAY_DEVICE_EVENT_REMOVED: if (DEBUG) { Slog.d(TAG, "Display device removed: " + device.getDisplayDeviceInfoLocked()); @@ -239,6 +232,15 @@ class LogicalDisplayMapper implements DisplayDeviceRepository.Listener { } } + @Override + public void onDisplayDeviceChangedLocked(DisplayDevice device, int diff) { + if (DEBUG) { + Slog.d(TAG, "Display device changed: " + device.getDisplayDeviceInfoLocked()); + } + finishStateTransitionLocked(false /*force*/); + updateLogicalDisplaysLocked(diff); + } + @Override public void onTraversalRequested() { mListener.onTraversalRequested(); @@ -649,13 +651,20 @@ class LogicalDisplayMapper implements DisplayDeviceRepository.Listener { } } + private void updateLogicalDisplaysLocked() { + updateLogicalDisplaysLocked(DisplayDeviceInfo.DIFF_EVERYTHING); + } + /** * Updates the rest of the display system once all the changes are applied for display * devices and logical displays. The includes releasing invalid/empty LogicalDisplays, * creating/adjusting/removing DisplayGroups, and notifying the rest of the system of the * relevant changes. + * + * @param diff The DisplayDeviceInfo.DIFF_* of what actually changed to enable finer-grained + * display update listeners */ - private void updateLogicalDisplaysLocked() { + private void updateLogicalDisplaysLocked(int diff) { // Go through all the displays and figure out if they need to be updated. // Loops in reverse so that displays can be removed during the loop without affecting the // rest of the loop. @@ -709,7 +718,13 @@ class LogicalDisplayMapper implements DisplayDeviceRepository.Listener { } else if (!mTempDisplayInfo.equals(newDisplayInfo)) { // FLAG_OWN_DISPLAY_GROUP could have changed, recalculate just in case assignDisplayGroupLocked(display); - mLogicalDisplaysToUpdate.put(displayId, LOGICAL_DISPLAY_EVENT_CHANGED); + // If only the hdr/sdr ratio changed, then send just the event for that case + if ((diff == DisplayDeviceInfo.DIFF_HDR_SDR_RATIO)) { + mLogicalDisplaysToUpdate.put(displayId, + LOGICAL_DISPLAY_EVENT_HDR_SDR_RATIO_CHANGED); + } else { + mLogicalDisplaysToUpdate.put(displayId, LOGICAL_DISPLAY_EVENT_CHANGED); + } // The display is involved in a display layout transition } else if (updateState == UPDATE_STATE_TRANSITION) { @@ -768,6 +783,7 @@ class LogicalDisplayMapper implements DisplayDeviceRepository.Listener { sendUpdatesForDisplaysLocked(LOGICAL_DISPLAY_EVENT_FRAME_RATE_OVERRIDES_CHANGED); sendUpdatesForDisplaysLocked(LOGICAL_DISPLAY_EVENT_SWAPPED); sendUpdatesForDisplaysLocked(LOGICAL_DISPLAY_EVENT_ADDED); + sendUpdatesForDisplaysLocked(LOGICAL_DISPLAY_EVENT_HDR_SDR_RATIO_CHANGED); sendUpdatesForGroupsLocked(DISPLAY_GROUP_EVENT_CHANGED); sendUpdatesForGroupsLocked(DISPLAY_GROUP_EVENT_REMOVED); @@ -1110,6 +1126,8 @@ class LogicalDisplayMapper implements DisplayDeviceRepository.Listener { return "swapped"; case LOGICAL_DISPLAY_EVENT_REMOVED: return "removed"; + case LOGICAL_DISPLAY_EVENT_HDR_SDR_RATIO_CHANGED: + return "hdr_sdr_ratio_changed"; } return null; } diff --git a/services/tests/mockingservicestests/src/com/android/server/display/LocalDisplayAdapterTest.java b/services/tests/mockingservicestests/src/com/android/server/display/LocalDisplayAdapterTest.java index 45247595b678a..95a5884979804 100644 --- a/services/tests/mockingservicestests/src/com/android/server/display/LocalDisplayAdapterTest.java +++ b/services/tests/mockingservicestests/src/com/android/server/display/LocalDisplayAdapterTest.java @@ -69,6 +69,8 @@ import org.mockito.quality.Strictness; import java.util.ArrayList; import java.util.Arrays; import java.util.LinkedList; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; @SmallTest @@ -692,7 +694,10 @@ public class LocalDisplayAdapterTest { // Verify that committed triggered a new change event and is set correctly. verify(mSurfaceControlProxy, never()).setDisplayPowerMode(display.token, Display.STATE_OFF); - assertThat(mListener.changedDisplays.size()).isEqualTo(1); + // We expect at least 1 update for the state change, but + // could get a second update for the initial brightness change if a nits mapping + // is available + assertThat(mListener.changedDisplays.size()).isAnyOf(1, 2); assertThat(displayDevice.getDisplayDeviceInfoLocked().state).isEqualTo(Display.STATE_OFF); assertThat(displayDevice.getDisplayDeviceInfoLocked().committedState).isEqualTo( Display.STATE_OFF); @@ -964,6 +969,43 @@ public class LocalDisplayAdapterTest { .isTrue(); } + @Test + public void testHdrSdrRatio_notifiesOnChange() throws Exception { + FakeDisplay display = new FakeDisplay(PORT_A); + setUpDisplay(display); + updateAvailableDisplays(); + mAdapter.registerLocked(); + waitForHandlerToComplete(mHandler, HANDLER_WAIT_MS); + assertThat(mListener.addedDisplays.size()).isEqualTo(1); + DisplayDevice displayDevice = mListener.addedDisplays.get(0); + + // Turn on / initialize + Runnable changeStateRunnable = displayDevice.requestDisplayStateLocked(Display.STATE_ON, 0, + 0); + changeStateRunnable.run(); + waitForHandlerToComplete(mHandler, HANDLER_WAIT_MS); + mListener.changedDisplays.clear(); + + assertEquals(1.0f, displayDevice.getDisplayDeviceInfoLocked().hdrSdrRatio, 0.001f); + + // HDR time! + Runnable goHdrRunnable = displayDevice.requestDisplayStateLocked(Display.STATE_ON, 1f, + 0); + waitForHandlerToComplete(mHandler, HANDLER_WAIT_MS); + // Display state didn't change, no listeners should have happened + assertThat(mListener.changedDisplays.size()).isEqualTo(0); + + // Execute hdr change. + goHdrRunnable.run(); + waitForHandlerToComplete(mHandler, HANDLER_WAIT_MS); + // Display state didn't change, expect to only get the HDR/SDR ratio change notification + assertThat(mListener.changedDisplays.size()).isEqualTo(1); + + final float expectedRatio = DISPLAY_RANGE_NITS[1] / DISPLAY_RANGE_NITS[0]; + assertEquals(expectedRatio, displayDevice.getDisplayDeviceInfoLocked().hdrSdrRatio, + 0.001f); + } + private void assertDisplayDpi(DisplayDeviceInfo info, int expectedPort, float expectedXdpi, float expectedYDpi, @@ -1107,15 +1149,9 @@ public class LocalDisplayAdapterTest { private static void waitForHandlerToComplete(Handler handler, long waitTimeMs) throws InterruptedException { - final Object lock = new Object(); - synchronized (lock) { - handler.post(() -> { - synchronized (lock) { - lock.notify(); - } - }); - lock.wait(waitTimeMs); - } + final CountDownLatch fence = new CountDownLatch(1); + handler.post(fence::countDown); + assertTrue(fence.await(waitTimeMs, TimeUnit.MILLISECONDS)); } private class HotplugTransmitter { diff --git a/tests/SilkFX/src/com/android/test/silkfx/common/ColorModeControls.kt b/tests/SilkFX/src/com/android/test/silkfx/common/ColorModeControls.kt index 046174cf83255..1bd8f6a68cbd7 100644 --- a/tests/SilkFX/src/com/android/test/silkfx/common/ColorModeControls.kt +++ b/tests/SilkFX/src/com/android/test/silkfx/common/ColorModeControls.kt @@ -20,12 +20,15 @@ import android.content.Context import android.content.pm.ActivityInfo import android.hardware.display.DisplayManager import android.util.AttributeSet +import android.util.Log +import android.view.Display import android.view.Window import android.widget.Button import android.widget.LinearLayout import android.widget.TextView import com.android.test.silkfx.R import com.android.test.silkfx.app.WindowObserver +import java.util.function.Consumer class ColorModeControls : LinearLayout, WindowObserver { private val COLOR_MODE_HDR10 = 3 @@ -66,6 +69,38 @@ class ColorModeControls : LinearLayout, WindowObserver { } } + private val hdrsdrListener = Consumer { display -> + Log.d("SilkFX", "HDR/SDR changed ${display.hdrSdrRatio}") + } + + private val displayChangedListener = object : DisplayManager.DisplayListener { + override fun onDisplayAdded(displayId: Int) { + Log.d("SilkFX", "onDisplayAdded") + } + + override fun onDisplayRemoved(displayId: Int) { + Log.d("SilkFX", "onDisplayRemoved") + } + + override fun onDisplayChanged(displayId: Int) { + Log.d("SilkFX", "onDisplayChanged") + } + } + + override fun onAttachedToWindow() { + super.onAttachedToWindow() + Log.d("SilkFX", "is hdr/sdr available: ${display.isHdrSdrRatioAvailable}; " + + "current ration = ${display.hdrSdrRatio}") + display.registerHdrSdrRatioChangedListener({ it.run() }, hdrsdrListener) + displayManager.registerDisplayListener(displayChangedListener, handler) + } + + override fun onDetachedFromWindow() { + super.onDetachedFromWindow() + display.unregisterHdrSdrRatioChangedListener(hdrsdrListener) + displayManager.unregisterDisplayListener(displayChangedListener) + } + private fun setColorMode(newMode: Int) { val window = window!! var sdrWhitepointChanged = false