Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit ae9cd8f4 authored by Marin Shalamanov's avatar Marin Shalamanov
Browse files

Invalidate desiredDisplayModeSpecs when SF specs are invalid

When a display change occurs we change if the SF display specs
are different than the current. However if the SF base mode is
no longer supported we don't invalidate. This CL fixes that and
adds a test for the broken scenario.

In order to test this scenario we introduce SurfaceControlProxy,
which we mock in the unit test. This is needed because the
code under test is accessing SurfaceControl from multiple threads
and we mock static method only works for the test thread.

Bug: 179926030
Test: atest LocalDisplayAdapterTest
Change-Id: I69d4dbb1e4ae59a6a38f35d2497a85cece9f4875
parent 463ad8ee
Loading
Loading
Loading
Loading
+96 −30
Original line number Diff line number Diff line
@@ -38,6 +38,7 @@ import android.view.DisplayEventReceiver;
import android.view.RoundedCorners;
import android.view.SurfaceControl;

import com.android.internal.annotations.VisibleForTesting;
import com.android.internal.display.BrightnessSynchronizer;
import com.android.internal.os.BackgroundThread;
import com.android.internal.util.function.pooled.PooledLambda;
@@ -72,6 +73,7 @@ final class LocalDisplayAdapter extends DisplayAdapter {

    private final Injector mInjector;

    private final SurfaceControlProxy mSurfaceControlProxy;

    // Called with SyncRoot lock held.
    public LocalDisplayAdapter(DisplayManagerService.SyncRoot syncRoot,
@@ -79,10 +81,12 @@ final class LocalDisplayAdapter extends DisplayAdapter {
        this(syncRoot, context, handler, listener, new Injector());
    }

    @VisibleForTesting
    LocalDisplayAdapter(DisplayManagerService.SyncRoot syncRoot,
            Context context, Handler handler, Listener listener, Injector injector) {
        super(syncRoot, context, handler, listener, TAG);
        mInjector = injector;
        mSurfaceControlProxy = mInjector.getSurfaceControlProxy();
    }

    @Override
@@ -92,22 +96,23 @@ final class LocalDisplayAdapter extends DisplayAdapter {
        mInjector.setDisplayEventListenerLocked(getHandler().getLooper(),
                new LocalDisplayEventListener());

        for (long physicalDisplayId : SurfaceControl.getPhysicalDisplayIds()) {
        for (long physicalDisplayId : mSurfaceControlProxy.getPhysicalDisplayIds()) {
            tryConnectDisplayLocked(physicalDisplayId);
        }
    }

    private void tryConnectDisplayLocked(long physicalDisplayId) {
        final IBinder displayToken = SurfaceControl.getPhysicalDisplayToken(physicalDisplayId);
        final IBinder displayToken =
                mSurfaceControlProxy.getPhysicalDisplayToken(physicalDisplayId);
        if (displayToken != null) {
            SurfaceControl.StaticDisplayInfo staticInfo =
                    SurfaceControl.getStaticDisplayInfo(displayToken);
                    mSurfaceControlProxy.getStaticDisplayInfo(displayToken);
            if (staticInfo == null) {
                Slog.w(TAG, "No valid static info found for display device " + physicalDisplayId);
                return;
            }
            SurfaceControl.DynamicDisplayInfo dynamicInfo =
                    SurfaceControl.getDynamicDisplayInfo(displayToken);
                    mSurfaceControlProxy.getDynamicDisplayInfo(displayToken);
            if (dynamicInfo == null) {
                Slog.w(TAG, "No valid dynamic info found for display device " + physicalDisplayId);
                return;
@@ -131,7 +136,7 @@ final class LocalDisplayAdapter extends DisplayAdapter {
                dynamicInfo.activeColorMode = Display.COLOR_MODE_INVALID;
            }
            SurfaceControl.DesiredDisplayModeSpecs modeSpecs =
                    SurfaceControl.getDesiredDisplayModeSpecs(displayToken);
                    mSurfaceControlProxy.getDesiredDisplayModeSpecs(displayToken);
            LocalDisplayDevice device = mDevices.get(physicalDisplayId);
            if (device == null) {
                // Display was added.
@@ -222,7 +227,8 @@ final class LocalDisplayAdapter extends DisplayAdapter {
            mIsDefaultDisplay = isDefaultDisplay;
            updateDisplayPropertiesLocked(staticDisplayInfo, dynamicInfo, modeSpecs);
            mSidekickInternal = LocalServices.getService(SidekickInternal.class);
            mBacklightAdapter = new BacklightAdapter(displayToken, isDefaultDisplay);
            mBacklightAdapter = new BacklightAdapter(displayToken, isDefaultDisplay,
                    mSurfaceControlProxy);
            mAllmSupported = SurfaceControl.getAutoLowLatencyModeSupport(displayToken);
            mGameContentTypeSupported = SurfaceControl.getGameContentTypeSupport(displayToken);
            mDisplayDeviceConfig = null;
@@ -338,8 +344,8 @@ final class LocalDisplayAdapter extends DisplayAdapter {
                // If we can't map the defaultMode index to a mode, then the physical display
                // modes must have changed, and the code below for handling changes to the
                // list of available modes will take care of updating display mode specs.
                if (activeBaseMode != NO_DISPLAY_MODE_ID) {
                    if (mDisplayModeSpecs.baseModeId != activeBaseMode
                if (activeBaseMode == NO_DISPLAY_MODE_ID
                        || mDisplayModeSpecs.baseModeId != activeBaseMode
                        || mDisplayModeSpecs.primaryRefreshRateRange.min
                                != modeSpecs.primaryRefreshRateMin
                        || mDisplayModeSpecs.primaryRefreshRateRange.max
@@ -352,7 +358,6 @@ final class LocalDisplayAdapter extends DisplayAdapter {
                    sendTraversalRequestLocked();
                }
            }
            }

            boolean recordsChanged = records.size() != mSupportedModes.size() || modesAdded;
            // If the records haven't changed then we're done here.
@@ -757,7 +762,7 @@ final class LocalDisplayAdapter extends DisplayAdapter {
                                + "id=" + physicalDisplayId
                                + ", state=" + Display.stateToString(state) + ")");
                        try {
                            SurfaceControl.setDisplayPowerMode(token, mode);
                            mSurfaceControlProxy.setDisplayPowerMode(token, mode);
                            Trace.traceCounter(Trace.TRACE_TAG_POWER, "DisplayPowerMode", mode);
                        } finally {
                            Trace.traceEnd(Trace.TRACE_TAG_POWER);
@@ -885,9 +890,10 @@ final class LocalDisplayAdapter extends DisplayAdapter {
                SurfaceControl.DesiredDisplayModeSpecs modeSpecs) {
            // Do not lock when calling these SurfaceControl methods because they are sync
            // operations that may block for a while when setting display power mode.
            SurfaceControl.setDesiredDisplayModeSpecs(displayToken, modeSpecs);
            final int sfActiveModeId =
                    SurfaceControl.getDynamicDisplayInfo(displayToken).activeDisplayModeId;
            mSurfaceControlProxy.setDesiredDisplayModeSpecs(displayToken, modeSpecs);

            final int sfActiveModeId = mSurfaceControlProxy
                    .getDynamicDisplayInfo(displayToken).activeDisplayModeId;
            synchronized (getSyncRoot()) {
                if (updateActiveModeLocked(sfActiveModeId)) {
                    updateDeviceInfoLocked();
@@ -955,7 +961,7 @@ final class LocalDisplayAdapter extends DisplayAdapter {
        private void requestColorModeAsync(IBinder displayToken, int colorMode) {
            // Do not lock when calling this SurfaceControl method because it is a sync operation
            // that may block for a while when setting display power mode.
            SurfaceControl.setActiveColorMode(displayToken, colorMode);
            mSurfaceControlProxy.setActiveColorMode(displayToken, colorMode);
            synchronized (getSyncRoot()) {
                updateDeviceInfoLocked();
            }
@@ -975,7 +981,7 @@ final class LocalDisplayAdapter extends DisplayAdapter {
                return;
            }

            SurfaceControl.setAutoLowLatencyMode(getDisplayTokenLocked(), on);
            mSurfaceControlProxy.setAutoLowLatencyMode(getDisplayTokenLocked(), on);
        }

        @Override
@@ -992,7 +998,7 @@ final class LocalDisplayAdapter extends DisplayAdapter {
                return;
            }

            SurfaceControl.setGameContentType(getDisplayTokenLocked(), on);
            mSurfaceControlProxy.setGameContentType(getDisplayTokenLocked(), on);
        }

        @Override
@@ -1136,6 +1142,9 @@ final class LocalDisplayAdapter extends DisplayAdapter {
        public void setDisplayEventListenerLocked(Looper looper, DisplayEventListener listener) {
            mReceiver = new ProxyDisplayEventReceiver(looper, listener);
        }
        public SurfaceControlProxy getSurfaceControlProxy() {
            return new SurfaceControlProxy();
        }
    }

    public interface DisplayEventListener {
@@ -1227,10 +1236,65 @@ final class LocalDisplayAdapter extends DisplayAdapter {
        }
    }

    @VisibleForTesting
    static class SurfaceControlProxy {
        public SurfaceControl.DynamicDisplayInfo getDynamicDisplayInfo(IBinder token) {
            return SurfaceControl.getDynamicDisplayInfo(token);
        }

        public long[] getPhysicalDisplayIds() {
            return SurfaceControl.getPhysicalDisplayIds();
        }

        public IBinder getPhysicalDisplayToken(long physicalDisplayId) {
            return SurfaceControl.getPhysicalDisplayToken(physicalDisplayId);
        }

        public SurfaceControl.StaticDisplayInfo getStaticDisplayInfo(IBinder displayToken) {
            return SurfaceControl.getStaticDisplayInfo(displayToken);
        }

        public SurfaceControl.DesiredDisplayModeSpecs getDesiredDisplayModeSpecs(
                IBinder displayToken) {
            return SurfaceControl.getDesiredDisplayModeSpecs(displayToken);
        }

        public boolean setDesiredDisplayModeSpecs(IBinder token,
                SurfaceControl.DesiredDisplayModeSpecs specs) {
            return SurfaceControl.setDesiredDisplayModeSpecs(token, specs);
        }

        public void setDisplayPowerMode(IBinder displayToken, int mode) {
            SurfaceControl.setDisplayPowerMode(displayToken, mode);
        }

        public boolean setActiveColorMode(IBinder displayToken, int colorMode) {
            return SurfaceControl.setActiveColorMode(displayToken, colorMode);
        }

        public void setAutoLowLatencyMode(IBinder displayToken, boolean on) {
            SurfaceControl.setAutoLowLatencyMode(displayToken, on);

        }

        public void setGameContentType(IBinder displayToken, boolean on) {
            SurfaceControl.setGameContentType(displayToken, on);
        }

        public boolean getDisplayBrightnessSupport(IBinder displayToken) {
            return SurfaceControl.getDisplayBrightnessSupport(displayToken);
        }

        public boolean setDisplayBrightness(IBinder displayToken, float brightness) {
            return SurfaceControl.setDisplayBrightness(displayToken, brightness);
        }
    }

    static class BacklightAdapter {
        private final IBinder mDisplayToken;
        private final LogicalLight mBacklight;
        private final boolean mUseSurfaceControlBrightness;
        private final SurfaceControlProxy mSurfaceControlProxy;

        private boolean mForceSurfaceControl = false;

@@ -1238,11 +1302,13 @@ final class LocalDisplayAdapter extends DisplayAdapter {
         * @param displayToken Token for display associated with this backlight.
         * @param isDefaultDisplay {@code true} if it is the default display.
         */
        BacklightAdapter(IBinder displayToken, boolean isDefaultDisplay) {
        BacklightAdapter(IBinder displayToken, boolean isDefaultDisplay,
                SurfaceControlProxy surfaceControlProxy) {
            mDisplayToken = displayToken;
            mSurfaceControlProxy = surfaceControlProxy;

            mUseSurfaceControlBrightness =
                    SurfaceControl.getDisplayBrightnessSupport(mDisplayToken);
            mUseSurfaceControlBrightness = mSurfaceControlProxy
                    .getDisplayBrightnessSupport(mDisplayToken);

            if (!mUseSurfaceControlBrightness && isDefaultDisplay) {
                LightsManager lights = LocalServices.getService(LightsManager.class);
@@ -1254,7 +1320,7 @@ final class LocalDisplayAdapter extends DisplayAdapter {

        void setBrightness(float brightness) {
            if (mUseSurfaceControlBrightness || mForceSurfaceControl) {
                SurfaceControl.setDisplayBrightness(mDisplayToken, brightness);
                mSurfaceControlProxy.setDisplayBrightness(mDisplayToken, brightness);
            } else if (mBacklight != null) {
                mBacklight.setBrightness(brightness);
            }
+93 −21
Original line number Diff line number Diff line
@@ -25,8 +25,10 @@ import static com.google.common.truth.Truth.assertThat;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;
import static org.mockito.ArgumentMatchers.anyFloat;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.when;

import android.content.Context;
import android.content.res.Resources;
@@ -94,11 +96,13 @@ public class LocalDisplayAdapterTest {

    private Injector mInjector;

    @Mock
    private LocalDisplayAdapter.SurfaceControlProxy mSurfaceControlProxy;

    @Before
    public void setUp() throws Exception {
        mMockitoSession = mockitoSession()
                .initMocks(this)
                .mockStatic(SurfaceControl.class)
                .strictness(Strictness.LENIENT)
                .startMocking();
        mHandler = new Handler(Looper.getMainLooper());
@@ -227,6 +231,7 @@ public class LocalDisplayAdapterTest {
        setUpDisplay(display);
        mInjector.getTransmitter().sendHotplug(display, /* connected */ true);
        waitForHandlerToComplete(mHandler, HANDLER_WAIT_MS);
        assertTrue(mListener.traversalRequested);
        assertThat(mListener.changedDisplays.size()).isGreaterThan(0);

        // Verify the supported modes are updated accordingly.
@@ -333,6 +338,7 @@ public class LocalDisplayAdapterTest {
        mInjector.getTransmitter().sendHotplug(display, /* connected */ true);
        waitForHandlerToComplete(mHandler, HANDLER_WAIT_MS);

        assertTrue(mListener.traversalRequested);
        assertThat(mListener.addedDisplays.size()).isEqualTo(1);
        assertThat(mListener.changedDisplays.size()).isEqualTo(1);

@@ -380,6 +386,7 @@ public class LocalDisplayAdapterTest {
        mInjector.getTransmitter().sendHotplug(display, /* connected */ true);
        waitForHandlerToComplete(mHandler, HANDLER_WAIT_MS);

        assertTrue(mListener.traversalRequested);
        assertThat(mListener.addedDisplays.size()).isEqualTo(1);
        assertThat(mListener.changedDisplays.size()).isEqualTo(1);

@@ -418,6 +425,7 @@ public class LocalDisplayAdapterTest {
        mInjector.getTransmitter().sendHotplug(display, /* connected */ true);
        waitForHandlerToComplete(mHandler, HANDLER_WAIT_MS);

        assertTrue(mListener.traversalRequested);
        assertThat(mListener.addedDisplays.size()).isEqualTo(1);
        assertThat(mListener.changedDisplays.size()).isEqualTo(1);

@@ -454,6 +462,7 @@ public class LocalDisplayAdapterTest {
        mInjector.getTransmitter().sendHotplug(display, /* connected */ true);
        waitForHandlerToComplete(mHandler, HANDLER_WAIT_MS);

        assertTrue(mListener.traversalRequested);
        assertThat(mListener.addedDisplays.size()).isEqualTo(1);
        assertThat(mListener.changedDisplays.size()).isEqualTo(1);

@@ -480,6 +489,30 @@ public class LocalDisplayAdapterTest {
        mAdapter.registerLocked();
        waitForHandlerToComplete(mHandler, HANDLER_WAIT_MS);

        assertThat(mListener.addedDisplays.size()).isEqualTo(1);
        DisplayDevice displayDevice = mListener.addedDisplays.get(0);

        int baseModeId = Arrays.stream(displayDevice.getDisplayDeviceInfoLocked().supportedModes)
                .filter(mode -> mode.getRefreshRate() == 60f)
                .findFirst()
                .get()
                .getModeId();

        displayDevice.setDesiredDisplayModeSpecsLocked(
                new DisplayModeDirector.DesiredDisplayModeSpecs(
                        /*baseModeId*/ baseModeId,
                        /*allowGroupSwitching*/ false,
                        new DisplayModeDirector.RefreshRateRange(60f, 60f),
                        new DisplayModeDirector.RefreshRateRange(60f, 60f)
                ));
        verify(mSurfaceControlProxy).setDesiredDisplayModeSpecs(display.token,
                new SurfaceControl.DesiredDisplayModeSpecs(
                        /* baseModeId */ 0,
                        /* allowGroupSwitching */ false,
                        /* primaryRange */ 60f, 60f,
                        /* appRange */ 60f, 60f
                ));

        // Change the display
        display.dynamicInfo.supportedDisplayModes = new SurfaceControl.DisplayMode[]{
                createFakeDisplayMode(2, 1920, 1080, 60f)
@@ -489,36 +522,64 @@ public class LocalDisplayAdapterTest {
        display.desiredDisplayModeSpecs.defaultMode = 1;

        setUpDisplay(display);
        updateAvailableDisplays();
        mInjector.getTransmitter().sendHotplug(display, /* connected */ true);
        waitForHandlerToComplete(mHandler, HANDLER_WAIT_MS);

        assertTrue(mListener.traversalRequested);

        displayDevice.applyPendingDisplayDeviceInfoChangesLocked();

        baseModeId = displayDevice.getDisplayDeviceInfoLocked().supportedModes[0].getModeId();

        // The traversal request will call setDesiredDisplayModeSpecsLocked on the display device
        displayDevice.setDesiredDisplayModeSpecsLocked(
                new DisplayModeDirector.DesiredDisplayModeSpecs(
                        /*baseModeId*/ baseModeId,
                        /*allowGroupSwitching*/ false,
                        new DisplayModeDirector.RefreshRateRange(60f, 60f),
                        new DisplayModeDirector.RefreshRateRange(60f, 60f)
                ));

        waitForHandlerToComplete(mHandler, HANDLER_WAIT_MS);

        // Verify that this will reapply the desired modes.
        verify(mSurfaceControlProxy).setDesiredDisplayModeSpecs(display.token,
                new SurfaceControl.DesiredDisplayModeSpecs(
                        /* baseModeId */ 2,
                        /* allowGroupSwitching */ false,
                        /* primaryRange */ 60f, 60f,
                        /* appRange */ 60f, 60f
                ));
    }

    @Test
    public void testBacklightAdapter_withSurfaceControlSupport() {
        final Binder displayToken = new Binder();
        doReturn(true).when(() -> SurfaceControl.getDisplayBrightnessSupport(displayToken));

        when(mSurfaceControlProxy.getDisplayBrightnessSupport(displayToken)).thenReturn(true);

        // Test as default display
        BacklightAdapter ba = new BacklightAdapter(displayToken, true /*isDefault*/);
        BacklightAdapter ba = new BacklightAdapter(displayToken, true /*isDefault*/,
                mSurfaceControlProxy);
        ba.setBrightness(0.514f);
        verify(() -> SurfaceControl.setDisplayBrightness(displayToken, 0.514f));
        verify(mSurfaceControlProxy).setDisplayBrightness(displayToken, 0.514f);

        // Test as not default display
        BacklightAdapter ba2 = new BacklightAdapter(displayToken,
                false /*isDefault*/);
        BacklightAdapter ba2 = new BacklightAdapter(displayToken, false /*isDefault*/,
                mSurfaceControlProxy);
        ba2.setBrightness(0.323f);
        verify(() -> SurfaceControl.setDisplayBrightness(displayToken, 0.323f));
        verify(mSurfaceControlProxy).setDisplayBrightness(displayToken, 0.323f);
    }

    @Test
    public void testBacklightAdapter_withoutSourceControlSupport_defaultDisplay() {
        final Binder displayToken = new Binder();
        doReturn(false).when(() -> SurfaceControl.getDisplayBrightnessSupport(displayToken));
        when(mSurfaceControlProxy.getDisplayBrightnessSupport(displayToken)).thenReturn(false);
        doReturn(mMockedBacklight).when(mMockedLightsManager)
                .getLight(LightsManager.LIGHT_ID_BACKLIGHT);

        BacklightAdapter ba = new BacklightAdapter(displayToken, true /*isDefault*/);
        BacklightAdapter ba = new BacklightAdapter(displayToken, true /*isDefault*/,
                mSurfaceControlProxy);
        ba.setBrightness(0.123f);
        verify(mMockedBacklight).setBrightness(0.123f);
    }
@@ -526,11 +587,12 @@ public class LocalDisplayAdapterTest {
    @Test
    public void testBacklightAdapter_withoutSourceControlSupport_nonDefaultDisplay() {
        final Binder displayToken = new Binder();
        doReturn(false).when(() -> SurfaceControl.getDisplayBrightnessSupport(displayToken));
        when(mSurfaceControlProxy.getDisplayBrightnessSupport(displayToken)).thenReturn(false);
        doReturn(mMockedBacklight).when(mMockedLightsManager)
                .getLight(LightsManager.LIGHT_ID_BACKLIGHT);

        BacklightAdapter ba = new BacklightAdapter(displayToken, false /*isDefault*/);
        BacklightAdapter ba = new BacklightAdapter(displayToken, false /*isDefault*/,
                mSurfaceControlProxy);
        ba.setBrightness(0.456f);

        // Adapter does not forward any brightness in this case.
@@ -618,13 +680,14 @@ public class LocalDisplayAdapterTest {

    private void setUpDisplay(FakeDisplay display) {
        mAddresses.add(display.address);
        doReturn(display.token).when(() ->
                SurfaceControl.getPhysicalDisplayToken(display.address.getPhysicalDisplayId()));
        doReturn(display.info).when(() -> SurfaceControl.getStaticDisplayInfo(display.token));
        doReturn(display.dynamicInfo).when(
                () -> SurfaceControl.getDynamicDisplayInfo(display.token));
        doReturn(display.desiredDisplayModeSpecs)
                .when(() -> SurfaceControl.getDesiredDisplayModeSpecs(display.token));
        when(mSurfaceControlProxy.getPhysicalDisplayToken(display.address.getPhysicalDisplayId()))
                .thenReturn(display.token);
        when(mSurfaceControlProxy.getStaticDisplayInfo(display.token))
                .thenReturn(display.info);
        when(mSurfaceControlProxy.getDynamicDisplayInfo(display.token))
                .thenReturn(display.dynamicInfo);
        when(mSurfaceControlProxy.getDesiredDisplayModeSpecs(display.token))
                .thenReturn(display.desiredDisplayModeSpecs);
    }

    private void updateAvailableDisplays() {
@@ -634,7 +697,7 @@ public class LocalDisplayAdapterTest {
            ids[i] = address.getPhysicalDisplayId();
            i++;
        }
        doReturn(ids).when(() -> SurfaceControl.getPhysicalDisplayIds());
        when(mSurfaceControlProxy.getPhysicalDisplayIds()).thenReturn(ids);
    }

    private static DisplayAddress.Physical createDisplayAddress(int port) {
@@ -649,7 +712,7 @@ public class LocalDisplayAdapterTest {

    private static SurfaceControl.DisplayMode createFakeDisplayMode(int id, int width, int height,
            float refreshRate) {
        return createFakeDisplayMode(id, width, height, refreshRate, 0);
        return createFakeDisplayMode(id, width, height, refreshRate, /* group */ 0);
    }

    private static SurfaceControl.DisplayMode createFakeDisplayMode(int id, int width, int height,
@@ -702,14 +765,21 @@ public class LocalDisplayAdapterTest {
                LocalDisplayAdapter.DisplayEventListener listener) {
            mTransmitter = new HotplugTransmitter(looper, listener);
        }

        public HotplugTransmitter getTransmitter() {
            return mTransmitter;
        }

        @Override
        public LocalDisplayAdapter.SurfaceControlProxy getSurfaceControlProxy() {
            return mSurfaceControlProxy;
        }
    }

    private class TestListener implements DisplayAdapter.Listener {
        public ArrayList<DisplayDevice> addedDisplays = new ArrayList<>();
        public ArrayList<DisplayDevice> changedDisplays = new ArrayList<>();
        public boolean traversalRequested = false;

        @Override
        public void onDisplayDeviceEvent(DisplayDevice device, int event) {
@@ -722,6 +792,8 @@ public class LocalDisplayAdapterTest {

        @Override
        public void onTraversalRequested() {
            traversalRequested = true;
        }
    }

}