From 3d75125a058ca1d4672266c843ed6435d4fb3bc2 Mon Sep 17 00:00:00 2001 From: Rupesh Bansal Date: Mon, 20 Mar 2023 04:39:45 +0000 Subject: [PATCH] Fix the issue where resetting the refresh rates and brightness thresholds in DeviceConfig wouldn't make the system to fallback to the default values Currently, if the DeviceConfig value is reset, we do not change the various different types of brightness thresholds and refresh rates to the default values, but rather gets stuck on the old stale value. The only was to recover from this is the device reboot. Bug: 265126371 Test: Manually changed the DeviceConfig values and confirmed the new behaviour. Test: atest DisplayModeDirectorTest Merged-In: I700d8581975fe6ac0c0db4ec800f3698ffa0c2d1 Change-Id: I700d8581975fe6ac0c0db4ec800f3698ffa0c2d1 --- .../server/display/DisplayModeDirector.java | 85 +++++++++++++------ .../display/DisplayModeDirectorTest.java | 43 ++++++++-- 2 files changed, 94 insertions(+), 34 deletions(-) diff --git a/services/core/java/com/android/server/display/DisplayModeDirector.java b/services/core/java/com/android/server/display/DisplayModeDirector.java index 1bbdc20c81e50..8c83be3ea8dd6 100644 --- a/services/core/java/com/android/server/display/DisplayModeDirector.java +++ b/services/core/java/com/android/server/display/DisplayModeDirector.java @@ -1230,13 +1230,12 @@ public class DisplayModeDirector { } public void onDeviceConfigDefaultPeakRefreshRateChanged(Float defaultPeakRefreshRate) { - if (defaultPeakRefreshRate == null) { - defaultPeakRefreshRate = (float) mContext.getResources().getInteger( - R.integer.config_defaultPeakRefreshRate); - } - - if (mDefaultPeakRefreshRate != defaultPeakRefreshRate) { - synchronized (mLock) { + synchronized (mLock) { + if (defaultPeakRefreshRate == null) { + setDefaultPeakRefreshRate(mDefaultDisplayDeviceConfig, + /* attemptLoadingFromDeviceConfig= */ false); + updateRefreshRateSettingLocked(); + } else if (mDefaultPeakRefreshRate != defaultPeakRefreshRate) { mDefaultPeakRefreshRate = defaultPeakRefreshRate; updateRefreshRateSettingLocked(); } @@ -1869,11 +1868,20 @@ public class DisplayModeDirector { mLowDisplayBrightnessThresholds = displayThresholds; mLowAmbientBrightnessThresholds = ambientThresholds; } else { - // Invalid or empty. Use device default. - mLowDisplayBrightnessThresholds = mContext.getResources().getIntArray( - R.array.config_brightnessThresholdsOfPeakRefreshRate); - mLowAmbientBrightnessThresholds = mContext.getResources().getIntArray( - R.array.config_ambientThresholdsOfPeakRefreshRate); + DisplayDeviceConfig displayDeviceConfig; + synchronized (mLock) { + displayDeviceConfig = mDefaultDisplayDeviceConfig; + } + mLowDisplayBrightnessThresholds = loadBrightnessThresholds( + () -> mDeviceConfigDisplaySettings.getLowDisplayBrightnessThresholds(), + () -> displayDeviceConfig.getLowDisplayBrightnessThresholds(), + R.array.config_brightnessThresholdsOfPeakRefreshRate, + displayDeviceConfig, /* attemptLoadingFromDeviceConfig= */ false); + mLowAmbientBrightnessThresholds = loadBrightnessThresholds( + () -> mDeviceConfigDisplaySettings.getLowAmbientBrightnessThresholds(), + () -> displayDeviceConfig.getLowAmbientBrightnessThresholds(), + R.array.config_ambientThresholdsOfPeakRefreshRate, + displayDeviceConfig, /* attemptLoadingFromDeviceConfig= */ false); } restartObserver(); } @@ -1883,24 +1891,41 @@ public class DisplayModeDirector { * DeviceConfig properties. */ public void onDeviceConfigRefreshRateInLowZoneChanged(int refreshRate) { - if (refreshRate != mRefreshRateInLowZone) { + if (refreshRate == -1) { + // Given there is no value available in DeviceConfig, lets not attempt loading it + // from there. + synchronized (mLock) { + loadRefreshRateInLowZone(mDefaultDisplayDeviceConfig, + /* attemptLoadingFromDeviceConfig= */ false); + } + restartObserver(); + } else if (refreshRate != mRefreshRateInLowZone) { mRefreshRateInLowZone = refreshRate; restartObserver(); } } - public void onDeviceConfigHighBrightnessThresholdsChanged(int[] displayThresholds, + private void onDeviceConfigHighBrightnessThresholdsChanged(int[] displayThresholds, int[] ambientThresholds) { if (displayThresholds != null && ambientThresholds != null && displayThresholds.length == ambientThresholds.length) { mHighDisplayBrightnessThresholds = displayThresholds; mHighAmbientBrightnessThresholds = ambientThresholds; } else { - // Invalid or empty. Use device default. - mHighDisplayBrightnessThresholds = mContext.getResources().getIntArray( - R.array.config_highDisplayBrightnessThresholdsOfFixedRefreshRate); - mHighAmbientBrightnessThresholds = mContext.getResources().getIntArray( - R.array.config_highAmbientBrightnessThresholdsOfFixedRefreshRate); + DisplayDeviceConfig displayDeviceConfig; + synchronized (mLock) { + displayDeviceConfig = mDefaultDisplayDeviceConfig; + } + mHighDisplayBrightnessThresholds = loadBrightnessThresholds( + () -> mDeviceConfigDisplaySettings.getHighDisplayBrightnessThresholds(), + () -> displayDeviceConfig.getHighDisplayBrightnessThresholds(), + R.array.config_highDisplayBrightnessThresholdsOfFixedRefreshRate, + displayDeviceConfig, /* attemptLoadingFromDeviceConfig= */ false); + mHighAmbientBrightnessThresholds = loadBrightnessThresholds( + () -> mDeviceConfigDisplaySettings.getHighAmbientBrightnessThresholds(), + () -> displayDeviceConfig.getHighAmbientBrightnessThresholds(), + R.array.config_highAmbientBrightnessThresholdsOfFixedRefreshRate, + displayDeviceConfig, /* attemptLoadingFromDeviceConfig= */ false); } restartObserver(); } @@ -1910,7 +1935,15 @@ public class DisplayModeDirector { * DeviceConfig properties. */ public void onDeviceConfigRefreshRateInHighZoneChanged(int refreshRate) { - if (refreshRate != mRefreshRateInHighZone) { + if (refreshRate == -1) { + // Given there is no value available in DeviceConfig, lets not attempt loading it + // from there. + synchronized (mLock) { + loadRefreshRateInHighZone(mDefaultDisplayDeviceConfig, + /* attemptLoadingFromDeviceConfig= */ false); + } + restartObserver(); + } else if (refreshRate != mRefreshRateInHighZone) { mRefreshRateInHighZone = refreshRate; restartObserver(); } @@ -2870,10 +2903,8 @@ public class DisplayModeDirector { new Pair<>(lowDisplayBrightnessThresholds, lowAmbientBrightnessThresholds)) .sendToTarget(); - if (refreshRateInLowZone != -1) { - mHandler.obtainMessage(MSG_REFRESH_RATE_IN_LOW_ZONE_CHANGED, refreshRateInLowZone, - 0).sendToTarget(); - } + mHandler.obtainMessage(MSG_REFRESH_RATE_IN_LOW_ZONE_CHANGED, refreshRateInLowZone, + 0).sendToTarget(); int[] highDisplayBrightnessThresholds = getHighDisplayBrightnessThresholds(); int[] highAmbientBrightnessThresholds = getHighAmbientBrightnessThresholds(); @@ -2883,10 +2914,8 @@ public class DisplayModeDirector { new Pair<>(highDisplayBrightnessThresholds, highAmbientBrightnessThresholds)) .sendToTarget(); - if (refreshRateInHighZone != -1) { - mHandler.obtainMessage(MSG_REFRESH_RATE_IN_HIGH_ZONE_CHANGED, refreshRateInHighZone, - 0).sendToTarget(); - } + mHandler.obtainMessage(MSG_REFRESH_RATE_IN_HIGH_ZONE_CHANGED, refreshRateInHighZone, + 0).sendToTarget(); synchronized (mLock) { final int refreshRateInHbmSunlight = diff --git a/services/tests/servicestests/src/com/android/server/display/DisplayModeDirectorTest.java b/services/tests/servicestests/src/com/android/server/display/DisplayModeDirectorTest.java index ff37564f46a4a..58f3db959afe4 100644 --- a/services/tests/servicestests/src/com/android/server/display/DisplayModeDirectorTest.java +++ b/services/tests/servicestests/src/com/android/server/display/DisplayModeDirectorTest.java @@ -1905,7 +1905,7 @@ public class DisplayModeDirectorTest { // We don't expect any interaction with DeviceConfig when the director is initialized // because we explicitly avoid doing this as this can lead to a latency spike in the // startup of DisplayManagerService - // Verify all the loaded values are from DisplayDeviceConfig + // Verify all the loaded values are from config.xml assertEquals(director.getSettingsObserver().getDefaultRefreshRate(), 45, 0.0); assertEquals(director.getSettingsObserver().getDefaultPeakRefreshRate(), 75, 0.0); @@ -1937,6 +1937,7 @@ public class DisplayModeDirectorTest { when(displayDeviceConfig.getDefaultRefreshRateInHbmSunlight()).thenReturn(75); director.defaultDisplayDeviceUpdated(displayDeviceConfig); + // Verify the new values are from the freshly loaded DisplayDeviceConfig. assertEquals(director.getSettingsObserver().getDefaultRefreshRate(), 60, 0.0); assertEquals(director.getSettingsObserver().getDefaultPeakRefreshRate(), 65, 0.0); @@ -1966,6 +1967,7 @@ public class DisplayModeDirectorTest { config.setRefreshRateInHbmSunlight(80); director.defaultDisplayDeviceUpdated(displayDeviceConfig); + // Verify the values are loaded from the DeviceConfig. assertEquals(director.getSettingsObserver().getDefaultRefreshRate(), 60, 0.0); assertEquals(director.getSettingsObserver().getDefaultPeakRefreshRate(), 60, 0.0); @@ -1981,6 +1983,35 @@ public class DisplayModeDirectorTest { new int[]{20}); assertEquals(director.getHbmObserver().getRefreshRateInHbmHdr(), 70); assertEquals(director.getHbmObserver().getRefreshRateInHbmSunlight(), 80); + + // Reset the DeviceConfig + config.setDefaultPeakRefreshRate(null); + config.setRefreshRateInHighZone(null); + config.setRefreshRateInLowZone(null); + config.setLowAmbientBrightnessThresholds(new int[]{}); + config.setLowDisplayBrightnessThresholds(new int[]{}); + config.setHighDisplayBrightnessThresholds(new int[]{}); + config.setHighAmbientBrightnessThresholds(new int[]{}); + config.setRefreshRateInHbmHdr(null); + config.setRefreshRateInHbmSunlight(null); + waitForIdleSync(); + + // verify the new values now fallback to DisplayDeviceConfig + assertEquals(director.getSettingsObserver().getDefaultRefreshRate(), 60, 0.0); + assertEquals(director.getSettingsObserver().getDefaultPeakRefreshRate(), 65, + 0.0); + assertEquals(director.getBrightnessObserver().getRefreshRateInHighZone(), 55); + assertEquals(director.getBrightnessObserver().getRefreshRateInLowZone(), 50); + assertArrayEquals(director.getBrightnessObserver().getHighDisplayBrightnessThreshold(), + new int[]{210}); + assertArrayEquals(director.getBrightnessObserver().getHighAmbientBrightnessThreshold(), + new int[]{2100}); + assertArrayEquals(director.getBrightnessObserver().getLowDisplayBrightnessThreshold(), + new int[]{25}); + assertArrayEquals(director.getBrightnessObserver().getLowAmbientBrightnessThreshold(), + new int[]{30}); + assertEquals(director.getHbmObserver().getRefreshRateInHbmHdr(), 65); + assertEquals(director.getHbmObserver().getRefreshRateInHbmSunlight(), 75); } @Test @@ -2140,18 +2171,18 @@ public class DisplayModeDirectorTest { super.addOnPropertiesChangedListener(namespace, executor, listener); } - void setRefreshRateInLowZone(int fps) { + void setRefreshRateInLowZone(Integer fps) { putPropertyAndNotify( DeviceConfig.NAMESPACE_DISPLAY_MANAGER, KEY_REFRESH_RATE_IN_LOW_ZONE, String.valueOf(fps)); } - void setRefreshRateInHbmSunlight(int fps) { + void setRefreshRateInHbmSunlight(Integer fps) { putPropertyAndNotify(DeviceConfig.NAMESPACE_DISPLAY_MANAGER, KEY_REFRESH_RATE_IN_HBM_SUNLIGHT, String.valueOf(fps)); } - void setRefreshRateInHbmHdr(int fps) { + void setRefreshRateInHbmHdr(Integer fps) { putPropertyAndNotify(DeviceConfig.NAMESPACE_DISPLAY_MANAGER, KEY_REFRESH_RATE_IN_HBM_HDR, String.valueOf(fps)); } @@ -2187,13 +2218,13 @@ public class DisplayModeDirectorTest { thresholds); } - void setRefreshRateInHighZone(int fps) { + void setRefreshRateInHighZone(Integer fps) { putPropertyAndNotify( DeviceConfig.NAMESPACE_DISPLAY_MANAGER, KEY_REFRESH_RATE_IN_HIGH_ZONE, String.valueOf(fps)); } - void setDefaultPeakRefreshRate(int fps) { + void setDefaultPeakRefreshRate(Integer fps) { putPropertyAndNotify( DeviceConfig.NAMESPACE_DISPLAY_MANAGER, KEY_PEAK_REFRESH_RATE_DEFAULT, String.valueOf(fps));