From 7d1962cada48f8c351909d1ccc14b13c3adaa542 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tor=20Arne=20Vestb=C3=B8?= Date: Mon, 22 Jan 2018 17:28:10 +0100 Subject: [PATCH 1/4] macOS: Handle update requests via setNeedsDisplay more carefully Instead of trying to detect situations where we need to send a real expose event instead of an update request in response to a drawRect call, we keep track of when we've asked the view to display due to a requestUpdate call, and only deliver the corresponding drawRect as an update request if no other code has asked the view to display. This should cover all cases of asking the view to display, issued from our code or from AppKit, such as the view changing its backing scale factor, or otherwise needing a real expose event. Task-number: QTBUG-65663 Change-Id: I1783787823aee889dad8e48f34a1cb0f1b7b06bd Reviewed-by: Shawn Rutledge --- src/plugins/platforms/cocoa/qcocoawindow.mm | 19 +-------- src/plugins/platforms/cocoa/qnsview.h | 3 ++ src/plugins/platforms/cocoa/qnsview.mm | 44 ++++++++++++++++----- 3 files changed, 39 insertions(+), 27 deletions(-) diff --git a/src/plugins/platforms/cocoa/qcocoawindow.mm b/src/plugins/platforms/cocoa/qcocoawindow.mm index 92df6eac498..af2931c747e 100644 --- a/src/plugins/platforms/cocoa/qcocoawindow.mm +++ b/src/plugins/platforms/cocoa/qcocoawindow.mm @@ -1157,23 +1157,6 @@ void QCocoaWindow::handleExposeEvent(const QRegion ®ion) m_exposedRect = QRect(); } - QWindowPrivate *windowPrivate = qt_window_private(window()); - if (windowPrivate->updateRequestPending) { - // We can only deliver update request events when the window is exposed, - // and we also have to make sure we deliver any change to the exposed - // rect as a real expose event (including going from non-exposed to - // exposed). FIXME: Should this logic live in QGuiApplication? - if (isExposed() && m_exposedRect == previouslyExposedRect) { - qCDebug(lcQpaCocoaWindow) << "QCocoaWindow::handleExposeEvent" << window() << region << "as update request"; - windowPrivate->deliverUpdateRequest(); - return; - } else { - // Since updateRequestPending is still set, we will issue a deferred setNeedsDisplay - // from drawRect and get back into this code on the next display cycle, delivering - // the pending update request. - } - } - qCDebug(lcQpaCocoaWindow) << "QCocoaWindow::handleExposeEvent" << window() << region << "isExposed" << isExposed(); QWindowSystemInterface::handleExposeEvent(window(), region); } @@ -1348,7 +1331,7 @@ void QCocoaWindow::recreateWindowIfNeeded() void QCocoaWindow::requestUpdate() { qCDebug(lcQpaCocoaWindow) << "QCocoaWindow::requestUpdate" << window(); - [m_view setNeedsDisplay:YES]; + [m_view requestUpdate]; } void QCocoaWindow::requestActivateWindow() diff --git a/src/plugins/platforms/cocoa/qnsview.h b/src/plugins/platforms/cocoa/qnsview.h index f8903725a65..e2ea862cd5b 100644 --- a/src/plugins/platforms/cocoa/qnsview.h +++ b/src/plugins/platforms/cocoa/qnsview.h @@ -81,6 +81,7 @@ Q_FORWARD_DECLARE_OBJC_CLASS(QT_MANGLE_NAMESPACE(QNSViewMouseMoveHelper)); NSEvent *m_currentlyInterpretedKeyEvent; bool m_isMenuView; QSet m_acceptedKeyDowns; + bool m_updateRequested; } @property (nonatomic, retain) NSCursor *cursor; @@ -105,6 +106,8 @@ Q_FORWARD_DECLARE_OBJC_CLASS(QT_MANGLE_NAMESPACE(QNSViewMouseMoveHelper)); - (void)resetMouseButtons; +- (void)requestUpdate; + - (void)handleMouseEvent:(NSEvent *)theEvent; - (bool)handleMouseDownEvent:(NSEvent *)theEvent withButton:(int)buttonNumber; - (bool)handleMouseDraggedEvent:(NSEvent *)theEvent withButton:(int)buttonNumber; diff --git a/src/plugins/platforms/cocoa/qnsview.mm b/src/plugins/platforms/cocoa/qnsview.mm index 61f0012a2d6..e3bee95ad9a 100644 --- a/src/plugins/platforms/cocoa/qnsview.mm +++ b/src/plugins/platforms/cocoa/qnsview.mm @@ -147,6 +147,7 @@ Q_LOGGING_CATEGORY(lcQpaTablet, "qt.qpa.input.tablet") m_isMenuView = false; self.focusRingType = NSFocusRingTypeNone; self.cursor = nil; + m_updateRequested = false; } return self; } @@ -299,6 +300,25 @@ Q_LOGGING_CATEGORY(lcQpaTablet, "qt.qpa.input.tablet") return m_platformWindow->isOpaque(); } +- (void)requestUpdate +{ + if (self.needsDisplay) { + // If the view already has needsDisplay set it means that there may be code waiting for + // a real expose event, so we can't issue setNeedsDisplay now as a way to trigger an + // update request. We will re-trigger requestUpdate from drawRect. + return; + } + + [self setNeedsDisplay:YES]; + m_updateRequested = true; +} + +- (void)setNeedsDisplayInRect:(NSRect)rect +{ + [super setNeedsDisplayInRect:rect]; + m_updateRequested = false; +} + - (void)drawRect:(NSRect)dirtyRect { Q_UNUSED(dirtyRect); @@ -322,18 +342,24 @@ Q_LOGGING_CATEGORY(lcQpaTablet, "qt.qpa.input.tablet") } #endif - m_platformWindow->handleExposeEvent(exposedRegion); + QWindowPrivate *windowPrivate = qt_window_private(m_platformWindow->window()); - if (qt_window_private(m_platformWindow->window())->updateRequestPending) { - // A call to QWindow::requestUpdate was issued during the expose event, or we - // had to deliver a real expose event and still need to deliver the update. - // But AppKit will reset the needsDisplay state of the view after completing + if (m_updateRequested) { + Q_ASSERT(windowPrivate->updateRequestPending); + qCDebug(lcQpaCocoaWindow) << "Delivering update request to" << m_platformWindow->window(); + windowPrivate->deliverUpdateRequest(); + m_updateRequested = false; + } else { + m_platformWindow->handleExposeEvent(exposedRegion); + } + + if (windowPrivate->updateRequestPending) { + // A call to QWindow::requestUpdate was issued during event delivery above, + // but AppKit will reset the needsDisplay state of the view after completing // the current display cycle, so we need to defer the request to redisplay. // FIXME: Perhaps this should be a trigger to enable CADisplayLink? - qCDebug(lcQpaCocoaWindow) << "[QNSView drawRect:] issuing deferred setNeedsDisplay due to pending update request"; - dispatch_async(dispatch_get_main_queue (), ^{ - [self setNeedsDisplay:YES]; - }); + qCDebug(lcQpaCocoaWindow) << "Pending update request, triggering re-display"; + dispatch_async(dispatch_get_main_queue (), ^{ [self requestUpdate]; }); } } From 538b1b50764fb3a1898d425a7155319afbcf3b25 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tor=20Arne=20Vestb=C3=B8?= Date: Thu, 1 Feb 2018 12:17:48 +0100 Subject: [PATCH 2/4] Android: Defer initialization of logging rules until qApp construction MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On Android, we load the application library, and its dependencies (Qt), on Android's main thread (thread 0), and then spin up a secondary thread (thread 1), that we call main() on. If any QObject is constructed during loading of the application library or any of Qt's libraries, via static initializers or constructor functions, we will set QCoreApplicationPrivate::theMainThread to thread 0, which will confuse Qt later on when it's being run on thread 1, and will result in a warning during QCoreApplication construction: QApplication was not created in the main() thread This situation can easily lead to a crash as well. Unfortunately logging via qDebug/qCDebug and friends will trigger this too, as they internally use QObject. Fixing the root cause of this is under investigation, but for now we will partially revert fa2a653b3b934783 for Android. The effect is that any qCDebug with a "qt.*" category before qApp construction will turn into a no-op, like it was before fa2a653b3b934783. This patch does not cover the case of a regular qDebug, or a qCDebug with a non-Qt category. Those will still produce the same symptom, as before fa2a653b3b934783. Task-number: QTBUG-65863 Change-Id: I95675731d233244530d0a2a1c82a9578d5599775 Reviewed-by: Eskil Abrahamsen Blomfeldt Reviewed-by: Tor Arne Vestbø --- src/corelib/io/qloggingregistry.cpp | 10 ++++++++++ src/corelib/kernel/qcoreapplication.cpp | 8 ++++++++ 2 files changed, 18 insertions(+) diff --git a/src/corelib/io/qloggingregistry.cpp b/src/corelib/io/qloggingregistry.cpp index b5f8e30b803..cd97268d716 100644 --- a/src/corelib/io/qloggingregistry.cpp +++ b/src/corelib/io/qloggingregistry.cpp @@ -44,6 +44,7 @@ #include #include #include +#include // We can't use the default macros because this would lead to recursion. // Instead let's define our own one that unconditionally logs... @@ -255,6 +256,15 @@ void QLoggingSettingsParser::parseNextLine(QStringRef line) QLoggingRegistry::QLoggingRegistry() : categoryFilter(defaultCategoryFilter) { +#if defined(Q_OS_ANDROID) + // Unless QCoreApplication has been constructed we can't be sure that + // we are on Qt's main thread. If we did allow logging here, we would + // potentially set Qt's main thread to Android's thread 0, which would + // confuse Qt later when running main(). + if (!qApp) + return; +#endif + initializeRules(); // Init on first use } diff --git a/src/corelib/kernel/qcoreapplication.cpp b/src/corelib/kernel/qcoreapplication.cpp index 9ec8262bf90..38148946304 100644 --- a/src/corelib/kernel/qcoreapplication.cpp +++ b/src/corelib/kernel/qcoreapplication.cpp @@ -802,6 +802,14 @@ void QCoreApplicationPrivate::init() if (!coreappdata()->applicationVersionSet) coreappdata()->applicationVersion = appVersion(); +#if defined(Q_OS_ANDROID) + // We've deferred initializing the logging registry due to not being + // able to guarantee that logging happened on the same thread as the + // Qt main thread, but now that the Qt main thread is set up, we can + // enable categorized logging. + QLoggingRegistry::instance()->initializeRules(); +#endif + #if QT_CONFIG(library) // Reset the lib paths, so that they will be recomputed, taking the availability of argv[0] // into account. If necessary, recompute right away and replay the manual changes on top of the From eadf9e542fcc42597bfe02df065fc4cefa94cd56 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Str=C3=B8mme?= Date: Tue, 30 Jan 2018 16:40:26 +0100 Subject: [PATCH 3/4] Android: Don't rely on QDir::homePath() to get the application directory MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If the QLoggingRegistry gets called as part of the static initialization phase, it would call into Android's QStandarPaths implementation, which assumed that the HOME env. variable was already set. Since the variable isn't set before main is called, QDir::homePath() returns the root path, which would be cached and always returned. With this fix we now call Android's getFilesDir() directly, which will always return the right path. Since the font locations are also relying on an environment variable being set, we no longer cache that either. Task-number: QTBUG-65820 Change-Id: If45f3d5f0e87b808a62118ae95c31b492885646a Reviewed-by: Tor Arne Vestbø Reviewed-by: Eskil Abrahamsen Blomfeldt --- src/corelib/io/qstandardpaths_android.cpp | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/corelib/io/qstandardpaths_android.cpp b/src/corelib/io/qstandardpaths_android.cpp index 2a44daf8b52..0667d170c71 100644 --- a/src/corelib/io/qstandardpaths_android.cpp +++ b/src/corelib/io/qstandardpaths_android.cpp @@ -217,7 +217,16 @@ static QString getFilesDir() if (!path.isEmpty()) return path; - return (path = QDir::homePath()); + QJNIObjectPrivate appCtx = applicationContext(); + if (!appCtx.isValid()) + return QString(); + + QJNIObjectPrivate file = appCtx.callObjectMethod("getFilesDir", + "()Ljava/io/File;"); + if (!file.isValid()) + return QString(); + + return (path = getAbsolutePath(file)); } QString QStandardPaths::writableLocation(StandardLocation type) @@ -319,7 +328,9 @@ QStringList QStandardPaths::standardLocations(StandardLocation type) if (!ba.isEmpty()) return QStringList((fontLocation = QDir::cleanPath(QString::fromLocal8Bit(ba)))); - return QStringList((fontLocation = QLatin1String("/system/fonts"))); + // Don't cache the fallback, as we might just have been called before + // QT_ANDROID_FONT_LOCATION has been set. + return QStringList(QLatin1String("/system/fonts")); } return QStringList(writableLocation(type)); From 69d2501ee1900c43a35802eae05a734e479a7776 Mon Sep 17 00:00:00 2001 From: Thiago Macieira Date: Mon, 29 Jan 2018 23:56:05 -0800 Subject: [PATCH 4/4] Revert "QVariant: convert QDateTime and QTime to string with milliseconds" This reverts commit ab1e50757454b5afda2f6dec52d2eb16a32d4798. That was supposed to be a minor behavior change, but ends up having visible effects such as QtXmlPatterns xs:dateTime type now reporting sub-second fractions. So we're reverting in 5.10 and re-applying in 5.11. Change-Id: I741e49459c9a688c1c329d6cbd521cd4a0b2aa84 Reviewed-by: Lars Knoll --- src/corelib/kernel/qvariant.cpp | 4 ++-- tests/auto/corelib/kernel/qvariant/tst_qvariant.cpp | 10 ++-------- 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/src/corelib/kernel/qvariant.cpp b/src/corelib/kernel/qvariant.cpp index 93736ae8072..29429b5e551 100644 --- a/src/corelib/kernel/qvariant.cpp +++ b/src/corelib/kernel/qvariant.cpp @@ -440,10 +440,10 @@ static bool convert(const QVariant::Private *d, int t, void *result, bool *ok) *str = v_cast(d)->toString(Qt::ISODate); break; case QVariant::Time: - *str = v_cast(d)->toString(Qt::ISODateWithMs); + *str = v_cast(d)->toString(Qt::ISODate); break; case QVariant::DateTime: - *str = v_cast(d)->toString(Qt::ISODateWithMs); + *str = v_cast(d)->toString(Qt::ISODate); break; #endif case QVariant::Bool: diff --git a/tests/auto/corelib/kernel/qvariant/tst_qvariant.cpp b/tests/auto/corelib/kernel/qvariant/tst_qvariant.cpp index 6ec90a66c85..0d45159d09c 100644 --- a/tests/auto/corelib/kernel/qvariant/tst_qvariant.cpp +++ b/tests/auto/corelib/kernel/qvariant/tst_qvariant.cpp @@ -1099,9 +1099,8 @@ void tst_QVariant::toString_data() QTest::newRow( "bool" ) << QVariant( true ) << QString( "true" ); QTest::newRow( "qdate" ) << QVariant( QDate( 2002, 1, 1 ) ) << QString( "2002-01-01" ); - QTest::newRow( "qtime" ) << QVariant( QTime( 12, 34, 56 ) ) << QString( "12:34:56.000" ); - QTest::newRow( "qtime-with-ms" ) << QVariant( QTime( 12, 34, 56, 789 ) ) << QString( "12:34:56.789" ); - QTest::newRow( "qdatetime" ) << QVariant( QDateTime( QDate( 2002, 1, 1 ), QTime( 12, 34, 56, 789 ) ) ) << QString( "2002-01-01T12:34:56.789" ); + QTest::newRow( "qtime" ) << QVariant( QTime( 12, 34, 56 ) ) << QString( "12:34:56" ); + QTest::newRow( "qdatetime" ) << QVariant( QDateTime( QDate( 2002, 1, 1 ), QTime( 12, 34, 56 ) ) ) << QString( "2002-01-01T12:34:56" ); QTest::newRow( "llong" ) << QVariant( (qlonglong)Q_INT64_C(123456789012) ) << QString( "123456789012" ); QTest::newRow("QJsonValue") << QVariant(QJsonValue(QString("hello"))) << QString("hello"); @@ -1152,7 +1151,6 @@ void tst_QVariant::toTime_data() QTest::newRow( "qtime" ) << QVariant( QTime( 12, 34, 56 ) ) << QTime( 12, 34, 56 ); QTest::newRow( "qdatetime" ) << QVariant( QDateTime( QDate( 2002, 10, 10 ), QTime( 12, 34, 56 ) ) ) << QTime( 12, 34, 56 ); QTest::newRow( "qstring" ) << QVariant( QString( "12:34:56" ) ) << QTime( 12, 34, 56 ); - QTest::newRow( "qstring-with-ms" ) << QVariant( QString( "12:34:56.789" ) ) << QTime( 12, 34, 56, 789 ); } void tst_QVariant::toTime() @@ -1173,10 +1171,6 @@ void tst_QVariant::toDateTime_data() << QDateTime( QDate( 2002, 10, 10 ), QTime( 12, 34, 56 ) ); QTest::newRow( "qdate" ) << QVariant( QDate( 2002, 10, 10 ) ) << QDateTime( QDate( 2002, 10, 10 ), QTime( 0, 0, 0 ) ); QTest::newRow( "qstring" ) << QVariant( QString( "2002-10-10T12:34:56" ) ) << QDateTime( QDate( 2002, 10, 10 ), QTime( 12, 34, 56 ) ); - QTest::newRow( "qstring-utc" ) << QVariant( QString( "2002-10-10T12:34:56Z" ) ) - << QDateTime( QDate( 2002, 10, 10 ), QTime( 12, 34, 56 ), Qt::UTC ); - QTest::newRow( "qstring-with-ms" ) << QVariant( QString( "2002-10-10T12:34:56.789" ) ) - << QDateTime( QDate( 2002, 10, 10 ), QTime( 12, 34, 56, 789 ) ); } void tst_QVariant::toDateTime()