diff --git a/services/core/java/com/android/server/display/DisplayManagerService.java b/services/core/java/com/android/server/display/DisplayManagerService.java index 5d92c7f8cf057..26b6cb0ebe9be 100644 --- a/services/core/java/com/android/server/display/DisplayManagerService.java +++ b/services/core/java/com/android/server/display/DisplayManagerService.java @@ -1781,7 +1781,24 @@ public final class DisplayManagerService extends SystemService { } else { configurePreferredDisplayModeLocked(display); } - addDisplayPowerControllerLocked(display); + DisplayPowerControllerInterface dpc = addDisplayPowerControllerLocked(display); + + if (dpc != null) { + final int leadDisplayId = display.getLeadDisplayIdLocked(); + updateDisplayPowerControllerLeaderLocked(dpc, leadDisplayId); + + // Loop through all the displays and check if any should follow this one - it could be + // that the follower display was added before the lead display. + mLogicalDisplayMapper.forEachLocked(d -> { + if (d.getLeadDisplayIdLocked() == displayId) { + DisplayPowerControllerInterface followerDpc = + mDisplayPowerControllers.get(d.getDisplayIdLocked()); + if (followerDpc != null) { + updateDisplayPowerControllerLeaderLocked(followerDpc, displayId); + } + } + }); + } mDisplayStates.append(displayId, Display.STATE_UNKNOWN); @@ -1832,8 +1849,8 @@ public final class DisplayManagerService extends SystemService { } } - private void updateDisplayPowerControllerLeaderLocked(DisplayPowerControllerInterface dpc, - int leadDisplayId) { + private void updateDisplayPowerControllerLeaderLocked( + @NonNull DisplayPowerControllerInterface dpc, int leadDisplayId) { if (dpc.getLeadDisplayId() == leadDisplayId) { // Lead display hasn't changed, nothing to do. return; @@ -1851,9 +1868,11 @@ public final class DisplayManagerService extends SystemService { // And then, if it's following, register it with the new one. if (leadDisplayId != Layout.NO_LEAD_DISPLAY) { - final DisplayPowerControllerInterface newLead = + final DisplayPowerControllerInterface newLeader = mDisplayPowerControllers.get(leadDisplayId); - newLead.addDisplayBrightnessFollower(dpc); + if (newLeader != null) { + newLeader.addDisplayBrightnessFollower(dpc); + } } } @@ -1872,6 +1891,7 @@ public final class DisplayManagerService extends SystemService { final DisplayPowerControllerInterface dpc = mDisplayPowerControllers.removeReturnOld(displayId); if (dpc != null) { + updateDisplayPowerControllerLeaderLocked(dpc, Layout.NO_LEAD_DISPLAY); dpc.stop(); } mDisplayStates.delete(displayId); @@ -3062,10 +3082,11 @@ public final class DisplayManagerService extends SystemService { } @RequiresPermission(Manifest.permission.READ_DEVICE_CONFIG) - private void addDisplayPowerControllerLocked(LogicalDisplay display) { + private DisplayPowerControllerInterface addDisplayPowerControllerLocked( + LogicalDisplay display) { if (mPowerHandler == null) { // initPowerManagement has not yet been called. - return; + return null; } if (mBrightnessTracker == null && display.getDisplayIdLocked() == Display.DEFAULT_DISPLAY) { @@ -3086,7 +3107,7 @@ public final class DisplayManagerService extends SystemService { if (hbmMetadata == null) { Slog.wtf(TAG, "High Brightness Mode Metadata is null in DisplayManagerService for " + "display: " + display.getDisplayIdLocked()); - return; + return null; } if (DeviceConfig.getBoolean("display_manager", "use_newly_structured_display_power_controller", true)) { @@ -3101,6 +3122,7 @@ public final class DisplayManagerService extends SystemService { () -> handleBrightnessChange(display), hbmMetadata, mBootCompleted); } mDisplayPowerControllers.append(display.getDisplayIdLocked(), displayPowerController); + return displayPowerController; } private void handleBrightnessChange(LogicalDisplay display) { diff --git a/services/core/java/com/android/server/display/DisplayPowerController.java b/services/core/java/com/android/server/display/DisplayPowerController.java index 1acc20880258b..0c1ace7395e5a 100644 --- a/services/core/java/com/android/server/display/DisplayPowerController.java +++ b/services/core/java/com/android/server/display/DisplayPowerController.java @@ -789,6 +789,17 @@ final class DisplayPowerController implements AutomaticBrightnessController.Call } } + @GuardedBy("mLock") + private void clearDisplayBrightnessFollowersLocked() { + for (int i = 0; i < mDisplayBrightnessFollowers.size(); i++) { + DisplayPowerControllerInterface follower = mDisplayBrightnessFollowers.valueAt(i); + mHandler.postAtTime(() -> follower.setBrightnessToFollow( + PowerManager.BRIGHTNESS_INVALID_FLOAT, /* nits= */ -1, + /* ambientLux= */ 0), mClock.uptimeMillis()); + } + mDisplayBrightnessFollowers.clear(); + } + @Nullable @Override public ParceledListSlice getAmbientBrightnessStats( @@ -946,6 +957,8 @@ final class DisplayPowerController implements AutomaticBrightnessController.Call @Override public void stop() { synchronized (mLock) { + clearDisplayBrightnessFollowersLocked(); + mStopped = true; Message msg = mHandler.obtainMessage(MSG_STOP); mHandler.sendMessageAtTime(msg, mClock.uptimeMillis()); diff --git a/services/core/java/com/android/server/display/DisplayPowerController2.java b/services/core/java/com/android/server/display/DisplayPowerController2.java index b36aedefa2a7e..65c9cfbc840b4 100644 --- a/services/core/java/com/android/server/display/DisplayPowerController2.java +++ b/services/core/java/com/android/server/display/DisplayPowerController2.java @@ -764,6 +764,8 @@ final class DisplayPowerController2 implements AutomaticBrightnessController.Cal @Override public void stop() { synchronized (mLock) { + clearDisplayBrightnessFollowersLocked(); + mStopped = true; Message msg = mHandler.obtainMessage(MSG_STOP); mHandler.sendMessageAtTime(msg, mClock.uptimeMillis()); @@ -2200,6 +2202,17 @@ final class DisplayPowerController2 implements AutomaticBrightnessController.Cal } } + @GuardedBy("mLock") + private void clearDisplayBrightnessFollowersLocked() { + for (int i = 0; i < mDisplayBrightnessFollowers.size(); i++) { + DisplayPowerControllerInterface follower = mDisplayBrightnessFollowers.valueAt(i); + mHandler.postAtTime(() -> follower.setBrightnessToFollow( + PowerManager.BRIGHTNESS_INVALID_FLOAT, /* nits= */ -1, + /* ambientLux= */ 0), mClock.uptimeMillis()); + } + mDisplayBrightnessFollowers.clear(); + } + @Override public void dump(final PrintWriter pw) { synchronized (mLock) { diff --git a/services/tests/mockingservicestests/src/com/android/server/display/DisplayPowerController2Test.java b/services/tests/mockingservicestests/src/com/android/server/display/DisplayPowerController2Test.java index ca857f121624b..c4aa0bbc24b5a 100644 --- a/services/tests/mockingservicestests/src/com/android/server/display/DisplayPowerController2Test.java +++ b/services/tests/mockingservicestests/src/com/android/server/display/DisplayPowerController2Test.java @@ -445,7 +445,7 @@ public final class DisplayPowerController2Test { } @Test - public void testDisplayBrightnessFollowersRemoval() { + public void testDisplayBrightnessFollowersRemoval_RemoveSingleFollower() { DisplayPowerControllerHolder followerDpc = createDisplayPowerController(FOLLOWER_DISPLAY_ID, FOLLOWER_UNIQUE_ID); DisplayPowerControllerHolder secondFollowerDpc = createDisplayPowerController( @@ -519,6 +519,78 @@ public final class DisplayPowerController2Test { verify(secondFollowerDpc.animator).animateTo(eq(brightness), anyFloat(), anyFloat()); } + @Test + public void testDisplayBrightnessFollowersRemoval_RemoveAllFollowers() { + DisplayPowerControllerHolder followerHolder = + createDisplayPowerController(FOLLOWER_DISPLAY_ID, FOLLOWER_UNIQUE_ID); + DisplayPowerControllerHolder secondFollowerHolder = + createDisplayPowerController(SECOND_FOLLOWER_DISPLAY_ID, + SECOND_FOLLOWER_UNIQUE_DISPLAY_ID); + + DisplayPowerRequest dpr = new DisplayPowerRequest(); + mHolder.dpc.requestPowerState(dpr, /* waitForNegativeProximity= */ false); + followerHolder.dpc.requestPowerState(dpr, /* waitForNegativeProximity= */ false); + secondFollowerHolder.dpc.requestPowerState(dpr, /* waitForNegativeProximity= */ false); + advanceTime(1); // Run updatePowerState + + ArgumentCaptor listenerCaptor = + ArgumentCaptor.forClass(BrightnessSetting.BrightnessSettingListener.class); + verify(mHolder.brightnessSetting).registerListener(listenerCaptor.capture()); + BrightnessSetting.BrightnessSettingListener listener = listenerCaptor.getValue(); + + // Set the initial brightness on the DPCs we're going to remove so we have a fixed value for + // it to return to. + listenerCaptor = ArgumentCaptor.forClass(BrightnessSetting.BrightnessSettingListener.class); + verify(followerHolder.brightnessSetting).registerListener(listenerCaptor.capture()); + BrightnessSetting.BrightnessSettingListener followerListener = listenerCaptor.getValue(); + listenerCaptor = ArgumentCaptor.forClass(BrightnessSetting.BrightnessSettingListener.class); + verify(secondFollowerHolder.brightnessSetting).registerListener(listenerCaptor.capture()); + BrightnessSetting.BrightnessSettingListener secondFollowerListener = + listenerCaptor.getValue(); + final float initialFollowerBrightness = 0.3f; + when(followerHolder.brightnessSetting.getBrightness()).thenReturn( + initialFollowerBrightness); + when(secondFollowerHolder.brightnessSetting.getBrightness()).thenReturn( + initialFollowerBrightness); + followerListener.onBrightnessChanged(initialFollowerBrightness); + secondFollowerListener.onBrightnessChanged(initialFollowerBrightness); + advanceTime(1); + verify(followerHolder.animator).animateTo(eq(initialFollowerBrightness), + anyFloat(), anyFloat()); + verify(secondFollowerHolder.animator).animateTo(eq(initialFollowerBrightness), + anyFloat(), anyFloat()); + + mHolder.dpc.addDisplayBrightnessFollower(followerHolder.dpc); + mHolder.dpc.addDisplayBrightnessFollower(secondFollowerHolder.dpc); + clearInvocations(followerHolder.animator, secondFollowerHolder.animator); + + // Validate both followers are correctly registered and receiving brightness updates + float brightness = 0.6f; + float nits = 600; + when(mHolder.automaticBrightnessController.convertToNits(brightness)).thenReturn(nits); + when(followerHolder.automaticBrightnessController.convertToFloatScale(nits)) + .thenReturn(brightness); + when(secondFollowerHolder.automaticBrightnessController.convertToFloatScale(nits)) + .thenReturn(brightness); + when(mHolder.brightnessSetting.getBrightness()).thenReturn(brightness); + listener.onBrightnessChanged(brightness); + advanceTime(1); // Send messages, run updatePowerState + verify(mHolder.animator).animateTo(eq(brightness), anyFloat(), anyFloat()); + verify(followerHolder.animator).animateTo(eq(brightness), anyFloat(), anyFloat()); + verify(secondFollowerHolder.animator).animateTo(eq(brightness), anyFloat(), anyFloat()); + + clearInvocations(mHolder.animator, followerHolder.animator, secondFollowerHolder.animator); + + // Stop the lead DPC and validate that the followers go back to their original brightness. + mHolder.dpc.stop(); + advanceTime(1); + verify(followerHolder.animator).animateTo(eq(initialFollowerBrightness), + anyFloat(), anyFloat()); + verify(secondFollowerHolder.animator).animateTo(eq(initialFollowerBrightness), + anyFloat(), anyFloat()); + clearInvocations(followerHolder.animator, secondFollowerHolder.animator); + } + @Test public void testDoesNotSetScreenStateForNonDefaultDisplayUntilBootCompleted() { // We should still set screen state for the default display diff --git a/services/tests/mockingservicestests/src/com/android/server/display/DisplayPowerControllerTest.java b/services/tests/mockingservicestests/src/com/android/server/display/DisplayPowerControllerTest.java index 0b97c5cb6ca1b..415adbbac91e0 100644 --- a/services/tests/mockingservicestests/src/com/android/server/display/DisplayPowerControllerTest.java +++ b/services/tests/mockingservicestests/src/com/android/server/display/DisplayPowerControllerTest.java @@ -91,7 +91,7 @@ public final class DisplayPowerControllerTest { private static final int DISPLAY_ID = Display.DEFAULT_DISPLAY; private static final String UNIQUE_ID = "unique_id_test123"; private static final int FOLLOWER_DISPLAY_ID = DISPLAY_ID + 1; - private static final String FOLLOWER_UNIQUE_DISPLAY_ID = "unique_id_456"; + private static final String FOLLOWER_UNIQUE_ID = "unique_id_456"; private static final int SECOND_FOLLOWER_DISPLAY_ID = FOLLOWER_DISPLAY_ID + 1; private static final String SECOND_FOLLOWER_UNIQUE_DISPLAY_ID = "unique_id_789"; private static final float PROX_SENSOR_MAX_RANGE = 5; @@ -279,7 +279,7 @@ public final class DisplayPowerControllerTest { @Test public void testProximitySensorListenerNotRegisteredForNonDefaultDisplay() { DisplayPowerControllerHolder followerDpc = - createDisplayPowerController(FOLLOWER_DISPLAY_ID, FOLLOWER_UNIQUE_DISPLAY_ID); + createDisplayPowerController(FOLLOWER_DISPLAY_ID, FOLLOWER_UNIQUE_ID); when(mHolder.displayPowerState.getScreenState()).thenReturn(Display.STATE_ON); // send a display power request @@ -298,7 +298,7 @@ public final class DisplayPowerControllerTest { @Test public void testDisplayBrightnessFollowers_BothDpcsSupportNits() { DisplayPowerControllerHolder followerDpc = - createDisplayPowerController(FOLLOWER_DISPLAY_ID, FOLLOWER_UNIQUE_DISPLAY_ID); + createDisplayPowerController(FOLLOWER_DISPLAY_ID, FOLLOWER_UNIQUE_ID); DisplayPowerRequest dpr = new DisplayPowerRequest(); mHolder.dpc.requestPowerState(dpr, /* waitForNegativeProximity= */ false); @@ -344,7 +344,7 @@ public final class DisplayPowerControllerTest { @Test public void testDisplayBrightnessFollowers_FollowerDoesNotSupportNits() { DisplayPowerControllerHolder followerDpc = - createDisplayPowerController(FOLLOWER_DISPLAY_ID, FOLLOWER_UNIQUE_DISPLAY_ID); + createDisplayPowerController(FOLLOWER_DISPLAY_ID, FOLLOWER_UNIQUE_ID); DisplayPowerRequest dpr = new DisplayPowerRequest(); mHolder.dpc.requestPowerState(dpr, /* waitForNegativeProximity= */ false); @@ -372,7 +372,7 @@ public final class DisplayPowerControllerTest { @Test public void testDisplayBrightnessFollowers_LeadDpcDoesNotSupportNits() { DisplayPowerControllerHolder followerDpc = - createDisplayPowerController(FOLLOWER_DISPLAY_ID, FOLLOWER_UNIQUE_DISPLAY_ID); + createDisplayPowerController(FOLLOWER_DISPLAY_ID, FOLLOWER_UNIQUE_ID); DisplayPowerRequest dpr = new DisplayPowerRequest(); mHolder.dpc.requestPowerState(dpr, /* waitForNegativeProximity= */ false); @@ -398,7 +398,7 @@ public final class DisplayPowerControllerTest { @Test public void testDisplayBrightnessFollowers_NeitherDpcSupportsNits() { DisplayPowerControllerHolder followerDpc = - createDisplayPowerController(FOLLOWER_DISPLAY_ID, FOLLOWER_UNIQUE_DISPLAY_ID); + createDisplayPowerController(FOLLOWER_DISPLAY_ID, FOLLOWER_UNIQUE_ID); DisplayPowerRequest dpr = new DisplayPowerRequest(); mHolder.dpc.requestPowerState(dpr, /* waitForNegativeProximity= */ false); @@ -449,9 +449,9 @@ public final class DisplayPowerControllerTest { } @Test - public void testDisplayBrightnessFollowersRemoval() { + public void testDisplayBrightnessFollowersRemoval_RemoveSingleFollower() { DisplayPowerControllerHolder followerHolder = - createDisplayPowerController(FOLLOWER_DISPLAY_ID, FOLLOWER_UNIQUE_DISPLAY_ID); + createDisplayPowerController(FOLLOWER_DISPLAY_ID, FOLLOWER_UNIQUE_ID); DisplayPowerControllerHolder secondFollowerHolder = createDisplayPowerController(SECOND_FOLLOWER_DISPLAY_ID, SECOND_FOLLOWER_UNIQUE_DISPLAY_ID); @@ -524,6 +524,78 @@ public final class DisplayPowerControllerTest { verify(secondFollowerHolder.animator).animateTo(eq(brightness), anyFloat(), anyFloat()); } + @Test + public void testDisplayBrightnessFollowersRemoval_RemoveAllFollowers() { + DisplayPowerControllerHolder followerHolder = + createDisplayPowerController(FOLLOWER_DISPLAY_ID, FOLLOWER_UNIQUE_ID); + DisplayPowerControllerHolder secondFollowerHolder = + createDisplayPowerController(SECOND_FOLLOWER_DISPLAY_ID, + SECOND_FOLLOWER_UNIQUE_DISPLAY_ID); + + DisplayPowerRequest dpr = new DisplayPowerRequest(); + mHolder.dpc.requestPowerState(dpr, /* waitForNegativeProximity= */ false); + followerHolder.dpc.requestPowerState(dpr, /* waitForNegativeProximity= */ false); + secondFollowerHolder.dpc.requestPowerState(dpr, /* waitForNegativeProximity= */ false); + advanceTime(1); // Run updatePowerState + + ArgumentCaptor listenerCaptor = + ArgumentCaptor.forClass(BrightnessSetting.BrightnessSettingListener.class); + verify(mHolder.brightnessSetting).registerListener(listenerCaptor.capture()); + BrightnessSetting.BrightnessSettingListener listener = listenerCaptor.getValue(); + + // Set the initial brightness on the DPCs we're going to remove so we have a fixed value for + // it to return to. + listenerCaptor = ArgumentCaptor.forClass(BrightnessSetting.BrightnessSettingListener.class); + verify(followerHolder.brightnessSetting).registerListener(listenerCaptor.capture()); + BrightnessSetting.BrightnessSettingListener followerListener = listenerCaptor.getValue(); + listenerCaptor = ArgumentCaptor.forClass(BrightnessSetting.BrightnessSettingListener.class); + verify(secondFollowerHolder.brightnessSetting).registerListener(listenerCaptor.capture()); + BrightnessSetting.BrightnessSettingListener secondFollowerListener = + listenerCaptor.getValue(); + final float initialFollowerBrightness = 0.3f; + when(followerHolder.brightnessSetting.getBrightness()).thenReturn( + initialFollowerBrightness); + when(secondFollowerHolder.brightnessSetting.getBrightness()).thenReturn( + initialFollowerBrightness); + followerListener.onBrightnessChanged(initialFollowerBrightness); + secondFollowerListener.onBrightnessChanged(initialFollowerBrightness); + advanceTime(1); + verify(followerHolder.animator).animateTo(eq(initialFollowerBrightness), + anyFloat(), anyFloat()); + verify(secondFollowerHolder.animator).animateTo(eq(initialFollowerBrightness), + anyFloat(), anyFloat()); + + mHolder.dpc.addDisplayBrightnessFollower(followerHolder.dpc); + mHolder.dpc.addDisplayBrightnessFollower(secondFollowerHolder.dpc); + clearInvocations(followerHolder.animator, secondFollowerHolder.animator); + + // Validate both followers are correctly registered and receiving brightness updates + float brightness = 0.6f; + float nits = 600; + when(mHolder.automaticBrightnessController.convertToNits(brightness)).thenReturn(nits); + when(followerHolder.automaticBrightnessController.convertToFloatScale(nits)) + .thenReturn(brightness); + when(secondFollowerHolder.automaticBrightnessController.convertToFloatScale(nits)) + .thenReturn(brightness); + when(mHolder.brightnessSetting.getBrightness()).thenReturn(brightness); + listener.onBrightnessChanged(brightness); + advanceTime(1); // Send messages, run updatePowerState + verify(mHolder.animator).animateTo(eq(brightness), anyFloat(), anyFloat()); + verify(followerHolder.animator).animateTo(eq(brightness), anyFloat(), anyFloat()); + verify(secondFollowerHolder.animator).animateTo(eq(brightness), anyFloat(), anyFloat()); + + clearInvocations(mHolder.animator, followerHolder.animator, secondFollowerHolder.animator); + + // Stop the lead DPC and validate that the followers go back to their original brightness. + mHolder.dpc.stop(); + advanceTime(1); + verify(followerHolder.animator).animateTo(eq(initialFollowerBrightness), + anyFloat(), anyFloat()); + verify(secondFollowerHolder.animator).animateTo(eq(initialFollowerBrightness), + anyFloat(), anyFloat()); + clearInvocations(followerHolder.animator, secondFollowerHolder.animator); + } + @Test public void testDoesNotSetScreenStateForNonDefaultDisplayUntilBootCompleted() { // We should still set screen state for the default display