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

Commit 5b764518 authored by Michael Wright's avatar Michael Wright
Browse files

Refine DisplayDeviceInfo#diff. 

Currently its diffing is too coarse, leading to confusing double
messages and non-specific logs for common operations like refresh rate
changes. Refine the diff to provide more detail so we can better log
what's going on on the device.

Fixes: 330169828
Flag: EXEMPT logging bug fix
Test: atest DisplayDeviceInfoTest
Change-Id: I6acc91d77b99ef1ae5cf52ba031b88fa70720b24
parent d5adf387

services/core/java/com/android/server/display/DisplayDeviceInfo.java

+42 −11
Original line number Diff line number Diff line
@@ -211,25 +211,47 @@ final class DisplayDeviceInfo { 
     */

    public static final int TOUCH_VIRTUAL = 3;



    /**

     * Diff result: Other fields differ.

     */

    public static final int DIFF_OTHER = 1 << 0;



    /**

     * Diff result: The {@link #state} or {@link #committedState} fields differ.

     */

    public static final int DIFF_STATE = 1 << 0;

    public static final int DIFF_STATE = 1 << 1;



    /**

     * Diff result: Other fields differ.

     * Diff result: The committed state differs. Note this is slightly different from the state,

     * which is what most of the device should care about.

     */

    public static final int DIFF_OTHER = 1 << 1;

    public static final int DIFF_COMMITTED_STATE = 1 << 2;



    /**

     * Diff result: The color mode fields differ.

     */

    public static final int DIFF_COLOR_MODE = 1 << 2;

    public static final int DIFF_COLOR_MODE = 1 << 3;



    /**

     * Diff result: The hdr/sdr ratio differs

     */

    public static final int DIFF_HDR_SDR_RATIO = 1 << 3;

    public static final int DIFF_HDR_SDR_RATIO = 1 << 4;



    /**

     * Diff result: The rotation differs

     */

    public static final int DIFF_ROTATION = 1 << 5;



    /**

     * Diff result: The render timings. Note this could be any of {@link #renderFrameRate},

     * {@link #presentationDeadlineNanos}, or {@link #appVsyncOffsetNanos}.

     */

    public static final int DIFF_RENDER_TIMINGS = 1 << 6;



    /**

     * Diff result: The mode ID differs.

     */

    public static final int DIFF_MODE_ID = 1 << 7;



    /**

     * Diff result: Catch-all for "everything changed"
@@ -462,21 +484,33 @@ final class DisplayDeviceInfo { 
     */

    public int diff(DisplayDeviceInfo other) {

        int diff = 0;

        if (state != other.state || committedState != other.committedState) {

        if (state != other.state) {

            diff |= DIFF_STATE;

        }

        if (committedState != other.committedState) {

            diff |= DIFF_COMMITTED_STATE;

        }

        if (colorMode != other.colorMode) {

            diff |= DIFF_COLOR_MODE;

        }

        if (!BrightnessSynchronizer.floatEquals(hdrSdrRatio, other.hdrSdrRatio)) {

            diff |= DIFF_HDR_SDR_RATIO;

        }

        if (rotation != other.rotation) {

            diff |= DIFF_ROTATION;

        }

        if (renderFrameRate != other.renderFrameRate

                || presentationDeadlineNanos != other.presentationDeadlineNanos

                || appVsyncOffsetNanos != other.appVsyncOffsetNanos) {

            diff |= DIFF_RENDER_TIMINGS;

        }

        if (modeId != other.modeId) {

            diff |= DIFF_MODE_ID;

        }

        if (!Objects.equals(name, other.name)

                || !Objects.equals(uniqueId, other.uniqueId)

                || width != other.width

                || height != other.height

                || modeId != other.modeId

                || renderFrameRate != other.renderFrameRate

                || defaultModeId != other.defaultModeId

                || userPreferredModeId != other.userPreferredModeId

                || !Arrays.equals(supportedModes, other.supportedModes)
@@ -487,12 +521,9 @@ final class DisplayDeviceInfo { 
                || densityDpi != other.densityDpi

                || xDpi != other.xDpi

                || yDpi != other.yDpi

                || appVsyncOffsetNanos != other.appVsyncOffsetNanos

                || presentationDeadlineNanos != other.presentationDeadlineNanos

                || flags != other.flags

                || !Objects.equals(displayCutout, other.displayCutout)

                || touch != other.touch

                || rotation != other.rotation

                || type != other.type

                || !Objects.equals(address, other.address)

                || !Objects.equals(deviceProductInfo, other.deviceProductInfo)

services/core/java/com/android/server/display/DisplayDeviceRepository.java

+15 −0
Original line number Diff line number Diff line
@@ -21,6 +21,7 @@ import android.os.Trace; 
import android.util.Slog;

import android.view.Display;

import android.view.DisplayAddress;

import android.view.Surface;



import com.android.internal.annotations.GuardedBy;

import com.android.server.display.DisplayManagerService.SyncRoot;
@@ -179,6 +180,20 @@ 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 == DisplayDeviceInfo.DIFF_ROTATION) {

                Slog.i(TAG, "Display device rotated: \"" + info.name

                        + "\", " + Surface.rotationToString(info.rotation));

            } else if (diff

                    == (DisplayDeviceInfo.DIFF_MODE_ID | DisplayDeviceInfo.DIFF_RENDER_TIMINGS)) {

                Slog.i(TAG, "Display device changed render timings: \"" + info.name

                        + "\", renderFrameRate=" + info.renderFrameRate

                        + ", presentationDeadlineNanos=" + info.presentationDeadlineNanos

                        + ", appVsyncOffsetNanos=" + info.appVsyncOffsetNanos);

            } else if (diff == DisplayDeviceInfo.DIFF_COMMITTED_STATE) {

                if (DEBUG) {

                    Slog.i(TAG, "Display device changed committed state: \"" + info.name

                            + "\", " + Display.stateToString(info.committedState));

                }

            } else if (diff != DisplayDeviceInfo.DIFF_HDR_SDR_RATIO) {

                Slog.i(TAG, "Display device changed: " + info);

            }

services/tests/displayservicetests/src/com/android/server/display/DisplayDeviceInfoTest.java

0 → 100644
+148 −0
Original line number Diff line number Diff line 
/*

 * Copyright (C) 2024 The Android Open Source Project

 *

 * Licensed under the Apache License, Version 2.0 (the "License");

 * you may not use this file except in compliance with the License.

 * You may obtain a copy of the License at

 *

 *      http://www.apache.org/licenses/LICENSE-2.0

 *

 * Unless required by applicable law or agreed to in writing, software

 * distributed under the License is distributed on an "AS IS" BASIS,

 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.

 * See the License for the specific language governing permissions and

 * limitations under the License.

 */



package com.android.server.display;



import static com.android.server.display.DisplayDeviceInfo.DIFF_COLOR_MODE;

import static com.android.server.display.DisplayDeviceInfo.DIFF_COMMITTED_STATE;

import static com.android.server.display.DisplayDeviceInfo.DIFF_HDR_SDR_RATIO;

import static com.android.server.display.DisplayDeviceInfo.DIFF_MODE_ID;

import static com.android.server.display.DisplayDeviceInfo.DIFF_RENDER_TIMINGS;

import static com.android.server.display.DisplayDeviceInfo.DIFF_ROTATION;

import static com.android.server.display.DisplayDeviceInfo.DIFF_STATE;



import static com.google.common.truth.Truth.assertThat;



import android.view.Display;

import android.view.Surface;



import androidx.test.filters.SmallTest;

import androidx.test.runner.AndroidJUnit4;



import org.junit.Test;

import org.junit.runner.RunWith;



@SmallTest

@RunWith(AndroidJUnit4.class)

public class DisplayDeviceInfoTest {



    @Test

    public void testDiff_noChange() {

        var oldDdi = createInfo();

        var newDdi = createInfo();



        assertThat(oldDdi.diff(newDdi)).isEqualTo(0);

    }



    @Test

    public void testDiff_state() {

        var oldDdi = createInfo();

        var newDdi = createInfo();



        newDdi.state = Display.STATE_VR;

        assertThat(oldDdi.diff(newDdi)).isEqualTo(DIFF_STATE);

    }



    @Test

    public void testDiff_committedState() {

        var oldDdi = createInfo();

        var newDdi = createInfo();



        newDdi.committedState = Display.STATE_UNKNOWN;

        assertThat(oldDdi.diff(newDdi)).isEqualTo(DIFF_COMMITTED_STATE);

    }



    @Test

    public void testDiff_colorMode() {

        var oldDdi = createInfo();

        var newDdi = createInfo();



        newDdi.colorMode = Display.COLOR_MODE_DISPLAY_P3;

        assertThat(oldDdi.diff(newDdi)).isEqualTo(DIFF_COLOR_MODE);

    }



    @Test

    public void testDiff_hdrSdrRatio() {

        var oldDdi = createInfo();

        var newDdi = createInfo();



        /* First change new ratio to non-NaN */

        newDdi.hdrSdrRatio = 2.3f;

        assertThat(oldDdi.diff(newDdi)).isEqualTo(DIFF_HDR_SDR_RATIO);



        /* Then change old to be non-NaN and also distinct */

        oldDdi.hdrSdrRatio = 1.1f;

        assertThat(oldDdi.diff(newDdi)).isEqualTo(DIFF_HDR_SDR_RATIO);



        /* Now make the new one NaN and the old one non-NaN */

        newDdi.hdrSdrRatio = Float.NaN;

        assertThat(oldDdi.diff(newDdi)).isEqualTo(DIFF_HDR_SDR_RATIO);

    }



    @Test

    public void testDiff_rotation() {

        var oldDdi = createInfo();

        var newDdi = createInfo();



        newDdi.rotation = Surface.ROTATION_270;

        assertThat(oldDdi.diff(newDdi)).isEqualTo(DIFF_ROTATION);

    }



    @Test

    public void testDiff_frameRate() {

        var oldDdi = createInfo();

        var newDdi = createInfo();



        newDdi.renderFrameRate = 123.4f;

        assertThat(oldDdi.diff(newDdi)).isEqualTo(DIFF_RENDER_TIMINGS);

        newDdi.renderFrameRate = oldDdi.renderFrameRate;



        newDdi.appVsyncOffsetNanos = 31222221;

        assertThat(oldDdi.diff(newDdi)).isEqualTo(DIFF_RENDER_TIMINGS);

        newDdi.appVsyncOffsetNanos = oldDdi.appVsyncOffsetNanos;



        newDdi.presentationDeadlineNanos = 23000000;

        assertThat(oldDdi.diff(newDdi)).isEqualTo(DIFF_RENDER_TIMINGS);

    }



    @Test

    public void testDiff_modeId() {

        var oldDdi = createInfo();

        var newDdi = createInfo();



        newDdi.modeId = 9;

        assertThat(oldDdi.diff(newDdi)).isEqualTo(DIFF_MODE_ID);

    }



    private static DisplayDeviceInfo createInfo() {

        var ddi = new DisplayDeviceInfo();

        ddi.name = "TestDisplayDeviceInfo";

        ddi.uniqueId = "test:51651561321";

        ddi.width = 671;

        ddi.height = 483;

        ddi.modeId = 2;

        ddi.renderFrameRate = 68.9f;

        ddi.supportedModes = new Display.Mode[] {

                new Display.Mode.Builder().setRefreshRate(68.9f).setResolution(671, 483).build(),

        };

        ddi.appVsyncOffsetNanos = 6233332;

        ddi.presentationDeadlineNanos = 11500000;

        ddi.rotation = Surface.ROTATION_90;

        ddi.state = Display.STATE_ON;

        ddi.committedState = Display.STATE_DOZE_SUSPEND;

        return ddi;

    }

}