From 7a9bc220c70745cd3689d51cd131efde86f078d7 Mon Sep 17 00:00:00 2001 From: Assam Boudjelthia Date: Thu, 30 Nov 2023 17:41:37 +0200 Subject: [PATCH] Android: fix and simplify the orientation change logic Move the orientation change handling to the display manager and call it from the relevant places to repeated and scattered code for the same functionality. Bring back part of 072387edecb2269097821e35f1f232da6c657650 which checks discard a resize event that's not valid that reports not up to date layout size. If DisplayManager.DisplayListener.onDisplay() initiates the orientation change, handle it immediately if we don't expect the size to change, i.e. if the orientation is changing from portrait to inverted portrait and vise versa for landscape. Otherwise, expect the change to be initiated again shortly after from QtLayout.onSizeChanged(). Also, add improve the unit test of the orientation change, and test for the widget size after such changes to make sure QTBUG-94459 is not reached again. Fixes: QTBUG-119430 Task-number: QTBUG-115019 Task-number: QTBUG-94459 Change-Id: I5f060d91531af677ddf891f2af360d5f399e26e5 Reviewed-by: Qt CI Bot Reviewed-by: Ville Voutilainen --- .../qt/android/QtActivityDelegate.java | 8 +- .../qt/android/QtDisplayManager.java | 105 +++++++++--------- .../org/qtproject/qt/android/QtLayout.java | 54 ++++----- tests/auto/corelib/platform/android/BLACKLIST | 2 - .../corelib/platform/android/CMakeLists.txt | 1 + .../corelib/platform/android/tst_android.cpp | 26 ++++- 6 files changed, 103 insertions(+), 93 deletions(-) delete mode 100644 tests/auto/corelib/platform/android/BLACKLIST diff --git a/src/android/jar/src/org/qtproject/qt/android/QtActivityDelegate.java b/src/android/jar/src/org/qtproject/qt/android/QtActivityDelegate.java index 5ebce6dc35f..6a87c187be0 100644 --- a/src/android/jar/src/org/qtproject/qt/android/QtActivityDelegate.java +++ b/src/android/jar/src/org/qtproject/qt/android/QtActivityDelegate.java @@ -135,7 +135,7 @@ class QtActivityDelegate { m_layout = new QtLayout(m_activity); - m_displayManager = new QtDisplayManager(m_activity, m_layout); + m_displayManager = new QtDisplayManager(m_activity); m_displayManager.registerDisplayListener(); QtInputDelegate.KeyboardVisibilityListener keyboardVisibilityListener = @@ -179,11 +179,7 @@ class QtActivityDelegate m_activity.setContentView(m_layout, new ViewGroup.LayoutParams(ViewGroup.LayoutParams.MATCH_PARENT, ViewGroup.LayoutParams.MATCH_PARENT)); - - int rotation = m_activity.getWindowManager().getDefaultDisplay().getRotation(); - int nativeOrientation = QtDisplayManager.getNativeOrientation(m_activity, rotation); - m_layout.setNativeOrientation(nativeOrientation); - QtDisplayManager.handleOrientationChanged(rotation, nativeOrientation); + QtDisplayManager.handleOrientationChanges(m_activity, false); handleUiModeChange(m_activity.getResources().getConfiguration().uiMode & Configuration.UI_MODE_NIGHT_MASK); diff --git a/src/android/jar/src/org/qtproject/qt/android/QtDisplayManager.java b/src/android/jar/src/org/qtproject/qt/android/QtDisplayManager.java index b5347596f85..80c89526ecb 100644 --- a/src/android/jar/src/org/qtproject/qt/android/QtDisplayManager.java +++ b/src/android/jar/src/org/qtproject/qt/android/QtDisplayManager.java @@ -44,45 +44,30 @@ class QtDisplayManager { public static final int SYSTEM_UI_VISIBILITY_TRANSLUCENT = 2; private int m_systemUiVisibility = SYSTEM_UI_VISIBILITY_NORMAL; + private static int m_previousRotation = -1; + private DisplayManager.DisplayListener m_displayListener = null; private final Activity m_activity; - QtDisplayManager(Activity activity, QtLayout layout) + QtDisplayManager(Activity activity) { m_activity = activity; - initDisplayListener(layout); + initDisplayListener(); } - private void initDisplayListener(QtLayout layout) { + private void initDisplayListener() { m_displayListener = new DisplayManager.DisplayListener() { @Override public void onDisplayAdded(int displayId) { QtDisplayManager.handleScreenAdded(displayId); } - private boolean isSimilarRotation(int r1, int r2) { - return (r1 == r2) - || (r1 == Surface.ROTATION_0 && r2 == Surface.ROTATION_180) - || (r1 == Surface.ROTATION_180 && r2 == Surface.ROTATION_0) - || (r1 == Surface.ROTATION_90 && r2 == Surface.ROTATION_270) - || (r1 == Surface.ROTATION_270 && r2 == Surface.ROTATION_90); - } - @Override public void onDisplayChanged(int displayId) { + handleOrientationChanges(m_activity, false); Display display = (Build.VERSION.SDK_INT < Build.VERSION_CODES.R) ? m_activity.getWindowManager().getDefaultDisplay() : m_activity.getDisplay(); - int rotation = display != null ? display.getRotation() : Surface.ROTATION_0; - layout.setActivityDisplayRotation(rotation); - // Process orientation change only if it comes after the size - // change, or if the screen is rotated by 180 degrees. - // Otherwise it will be processed in QtLayout. - if (isSimilarRotation(rotation, layout.displayRotation())) { - QtDisplayManager.handleOrientationChanged(rotation, - getNativeOrientation(m_activity, rotation)); - } - float refreshRate = getRefreshRate(display); QtDisplayManager.handleRefreshRateChanged(refreshRate); QtDisplayManager.handleScreenChanged(displayId); @@ -95,6 +80,53 @@ class QtDisplayManager { }; } + private static boolean isSameSizeForOrientations(int r1, int r2) { + return (r1 == r2) || + (r1 == Surface.ROTATION_0 && r2 == Surface.ROTATION_180) + || (r1 == Surface.ROTATION_180 && r2 == Surface.ROTATION_0) + || (r1 == Surface.ROTATION_90 && r2 == Surface.ROTATION_270) + || (r1 == Surface.ROTATION_270 && r2 == Surface.ROTATION_90); + } + + static void handleOrientationChanges(Activity activity, boolean sizeChanged) + { + int currentRotation = getDisplayRotation(activity); + int nativeOrientation = getNativeOrientation(activity, currentRotation); + + if (m_previousRotation == currentRotation) + return; + + // If the the current and previous rotations are similar then QtLayout.onSizeChanged() + // might not be called, so we can already update the orientation, and rely on this to + // called again once the resize event is sent. + // Note: Android 10 emulator seems to not always send an event when the orientation + // changes, could be a bug in the emulator. + boolean noResizeNeeded = isSameSizeForOrientations(m_previousRotation, currentRotation); + if (m_previousRotation == -1 || sizeChanged || noResizeNeeded) { + QtDisplayManager.handleOrientationChanged(currentRotation, nativeOrientation); + m_previousRotation = currentRotation; + } + } + + private static int getDisplayRotation(Activity activity) { + Display display = Build.VERSION.SDK_INT < Build.VERSION_CODES.R ? + activity.getWindowManager().getDefaultDisplay() : + activity.getDisplay(); + + return display != null ? display.getRotation() : 0; + } + + private static int getNativeOrientation(Activity activity, int rotation) + { + int orientation = activity.getResources().getConfiguration().orientation; + boolean rot90 = (rotation == Surface.ROTATION_90 || rotation == Surface.ROTATION_270); + boolean isLandscape = (orientation == Configuration.ORIENTATION_LANDSCAPE); + if ((isLandscape && !rot90) || (!isLandscape && rot90)) + return Configuration.ORIENTATION_LANDSCAPE; + + return Configuration.ORIENTATION_PORTRAIT; + } + static float getRefreshRate(Display display) { return display != null ? display.getRefreshRate() : 60.0f; @@ -114,21 +146,6 @@ class QtDisplayManager { displayManager.unregisterDisplayListener(m_displayListener); } - public static int getNativeOrientation(Activity activity, int rotation) - { - int nativeOrientation; - - int orientation = activity.getResources().getConfiguration().orientation; - boolean rot90 = (rotation == Surface.ROTATION_90 || rotation == Surface.ROTATION_270); - boolean isLandscape = (orientation == Configuration.ORIENTATION_LANDSCAPE); - if ((isLandscape && !rot90) || (!isLandscape && rot90)) - nativeOrientation = Configuration.ORIENTATION_LANDSCAPE; - else - nativeOrientation = Configuration.ORIENTATION_PORTRAIT; - - return nativeOrientation; - } - public void setSystemUiVisibility(int systemUiVisibility) { if (m_systemUiVisibility == systemUiVisibility) @@ -283,20 +300,4 @@ class QtDisplayManager { width, height, xdpi, ydpi, scaledDensity, density, getRefreshRate(display)); } - - public static int getDisplayRotation(Activity activity) { - Display display; - if (Build.VERSION.SDK_INT < Build.VERSION_CODES.R) { - final WindowManager windowManager = activity.getWindowManager(); - display = windowManager.getDefaultDisplay(); - } else { - display = activity.getDisplay(); - } - - int newRotation = 0; - if (display != null) { - newRotation = display.getRotation(); - } - return newRotation; - } } diff --git a/src/android/jar/src/org/qtproject/qt/android/QtLayout.java b/src/android/jar/src/org/qtproject/qt/android/QtLayout.java index 8732a5f6121..2643a9afcf7 100644 --- a/src/android/jar/src/org/qtproject/qt/android/QtLayout.java +++ b/src/android/jar/src/org/qtproject/qt/android/QtLayout.java @@ -1,4 +1,4 @@ -// Copyright (C) 2022 The Qt Company Ltd. +// Copyright (C) 2023 The Qt Company Ltd. // Copyright (C) 2012 BogDan Vatra // SPDX-License-Identifier: LicenseRef-Qt-Commercial OR LGPL-3.0-only OR GPL-2.0-only OR GPL-3.0-only @@ -6,31 +6,15 @@ package org.qtproject.qt.android; import android.app.Activity; import android.content.Context; +import android.os.Build; import android.util.AttributeSet; +import android.util.DisplayMetrics; +import android.view.Display; import android.view.View; import android.view.ViewGroup; class QtLayout extends ViewGroup { - private int m_activityDisplayRotation = -1; - private int m_ownDisplayRotation = -1; - private int m_nativeOrientation = -1; - - public void setActivityDisplayRotation(int rotation) - { - m_activityDisplayRotation = rotation; - } - - public void setNativeOrientation(int orientation) - { - m_nativeOrientation = orientation; - } - - public int displayRotation() - { - return m_ownDisplayRotation; - } - public QtLayout(Context context) { super(context); @@ -53,18 +37,28 @@ class QtLayout extends ViewGroup if (activity == null) return; - QtDisplayManager.setApplicationDisplayMetrics(activity, w, h); + DisplayMetrics realMetrics = new DisplayMetrics(); + Display display = (Build.VERSION.SDK_INT < Build.VERSION_CODES.R) + ? activity.getWindowManager().getDefaultDisplay() + : activity.getDisplay(); - int newRotation = QtDisplayManager.getDisplayRotation(activity); - if (m_ownDisplayRotation != m_activityDisplayRotation - && newRotation == m_activityDisplayRotation) { - // If the saved rotation value does not match the one from the - // activity, it means that we got orientation change before size - // change, and the value was cached. So we need to notify about - // orientation change now. - QtDisplayManager.handleOrientationChanged(newRotation, m_nativeOrientation); + if (display == null) + return; + + display.getRealMetrics(realMetrics); + if ((realMetrics.widthPixels > realMetrics.heightPixels) != (w > h)) { + // This is an intermediate state during display rotation. + // The new size is still reported for old orientation, while + // realMetrics contain sizes for new orientation. Setting + // such parameters will produce inconsistent results, so + // we just skip them. + // We will have another onSizeChanged() with normal values + // a bit later. + return; } - m_ownDisplayRotation = newRotation; + + QtDisplayManager.setApplicationDisplayMetrics(activity, w, h); + QtDisplayManager.handleOrientationChanges(activity, true); } @Override diff --git a/tests/auto/corelib/platform/android/BLACKLIST b/tests/auto/corelib/platform/android/BLACKLIST deleted file mode 100644 index 8b5c0990f12..00000000000 --- a/tests/auto/corelib/platform/android/BLACKLIST +++ /dev/null @@ -1,2 +0,0 @@ -[orientationChange] -android diff --git a/tests/auto/corelib/platform/android/CMakeLists.txt b/tests/auto/corelib/platform/android/CMakeLists.txt index 0f9f670cb03..6b7e9b29017 100644 --- a/tests/auto/corelib/platform/android/CMakeLists.txt +++ b/tests/auto/corelib/platform/android/CMakeLists.txt @@ -18,6 +18,7 @@ qt_internal_add_test(tst_android Qt::CorePrivate Qt::Gui Qt::GuiPrivate + Qt::Widgets ) if(ANDROID) diff --git a/tests/auto/corelib/platform/android/tst_android.cpp b/tests/auto/corelib/platform/android/tst_android.cpp index 9a503fafc58..02204abcb5d 100644 --- a/tests/auto/corelib/platform/android/tst_android.cpp +++ b/tests/auto/corelib/platform/android/tst_android.cpp @@ -12,6 +12,8 @@ #include #include #include +#include +#include using namespace Qt::StringLiterals; @@ -305,19 +307,37 @@ void tst_Android::orientationChange_data() { QTest::addColumn("nativeOrientation"); QTest::addColumn("expected"); + QTest::addColumn("screenSize"); - QTest::newRow("Landscape") << 0 << Qt::LandscapeOrientation; - QTest::newRow("Portrait") << 1 << Qt::PortraitOrientation; + const QSize portraitSize = QGuiApplication::primaryScreen()->size(); + const QSize landscapeSize = QSize(portraitSize.height(), portraitSize.width()); + + QTest::newRow("InvertedLandscape") << 8 << Qt::InvertedLandscapeOrientation << landscapeSize; + QTest::newRow("InvertedPortrait") << 9 << Qt::InvertedPortraitOrientation << portraitSize; + QTest::newRow("Landscape") << 0 << Qt::LandscapeOrientation << landscapeSize; + // Leave Portrait till the end + QTest::newRow("Portrait") << 1 << Qt::PortraitOrientation << portraitSize; } void tst_Android::orientationChange() { QFETCH(int, nativeOrientation); QFETCH(Qt::ScreenOrientation, expected); + QFETCH(QSize, screenSize); + + // For QTBUG-94459 to check that the widget size are consistent after orientation changes + QWidget widget; + widget.show(); auto context = QNativeInterface::QAndroidApplication::context(); context.callMethod("setRequestedOrientation", nativeOrientation); - QTRY_COMPARE(qGuiApp->primaryScreen()->orientation(), expected); + + QScreen *screen = QGuiApplication::primaryScreen(); + QSignalSpy orientationSpy(screen, SIGNAL(orientationChanged(Qt::ScreenOrientation))); + QTRY_COMPARE(screen->orientation(), expected); + QCOMPARE(orientationSpy.size(), 1); + QCOMPARE(screen->size(), screenSize); + QCOMPARE(widget.size(), screen->availableSize()); } QTEST_MAIN(tst_Android)