From d2ae838a7c6a49eb826ddf8de115306d11dd4d30 Mon Sep 17 00:00:00 2001 From: Eirik Aavitsland Date: Tue, 8 Oct 2019 13:47:28 +0200 Subject: Item views: fix regression causing unexpected widget overlapping A certain geometry adjustment was (practically) introduced in Qt 5.11, and caused very surprising behavior, where item widgets will often overwrite neighbouring cells. This has resulted in a number of bug reports. Since the adjustment has such serious side effects, and does not seem to be relevant any longer for the issue for which it was intended, remove it here. More details: From early Qt 4 times, QStyledItemDelegate would do some automatic expansion of the geometry of editor widgets - but only if the layout was RightToLeft. Hence, the effect of it was rarely seen. QTBUG-37433 did, for Qt 5.10, and complained about it. However, the resulting code change did not remove the adjustment, but instead extended it to apply to the normal LeftToRight layout also. Hence, more users experienced it, and reported it as a regression. Also, now in Qt 5.13, it seems Qt has changed in other ways, and the geometry adjustment no longer seems to help (or indeed make any difference to) the original case in QTBUG-37433. Fixes: QTBUG-78495 Fixes: QTBUG-76011 Fixes: QTBUG-68476 Change-Id: I4a4e873969eb1d89843f98fc63d90371207515d1 Reviewed-by: Paul Olav Tvete --- src/widgets/itemviews/qstyleditemdelegate.cpp | 9 --------- 1 file changed, 9 deletions(-) diff --git a/src/widgets/itemviews/qstyleditemdelegate.cpp b/src/widgets/itemviews/qstyleditemdelegate.cpp index 22067851cb..702e290da3 100644 --- a/src/widgets/itemviews/qstyleditemdelegate.cpp +++ b/src/widgets/itemviews/qstyleditemdelegate.cpp @@ -514,15 +514,6 @@ void QStyledItemDelegate::updateEditorGeometry(QWidget *editor, QStyle *style = widget ? widget->style() : QApplication::style(); QRect geom = style->subElementRect(QStyle::SE_ItemViewItemText, &opt, widget); - const int delta = qSmartMinSize(editor).width() - geom.width(); - if (delta > 0) { - //we need to widen the geometry - if (editor->layoutDirection() == Qt::RightToLeft) - geom.adjust(-delta, 0, 0, 0); - else - geom.adjust(0, 0, delta, 0); - } - editor->setGeometry(geom); } -- cgit v1.2.3 From 9845c06d1f8c3b30697cdf96b569849ee2372f47 Mon Sep 17 00:00:00 2001 From: Timur Pocheptsov Date: Thu, 10 Oct 2019 10:41:33 +0200 Subject: DTLS auto-test(s) - fix a buggy logic with pending datagrams MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes: QTBUG-79128 Change-Id: Ifebd5b056541b7732b15b5cf063ad22ab754a64c Reviewed-by: Mårten Nordheim --- tests/auto/network/ssl/qdtls/tst_qdtls.cpp | 2 +- tests/auto/network/ssl/qdtlscookie/tst_qdtlscookie.cpp | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/auto/network/ssl/qdtls/tst_qdtls.cpp b/tests/auto/network/ssl/qdtls/tst_qdtls.cpp index 6a94eee389..4dfdf14e5b 100644 --- a/tests/auto/network/ssl/qdtls/tst_qdtls.cpp +++ b/tests/auto/network/ssl/qdtls/tst_qdtls.cpp @@ -1131,7 +1131,7 @@ void tst_QDtls::handshakeReadyRead() QUdpSocket *socket = qobject_cast(sender()); Q_ASSERT(socket); - if (!socket->pendingDatagramSize()) + if (socket->pendingDatagramSize() <= 0) return; const bool isServer = socket == &serverSocket; diff --git a/tests/auto/network/ssl/qdtlscookie/tst_qdtlscookie.cpp b/tests/auto/network/ssl/qdtlscookie/tst_qdtlscookie.cpp index c90e9cb2c8..a273ceaa17 100644 --- a/tests/auto/network/ssl/qdtlscookie/tst_qdtlscookie.cpp +++ b/tests/auto/network/ssl/qdtlscookie/tst_qdtlscookie.cpp @@ -352,7 +352,7 @@ void tst_QDtlsCookie::receiveMessage(QUdpSocket *socket, QByteArray *message, { Q_ASSERT(socket && message); - if (!socket->pendingDatagramSize()) + if (socket->pendingDatagramSize() <= 0) testLoop.enterLoopMSecs(handshakeTimeoutMS); QVERIFY(!testLoop.timeout()); @@ -377,7 +377,7 @@ void tst_QDtlsCookie::serverReadyRead() { Q_ASSERT(clientsToWait); - if (!serverSocket.pendingDatagramSize()) + if (serverSocket.pendingDatagramSize() <= 0) return; QByteArray hello; @@ -410,7 +410,7 @@ void tst_QDtlsCookie::clientReadyRead() QUdpSocket *clientSocket = qobject_cast(sender()); Q_ASSERT(clientSocket); - if (!clientSocket->pendingDatagramSize()) + if (clientSocket->pendingDatagramSize() <= 0) return; QDtls *handshake = nullptr; -- cgit v1.2.3 From 3dbc7596a3b7aca37e80f76288d3ab761ddd247a Mon Sep 17 00:00:00 2001 From: Volker Hilsheimer Date: Tue, 8 Oct 2019 10:56:44 +0200 Subject: Drag'n'Drop: fix crash when widgets are destroyed during event handling Widgets might be destroyed when handling a dragMoveEvent, in which case the following code will operate on dangling pointers or null pointers. Use a QPointer to watch for the original event receiver to disappear, and add the necessary checks for the objects we deliver events to being null. Change-Id: I4ca2f182540ae21113f4bea4e5c569e983cc58bf Fixes: QTBUG-78907 Reviewed-by: Gatis Paeglis --- src/widgets/kernel/qwidgetwindow.cpp | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/src/widgets/kernel/qwidgetwindow.cpp b/src/widgets/kernel/qwidgetwindow.cpp index fbc71cd0ea..aad505ed29 100644 --- a/src/widgets/kernel/qwidgetwindow.cpp +++ b/src/widgets/kernel/qwidgetwindow.cpp @@ -885,7 +885,7 @@ void QWidgetWindow::handleDragEnterEvent(QDragEnterEvent *event, QWidget *widget void QWidgetWindow::handleDragMoveEvent(QDragMoveEvent *event) { - auto *widget = findDnDTarget(m_widget, event->pos()); + QPointer widget = findDnDTarget(m_widget, event->pos()); if (!widget) { event->ignore(); if (m_dragTarget) { // Send DragLeave to previous @@ -908,14 +908,18 @@ void QWidgetWindow::handleDragMoveEvent(QDragMoveEvent *event) QGuiApplication::forwardEvent(m_dragTarget, &leaveEvent, event); m_dragTarget = nullptr; } - // Send DragEnter to new widget. - handleDragEnterEvent(static_cast(event), widget); - // Handling 'DragEnter' should suffice for the application. - translated.setDropAction(event->dropAction()); - translated.setAccepted(event->isAccepted()); - // The drag enter event is always immediately followed by a drag move event, - // see QDragEnterEvent documentation. - QGuiApplication::forwardEvent(m_dragTarget, &translated, event); + // widget might have been deleted when handling the leaveEvent + if (widget) { + // Send DragEnter to new widget. + handleDragEnterEvent(static_cast(event), widget); + // Handling 'DragEnter' should suffice for the application. + translated.setDropAction(event->dropAction()); + translated.setAccepted(event->isAccepted()); + // The drag enter event is always immediately followed by a drag move event, + // see QDragEnterEvent documentation. + if (m_dragTarget) + QGuiApplication::forwardEvent(m_dragTarget, &translated, event); + } } event->setAccepted(translated.isAccepted()); event->setDropAction(translated.dropAction()); -- cgit v1.2.3 From 472f5331ca8091b191944650d043a288dac7c3e8 Mon Sep 17 00:00:00 2001 From: Joerg Bornemann Date: Mon, 7 Oct 2019 09:15:37 +0200 Subject: Doc: Fix QMAKE_EXTRA_TARGETS snippet description The snippet uses the Unix touch command, not qmake's touch function. Change-Id: I71d1460447249b8941ce4bdbb494bb419e13b119 Reviewed-by: Oliver Wolff Reviewed-by: Edward Welbourne Reviewed-by: Paul Wicking --- qmake/doc/src/qmake-manual.qdoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qmake/doc/src/qmake-manual.qdoc b/qmake/doc/src/qmake-manual.qdoc index da3aa6b248..ed7fe60fdc 100644 --- a/qmake/doc/src/qmake-manual.qdoc +++ b/qmake/doc/src/qmake-manual.qdoc @@ -4690,7 +4690,7 @@ The definitions above define a qmake target called \c mytarget, containing a Makefile target called \c{.buildfile} which in turn is generated with the - \l{touchfunction}{touch()} function. Finally, the + \c touch command. Finally, the \c{.depends} member specifies that \c mytarget depends on \c mytarget2, another target that is defined afterwards. \c mytarget2 is a dummy target. It is only defined to echo some text to the console. -- cgit v1.2.3 From 014d7ac65417ed9b5ffb85cca24d16564ff5005a Mon Sep 17 00:00:00 2001 From: Milian Wolff Date: Thu, 5 Sep 2019 09:58:30 +0200 Subject: Q{Shared,Weak}Pointer: Reduce overload sets in implicit conversions Only allow implicit conversions when the types involved are compatible. That means, only allow construction and copy assignment when the type X* is convertible to type T*. This is done using SFINAE and the std::is_convertible type trait, which makes the previous QSHAREDPOINTER_VERIFY_AUTO_CAST obsolete. This patch fixes compilation when a function is overloaded with Q{Shared,Weak}Pointer of different, incompatible types. Previously, this resulted in a compilation error due to an ambiguous overload. Change-Id: I069d22f3582e69842f14284d4f27827326597ca2 Fixes: QTBUG-75222 Reviewed-by: Thiago Macieira --- src/corelib/tools/qsharedpointer_impl.h | 51 ++++++++------------ .../tools/qsharedpointer/tst_qsharedpointer.cpp | 54 ++++++++++++++++++++++ 2 files changed, 74 insertions(+), 31 deletions(-) diff --git a/src/corelib/tools/qsharedpointer_impl.h b/src/corelib/tools/qsharedpointer_impl.h index bccf8c5740..f2158436fd 100644 --- a/src/corelib/tools/qsharedpointer_impl.h +++ b/src/corelib/tools/qsharedpointer_impl.h @@ -69,21 +69,6 @@ QT_END_NAMESPACE QT_BEGIN_NAMESPACE - -// Macro QSHAREDPOINTER_VERIFY_AUTO_CAST -// generates a compiler error if the following construct isn't valid: -// T *ptr1; -// X *ptr2 = ptr1; -// -#ifdef QT_NO_DEBUG -# define QSHAREDPOINTER_VERIFY_AUTO_CAST(T, X) qt_noop() -#else - -template inline void qt_sharedpointer_cast_check(T *) { } -# define QSHAREDPOINTER_VERIFY_AUTO_CAST(T, X) \ - qt_sharedpointer_cast_check(static_cast(0)) -#endif - // // forward declarations // @@ -293,6 +278,9 @@ template class QSharedPointer { typedef T *QSharedPointer:: *RestrictedBool; typedef QtSharedPointer::ExternalRefCountData Data; + template + using IfCompatible = typename std::enable_if::value, bool>::type; + public: typedef T Type; typedef T element_type; @@ -316,11 +304,11 @@ public: Q_DECL_CONSTEXPR QSharedPointer(std::nullptr_t) Q_DECL_NOTHROW : value(nullptr), d(nullptr) { } - template + template = true> inline explicit QSharedPointer(X *ptr) : value(ptr) // noexcept { internalConstruct(ptr, QtSharedPointer::NormalDeleter()); } - template + template = true> inline QSharedPointer(X *ptr, Deleter deleter) : value(ptr) // throws { internalConstruct(ptr, deleter); } @@ -349,7 +337,7 @@ public: return *this; } - template + template = true> QSharedPointer(QSharedPointer &&other) Q_DECL_NOTHROW : value(other.value), d(other.d) { @@ -357,7 +345,7 @@ public: other.value = nullptr; } - template + template = true> QSharedPointer &operator=(QSharedPointer &&other) Q_DECL_NOTHROW { QSharedPointer moved(std::move(other)); @@ -367,11 +355,11 @@ public: #endif - template + template = true> QSharedPointer(const QSharedPointer &other) Q_DECL_NOTHROW : value(other.value), d(other.d) { if (d) ref(); } - template + template = true> inline QSharedPointer &operator=(const QSharedPointer &other) { QSharedPointer copy(other); @@ -379,11 +367,11 @@ public: return *this; } - template + template = true> inline QSharedPointer(const QWeakPointer &other) : value(nullptr), d(nullptr) { *this = other; } - template + template = true> inline QSharedPointer &operator=(const QWeakPointer &other) { internalSet(other.d, other.value); return *this; } @@ -553,6 +541,8 @@ class QWeakPointer { typedef T *QWeakPointer:: *RestrictedBool; typedef QtSharedPointer::ExternalRefCountData Data; + template + using IfCompatible = typename std::enable_if::value, bool>::type; public: typedef T element_type; @@ -574,14 +564,14 @@ public: #ifndef QT_NO_QOBJECT // special constructor that is enabled only if X derives from QObject #if QT_DEPRECATED_SINCE(5, 0) - template + template = true> QT_DEPRECATED inline QWeakPointer(X *ptr) : d(ptr ? Data::getAndRef(ptr) : nullptr), value(ptr) { } #endif #endif #if QT_DEPRECATED_SINCE(5, 0) - template + template = true> QT_DEPRECATED inline QWeakPointer &operator=(X *ptr) { return *this = QWeakPointer(ptr); } #endif @@ -619,11 +609,11 @@ public: return *this; } - template + template = true> inline QWeakPointer(const QWeakPointer &o) : d(nullptr), value(nullptr) { *this = o; } - template + template = true> inline QWeakPointer &operator=(const QWeakPointer &o) { // conversion between X and T could require access to the virtual table @@ -640,14 +630,13 @@ public: bool operator!=(const QWeakPointer &o) const Q_DECL_NOTHROW { return !(*this == o); } - template + template = true> inline QWeakPointer(const QSharedPointer &o) : d(nullptr), value(nullptr) { *this = o; } - template + template = true> inline QWeakPointer &operator=(const QSharedPointer &o) { - QSHAREDPOINTER_VERIFY_AUTO_CAST(T, X); // if you get an error in this line, the cast is invalid internalSet(o.d, o.data()); return *this; } @@ -684,7 +673,7 @@ public: { return *this = QWeakPointer(ptr, true); } #ifndef QT_NO_QOBJECT - template + template = true> inline QWeakPointer(X *ptr, bool) : d(ptr ? Data::getAndRef(ptr) : nullptr), value(ptr) { } #endif diff --git a/tests/auto/corelib/tools/qsharedpointer/tst_qsharedpointer.cpp b/tests/auto/corelib/tools/qsharedpointer/tst_qsharedpointer.cpp index e97848fb1c..0fe24ed5cb 100644 --- a/tests/auto/corelib/tools/qsharedpointer/tst_qsharedpointer.cpp +++ b/tests/auto/corelib/tools/qsharedpointer/tst_qsharedpointer.cpp @@ -40,6 +40,7 @@ #include "nontracked.h" #include "wrapper.h" +#include #include #include #include @@ -105,12 +106,15 @@ private slots: void sharedFromThis(); void constructorThrow(); + void overloads(); void threadStressTest_data(); void threadStressTest(); void validConstructs(); void invalidConstructs_data(); void invalidConstructs(); + + // let invalidConstructs be the last test, because it's the slowest; // add new tests above this block public slots: @@ -2250,6 +2254,11 @@ void tst_QSharedPointer::invalidConstructs_data() << &QTest::QExternalTest::tryCompileFail << "QSharedPointer ptr(new Data, [](int *) {});\n"; #endif + + QTest::newRow("incompatible-overload") + << &QTest::QExternalTest::tryCompileFail + << "void foo(QSharedPointer) {}\n" + "void bar() { foo(QSharedPointer()); }\n"; } void tst_QSharedPointer::invalidConstructs() @@ -2748,5 +2757,50 @@ void tst_QSharedPointer::reentrancyWhileDestructing() ReentrancyWhileDestructing::A obj; } +namespace { +struct Base1 {}; +struct Base2 {}; + +struct Child1 : Base1 {}; +struct Child2 : Base2 {}; + +template class SmartPtr> +struct Overloaded +{ + std::array call(const SmartPtr &) + { + return {}; + } + std::array call(const SmartPtr &) + { + return {}; + } + static const Q_CONSTEXPR uint base1Called = sizeof(std::array); + static const Q_CONSTEXPR uint base2Called = sizeof(std::array); + + void test() + { +#define QVERIFY_CALLS(expr, base) Q_STATIC_ASSERT(sizeof(call(expr)) == base##Called) + QVERIFY_CALLS(SmartPtr{}, base1); + QVERIFY_CALLS(SmartPtr{}, base2); + QVERIFY_CALLS(SmartPtr{}, base1); + QVERIFY_CALLS(SmartPtr{}, base2); + QVERIFY_CALLS(SmartPtr{}, base1); + QVERIFY_CALLS(SmartPtr{}, base2); + QVERIFY_CALLS(SmartPtr{}, base1); + QVERIFY_CALLS(SmartPtr{}, base2); +#undef QVERIFY_CALLS + } +}; +} + +void tst_QSharedPointer::overloads() +{ + Overloaded sharedOverloaded; + sharedOverloaded.test(); + Overloaded weakOverloaded; + weakOverloaded.test(); +} + QTEST_MAIN(tst_QSharedPointer) #include "tst_qsharedpointer.moc" -- cgit v1.2.3 From 31012df705c37503be00f0ddc2c323e73fd28067 Mon Sep 17 00:00:00 2001 From: Andre de la Rocha Date: Mon, 2 Sep 2019 16:46:07 +0200 Subject: Fix QGraphicsScene::update() performance A previous fix has caused a performance degradation while adding a check for avoiding adding duplicated rectangles to the update list. This patch fixes it by using a std::set instead of a QList, avoiding duplication while using an O(log N) operation, instead of the O(N) used before. Fixes: QTBUG-77952 Change-Id: Ifa9fbf110e0bad60ee02a42d91281981fd98ceab Reviewed-by: Volker Hilsheimer --- src/widgets/graphicsview/qgraphicsscene.cpp | 13 ++++++++++--- src/widgets/graphicsview/qgraphicsscene_p.h | 17 ++++++++++++++++- .../graphicsview/qgraphicsscene/tst_qgraphicsscene.cpp | 2 -- 3 files changed, 26 insertions(+), 6 deletions(-) diff --git a/src/widgets/graphicsview/qgraphicsscene.cpp b/src/widgets/graphicsview/qgraphicsscene.cpp index 5238ea2f5c..03717d5d1a 100644 --- a/src/widgets/graphicsview/qgraphicsscene.cpp +++ b/src/widgets/graphicsview/qgraphicsscene.cpp @@ -386,7 +386,15 @@ void QGraphicsScenePrivate::_q_emitUpdated() // Notify the changes to anybody interested. QList oldUpdatedRects; - oldUpdatedRects = updateAll ? (QList() << q->sceneRect()) : updatedRects; + if (updateAll) { + oldUpdatedRects << q->sceneRect(); + } else { + // Switch to a ranged constructor in Qt 6... + oldUpdatedRects.reserve(int(updatedRects.size())); + std::copy(updatedRects.cbegin(), updatedRects.cend(), + std::back_inserter(oldUpdatedRects)); + } + updateAll = false; updatedRects.clear(); emit q->changed(oldUpdatedRects); @@ -3219,8 +3227,7 @@ void QGraphicsScene::update(const QRectF &rect) view->d_func()->updateRectF(rect); } } else { - if (!d->updatedRects.contains(rect)) - d->updatedRects << rect; + d->updatedRects.insert(rect); } } diff --git a/src/widgets/graphicsview/qgraphicsscene_p.h b/src/widgets/graphicsview/qgraphicsscene_p.h index a2d13436fc..14bafe6678 100644 --- a/src/widgets/graphicsview/qgraphicsscene_p.h +++ b/src/widgets/graphicsview/qgraphicsscene_p.h @@ -69,6 +69,9 @@ #include #include +#include +#include + QT_REQUIRE_CONFIG(graphicsview); QT_BEGIN_NAMESPACE @@ -122,7 +125,19 @@ public: QRectF growingItemsBoundingRect; void _q_emitUpdated(); - QList updatedRects; + + struct UpdatedRectsCmp + { + bool operator() (const QRectF &a, const QRectF &b) const noexcept + { + return std::make_tuple(a.y(), a.x(), a.height(), a.width()) + < std::make_tuple(b.y(), b.x(), b.height(), b.width()); + } + }; + + // std::set was used here instead of std::unordered_set due to requiring only a comparator and + // showing equivalent performance in empirical measurements within the ranges of interest... + std::set updatedRects; QPainterPath selectionArea; int selectionChanging; diff --git a/tests/auto/widgets/graphicsview/qgraphicsscene/tst_qgraphicsscene.cpp b/tests/auto/widgets/graphicsview/qgraphicsscene/tst_qgraphicsscene.cpp index 46f1d5df5c..4f8741a5aa 100644 --- a/tests/auto/widgets/graphicsview/qgraphicsscene/tst_qgraphicsscene.cpp +++ b/tests/auto/widgets/graphicsview/qgraphicsscene/tst_qgraphicsscene.cpp @@ -3685,8 +3685,6 @@ void tst_QGraphicsScene::changedSignal() qApp->processEvents(); QCOMPARE(cl.changes.size(), 2); QCOMPARE(cl.changes.at(1).size(), 2); - QCOMPARE(cl.changes.at(1).first(), QRectF(0, 0, 10, 10)); - QCOMPARE(cl.changes.at(1).last(), QRectF(20, 0, 10, 10)); QCOMPARE(scene.sceneRect(), QRectF(0, 0, 30, 10)); -- cgit v1.2.3 From 365f70be6ed53bf8b9ba7c5a78d5683e4ff8fe79 Mon Sep 17 00:00:00 2001 From: Nico Vertriest Date: Thu, 10 Oct 2019 14:45:21 +0200 Subject: Doc: Correct snippet about customizing QMenuBar Task-number: QTBUG-79129 Change-Id: I1f8da3b429ab8543ca1f0b7079d0f50bbeea8eb5 Reviewed-by: Paul Wicking Reviewed-by: Volker Hilsheimer --- src/widgets/doc/snippets/code/doc_src_stylesheet.qdoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/widgets/doc/snippets/code/doc_src_stylesheet.qdoc b/src/widgets/doc/snippets/code/doc_src_stylesheet.qdoc index 098eaf4717..b0d042566f 100644 --- a/src/widgets/doc/snippets/code/doc_src_stylesheet.qdoc +++ b/src/widgets/doc/snippets/code/doc_src_stylesheet.qdoc @@ -1107,10 +1107,10 @@ QMenu::indicator:exclusive:checked:selected { QMenuBar { background-color: qlineargradient(x1:0, y1:0, x2:0, y2:1, stop:0 lightgray, stop:1 darkgray); + spacing: 3px; /* spacing between menu bar items */ } QMenuBar::item { - spacing: 3px; /* spacing between menu bar items */ padding: 1px 4px; background: transparent; border-radius: 4px; -- cgit v1.2.3 From 2e12825b0b4457d709d6d467c84f30ce35336ff3 Mon Sep 17 00:00:00 2001 From: Leena Miettinen Date: Wed, 2 Oct 2019 13:22:39 +0200 Subject: Doc: Describe initTestCase_data() function and QFETCH_GLOBAL macro Fixes: QTBUG-24342 Change-Id: I8f8f3726c5d31e34af9bfe054572c08fc07e01e0 Reviewed-by: Edward Welbourne --- .../doc/snippets/code/src_qtestlib_qtestcase.cpp | 30 ++++++++++++- src/testlib/doc/src/qttestlib-manual.qdoc | 49 ++++++++++++++++++---- src/testlib/qtestcase.qdoc | 33 +++++++++++++-- 3 files changed, 100 insertions(+), 12 deletions(-) diff --git a/src/testlib/doc/snippets/code/src_qtestlib_qtestcase.cpp b/src/testlib/doc/snippets/code/src_qtestlib_qtestcase.cpp index 202f87af52..5f71828595 100644 --- a/src/testlib/doc/snippets/code/src_qtestlib_qtestcase.cpp +++ b/src/testlib/doc/snippets/code/src_qtestlib_qtestcase.cpp @@ -1,6 +1,6 @@ /**************************************************************************** ** -** Copyright (C) 2016 The Qt Company Ltd. +** Copyright (C) 2019 The Qt Company Ltd. ** Contact: https://www.qt.io/licensing/ ** ** This file is part of the documentation of the Qt Toolkit. @@ -296,3 +296,31 @@ QTest::keyClick(myWindow, Qt::Key_Escape, Qt::ShiftModifier, 200); } +//! [30] +void TestQLocale::initTestCase_data() +{ + QTest::addColumn("locale"); + QTest::newRow("C") << QLocale::c(); + QTest::newRow("UKish") << QLocale("en_GB"); + QTest::newRow("USAish") << QLocale(QLocale::English); +} + +void TestQLocale::roundTripInt_data() +{ + QTest::addColumn("number"); + QTest::newRow("one") << 1; + QTest::newRow("two") << 2; + QTest::newRow("ten") << 10; +} +//! [30] + +//! [31] +void TestQLocale::roundTripInt() +{ + QFETCH_GLOBAL(QLocale, locale); + QFETCH(int, number); + bool ok; + QCOMPARE(locale.toInt(locale.toString(number), &ok), number); + QVERIFY(ok); +} +//! [31] diff --git a/src/testlib/doc/src/qttestlib-manual.qdoc b/src/testlib/doc/src/qttestlib-manual.qdoc index 71b4541313..a3644c1623 100644 --- a/src/testlib/doc/src/qttestlib-manual.qdoc +++ b/src/testlib/doc/src/qttestlib-manual.qdoc @@ -1,6 +1,6 @@ /**************************************************************************** ** -** Copyright (C) 2016 The Qt Company Ltd. +** Copyright (C) 2019 The Qt Company Ltd. ** Copyright (C) 2016 Intel Corporation. ** Contact: https://www.qt.io/licensing/ ** @@ -89,12 +89,14 @@ private slot is a test function in your test. QTest::qExec() can be used to execute all test functions in the test object. - In addition, there are four private slots that are \e not treated as test functions. - They will be executed by the testing framework and can be used to initialize and - clean up either the entire test or the current test function. + In addition, you can define the following private slots that are \e not + treated as test functions. When present, they will be executed by the + testing framework and can be used to initialize and clean up either the + entire test or the current test function. \list \li \c{initTestCase()} will be called before the first test function is executed. + \li \c{initTestCase_data()} will be called to create a global test data table. \li \c{cleanupTestCase()} will be called after the last test function was executed. \li \c{init()} will be called before each test function is executed. \li \c{cleanup()} will be called after every test function. @@ -365,6 +367,34 @@ See \l {Chapter 5: Writing a Benchmark}{Writing a Benchmark} in the Qt Test Tutorial for more benchmarking examples. + + \section1 Using Global Test Data + + You can define \c{initTestCase_data()} to set up a global test data table. + Each test is run once for each row in the global test data table. When the + test function itself \l{Chapter 2: Data-driven Testing}{is data-driven}, + it is run for each local data row, for each global data row. So, if there + are \c g rows in the global data table and \c d rows in the test's own + data-table, the number of runs of this test is \c g times \c d. + + Global data is fetched from the table using the \l QFETCH_GLOBAL() macro. + + The following are typical use cases for global test data: + + \list + \li Selecting among the available database backends in QSql tests to run + every test against every database. + \li Doing all networking tests with and without SSL (HTTP versus HTTPS) + and proxying. + \li Testing a timer with a high precision clock and with a coarse one. + \li Selecting whether a parser shall read from a QByteArray or from a + QIODevice. + \endlist + + For example, to test each number provided by \c {roundTripInt_data()} with + each locale provided by \c {initTestCase_data()}: + + \snippet code/src_qtestlib_qtestcase.cpp 31 */ /*! @@ -508,10 +538,9 @@ QTest::newRow() function. Each set of data will become a separate row in the test table. - \l QTest::newRow() takes one argument: a name that will be - associated with the data set. If the test fails, the name will be - used in the test log, referencing the failed data. Then we - stream the data set into the new table row. First an arbitrary + \l QTest::newRow() takes one argument: a name that will be associated + with the data set and used in the test log to identify the data set. + Then we stream the data set into the new table row. First an arbitrary string, and then the expected result of applying the QString::toUpper() function to that string. @@ -543,6 +572,10 @@ \li HELLO \endtable + When data is streamed into the row, each datum is asserted to match + the type of the column whose value it supplies. If any assertion fails, + the test is aborted. + \section1 Rewriting the Test Function Our test function can now be rewritten: diff --git a/src/testlib/qtestcase.qdoc b/src/testlib/qtestcase.qdoc index 2af016304d..f3761cdfa8 100644 --- a/src/testlib/qtestcase.qdoc +++ b/src/testlib/qtestcase.qdoc @@ -1,6 +1,6 @@ /**************************************************************************** ** -** Copyright (C) 2016 The Qt Company Ltd. +** Copyright (C) 2019 The Qt Company Ltd. ** Contact: https://www.qt.io/licensing/ ** ** This file is part of the documentation of the Qt Toolkit. @@ -220,8 +220,8 @@ \relates QTest The fetch macro creates a local variable named \a name with the type \a type - on the stack. \a name has to match the element name from the test's data. - If no such element exists, the test will assert. + on the stack. The \a name and \a type must match a column from the test's + data table. This is asserted and the test will abort if the assertion fails. Assuming a test has the following data: @@ -239,6 +239,33 @@ by the test framework. The test function must have a _data function. */ +/*! \macro QFETCH_GLOBAL(type, name) + + \relates QTest + + This macro fetches a variable named \a name with the type \a type from + a row in the global data table. The \a name and \a type must match a + column in the global data table. This is asserted and the test will abort + if the assertion fails. + + Assuming a test has the following data: + + \snippet code/src_qtestlib_qtestcase.cpp 30 + + The test's own data is a single number per row. In this case, + \c initTestCase_data() also supplies a locale per row. Therefore, + this test will be run with every combination of locale from the + latter and number from the former. + + \snippet code/src_qtestlib_qtestcase.cpp 31 + + The locale is read from the global data table using QFETCH_GLOBAL(), + and the number is read from the local data table using QFETCH(). + + \note This macro can only be used in test methods of a class with an + \c initTestCase_data() method. +*/ + /*! \macro QWARN(message) \relates QTest -- cgit v1.2.3 From 55427858e3f9d98dc7a75638c54a4fffde6be73a Mon Sep 17 00:00:00 2001 From: Samuli Piippo Date: Wed, 20 Mar 2019 15:39:59 +0200 Subject: QStandardPaths: Correct handling for XDG_RUNTIME_DIR Always try to create the runtime directory and never change the permissions of an existing directory. Conform to the XDG Base Directory Specification: "If, when attempting to write a file, the destination directory is non-existent an attempt should be made to create it with permission 0700. If the destination directory exists already the permissions should not be changed." Fixes: QTBUG-68338 Change-Id: Iaf854d69225fc46e43abae86232d749e5c247df0 Reviewed-by: David Faure Reviewed-by: Thiago Macieira --- src/corelib/io/qstandardpaths_unix.cpp | 51 ++++++++++++---------- .../io/qstandardpaths/tst_qstandardpaths.cpp | 34 +++++++++------ 2 files changed, 48 insertions(+), 37 deletions(-) diff --git a/src/corelib/io/qstandardpaths_unix.cpp b/src/corelib/io/qstandardpaths_unix.cpp index 3d4a349c8c..cc656954d9 100644 --- a/src/corelib/io/qstandardpaths_unix.cpp +++ b/src/corelib/io/qstandardpaths_unix.cpp @@ -120,54 +120,59 @@ QString QStandardPaths::writableLocation(StandardLocation type) } case RuntimeLocation: { - const uint myUid = uint(geteuid()); // http://standards.freedesktop.org/basedir-spec/latest/ + const uint myUid = uint(geteuid()); + // since the current user is the owner, set both xxxUser and xxxOwner + const QFile::Permissions wantedPerms = QFile::ReadUser | QFile::WriteUser | QFile::ExeUser + | QFile::ReadOwner | QFile::WriteOwner | QFile::ExeOwner; QFileInfo fileInfo; QString xdgRuntimeDir = QFile::decodeName(qgetenv("XDG_RUNTIME_DIR")); if (xdgRuntimeDir.isEmpty()) { const QString userName = QFileSystemEngine::resolveUserName(myUid); xdgRuntimeDir = QDir::tempPath() + QLatin1String("/runtime-") + userName; fileInfo.setFile(xdgRuntimeDir); - if (!fileInfo.isDir()) { - if (!QDir().mkdir(xdgRuntimeDir)) { - qWarning("QStandardPaths: error creating runtime directory %s: %s", qPrintable(xdgRuntimeDir), qPrintable(qt_error_string(errno))); - return QString(); - } - } #ifndef Q_OS_WASM qWarning("QStandardPaths: XDG_RUNTIME_DIR not set, defaulting to '%s'", qPrintable(xdgRuntimeDir)); #endif } else { fileInfo.setFile(xdgRuntimeDir); - if (!fileInfo.exists()) { - qWarning("QStandardPaths: XDG_RUNTIME_DIR points to non-existing path '%s', " - "please create it with 0700 permissions.", qPrintable(xdgRuntimeDir)); - return QString(); - } + } + if (fileInfo.exists()) { if (!fileInfo.isDir()) { qWarning("QStandardPaths: XDG_RUNTIME_DIR points to '%s' which is not a directory", qPrintable(xdgRuntimeDir)); return QString(); } + } else { + QFileSystemEntry entry(xdgRuntimeDir); + if (!QFileSystemEngine::createDirectory(entry, false)) { + if (errno != EEXIST) { + qWarning("QStandardPaths: error creating runtime directory %s: %s", + qPrintable(xdgRuntimeDir), qPrintable(qt_error_string(errno))); + return QString(); + } + } else { + QSystemError error; + if (!QFileSystemEngine::setPermissions(entry, wantedPerms, error)) { + qWarning("QStandardPaths: could not set correct permissions on runtime directory %s: %s", + qPrintable(xdgRuntimeDir), qPrintable(error.toString())); + return QString(); + } + } } // "The directory MUST be owned by the user" if (fileInfo.ownerId() != myUid) { - qWarning("QStandardPaths: wrong ownership on runtime directory %s, %d instead of %d", qPrintable(xdgRuntimeDir), - fileInfo.ownerId(), myUid); + qWarning("QStandardPaths: wrong ownership on runtime directory %s, %d instead of %d", + qPrintable(xdgRuntimeDir), fileInfo.ownerId(), myUid); return QString(); } // "and he MUST be the only one having read and write access to it. Its Unix access mode MUST be 0700." - // since the current user is the owner, set both xxxUser and xxxOwner - const QFile::Permissions wantedPerms = QFile::ReadUser | QFile::WriteUser | QFile::ExeUser - | QFile::ReadOwner | QFile::WriteOwner | QFile::ExeOwner; if (fileInfo.permissions() != wantedPerms) { - QFile file(xdgRuntimeDir); - if (!file.setPermissions(wantedPerms)) { - qWarning("QStandardPaths: could not set correct permissions on runtime directory %s: %s", - qPrintable(xdgRuntimeDir), qPrintable(file.errorString())); - return QString(); - } + qWarning("QStandardPaths: wrong permissions on runtime directory %s, %x instead of %x", + qPrintable(xdgRuntimeDir), uint(fileInfo.permissions()), uint(wantedPerms)); + return QString(); } + return xdgRuntimeDir; } default: diff --git a/tests/auto/corelib/io/qstandardpaths/tst_qstandardpaths.cpp b/tests/auto/corelib/io/qstandardpaths/tst_qstandardpaths.cpp index 1379c788d1..40023d7fea 100644 --- a/tests/auto/corelib/io/qstandardpaths/tst_qstandardpaths.cpp +++ b/tests/auto/corelib/io/qstandardpaths/tst_qstandardpaths.cpp @@ -465,16 +465,6 @@ void tst_qstandardpaths::testRuntimeDirectory() #ifdef Q_XDG_PLATFORM const QString runtimeDir = QStandardPaths::writableLocation(QStandardPaths::RuntimeLocation); QVERIFY(!runtimeDir.isEmpty()); - - // Check that it can automatically fix permissions - QFile file(runtimeDir); - const QFile::Permissions wantedPerms = QFile::ReadUser | QFile::WriteUser | QFile::ExeUser; - const QFile::Permissions additionalPerms = QFile::ReadOwner | QFile::WriteOwner | QFile::ExeOwner; - QCOMPARE(file.permissions(), wantedPerms | additionalPerms); - QVERIFY(file.setPermissions(wantedPerms | QFile::ExeGroup)); - const QString runtimeDirAgain = QStandardPaths::writableLocation(QStandardPaths::RuntimeLocation); - QCOMPARE(runtimeDirAgain, runtimeDir); - QCOMPARE(QFile(runtimeDirAgain).permissions(), wantedPerms | additionalPerms); #endif } @@ -516,11 +506,27 @@ void tst_qstandardpaths::testCustomRuntimeDirectory() const QString runtimeDir = QStandardPaths::writableLocation(QStandardPaths::RuntimeLocation); QVERIFY2(runtimeDir.isEmpty(), qPrintable(runtimeDir)); - // When $XDG_RUNTIME_DIR points to a non-existing directory, QStandardPaths should warn (QTBUG-48771) - qputenv("XDG_RUNTIME_DIR", "does_not_exist"); - QTest::ignoreMessage(QtWarningMsg, "QStandardPaths: XDG_RUNTIME_DIR points to non-existing path 'does_not_exist', please create it with 0700 permissions."); + // When $XDG_RUNTIME_DIR points to a directory with wrong permissions, QStandardPaths should warn + const QByteArray wrongPermissionFileName = "wrong_permissions"; + QDir::current().mkdir(wrongPermissionFileName); + QFile wrongPermissionFile(wrongPermissionFileName); + const QFile::Permissions wantedPerms = QFile::ReadUser | QFile::WriteUser | QFile::ExeUser; + QVERIFY(wrongPermissionFile.setPermissions(wantedPerms | QFile::ExeGroup)); + + qputenv("XDG_RUNTIME_DIR", wrongPermissionFileName); + QTest::ignoreMessage(QtWarningMsg, + qPrintable(QString::fromLatin1("QStandardPaths: wrong permissions on runtime directory " + wrongPermissionFileName + ", 7710 instead of 7700"))); + const QString wrongPermissionRuntimeDir = QStandardPaths::writableLocation(QStandardPaths::RuntimeLocation); + QVERIFY(wrongPermissionRuntimeDir.isEmpty()); + QDir::current().rmdir(wrongPermissionFileName); + + // When $XDG_RUNTIME_DIR points to a non-existing directory, QStandardPaths should create it first + const QByteArray nonExistingDir = "does_not_exist"; + qputenv("XDG_RUNTIME_DIR", nonExistingDir); const QString nonExistingRuntimeDir = QStandardPaths::writableLocation(QStandardPaths::RuntimeLocation); - QVERIFY2(nonExistingRuntimeDir.isEmpty(), qPrintable(nonExistingRuntimeDir)); + QVERIFY2(!nonExistingRuntimeDir.compare(nonExistingDir), qPrintable(nonExistingRuntimeDir)); + QVERIFY(QDir::current().exists(nonExistingRuntimeDir)); + QDir::current().rmdir(nonExistingRuntimeDir); // When $XDG_RUNTIME_DIR points to a file, QStandardPaths should warn const QString file = QFINDTESTDATA("tst_qstandardpaths.cpp"); -- cgit v1.2.3 From 689405fb5ce5a2ff7446e1d110a7686184270f89 Mon Sep 17 00:00:00 2001 From: Edward Welbourne Date: Thu, 10 Oct 2019 17:17:29 +0200 Subject: Use arrays rather than assigning literals to char* (warning-fix) Integrity has a hack, to let us link to the library mmap is in, that depends on two extern "C" symbols of type char *; but assigning a string literal to a char * variable as initializer is a const-ness violation (as the Integrity compiler does point out), so change the two variables to be char[] instead of char *, so that the literals populate (and determine the size of) the arrays, instead. Change-Id: Iab34fb378bc0522e14539592ead066f068751ad0 Reviewed-by: Qt CI Bot Reviewed-by: Thiago Macieira --- src/corelib/global/qglobal.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/corelib/global/qglobal.cpp b/src/corelib/global/qglobal.cpp index 30a44aeef8..78ad2a5421 100644 --- a/src/corelib/global/qglobal.cpp +++ b/src/corelib/global/qglobal.cpp @@ -110,8 +110,8 @@ extern "C" { // without full system POSIX. # pragma weak shm_area_password # pragma weak shm_area_name - char *shm_area_password = "dummy"; - char *shm_area_name = "dummy"; + char shm_area_password[] = "dummy"; + char shm_area_name[] = "dummy"; } #endif -- cgit v1.2.3 From 79d7487c89aead1a736f75a5fbf8c154c09924e0 Mon Sep 17 00:00:00 2001 From: Kari Hormi Date: Thu, 10 Oct 2019 17:49:43 +0300 Subject: Fix cursor not showing in empty block preceding a table When an empty text block precedes a table in QTextEdit, the cursor in the said text block is drawn twice (in order to make sure that the cursor is drawn on top of the table) with inverted colors, resulting in nothing showing up. This commit checks for an empty block before the table and skips the first drawing of the cursor if that's what it finds. Fixes: QTBUG-62919 Change-Id: I828d06e0645007ac42e3f308a35868b4f0db1380 Reviewed-by: Edward Welbourne Reviewed-by: Ville Voutilainen --- src/gui/text/qtextdocumentlayout.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/gui/text/qtextdocumentlayout.cpp b/src/gui/text/qtextdocumentlayout.cpp index cf02e2f9c8..65cafc7bc2 100644 --- a/src/gui/text/qtextdocumentlayout.cpp +++ b/src/gui/text/qtextdocumentlayout.cpp @@ -1336,8 +1336,12 @@ void QTextDocumentLayoutPrivate::drawBlock(const QPointF &offset, QPainter *pain tl->draw(painter, offset, selections, context.clip.isValid() ? (context.clip & clipRect) : clipRect); - if ((context.cursorPosition >= blpos && context.cursorPosition < blpos + bllen) - || (context.cursorPosition < -1 && !tl->preeditAreaText().isEmpty())) { + // if the block is empty and it precedes a table, do not draw the cursor. + // the cursor is drawn later after the table has been drawn so no need + // to draw it here. + if (!isEmptyBlockBeforeTable(frameIteratorForTextPosition(blpos)) + && ((context.cursorPosition >= blpos && context.cursorPosition < blpos + bllen) + || (context.cursorPosition < -1 && !tl->preeditAreaText().isEmpty()))) { int cpos = context.cursorPosition; if (cpos < -1) cpos = tl->preeditAreaPosition() - (cpos + 2); -- cgit v1.2.3 From 3478f6c4474e86f2431c22533dbffefb48af93a9 Mon Sep 17 00:00:00 2001 From: Leena Miettinen Date: Fri, 11 Oct 2019 16:30:33 +0200 Subject: Doc: Describe using QVERIFY to verify validity of QSignalSpy From https://wiki.qt.io/Writing_Unit_Tests Change-Id: I3186efe30cde465766800aee1f0a530fb80907fb Reviewed-by: Edward Welbourne --- src/testlib/qsignalspy.qdoc | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/src/testlib/qsignalspy.qdoc b/src/testlib/qsignalspy.qdoc index 3352307d69..39639d0a09 100644 --- a/src/testlib/qsignalspy.qdoc +++ b/src/testlib/qsignalspy.qdoc @@ -1,6 +1,6 @@ /**************************************************************************** ** -** Copyright (C) 2016 The Qt Company Ltd. +** Copyright (C) 2019 The Qt Company Ltd. ** Contact: https://www.qt.io/licensing/ ** ** This file is part of the documentation of the Qt Toolkit. @@ -48,7 +48,7 @@ \snippet code/doc_src_qsignalspy.cpp 1 - \b {Note:} Non-standard data types need to be registered, using + \note Non-standard data types need to be registered, using the qRegisterMetaType() function, before you can create a QSignalSpy. For example: @@ -57,6 +57,18 @@ To retrieve the instance, you can use qvariant_cast: \snippet code/doc_src_qsignalspy.cpp 3 + + \section1 Verifying Signal Emissions + + The QSignalSpy class provides an elegant mechanism for capturing the list + of signals emitted by an object. However, you should verify its validity + after construction. The constructor does a number of sanity checks, such as + verifying that the signal to be spied upon actually exists. To make the + diagnosis of test failures easier, the results of these checks should be + checked by calling \c QVERIFY(spy.isValid()) before proceeding further with + a test. + + \sa QVERIFY() */ /*! \fn QSignalSpy::QSignalSpy(const QObject *object, const char *signal) -- cgit v1.2.3 From 50481fb909c2bbbc26a193e23783e5b0151168b9 Mon Sep 17 00:00:00 2001 From: Giuseppe D'Angelo Date: Thu, 10 Oct 2019 17:29:17 +0200 Subject: QEndian: do not use "raw" constexpr Use the Qt's macros instead, since constexpr support may be revoked on certain compilers. Amends d26289ffb43a5fcf34e855db1dfbf42aa03c4f5a. Change-Id: I62354b14b57ae5fcbf3f1186ddb48bcf26535e90 Reviewed-by: Edward Welbourne --- src/corelib/global/qendian.h | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/corelib/global/qendian.h b/src/corelib/global/qendian.h index 615f523888..5cd9d3160b 100644 --- a/src/corelib/global/qendian.h +++ b/src/corelib/global/qendian.h @@ -327,9 +327,9 @@ public: return pre; } - static constexpr QSpecialInteger max() + static Q_DECL_CONSTEXPR QSpecialInteger max() { return QSpecialInteger(std::numeric_limits::max()); } - static constexpr QSpecialInteger min() + static Q_DECL_CONSTEXPR QSpecialInteger min() { return QSpecialInteger(std::numeric_limits::min()); } }; @@ -373,8 +373,8 @@ public: QLEInteger &operator ++(int); QLEInteger &operator --(int); - static constexpr QLEInteger max(); - static constexpr QLEInteger min(); + static Q_DECL_CONSTEXPR QLEInteger max(); + static Q_DECL_CONSTEXPR QLEInteger min(); }; template @@ -400,8 +400,8 @@ public: QBEInteger &operator ++(int); QBEInteger &operator --(int); - static constexpr QBEInteger max(); - static constexpr QBEInteger min(); + static Q_DECL_CONSTEXPR QBEInteger max(); + static Q_DECL_CONSTEXPR QBEInteger min(); }; #else -- cgit v1.2.3 From 2ec93e29f75e0cb37650255d478b8e3a9da4dc25 Mon Sep 17 00:00:00 2001 From: Andy Shaw Date: Wed, 9 Oct 2019 15:48:03 +0200 Subject: Cocoa: Update the PPK_PrinterName property if one is explicitly set MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a QPrinterInfo is passed in to the QPrinter then it needs to ensure that the underlying session is set up to use the specified printer, otherwise it uses the default one as it has not been changed. Change-Id: I90012223e9831303d02fd3ffc68223dc492ece0c Reviewed-by: Tor Arne Vestbø --- src/plugins/platforms/cocoa/qprintengine_mac.mm | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/plugins/platforms/cocoa/qprintengine_mac.mm b/src/plugins/platforms/cocoa/qprintengine_mac.mm index 58ad53a6e1..dcb9a85a3c 100644 --- a/src/plugins/platforms/cocoa/qprintengine_mac.mm +++ b/src/plugins/platforms/cocoa/qprintengine_mac.mm @@ -59,6 +59,8 @@ QMacPrintEngine::QMacPrintEngine(QPrinter::PrinterMode mode, const QString &devi QString id = deviceId; if (id.isEmpty()) id = QCocoaPrinterSupport().defaultPrintDeviceId(); + else + setProperty(QPrintEngine::PPK_PrinterName, deviceId); d->m_printDevice.reset(new QCocoaPrintDevice(id)); d->m_pageLayout.setPageSize(d->m_printDevice->defaultPageSize()); d->initialize(); -- cgit v1.2.3 From ed7dd9a6edd7afd8798751c8b1d3ee7802ae253f Mon Sep 17 00:00:00 2001 From: Christian Ehrlicher Date: Sun, 6 Oct 2019 12:56:34 +0200 Subject: QODBC: Fix crash when a prepared statement is deleted after the db was removed When a prepared statement is still alive after the database was removed with QSqlDatabase::removeDatabase(), the cleanup routine is trying to access the driver which is no longer alive which results in a crash. Fixes: QTBUG-79019 Change-Id: I4630e3b947a12b23ed062f015abc373fc0e246c1 Reviewed-by: Andy Shaw --- src/plugins/sqldrivers/odbc/qsql_odbc.cpp | 10 ++++----- tests/auto/sql/kernel/qsqlquery/tst_qsqlquery.cpp | 27 +++++++++++------------ 2 files changed, 18 insertions(+), 19 deletions(-) diff --git a/src/plugins/sqldrivers/odbc/qsql_odbc.cpp b/src/plugins/sqldrivers/odbc/qsql_odbc.cpp index 7f98efccba..7709b13cd1 100644 --- a/src/plugins/sqldrivers/odbc/qsql_odbc.cpp +++ b/src/plugins/sqldrivers/odbc/qsql_odbc.cpp @@ -221,18 +221,18 @@ public: int disconnectCount; bool hasSQLFetchScroll; - bool isStmtHandleValid(); + bool isStmtHandleValid() const; void updateStmtHandleState(); }; -bool QODBCResultPrivate::isStmtHandleValid() +bool QODBCResultPrivate::isStmtHandleValid() const { - return disconnectCount == drv_d_func()->disconnectCount; + return drv_d_func() && disconnectCount == drv_d_func()->disconnectCount; } void QODBCResultPrivate::updateStmtHandleState() { - disconnectCount = drv_d_func()->disconnectCount; + disconnectCount = drv_d_func() ? drv_d_func()->disconnectCount : 0; } static QString qWarnODBCHandle(int handleType, SQLHANDLE handle, int *nativeCode = 0) @@ -975,7 +975,7 @@ QODBCResult::QODBCResult(const QODBCDriver *db) QODBCResult::~QODBCResult() { Q_D(QODBCResult); - if (d->hStmt && d->isStmtHandleValid() && driver()->isOpen()) { + if (d->hStmt && d->isStmtHandleValid() && driver() && driver()->isOpen()) { SQLRETURN r = SQLFreeHandle(SQL_HANDLE_STMT, d->hStmt); if (r != SQL_SUCCESS) qSqlWarning(QLatin1String("QODBCDriver: Unable to free statement handle ") diff --git a/tests/auto/sql/kernel/qsqlquery/tst_qsqlquery.cpp b/tests/auto/sql/kernel/qsqlquery/tst_qsqlquery.cpp index 784d0a70d7..fd7f24f308 100644 --- a/tests/auto/sql/kernel/qsqlquery/tst_qsqlquery.cpp +++ b/tests/auto/sql/kernel/qsqlquery/tst_qsqlquery.cpp @@ -196,8 +196,7 @@ private slots: void task_250026_data() { generic_data("QODBC"); } void task_250026(); - void task_205701_data() { generic_data("QMYSQL"); } - void task_205701(); + void crashQueryOnCloseDatabase(); void task_233829_data() { generic_data("QPSQL"); } void task_233829(); @@ -311,6 +310,8 @@ void tst_QSqlQuery::init() void tst_QSqlQuery::cleanup() { + if (QTest::currentTestFunction() == QLatin1String("crashQueryOnCloseDatabase")) + return; QFETCH( QString, dbName ); QSqlDatabase db = QSqlDatabase::database( dbName ); CHECK_DATABASE( db ); @@ -3448,19 +3449,17 @@ void tst_QSqlQuery::task_250026() QCOMPARE( q.value( 0 ).toString().length(), data1026.length() ); } -void tst_QSqlQuery::task_205701() +void tst_QSqlQuery::crashQueryOnCloseDatabase() { - QSqlDatabase qsdb = QSqlDatabase::addDatabase("QMYSQL", "atest"); - qsdb.setHostName("test"); - qsdb.setDatabaseName("test"); - qsdb.setUserName("test"); - qsdb.setPassword("test"); - qsdb.open(); - -// { - QSqlQuery query(qsdb); -// } - QSqlDatabase::removeDatabase("atest"); + for (const auto &dbName : qAsConst(dbs.dbNames)) { + QSqlDatabase clonedDb = QSqlDatabase::cloneDatabase( + QSqlDatabase::database(dbName), "crashTest"); + qDebug() << "Testing crash in sqlquery dtor for driver" << clonedDb.driverName(); + QVERIFY(clonedDb.open()); + QSqlQuery q(clonedDb); + clonedDb.close(); + QSqlDatabase::removeDatabase("crashTest"); + } } #ifdef NOT_READY_YET -- cgit v1.2.3 From d3398207d0384e786f466662fa40c6fb86844fb6 Mon Sep 17 00:00:00 2001 From: Eskil Abrahamsen Blomfeldt Date: Wed, 16 Oct 2019 11:34:29 +0200 Subject: Revert "Release left button before showing the popup context menu" This partially reverts commit 5e8b16f0e4247cc978b08480450526cfa3b25029. Releasing the mouse button synthetically made it impossible to use tap and hold gestures. When investigating, it seems that other changes have fixed the original issue that 5e8b16f0e4247cc978b08480450526cfa3b25029 was meant to address, so this is no longer needed. [ChangeLog][Android] Fixed regression that made it impossible for an application to use the tap-and-hold gesture. Fixes: QTBUG-72408 Change-Id: I53f687d047a4ad0fdf3c8c96a00ed1b11d09f047 Reviewed-by: BogDan Vatra --- src/plugins/platforms/android/androidjniinput.cpp | 14 -------------- src/plugins/platforms/android/androidjniinput.h | 2 -- src/plugins/platforms/android/qandroidinputcontext.cpp | 3 --- 3 files changed, 19 deletions(-) diff --git a/src/plugins/platforms/android/androidjniinput.cpp b/src/plugins/platforms/android/androidjniinput.cpp index 9cdc5de0e1..56885f2e23 100644 --- a/src/plugins/platforms/android/androidjniinput.cpp +++ b/src/plugins/platforms/android/androidjniinput.cpp @@ -195,20 +195,6 @@ namespace QtAndroidInput angleDelta); } - void releaseMouse(int x, int y) - { - m_ignoreMouseEvents = true; - QPoint globalPos(x,y); - QWindow *tlw = topLevelWindowAt(globalPos); - QPoint localPos = tlw ? (globalPos-tlw->position()) : globalPos; - - // Release left button - QWindowSystemInterface::handleMouseEvent(tlw, - localPos, - globalPos, - Qt::MouseButtons(Qt::NoButton)); - } - static void longPress(JNIEnv */*env*/, jobject /*thiz*/, jint /*winId*/, jint x, jint y) { QAndroidInputContext *inputContext = QAndroidInputContext::androidInputContext(); diff --git a/src/plugins/platforms/android/androidjniinput.h b/src/plugins/platforms/android/androidjniinput.h index 2e2470ae8f..cc3070c4aa 100644 --- a/src/plugins/platforms/android/androidjniinput.h +++ b/src/plugins/platforms/android/androidjniinput.h @@ -61,8 +61,6 @@ namespace QtAndroidInput void updateHandles(int handleCount, QPoint editMenuPos = QPoint(), uint32_t editButtons = 0, QPoint cursor = QPoint(), QPoint anchor = QPoint(), bool rtl = false); bool registerNatives(JNIEnv *env); - - void releaseMouse(int x, int y); } QT_END_NAMESPACE diff --git a/src/plugins/platforms/android/qandroidinputcontext.cpp b/src/plugins/platforms/android/qandroidinputcontext.cpp index 5614d3b04f..e78c317863 100644 --- a/src/plugins/platforms/android/qandroidinputcontext.cpp +++ b/src/plugins/platforms/android/qandroidinputcontext.cpp @@ -827,9 +827,6 @@ void QAndroidInputContext::longPress(int x, int y) focusObjectStopComposing(); - // Release left button, otherwise the following events will cancel the menu popup - QtAndroidInput::releaseMouse(x, y); - const double pixelDensity = QGuiApplication::focusWindow() ? QHighDpiScaling::factor(QGuiApplication::focusWindow()) -- cgit v1.2.3 From a9dcd772c032b5cb52f86e6cd898db3d3bc81023 Mon Sep 17 00:00:00 2001 From: Leena Miettinen Date: Fri, 18 Oct 2019 16:11:55 +0200 Subject: Doc: Replace \b {Note:} with \note Change-Id: I1f6947acec4494c151317e1faf79720dad0da6bb Reviewed-by: Martin Smith --- src/testlib/doc/src/qttestlib-manual.qdoc | 4 +--- src/testlib/qtestcase.qdoc | 24 ++++++++++++------------ 2 files changed, 13 insertions(+), 15 deletions(-) diff --git a/src/testlib/doc/src/qttestlib-manual.qdoc b/src/testlib/doc/src/qttestlib-manual.qdoc index a3644c1623..ee7767b5a5 100644 --- a/src/testlib/doc/src/qttestlib-manual.qdoc +++ b/src/testlib/doc/src/qttestlib-manual.qdoc @@ -355,15 +355,13 @@ counters can be obtained by running any benchmark executable with the option \c -perfcounterlist. - \list - \li \b Notes: + \note \list \li Using the performance counter may require enabling access to non-privileged applications. \li Devices that do not support high-resolution timers default to one-millisecond granularity. \endlist - \endlist See \l {Chapter 5: Writing a Benchmark}{Writing a Benchmark} in the Qt Test Tutorial for more benchmarking examples. diff --git a/src/testlib/qtestcase.qdoc b/src/testlib/qtestcase.qdoc index f3761cdfa8..9006d7b401 100644 --- a/src/testlib/qtestcase.qdoc +++ b/src/testlib/qtestcase.qdoc @@ -235,7 +235,7 @@ \c aString and \c expected are variables on the stack that are initialized with the current test data. - \b {Note:} This macro can only be used in a test function that is invoked + \note This macro can only be used in a test function that is invoked by the test framework. The test function must have a _data function. */ @@ -282,7 +282,7 @@ This macro can be used to force a test failure. The test stops executing and the failure \a message is appended to the test log. - \b {Note:} This macro can only be used in a test function that is invoked + \note This macro can only be used in a test function that is invoked by the test framework. Example: @@ -359,7 +359,7 @@ \a mode is a \l QTest::TestFailMode and sets whether the test should continue to execute or not. - \b {Note:} This macro can only be used in a test function that is invoked + \note This macro can only be used in a test function that is invoked by the test framework. Example 1: @@ -421,13 +421,13 @@ test has been installed, and regardless of whether the test's build tree is equal to the test's source tree. - \b {Note:} reliable detection of testdata from the source directory requires + \note reliable detection of testdata from the source directory requires either that qmake is used, or the \c{QT_TESTCASE_BUILDDIR} macro is defined to point to the working directory from which the compiler is invoked, or only absolute paths to the source files are passed to the compiler. Otherwise, the absolute path of the source directory cannot be determined. - \b {Note:} For tests that use the \l QTEST_APPLESS_MAIN() macro to generate a + \note For tests that use the \l QTEST_APPLESS_MAIN() macro to generate a \c{main()} function, \c{QFINDTESTDATA} will not attempt to find test data relative to QCoreApplication::applicationDirPath(). In practice, this means that tests using \c{QTEST_APPLESS_MAIN()} will fail to find their test data @@ -449,7 +449,7 @@ Similarly, if qmake is used and the configuration includes \c{QT += gui}, then \c QT_GUI_LIB will be defined automatically. - \b {Note:} On platforms that have keypad navigation enabled by default, + \note On platforms that have keypad navigation enabled by default, this macro will forcefully disable it if \c QT_WIDGETS_LIB is defined. This is done to simplify the usage of key events when writing autotests. If you wish to write a test case that uses keypad navigation, you should enable it either in the @@ -689,7 +689,7 @@ Simulates pressing a \a key with an optional \a modifier on a \a widget. If \a delay is larger than 0, the test will wait for \a delay milliseconds before pressing the key. - \b {Note:} At some point you should release the key using \l keyRelease(). + \note At some point you should release the key using \l keyRelease(). \sa QTest::keyRelease(), QTest::keyClick() */ @@ -701,7 +701,7 @@ If \a delay is larger than 0, the test will wait for \a delay milliseconds before pressing the key. - \b {Note:} At some point you should release the key using \l keyRelease(). + \note At some point you should release the key using \l keyRelease(). \sa QTest::keyRelease(), QTest::keyClick() */ @@ -713,7 +713,7 @@ Simulates pressing a \a key with an optional \a modifier on a \a window. If \a delay is larger than 0, the test will wait for \a delay milliseconds before pressing the key. - \b {Note:} At some point you should release the key using \l keyRelease(). + \note At some point you should release the key using \l keyRelease(). \sa QTest::keyRelease(), QTest::keyClick() */ @@ -726,7 +726,7 @@ If \a delay is larger than 0, the test will wait for \a delay milliseconds before pressing the key. - \b {Note:} At some point you should release the key using \l keyRelease(). + \note At some point you should release the key using \l keyRelease(). \sa QTest::keyRelease(), QTest::keyClick() */ @@ -947,12 +947,12 @@ You can add specializations or overloads of this function to your test to enable verbose output. - \b {Note:} Starting with Qt 5.5, you should prefer to provide a toString() function + \note Starting with Qt 5.5, you should prefer to provide a toString() function in the type's namespace instead of specializing this template. If your code needs to continue to work with the QTestLib from Qt 5.4 or earlier, you need to continue to use specialization. - \b {Note:} The caller of toString() must delete the returned data + \note The caller of toString() must delete the returned data using \c{delete[]}. Your implementation should return a string created with \c{new[]} or qstrdup(). The easiest way to do so is to create a QByteArray or QString and calling QTest::toString() on it -- cgit v1.2.3 From b61c6164c100defc519b178d73858df59cffc48d Mon Sep 17 00:00:00 2001 From: Hernan Martinez Date: Sat, 12 Oct 2019 02:40:29 -0400 Subject: QtGui: Disable Windows on ARM64 preprocessor conflict in QtOpenGL The Windows API MemoryBarrier function is actually a macro when _M_ARM64 is defined and it conflicts with the MemoryBarrier method when it's declared and used. Task-number: QTBUG-77388 Change-Id: I762edfc4ca1a44cbe095724de708c7cdad34ae65 Reviewed-by: Laszlo Agocs --- src/gui/opengl/qopenglfunctions_4_2_compatibility.h | 10 ++++++++++ src/gui/opengl/qopenglfunctions_4_2_core.h | 10 ++++++++++ src/gui/opengl/qopenglfunctions_4_3_compatibility.h | 10 ++++++++++ src/gui/opengl/qopenglfunctions_4_3_core.h | 11 +++++++++++ src/gui/opengl/qopenglfunctions_4_4_compatibility.h | 10 ++++++++++ src/gui/opengl/qopenglfunctions_4_4_core.h | 10 ++++++++++ src/gui/opengl/qopenglfunctions_4_5_compatibility.h | 10 ++++++++++ src/gui/opengl/qopenglfunctions_4_5_core.h | 11 +++++++++++ src/gui/opengl/qopenglversionfunctions.h | 10 ++++++++++ src/openglextensions/qopenglextensions.cpp | 9 +++++++++ src/openglextensions/qopenglextensions.h | 10 ++++++++++ 11 files changed, 111 insertions(+) diff --git a/src/gui/opengl/qopenglfunctions_4_2_compatibility.h b/src/gui/opengl/qopenglfunctions_4_2_compatibility.h index 6726d5fc44..a48d581c2d 100644 --- a/src/gui/opengl/qopenglfunctions_4_2_compatibility.h +++ b/src/gui/opengl/qopenglfunctions_4_2_compatibility.h @@ -57,6 +57,12 @@ #include #include +// MemoryBarrier is a macro on some architectures on Windows +#ifdef Q_OS_WIN +#pragma push_macro("MemoryBarrier") +#undef MemoryBarrier +#endif + QT_BEGIN_NAMESPACE class Q_GUI_EXPORT QOpenGLFunctions_4_2_Compatibility : public QAbstractOpenGLFunctions @@ -5632,6 +5638,10 @@ inline void QOpenGLFunctions_4_2_Compatibility::glVertexAttribI1i(GLuint index, QT_END_NAMESPACE +#ifdef Q_OS_WIN +#pragma pop_macro("MemoryBarrier") +#endif + #endif // QT_NO_OPENGL && !QT_OPENGL_ES_2 #endif diff --git a/src/gui/opengl/qopenglfunctions_4_2_core.h b/src/gui/opengl/qopenglfunctions_4_2_core.h index a921329741..5ca98e9808 100644 --- a/src/gui/opengl/qopenglfunctions_4_2_core.h +++ b/src/gui/opengl/qopenglfunctions_4_2_core.h @@ -57,6 +57,12 @@ #include #include +// MemoryBarrier is a macro on some architectures on Windows +#ifdef Q_OS_WIN +#pragma push_macro("MemoryBarrier") +#undef MemoryBarrier +#endif + QT_BEGIN_NAMESPACE class Q_GUI_EXPORT QOpenGLFunctions_4_2_Core : public QAbstractOpenGLFunctions @@ -3027,6 +3033,10 @@ inline void QOpenGLFunctions_4_2_Core::glDrawArraysInstancedBaseInstance(GLenum QT_END_NAMESPACE +#ifdef Q_OS_WIN +#pragma pop_macro("MemoryBarrier") +#endif + #endif // QT_NO_OPENGL && !QT_OPENGL_ES_2 #endif diff --git a/src/gui/opengl/qopenglfunctions_4_3_compatibility.h b/src/gui/opengl/qopenglfunctions_4_3_compatibility.h index b9d4eb1d6f..d969f5b3b4 100644 --- a/src/gui/opengl/qopenglfunctions_4_3_compatibility.h +++ b/src/gui/opengl/qopenglfunctions_4_3_compatibility.h @@ -57,6 +57,12 @@ #include #include +// MemoryBarrier is a macro on some architectures on Windows +#ifdef Q_OS_WIN +#pragma push_macro("MemoryBarrier") +#undef MemoryBarrier +#endif + QT_BEGIN_NAMESPACE class Q_GUI_EXPORT QOpenGLFunctions_4_3_Compatibility : public QAbstractOpenGLFunctions @@ -5839,6 +5845,10 @@ inline void QOpenGLFunctions_4_3_Compatibility::glVertexAttribI1i(GLuint index, QT_END_NAMESPACE +#ifdef Q_OS_WIN +#pragma pop_macro("MemoryBarrier") +#endif + #endif // QT_NO_OPENGL && !QT_OPENGL_ES_2 #endif diff --git a/src/gui/opengl/qopenglfunctions_4_3_core.h b/src/gui/opengl/qopenglfunctions_4_3_core.h index da552d64af..13675caf62 100644 --- a/src/gui/opengl/qopenglfunctions_4_3_core.h +++ b/src/gui/opengl/qopenglfunctions_4_3_core.h @@ -57,6 +57,13 @@ #include #include +// MemoryBarrier is a macro on some architectures on Windows +#ifdef Q_OS_WIN +#pragma push_macro("MemoryBarrier") +#undef MemoryBarrier +#endif + + QT_BEGIN_NAMESPACE class Q_GUI_EXPORT QOpenGLFunctions_4_3_Core : public QAbstractOpenGLFunctions @@ -3230,6 +3237,10 @@ inline void QOpenGLFunctions_4_3_Core::glClearBufferData(GLenum target, GLenum i QT_END_NAMESPACE +#ifdef Q_OS_WIN +#pragma pop_macro("MemoryBarrier") +#endif + #endif // QT_NO_OPENGL && !QT_OPENGL_ES_2 #endif diff --git a/src/gui/opengl/qopenglfunctions_4_4_compatibility.h b/src/gui/opengl/qopenglfunctions_4_4_compatibility.h index 7a05bd802d..0acab349a1 100644 --- a/src/gui/opengl/qopenglfunctions_4_4_compatibility.h +++ b/src/gui/opengl/qopenglfunctions_4_4_compatibility.h @@ -59,6 +59,12 @@ QT_BEGIN_NAMESPACE +// MemoryBarrier is a macro on some architectures on Windows +#ifdef Q_OS_WIN +#pragma push_macro("MemoryBarrier") +#undef MemoryBarrier +#endif + class Q_GUI_EXPORT QOpenGLFunctions_4_4_Compatibility : public QAbstractOpenGLFunctions { public: @@ -5961,6 +5967,10 @@ inline void QOpenGLFunctions_4_4_Compatibility::glVertexP2ui(GLenum type, GLuint QT_END_NAMESPACE +#ifdef Q_OS_WIN +#pragma pop_macro("MemoryBarrier") +#endif + #endif // QT_NO_OPENGL && !QT_OPENGL_ES_2 #endif diff --git a/src/gui/opengl/qopenglfunctions_4_4_core.h b/src/gui/opengl/qopenglfunctions_4_4_core.h index 6b29a9659b..1ad6f40214 100644 --- a/src/gui/opengl/qopenglfunctions_4_4_core.h +++ b/src/gui/opengl/qopenglfunctions_4_4_core.h @@ -57,6 +57,12 @@ #include #include +// MemoryBarrier is a macro on some architectures on Windows +#ifdef Q_OS_WIN +#pragma push_macro("MemoryBarrier") +#undef MemoryBarrier +#endif + QT_BEGIN_NAMESPACE class Q_GUI_EXPORT QOpenGLFunctions_4_4_Core : public QAbstractOpenGLFunctions @@ -3415,6 +3421,10 @@ inline void QOpenGLFunctions_4_4_Core::glBufferStorage(GLenum target, GLsizeiptr QT_END_NAMESPACE +#ifdef Q_OS_WIN +#pragma pop_macro("MemoryBarrier") +#endif + #endif // QT_NO_OPENGL && !QT_OPENGL_ES_2 #endif diff --git a/src/gui/opengl/qopenglfunctions_4_5_compatibility.h b/src/gui/opengl/qopenglfunctions_4_5_compatibility.h index a809c1c90b..9d9d14548b 100644 --- a/src/gui/opengl/qopenglfunctions_4_5_compatibility.h +++ b/src/gui/opengl/qopenglfunctions_4_5_compatibility.h @@ -57,6 +57,12 @@ #include #include +// MemoryBarrier is a macro on some architectures on Windows +#ifdef Q_OS_WIN +#pragma push_macro("MemoryBarrier") +#undef MemoryBarrier +#endif + QT_BEGIN_NAMESPACE class Q_GUI_EXPORT QOpenGLFunctions_4_5_Compatibility : public QAbstractOpenGLFunctions @@ -6679,6 +6685,10 @@ inline void QOpenGLFunctions_4_5_Compatibility::glGetnMapdv(GLenum target, GLenu QT_END_NAMESPACE +#ifdef Q_OS_WIN +#pragma pop_macro("MemoryBarrier") +#endif + #endif // QT_NO_OPENGL && !QT_OPENGL_ES_2 #endif diff --git a/src/gui/opengl/qopenglfunctions_4_5_core.h b/src/gui/opengl/qopenglfunctions_4_5_core.h index bb1b17f7b1..bf872c628b 100644 --- a/src/gui/opengl/qopenglfunctions_4_5_core.h +++ b/src/gui/opengl/qopenglfunctions_4_5_core.h @@ -57,6 +57,12 @@ #include #include +// MemoryBarrier is a macro on some architectures on Windows +#ifdef Q_OS_WIN +#pragma push_macro("MemoryBarrier") +#undef MemoryBarrier +#endif + QT_BEGIN_NAMESPACE class Q_GUI_EXPORT QOpenGLFunctions_4_5_Core : public QAbstractOpenGLFunctions @@ -4056,6 +4062,11 @@ inline void QOpenGLFunctions_4_5_Core::glClipControl(GLenum origin, GLenum depth QT_END_NAMESPACE +#ifdef Q_OS_WIN +#pragma pop_macro("MemoryBarrier") +#endif + + #endif // QT_NO_OPENGL && !QT_OPENGL_ES_2 #endif diff --git a/src/gui/opengl/qopenglversionfunctions.h b/src/gui/opengl/qopenglversionfunctions.h index f828e5668b..99c8565fbc 100644 --- a/src/gui/opengl/qopenglversionfunctions.h +++ b/src/gui/opengl/qopenglversionfunctions.h @@ -61,6 +61,12 @@ #include #include +// MemoryBarrier is a macro on some architectures on Windows +#ifdef Q_OS_WIN +#pragma push_macro("MemoryBarrier") +#undef MemoryBarrier +#endif + QT_BEGIN_NAMESPACE class QOpenGLContext; @@ -1897,6 +1903,10 @@ public: QT_END_NAMESPACE +#ifdef Q_OS_WIN +#pragma pop_macro("MemoryBarrier") +#endif + #endif // QT_NO_OPENGL #endif diff --git a/src/openglextensions/qopenglextensions.cpp b/src/openglextensions/qopenglextensions.cpp index 6413ae4a78..1660181e97 100644 --- a/src/openglextensions/qopenglextensions.cpp +++ b/src/openglextensions/qopenglextensions.cpp @@ -60,6 +60,12 @@ #include "qopenglextensions.h" #include +// MemoryBarrier is a macro on some architectures on Windows +#ifdef Q_OS_WIN +#pragma push_macro("MemoryBarrier") +#undef MemoryBarrier +#endif + QT_BEGIN_NAMESPACE QAbstractOpenGLExtension::~QAbstractOpenGLExtension() @@ -7720,3 +7726,6 @@ bool QOpenGLExtension_QCOM_tiled_rendering::initializeOpenGLFunctions() QT_END_NAMESPACE +#ifdef Q_OS_WIN +#pragma pop_macro("MemoryBarrier") +#endif diff --git a/src/openglextensions/qopenglextensions.h b/src/openglextensions/qopenglextensions.h index 439e0e6530..eb473f3699 100644 --- a/src/openglextensions/qopenglextensions.h +++ b/src/openglextensions/qopenglextensions.h @@ -66,6 +66,12 @@ #include +// MemoryBarrier is a macro on some architectures on Windows +#ifdef Q_OS_WIN +#pragma push_macro("MemoryBarrier") +#undef MemoryBarrier +#endif + QT_BEGIN_NAMESPACE class QOpenGLContext; @@ -19473,6 +19479,10 @@ inline void QOpenGLExtension_QCOM_tiled_rendering::glEndTilingQCOM(GLbitfield pr QT_END_NAMESPACE +#ifdef Q_OS_WIN +#pragma pop_macro("MemoryBarrier") +#endif + #endif // QT_NO_OPENGL #endif -- cgit v1.2.3 From 03717be7885d84783bc8ea32a65e42e4970f59d6 Mon Sep 17 00:00:00 2001 From: Eirik Aavitsland Date: Wed, 23 Oct 2019 13:35:13 +0200 Subject: Fix: confusion in QImage paintEngine creation on shared images During the creation of a raster paint engine in QImage::paintEngine(), the QImage will be detached. At least old gcc versions would get confused so that the newly created paintengine would end up in the old QImage copy insteads of the newly detached one. Work around by dropping the temporary engine pointer. Fixes: QTBUG-79383 Change-Id: I27b1f24312269bc2bcc641dc4334397a92e3bfbb Reviewed-by: Allan Sandfeld Jensen --- src/gui/image/qimage.cpp | 6 +++--- tests/auto/gui/image/qimage/tst_qimage.cpp | 6 ++++++ 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/gui/image/qimage.cpp b/src/gui/image/qimage.cpp index dda407181a..2779b97fbd 100644 --- a/src/gui/image/qimage.cpp +++ b/src/gui/image/qimage.cpp @@ -4145,11 +4145,11 @@ QPaintEngine *QImage::paintEngine() const if (!d->paintEngine) { QPaintDevice *paintDevice = const_cast(this); - QPaintEngine *paintEngine = 0; QPlatformIntegration *platformIntegration = QGuiApplicationPrivate::platformIntegration(); if (platformIntegration) - paintEngine = platformIntegration->createImagePaintEngine(paintDevice); - d->paintEngine = paintEngine ? paintEngine : new QRasterPaintEngine(paintDevice); + d->paintEngine = platformIntegration->createImagePaintEngine(paintDevice); + if (!d->paintEngine) + d->paintEngine = new QRasterPaintEngine(paintDevice); } return d->paintEngine; diff --git a/tests/auto/gui/image/qimage/tst_qimage.cpp b/tests/auto/gui/image/qimage/tst_qimage.cpp index b84aa52465..2a9b92ed35 100644 --- a/tests/auto/gui/image/qimage/tst_qimage.cpp +++ b/tests/auto/gui/image/qimage/tst_qimage.cpp @@ -2132,6 +2132,12 @@ void tst_QImage::paintEngine() QCOMPARE(engine, img.paintEngine()); QCOMPARE(img, expected); + + { + QImage img1(16, 16, QImage::Format_ARGB32); + QImage img2 = img1; + QVERIFY(img2.paintEngine()); + } } void tst_QImage::setAlphaChannelWhilePainting() -- cgit v1.2.3 From 6326d1df39fc68e47d1cd34d6cd8b021bdd66045 Mon Sep 17 00:00:00 2001 From: Laszlo Agocs Date: Tue, 22 Oct 2019 10:47:35 +0200 Subject: Store a native resource binding map in QShader MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The deserializer remains compatible with .qsb files without this additional section. Task-number: QTBUG-79368 Change-Id: I03e2a634febbd88da7f6a4369f104855ea31e3af Reviewed-by: Christian Strømme --- src/gui/rhi/qshader.cpp | 144 ++++++++++++++++++++++++++++++++++++++++------ src/gui/rhi/qshader_p.h | 5 ++ src/gui/rhi/qshader_p_p.h | 4 +- 3 files changed, 135 insertions(+), 18 deletions(-) diff --git a/src/gui/rhi/qshader.cpp b/src/gui/rhi/qshader.cpp index 6a2c596557..c22b029dc8 100644 --- a/src/gui/rhi/qshader.cpp +++ b/src/gui/rhi/qshader.cpp @@ -214,7 +214,8 @@ QT_BEGIN_NAMESPACE QShader, it indicates no shader code was found for the requested key. */ -static const int QSB_VERSION = 1; +static const int QSB_VERSION = 2; +static const int QSB_VERSION_WITHOUT_BINDINGS = 1; /*! Constructs a new, empty (and thus invalid) QShader instance. @@ -345,6 +346,14 @@ void QShader::removeShader(const QShaderKey &key) d->shaders.erase(it); } +static void writeShaderKey(QDataStream *ds, const QShaderKey &k) +{ + *ds << k.source(); + *ds << k.sourceVersion().version(); + *ds << k.sourceVersion().flags(); + *ds << k.sourceVariant(); +} + /*! \return a serialized binary version of all the data held by the QShader, suitable for writing to files or other I/O devices. @@ -365,18 +374,42 @@ QByteArray QShader::serialized() const ds << d->shaders.count(); for (auto it = d->shaders.cbegin(), itEnd = d->shaders.cend(); it != itEnd; ++it) { const QShaderKey &k(it.key()); - ds << k.source(); - ds << k.sourceVersion().version(); - ds << k.sourceVersion().flags(); - ds << k.sourceVariant(); + writeShaderKey(&ds, k); const QShaderCode &shader(d->shaders.value(k)); ds << shader.shader(); ds << shader.entryPoint(); } + ds << d->bindings.count(); + for (auto it = d->bindings.cbegin(), itEnd = d->bindings.cend(); it != itEnd; ++it) { + const QShaderKey &k(it.key()); + writeShaderKey(&ds, k); + const NativeResourceBindingMap &map(it.value()); + ds << map.count(); + for (auto mapIt = map.cbegin(), mapItEnd = map.cend(); mapIt != mapItEnd; ++mapIt) { + ds << mapIt.key(); + ds << mapIt.value().first; + ds << mapIt.value().second; + } + } return qCompress(buf.buffer()); } +static void readShaderKey(QDataStream *ds, QShaderKey *k) +{ + int intVal; + *ds >> intVal; + k->setSource(QShader::Source(intVal)); + QShaderVersion ver; + *ds >> intVal; + ver.setVersion(intVal); + *ds >> intVal; + ver.setFlags(QShaderVersion::Flags(intVal)); + k->setSourceVersion(ver); + *ds >> intVal; + k->setSourceVariant(QShader::Variant(intVal)); +} + /*! Creates a new QShader instance from the given \a data. @@ -396,8 +429,11 @@ QShader QShader::fromSerialized(const QByteArray &data) Q_ASSERT(d->ref.loadRelaxed() == 1); // must be detached int intVal; ds >> intVal; - if (intVal != QSB_VERSION) + const int qsbVersion = intVal; + if (qsbVersion != QSB_VERSION && qsbVersion != QSB_VERSION_WITHOUT_BINDINGS) { + qWarning("Attempted to deserialize QShader with unknown version %d.", qsbVersion); return QShader(); + } ds >> intVal; d->stage = Stage(intVal); @@ -408,16 +444,7 @@ QShader QShader::fromSerialized(const QByteArray &data) ds >> count; for (int i = 0; i < count; ++i) { QShaderKey k; - ds >> intVal; - k.setSource(Source(intVal)); - QShaderVersion ver; - ds >> intVal; - ver.setVersion(intVal); - ds >> intVal; - ver.setFlags(QShaderVersion::Flags(intVal)); - k.setSourceVersion(ver); - ds >> intVal; - k.setSourceVariant(Variant(intVal)); + readShaderKey(&ds, &k); QShaderCode shader; QByteArray s; ds >> s; @@ -427,6 +454,27 @@ QShader QShader::fromSerialized(const QByteArray &data) d->shaders[k] = shader; } + if (qsbVersion != QSB_VERSION_WITHOUT_BINDINGS) { + ds >> count; + for (int i = 0; i < count; ++i) { + QShaderKey k; + readShaderKey(&ds, &k); + NativeResourceBindingMap map; + int mapSize; + ds >> mapSize; + for (int b = 0; b < mapSize; ++b) { + int binding; + ds >> binding; + int firstNativeBinding; + ds >> firstNativeBinding; + int secondNativeBinding; + ds >> secondNativeBinding; + map.insert(binding, { firstNativeBinding, secondNativeBinding }); + } + d->bindings.insert(k, map); + } + } + return bs; } @@ -460,7 +508,7 @@ bool operator==(const QShader &lhs, const QShader &rhs) Q_DECL_NOTHROW { return lhs.d->stage == rhs.d->stage && lhs.d->shaders == rhs.d->shaders; - // do not bother with desc, if the shader code is the same, the description must match too + // do not bother with desc and bindings, if the shader code is the same, the description must match too } /*! @@ -586,4 +634,66 @@ QDebug operator<<(QDebug dbg, const QShaderVersion &v) } #endif // QT_NO_DEBUG_STREAM +/*! + \typedef QShader::NativeResourceBindingMap + + Synonym for QHash>. + + The resource binding model QRhi assumes is based on SPIR-V. This means that + uniform buffers, storage buffers, combined image samplers, and storage + images share a common binding point space. The binding numbers in + QShaderDescription and QRhiShaderResourceBinding are expected to match the + \c binding layout qualifier in the Vulkan-compatible GLSL shader. + + Graphics APIs other than Vulkan may use a resource binding model that is + not fully compatible with this. In addition, the generator of the shader + code translated from SPIR-V may choose not to take the SPIR-V binding + qualifiers into account, for various reasons. (this is the case with the + Metal backend of SPIRV-Cross, for example). + + Therefore, a QShader may expose an additional map that describes what the + native binding point for a given SPIR-V binding is. The QRhi backends are + expected to use this map automatically, as appropriate. The value is a + pair, because combined image samplers may map to two native resources (a + texture and a sampler) in some shading languages. In that case the second + value refers to the sampler. +*/ + +/*! + \return the native binding map for \a key or null if no extra mapping is + available, or is not applicable. + */ +const QShader::NativeResourceBindingMap *QShader::nativeResourceBindingMap(const QShaderKey &key) const +{ + auto it = d->bindings.constFind(key); + if (it == d->bindings.cend()) + return nullptr; + + return &it.value(); +} + +/*! + Stores the given native resource binding \a map associated with \a key. + + \sa nativeResourceBindingMap() + */ +void QShader::setResourceBindingMap(const QShaderKey &key, const NativeResourceBindingMap &map) +{ + detach(); + d->bindings[key] = map; +} + +/*! + Removes the native resource binding map for \a key. + */ +void QShader::removeResourceBindingMap(const QShaderKey &key) +{ + auto it = d->bindings.find(key); + if (it == d->bindings.end()) + return; + + detach(); + d->bindings.erase(it); +} + QT_END_NAMESPACE diff --git a/src/gui/rhi/qshader_p.h b/src/gui/rhi/qshader_p.h index 243842a95a..4b561b6fa9 100644 --- a/src/gui/rhi/qshader_p.h +++ b/src/gui/rhi/qshader_p.h @@ -149,6 +149,11 @@ public: QByteArray serialized() const; static QShader fromSerialized(const QByteArray &data); + using NativeResourceBindingMap = QHash >; // binding -> native_binding[, native_binding] + const NativeResourceBindingMap *nativeResourceBindingMap(const QShaderKey &key) const; + void setResourceBindingMap(const QShaderKey &key, const NativeResourceBindingMap &map); + void removeResourceBindingMap(const QShaderKey &key); + private: QShaderPrivate *d; friend struct QShaderPrivate; diff --git a/src/gui/rhi/qshader_p_p.h b/src/gui/rhi/qshader_p_p.h index 6473590e95..4535e01491 100644 --- a/src/gui/rhi/qshader_p_p.h +++ b/src/gui/rhi/qshader_p_p.h @@ -66,7 +66,8 @@ struct Q_GUI_EXPORT QShaderPrivate : ref(1), stage(other->stage), desc(other->desc), - shaders(other->shaders) + shaders(other->shaders), + bindings(other->bindings) { } @@ -77,6 +78,7 @@ struct Q_GUI_EXPORT QShaderPrivate QShader::Stage stage = QShader::VertexStage; QShaderDescription desc; QHash shaders; + QHash bindings; }; QT_END_NAMESPACE -- cgit v1.2.3 From cc631c15a00f26fa8f6aa703fd7cf9e715be8f3c Mon Sep 17 00:00:00 2001 From: Laszlo Agocs Date: Tue, 22 Oct 2019 12:37:34 +0200 Subject: rhi: metal: Remap resource bindings based on the QShader table MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ...when available. Fall back to the QRhi (i.e. SPIR-V) binding point otherwise (which becomes unsafe once shadertools bumps its SPIRV-Cross snapshot, but is fine for existing .qsb files) Task-number: QTBUG-79368 Change-Id: I2d452fdd4efb484867732c358171a800d3261dcd Reviewed-by: Christian Strømme --- src/gui/rhi/qrhimetal.mm | 97 +++++++++++++++++++++++++++++++-------------- src/gui/rhi/qrhimetal_p_p.h | 7 +++- 2 files changed, 73 insertions(+), 31 deletions(-) diff --git a/src/gui/rhi/qrhimetal.mm b/src/gui/rhi/qrhimetal.mm index 5f14d917b8..b2d56b43af 100644 --- a/src/gui/rhi/qrhimetal.mm +++ b/src/gui/rhi/qrhimetal.mm @@ -35,8 +35,6 @@ ****************************************************************************/ #include "qrhimetal_p_p.h" -#include "qshader_p.h" -#include "qshaderdescription_p.h" #include #include #include @@ -143,8 +141,10 @@ struct QMetalShader id lib = nil; id func = nil; std::array localSize; + QShader::NativeResourceBindingMap nativeResourceBindingMap; void release() { + nativeResourceBindingMap.clear(); [lib release]; lib = nil; [func release]; @@ -164,7 +164,7 @@ struct QRhiMetalData const QRhiDepthStencilClearValue &depthStencilClearValue, int colorAttCount); id createMetalLib(const QShader &shader, QShader::Variant shaderVariant, - QString *error, QByteArray *entryPoint); + QString *error, QByteArray *entryPoint, QShaderKey *activeKey); id createMSLShaderFunction(id lib, const QByteArray &entryPoint); struct DeferredReleaseEntry { @@ -653,18 +653,39 @@ QRhiShaderResourceBindings *QRhiMetal::createShaderResourceBindings() return new QMetalShaderResourceBindings(this); } -void QRhiMetal::enqueueShaderResourceBindings(QMetalShaderResourceBindings *srbD, QMetalCommandBuffer *cbD, +enum class BindingType { + Buffer, + Texture, + Sampler +}; + +static inline int mapBinding(int binding, + int stageIndex, + const QShader::NativeResourceBindingMap *nativeResourceBindingMaps[], + BindingType type) +{ + const QShader::NativeResourceBindingMap *map = nativeResourceBindingMaps[stageIndex]; + if (map) { + auto it = map->constFind(binding); + if (it != map->cend()) + return type == BindingType::Sampler ? it->second : it->first; + } + return binding; +} + +void QRhiMetal::enqueueShaderResourceBindings(QMetalShaderResourceBindings *srbD, + QMetalCommandBuffer *cbD, int dynamicOffsetCount, const QRhiCommandBuffer::DynamicOffset *dynamicOffsets, - bool offsetOnlyChange) + bool offsetOnlyChange, + const QShader::NativeResourceBindingMap *nativeResourceBindingMaps[SUPPORTED_STAGES]) { - static const int KNOWN_STAGES = 3; struct { QRhiBatchedBindings > buffers; QRhiBatchedBindings bufferOffsets; QRhiBatchedBindings > textures; QRhiBatchedBindings > samplers; - } res[KNOWN_STAGES]; + } res[SUPPORTED_STAGES]; for (const QRhiShaderResourceBinding &binding : qAsConst(srbD->sortedBindings)) { const QRhiShaderResourceBinding::Data *b = binding.data(); @@ -682,15 +703,15 @@ void QRhiMetal::enqueueShaderResourceBindings(QMetalShaderResourceBindings *srbD } } if (b->stage.testFlag(QRhiShaderResourceBinding::VertexStage)) { - res[0].buffers.feed(b->binding, mtlbuf); + res[0].buffers.feed(mapBinding(b->binding, 0, nativeResourceBindingMaps, BindingType::Buffer), mtlbuf); res[0].bufferOffsets.feed(b->binding, offset); } if (b->stage.testFlag(QRhiShaderResourceBinding::FragmentStage)) { - res[1].buffers.feed(b->binding, mtlbuf); + res[1].buffers.feed(mapBinding(b->binding, 1, nativeResourceBindingMaps, BindingType::Buffer), mtlbuf); res[1].bufferOffsets.feed(b->binding, offset); } if (b->stage.testFlag(QRhiShaderResourceBinding::ComputeStage)) { - res[2].buffers.feed(b->binding, mtlbuf); + res[2].buffers.feed(mapBinding(b->binding, 2, nativeResourceBindingMaps, BindingType::Buffer), mtlbuf); res[2].bufferOffsets.feed(b->binding, offset); } } @@ -700,16 +721,16 @@ void QRhiMetal::enqueueShaderResourceBindings(QMetalShaderResourceBindings *srbD QMetalTexture *texD = QRHI_RES(QMetalTexture, b->u.stex.tex); QMetalSampler *samplerD = QRHI_RES(QMetalSampler, b->u.stex.sampler); if (b->stage.testFlag(QRhiShaderResourceBinding::VertexStage)) { - res[0].textures.feed(b->binding, texD->d->tex); - res[0].samplers.feed(b->binding, samplerD->d->samplerState); + res[0].textures.feed(mapBinding(b->binding, 0, nativeResourceBindingMaps, BindingType::Texture), texD->d->tex); + res[0].samplers.feed(mapBinding(b->binding, 0, nativeResourceBindingMaps, BindingType::Sampler), samplerD->d->samplerState); } if (b->stage.testFlag(QRhiShaderResourceBinding::FragmentStage)) { - res[1].textures.feed(b->binding, texD->d->tex); - res[1].samplers.feed(b->binding, samplerD->d->samplerState); + res[1].textures.feed(mapBinding(b->binding, 1, nativeResourceBindingMaps, BindingType::Texture), texD->d->tex); + res[1].samplers.feed(mapBinding(b->binding, 1, nativeResourceBindingMaps, BindingType::Sampler), samplerD->d->samplerState); } if (b->stage.testFlag(QRhiShaderResourceBinding::ComputeStage)) { - res[2].textures.feed(b->binding, texD->d->tex); - res[2].samplers.feed(b->binding, samplerD->d->samplerState); + res[2].textures.feed(mapBinding(b->binding, 2, nativeResourceBindingMaps, BindingType::Texture), texD->d->tex); + res[2].samplers.feed(mapBinding(b->binding, 2, nativeResourceBindingMaps, BindingType::Sampler), samplerD->d->samplerState); } } break; @@ -722,11 +743,11 @@ void QRhiMetal::enqueueShaderResourceBindings(QMetalShaderResourceBindings *srbD QMetalTexture *texD = QRHI_RES(QMetalTexture, b->u.simage.tex); id t = texD->d->viewForLevel(b->u.simage.level); if (b->stage.testFlag(QRhiShaderResourceBinding::VertexStage)) - res[0].textures.feed(b->binding, t); + res[0].textures.feed(mapBinding(b->binding, 0, nativeResourceBindingMaps, BindingType::Texture), t); if (b->stage.testFlag(QRhiShaderResourceBinding::FragmentStage)) - res[1].textures.feed(b->binding, t); + res[1].textures.feed(mapBinding(b->binding, 1, nativeResourceBindingMaps, BindingType::Texture), t); if (b->stage.testFlag(QRhiShaderResourceBinding::ComputeStage)) - res[2].textures.feed(b->binding, t); + res[2].textures.feed(mapBinding(b->binding, 2, nativeResourceBindingMaps, BindingType::Texture), t); } break; case QRhiShaderResourceBinding::BufferLoad: @@ -739,15 +760,15 @@ void QRhiMetal::enqueueShaderResourceBindings(QMetalShaderResourceBindings *srbD id mtlbuf = bufD->d->buf[0]; uint offset = uint(b->u.sbuf.offset); if (b->stage.testFlag(QRhiShaderResourceBinding::VertexStage)) { - res[0].buffers.feed(b->binding, mtlbuf); + res[0].buffers.feed(mapBinding(b->binding, 0, nativeResourceBindingMaps, BindingType::Buffer), mtlbuf); res[0].bufferOffsets.feed(b->binding, offset); } if (b->stage.testFlag(QRhiShaderResourceBinding::FragmentStage)) { - res[1].buffers.feed(b->binding, mtlbuf); + res[1].buffers.feed(mapBinding(b->binding, 1, nativeResourceBindingMaps, BindingType::Buffer), mtlbuf); res[1].bufferOffsets.feed(b->binding, offset); } if (b->stage.testFlag(QRhiShaderResourceBinding::ComputeStage)) { - res[2].buffers.feed(b->binding, mtlbuf); + res[2].buffers.feed(mapBinding(b->binding, 2, nativeResourceBindingMaps, BindingType::Buffer), mtlbuf); res[2].bufferOffsets.feed(b->binding, offset); } } @@ -758,7 +779,7 @@ void QRhiMetal::enqueueShaderResourceBindings(QMetalShaderResourceBindings *srbD } } - for (int idx = 0; idx < KNOWN_STAGES; ++idx) { + for (int idx = 0; idx < SUPPORTED_STAGES; ++idx) { res[idx].buffers.finish(); res[idx].bufferOffsets.finish(); @@ -973,18 +994,22 @@ void QRhiMetal::setShaderResources(QRhiCommandBuffer *cb, QRhiShaderResourceBind // dynamic uniform buffer offsets always trigger a rebind if (hasDynamicOffsetInSrb || resNeedsRebind || srbChanged || srbRebuilt) { + const QShader::NativeResourceBindingMap *resBindMaps[SUPPORTED_STAGES] = { nullptr, nullptr, nullptr }; if (gfxPsD) { cbD->currentGraphicsSrb = srb; cbD->currentComputeSrb = nullptr; + resBindMaps[0] = &gfxPsD->d->vs.nativeResourceBindingMap; + resBindMaps[1] = &gfxPsD->d->fs.nativeResourceBindingMap; } else { cbD->currentGraphicsSrb = nullptr; cbD->currentComputeSrb = srb; + resBindMaps[2] = &compPsD->d->cs.nativeResourceBindingMap; } cbD->currentSrbGeneration = srbD->generation; cbD->currentResSlot = resSlot; const bool offsetOnlyChange = hasDynamicOffsetInSrb && !resNeedsRebind && !srbChanged && !srbRebuilt; - enqueueShaderResourceBindings(srbD, cbD, dynamicOffsetCount, dynamicOffsets, offsetOnlyChange); + enqueueShaderResourceBindings(srbD, cbD, dynamicOffsetCount, dynamicOffsets, offsetOnlyChange, resBindMaps); } } @@ -3081,9 +3106,10 @@ static inline MTLCullMode toMetalCullMode(QRhiGraphicsPipeline::CullMode c) } id QRhiMetalData::createMetalLib(const QShader &shader, QShader::Variant shaderVariant, - QString *error, QByteArray *entryPoint) + QString *error, QByteArray *entryPoint, QShaderKey *activeKey) { - QShaderCode mtllib = shader.shader({ QShader::MetalLibShader, 12, shaderVariant }); + QShaderKey key = { QShader::MetalLibShader, 12, shaderVariant }; + QShaderCode mtllib = shader.shader(key); if (!mtllib.shader().isEmpty()) { dispatch_data_t data = dispatch_data_create(mtllib.shader().constData(), size_t(mtllib.shader().size()), @@ -3094,6 +3120,7 @@ id QRhiMetalData::createMetalLib(const QShader &shader, QShader::Var dispatch_release(data); if (!err) { *entryPoint = mtllib.entryPoint(); + *activeKey = key; return lib; } else { const QString msg = QString::fromNSString(err.localizedDescription); @@ -3101,7 +3128,8 @@ id QRhiMetalData::createMetalLib(const QShader &shader, QShader::Var } } - QShaderCode mslSource = shader.shader({ QShader::MslShader, 12, shaderVariant }); + key = { QShader::MslShader, 12, shaderVariant }; + QShaderCode mslSource = shader.shader(key); if (mslSource.shader().isEmpty()) { qWarning() << "No MSL 1.2 code found in baked shader" << shader; return nil; @@ -3122,6 +3150,7 @@ id QRhiMetalData::createMetalLib(const QShader &shader, QShader::Var } *entryPoint = mslSource.entryPoint(); + *activeKey = key; return lib; } @@ -3195,9 +3224,12 @@ bool QMetalGraphicsPipeline::build() break; } } else { + const QShader shader = shaderStage.shader(); QString error; QByteArray entryPoint; - id lib = rhiD->d->createMetalLib(shaderStage.shader(), shaderStage.shaderVariant(), &error, &entryPoint); + QShaderKey activeKey; + id lib = rhiD->d->createMetalLib(shader, shaderStage.shaderVariant(), + &error, &entryPoint, &activeKey); if (!lib) { qWarning("MSL shader compilation failed: %s", qPrintable(error)); return false; @@ -3218,6 +3250,8 @@ bool QMetalGraphicsPipeline::build() case QRhiShaderStage::Vertex: d->vs.lib = lib; d->vs.func = func; + if (const QShader::NativeResourceBindingMap *map = shader.nativeResourceBindingMap(activeKey)) + d->vs.nativeResourceBindingMap = *map; rhiD->d->shaderCache.insert(shaderStage, d->vs); [d->vs.lib retain]; [d->vs.func retain]; @@ -3226,6 +3260,8 @@ bool QMetalGraphicsPipeline::build() case QRhiShaderStage::Fragment: d->fs.lib = lib; d->fs.func = func; + if (const QShader::NativeResourceBindingMap *map = shader.nativeResourceBindingMap(activeKey)) + d->fs.nativeResourceBindingMap = *map; rhiD->d->shaderCache.insert(shaderStage, d->fs); [d->fs.lib retain]; [d->fs.func retain]; @@ -3360,8 +3396,9 @@ bool QMetalComputePipeline::build() const QShader shader = m_shaderStage.shader(); QString error; QByteArray entryPoint; + QShaderKey activeKey; id lib = rhiD->d->createMetalLib(shader, m_shaderStage.shaderVariant(), - &error, &entryPoint); + &error, &entryPoint, &activeKey); if (!lib) { qWarning("MSL shader compilation failed: %s", qPrintable(error)); return false; @@ -3375,6 +3412,8 @@ bool QMetalComputePipeline::build() d->cs.lib = lib; d->cs.func = func; d->cs.localSize = shader.description().computeShaderLocalSize(); + if (const QShader::NativeResourceBindingMap *map = shader.nativeResourceBindingMap(activeKey)) + d->cs.nativeResourceBindingMap = *map; if (rhiD->d->shaderCache.count() >= QRhiMetal::MAX_SHADER_CACHE_ENTRIES) { for (QMetalShader &s : rhiD->d->shaderCache) diff --git a/src/gui/rhi/qrhimetal_p_p.h b/src/gui/rhi/qrhimetal_p_p.h index 688fec8147..2be86db5c8 100644 --- a/src/gui/rhi/qrhimetal_p_p.h +++ b/src/gui/rhi/qrhimetal_p_p.h @@ -433,10 +433,13 @@ public: qsizetype *curOfs); void enqueueResourceUpdates(QRhiCommandBuffer *cb, QRhiResourceUpdateBatch *resourceUpdates); void executeBufferHostWritesForCurrentFrame(QMetalBuffer *bufD); - void enqueueShaderResourceBindings(QMetalShaderResourceBindings *srbD, QMetalCommandBuffer *cbD, + static const int SUPPORTED_STAGES = 3; + void enqueueShaderResourceBindings(QMetalShaderResourceBindings *srbD, + QMetalCommandBuffer *cbD, int dynamicOffsetCount, const QRhiCommandBuffer::DynamicOffset *dynamicOffsets, - bool offsetOnlyChange); + bool offsetOnlyChange, + const QShader::NativeResourceBindingMap *nativeResourceBindingMaps[SUPPORTED_STAGES]); int effectiveSampleCount(int sampleCount) const; bool importedDevice = false; -- cgit v1.2.3 From 4106275a7fc9ab3abe2fa2ca0b107a3e96e36ca0 Mon Sep 17 00:00:00 2001 From: Laszlo Agocs Date: Wed, 23 Oct 2019 10:21:24 +0200 Subject: rhi: metal: Fix and clean up committing resource bindings Make it readable by using names instead of mere indices for the stages. There is an important fix in there as well: when in a render pass, only resource for VERTEX and FRAGMENT are taken into account, while in a compute pass those are skipped. This ensures that we do not send messages to a nil or invalid MTLRender/ComputeCommandEncoder. (nil would not be an error but the other is fatal) Task-number: QTBUG-79447 Change-Id: Ibef108cb7c82b5b0fdd2a299cd89fbebe8c3606a Reviewed-by: Paul Olav Tvete --- src/gui/rhi/qrhimetal.mm | 96 +++++++++++++++++++++++++----------------------- 1 file changed, 51 insertions(+), 45 deletions(-) diff --git a/src/gui/rhi/qrhimetal.mm b/src/gui/rhi/qrhimetal.mm index b2d56b43af..131b2da802 100644 --- a/src/gui/rhi/qrhimetal.mm +++ b/src/gui/rhi/qrhimetal.mm @@ -686,6 +686,7 @@ void QRhiMetal::enqueueShaderResourceBindings(QMetalShaderResourceBindings *srbD QRhiBatchedBindings > textures; QRhiBatchedBindings > samplers; } res[SUPPORTED_STAGES]; + enum { VERTEX = 0, FRAGMENT = 1, COMPUTE = 2 }; for (const QRhiShaderResourceBinding &binding : qAsConst(srbD->sortedBindings)) { const QRhiShaderResourceBinding::Data *b = binding.data(); @@ -703,16 +704,16 @@ void QRhiMetal::enqueueShaderResourceBindings(QMetalShaderResourceBindings *srbD } } if (b->stage.testFlag(QRhiShaderResourceBinding::VertexStage)) { - res[0].buffers.feed(mapBinding(b->binding, 0, nativeResourceBindingMaps, BindingType::Buffer), mtlbuf); - res[0].bufferOffsets.feed(b->binding, offset); + res[VERTEX].buffers.feed(mapBinding(b->binding, VERTEX, nativeResourceBindingMaps, BindingType::Buffer), mtlbuf); + res[VERTEX].bufferOffsets.feed(b->binding, offset); } if (b->stage.testFlag(QRhiShaderResourceBinding::FragmentStage)) { - res[1].buffers.feed(mapBinding(b->binding, 1, nativeResourceBindingMaps, BindingType::Buffer), mtlbuf); - res[1].bufferOffsets.feed(b->binding, offset); + res[FRAGMENT].buffers.feed(mapBinding(b->binding, FRAGMENT, nativeResourceBindingMaps, BindingType::Buffer), mtlbuf); + res[FRAGMENT].bufferOffsets.feed(b->binding, offset); } if (b->stage.testFlag(QRhiShaderResourceBinding::ComputeStage)) { - res[2].buffers.feed(mapBinding(b->binding, 2, nativeResourceBindingMaps, BindingType::Buffer), mtlbuf); - res[2].bufferOffsets.feed(b->binding, offset); + res[COMPUTE].buffers.feed(mapBinding(b->binding, COMPUTE, nativeResourceBindingMaps, BindingType::Buffer), mtlbuf); + res[COMPUTE].bufferOffsets.feed(b->binding, offset); } } break; @@ -721,16 +722,16 @@ void QRhiMetal::enqueueShaderResourceBindings(QMetalShaderResourceBindings *srbD QMetalTexture *texD = QRHI_RES(QMetalTexture, b->u.stex.tex); QMetalSampler *samplerD = QRHI_RES(QMetalSampler, b->u.stex.sampler); if (b->stage.testFlag(QRhiShaderResourceBinding::VertexStage)) { - res[0].textures.feed(mapBinding(b->binding, 0, nativeResourceBindingMaps, BindingType::Texture), texD->d->tex); - res[0].samplers.feed(mapBinding(b->binding, 0, nativeResourceBindingMaps, BindingType::Sampler), samplerD->d->samplerState); + res[VERTEX].textures.feed(mapBinding(b->binding, VERTEX, nativeResourceBindingMaps, BindingType::Texture), texD->d->tex); + res[VERTEX].samplers.feed(mapBinding(b->binding, VERTEX, nativeResourceBindingMaps, BindingType::Sampler), samplerD->d->samplerState); } if (b->stage.testFlag(QRhiShaderResourceBinding::FragmentStage)) { - res[1].textures.feed(mapBinding(b->binding, 1, nativeResourceBindingMaps, BindingType::Texture), texD->d->tex); - res[1].samplers.feed(mapBinding(b->binding, 1, nativeResourceBindingMaps, BindingType::Sampler), samplerD->d->samplerState); + res[FRAGMENT].textures.feed(mapBinding(b->binding, FRAGMENT, nativeResourceBindingMaps, BindingType::Texture), texD->d->tex); + res[FRAGMENT].samplers.feed(mapBinding(b->binding, FRAGMENT, nativeResourceBindingMaps, BindingType::Sampler), samplerD->d->samplerState); } if (b->stage.testFlag(QRhiShaderResourceBinding::ComputeStage)) { - res[2].textures.feed(mapBinding(b->binding, 2, nativeResourceBindingMaps, BindingType::Texture), texD->d->tex); - res[2].samplers.feed(mapBinding(b->binding, 2, nativeResourceBindingMaps, BindingType::Sampler), samplerD->d->samplerState); + res[COMPUTE].textures.feed(mapBinding(b->binding, COMPUTE, nativeResourceBindingMaps, BindingType::Texture), texD->d->tex); + res[COMPUTE].samplers.feed(mapBinding(b->binding, COMPUTE, nativeResourceBindingMaps, BindingType::Sampler), samplerD->d->samplerState); } } break; @@ -743,11 +744,11 @@ void QRhiMetal::enqueueShaderResourceBindings(QMetalShaderResourceBindings *srbD QMetalTexture *texD = QRHI_RES(QMetalTexture, b->u.simage.tex); id t = texD->d->viewForLevel(b->u.simage.level); if (b->stage.testFlag(QRhiShaderResourceBinding::VertexStage)) - res[0].textures.feed(mapBinding(b->binding, 0, nativeResourceBindingMaps, BindingType::Texture), t); + res[VERTEX].textures.feed(mapBinding(b->binding, VERTEX, nativeResourceBindingMaps, BindingType::Texture), t); if (b->stage.testFlag(QRhiShaderResourceBinding::FragmentStage)) - res[1].textures.feed(mapBinding(b->binding, 1, nativeResourceBindingMaps, BindingType::Texture), t); + res[FRAGMENT].textures.feed(mapBinding(b->binding, FRAGMENT, nativeResourceBindingMaps, BindingType::Texture), t); if (b->stage.testFlag(QRhiShaderResourceBinding::ComputeStage)) - res[2].textures.feed(mapBinding(b->binding, 2, nativeResourceBindingMaps, BindingType::Texture), t); + res[COMPUTE].textures.feed(mapBinding(b->binding, COMPUTE, nativeResourceBindingMaps, BindingType::Texture), t); } break; case QRhiShaderResourceBinding::BufferLoad: @@ -760,16 +761,16 @@ void QRhiMetal::enqueueShaderResourceBindings(QMetalShaderResourceBindings *srbD id mtlbuf = bufD->d->buf[0]; uint offset = uint(b->u.sbuf.offset); if (b->stage.testFlag(QRhiShaderResourceBinding::VertexStage)) { - res[0].buffers.feed(mapBinding(b->binding, 0, nativeResourceBindingMaps, BindingType::Buffer), mtlbuf); - res[0].bufferOffsets.feed(b->binding, offset); + res[VERTEX].buffers.feed(mapBinding(b->binding, VERTEX, nativeResourceBindingMaps, BindingType::Buffer), mtlbuf); + res[VERTEX].bufferOffsets.feed(b->binding, offset); } if (b->stage.testFlag(QRhiShaderResourceBinding::FragmentStage)) { - res[1].buffers.feed(mapBinding(b->binding, 1, nativeResourceBindingMaps, BindingType::Buffer), mtlbuf); - res[1].bufferOffsets.feed(b->binding, offset); + res[FRAGMENT].buffers.feed(mapBinding(b->binding, FRAGMENT, nativeResourceBindingMaps, BindingType::Buffer), mtlbuf); + res[FRAGMENT].bufferOffsets.feed(b->binding, offset); } if (b->stage.testFlag(QRhiShaderResourceBinding::ComputeStage)) { - res[2].buffers.feed(mapBinding(b->binding, 2, nativeResourceBindingMaps, BindingType::Buffer), mtlbuf); - res[2].bufferOffsets.feed(b->binding, offset); + res[COMPUTE].buffers.feed(mapBinding(b->binding, COMPUTE, nativeResourceBindingMaps, BindingType::Buffer), mtlbuf); + res[COMPUTE].bufferOffsets.feed(b->binding, offset); } } break; @@ -779,25 +780,30 @@ void QRhiMetal::enqueueShaderResourceBindings(QMetalShaderResourceBindings *srbD } } - for (int idx = 0; idx < SUPPORTED_STAGES; ++idx) { - res[idx].buffers.finish(); - res[idx].bufferOffsets.finish(); + for (int stage = 0; stage < SUPPORTED_STAGES; ++stage) { + if (cbD->recordingPass != QMetalCommandBuffer::RenderPass && (stage == VERTEX || stage == FRAGMENT)) + continue; + if (cbD->recordingPass != QMetalCommandBuffer::ComputePass && stage == COMPUTE) + continue; + + res[stage].buffers.finish(); + res[stage].bufferOffsets.finish(); - for (int i = 0, ie = res[idx].buffers.batches.count(); i != ie; ++i) { - const auto &bufferBatch(res[idx].buffers.batches[i]); - const auto &offsetBatch(res[idx].bufferOffsets.batches[i]); - switch (idx) { - case 0: + for (int i = 0, ie = res[stage].buffers.batches.count(); i != ie; ++i) { + const auto &bufferBatch(res[stage].buffers.batches[i]); + const auto &offsetBatch(res[stage].bufferOffsets.batches[i]); + switch (stage) { + case VERTEX: [cbD->d->currentRenderPassEncoder setVertexBuffers: bufferBatch.resources.constData() offsets: offsetBatch.resources.constData() withRange: NSMakeRange(bufferBatch.startBinding, NSUInteger(bufferBatch.resources.count()))]; break; - case 1: + case FRAGMENT: [cbD->d->currentRenderPassEncoder setFragmentBuffers: bufferBatch.resources.constData() offsets: offsetBatch.resources.constData() withRange: NSMakeRange(bufferBatch.startBinding, NSUInteger(bufferBatch.resources.count()))]; break; - case 2: + case COMPUTE: [cbD->d->currentComputePassEncoder setBuffers: bufferBatch.resources.constData() offsets: offsetBatch.resources.constData() withRange: NSMakeRange(bufferBatch.startBinding, NSUInteger(bufferBatch.resources.count()))]; @@ -811,21 +817,21 @@ void QRhiMetal::enqueueShaderResourceBindings(QMetalShaderResourceBindings *srbD if (offsetOnlyChange) continue; - res[idx].textures.finish(); - res[idx].samplers.finish(); + res[stage].textures.finish(); + res[stage].samplers.finish(); - for (int i = 0, ie = res[idx].textures.batches.count(); i != ie; ++i) { - const auto &batch(res[idx].textures.batches[i]); - switch (idx) { - case 0: + for (int i = 0, ie = res[stage].textures.batches.count(); i != ie; ++i) { + const auto &batch(res[stage].textures.batches[i]); + switch (stage) { + case VERTEX: [cbD->d->currentRenderPassEncoder setVertexTextures: batch.resources.constData() withRange: NSMakeRange(batch.startBinding, NSUInteger(batch.resources.count()))]; break; - case 1: + case FRAGMENT: [cbD->d->currentRenderPassEncoder setFragmentTextures: batch.resources.constData() withRange: NSMakeRange(batch.startBinding, NSUInteger(batch.resources.count()))]; break; - case 2: + case COMPUTE: [cbD->d->currentComputePassEncoder setTextures: batch.resources.constData() withRange: NSMakeRange(batch.startBinding, NSUInteger(batch.resources.count()))]; break; @@ -834,18 +840,18 @@ void QRhiMetal::enqueueShaderResourceBindings(QMetalShaderResourceBindings *srbD break; } } - for (int i = 0, ie = res[idx].samplers.batches.count(); i != ie; ++i) { - const auto &batch(res[idx].samplers.batches[i]); - switch (idx) { - case 0: + for (int i = 0, ie = res[stage].samplers.batches.count(); i != ie; ++i) { + const auto &batch(res[stage].samplers.batches[i]); + switch (stage) { + case VERTEX: [cbD->d->currentRenderPassEncoder setVertexSamplerStates: batch.resources.constData() withRange: NSMakeRange(batch.startBinding, NSUInteger(batch.resources.count()))]; break; - case 1: + case FRAGMENT: [cbD->d->currentRenderPassEncoder setFragmentSamplerStates: batch.resources.constData() withRange: NSMakeRange(batch.startBinding, NSUInteger(batch.resources.count()))]; break; - case 2: + case COMPUTE: [cbD->d->currentComputePassEncoder setSamplerStates: batch.resources.constData() withRange: NSMakeRange(batch.startBinding, NSUInteger(batch.resources.count()))]; break; -- cgit v1.2.3 From c46eec096f30b69c1b683f8417d27e0234590bfa Mon Sep 17 00:00:00 2001 From: Robert Loehning Date: Fri, 18 Oct 2019 16:44:55 +0200 Subject: QStateMachine: Don't scream at the user Change-Id: I171606d10985bc7338b0f24ceb142fc0d88e7932 Reviewed-by: Thiago Macieira Reviewed-by: Leena Miettinen --- src/corelib/statemachine/qstatemachine.cpp | 4 ++-- .../auto/corelib/statemachine/qstatemachine/tst_qstatemachine.cpp | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/corelib/statemachine/qstatemachine.cpp b/src/corelib/statemachine/qstatemachine.cpp index 945e36968f..a97700e5d0 100644 --- a/src/corelib/statemachine/qstatemachine.cpp +++ b/src/corelib/statemachine/qstatemachine.cpp @@ -1498,7 +1498,7 @@ void QStateMachinePrivate::setError(QStateMachine::Error errorCode, QAbstractSta case QStateMachine::StateMachineChildModeSetToParallelError: Q_ASSERT(currentContext != nullptr); - errorString = QStateMachine::tr("Child mode of state machine '%1' is not 'ExclusiveStates'!") + errorString = QStateMachine::tr("Child mode of state machine '%1' is not 'ExclusiveStates'.") .arg(currentContext->objectName()); break; @@ -2469,7 +2469,7 @@ QStateMachine::QStateMachine(QObject *parent) and \a parent. \warning Do not set the \a childMode to anything else than \l{ExclusiveStates}, otherwise the - state machine is invalid, and might work incorrectly! + state machine is invalid, and might work incorrectly. */ QStateMachine::QStateMachine(QState::ChildMode childMode, QObject *parent) : QState(*new QStateMachinePrivate, /*parentState=*/0) diff --git a/tests/auto/corelib/statemachine/qstatemachine/tst_qstatemachine.cpp b/tests/auto/corelib/statemachine/qstatemachine/tst_qstatemachine.cpp index 72a6f0360d..4b13ac45cc 100644 --- a/tests/auto/corelib/statemachine/qstatemachine/tst_qstatemachine.cpp +++ b/tests/auto/corelib/statemachine/qstatemachine/tst_qstatemachine.cpp @@ -338,7 +338,7 @@ void tst_QStateMachine::transitionToRootState() machine.postEvent(new QEvent(QEvent::User)); QTest::ignoreMessage(QtWarningMsg, "Unrecoverable error detected in running state machine: " - "Child mode of state machine 'machine' is not 'ExclusiveStates'!"); + "Child mode of state machine 'machine' is not 'ExclusiveStates'."); QCoreApplication::processEvents(); QVERIFY(machine.configuration().isEmpty()); QVERIFY(!machine.isRunning()); @@ -1064,7 +1064,7 @@ void tst_QStateMachine::transitionToStateNotInGraph() machine.start(); QTest::ignoreMessage(QtWarningMsg, "Unrecoverable error detected in running state machine: " - "Child mode of state machine '' is not 'ExclusiveStates'!"); + "Child mode of state machine '' is not 'ExclusiveStates'."); QCoreApplication::processEvents(); QCOMPARE(machine.isRunning(), false); @@ -2103,7 +2103,7 @@ void tst_QStateMachine::parallelRootState() QVERIFY(finishedSpy.isValid()); machine.start(); QTest::ignoreMessage(QtWarningMsg, "Unrecoverable error detected in running state machine: " - "Child mode of state machine '' is not 'ExclusiveStates'!"); + "Child mode of state machine '' is not 'ExclusiveStates'."); QTRY_COMPARE(startedSpy.count(), 1); QCOMPARE(machine.configuration().size(), 4); QVERIFY(machine.configuration().contains(s1)); @@ -3316,7 +3316,7 @@ void tst_QStateMachine::targetStateWithNoParent() machine.start(); QTest::ignoreMessage(QtWarningMsg, "Unrecoverable error detected in running state machine: " - "Child mode of state machine '' is not 'ExclusiveStates'!"); + "Child mode of state machine '' is not 'ExclusiveStates'."); TEST_ACTIVE_CHANGED(s1, 2); QTRY_COMPARE(startedSpy.count(), 1); QCOMPARE(machine.isRunning(), false); -- cgit v1.2.3 From 86a0f1cfd7942e31d1462f74769eef5a5cf3c9ab Mon Sep 17 00:00:00 2001 From: BogDan Vatra Date: Tue, 22 Oct 2019 12:41:26 +0300 Subject: Add build-id flag This flag is needed by LLDB & simple perf tools to locate the right binary. Change-Id: Iffa1b0678663cfb9d1d699da5ad6fe672863918c Reviewed-by: Eskil Abrahamsen Blomfeldt --- mkspecs/android-clang/qmake.conf | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mkspecs/android-clang/qmake.conf b/mkspecs/android-clang/qmake.conf index 81609c3962..ec6c765799 100644 --- a/mkspecs/android-clang/qmake.conf +++ b/mkspecs/android-clang/qmake.conf @@ -70,8 +70,8 @@ QMAKE_CFLAGS_THREAD = -D_REENTRANT QMAKE_CFLAGS_HIDESYMS = -fvisibility=hidden QMAKE_CFLAGS_NEON = -mfpu=neon -QMAKE_LFLAGS_APP = -Wl,--no-undefined -Wl,-z,noexecstack -shared -QMAKE_LFLAGS_SHLIB = -Wl,--no-undefined -Wl,-z,noexecstack -shared +QMAKE_LFLAGS_APP = -Wl,--build-id=sha1 -Wl,--no-undefined -Wl,-z,noexecstack -shared +QMAKE_LFLAGS_SHLIB = -Wl,--build-id=sha1 -Wl,--no-undefined -Wl,-z,noexecstack -shared QMAKE_LFLAGS_PLUGIN = $$QMAKE_LFLAGS_SHLIB QMAKE_LFLAGS_NOUNDEF = -Wl,--no-undefined QMAKE_LFLAGS_RPATH = -Wl,-rpath= -- cgit v1.2.3 From 444e947aadc82e2aeb72fa5b3e6a352ff02441f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tor=20Arne=20Vestb=C3=B8?= Date: Wed, 23 Oct 2019 15:33:46 +0200 Subject: macOS: Improve handling of wantsBestResolutionOpenGLSurface We were disabling wantsBestResolutionOpenGLSurface whenever we detected the Apple software renderer, but this isn't needed when layer-backed, and did in fact result in the exact same visual result as the bug the code was working around -- only rendering to a quarter of the viewport. We now apply the workaround only when software rendering is combined with surface-backed views. The logic has also been improved to not rely on string comparison to look for the software renderer, but instead uses the renderer ID that the context provides. Since tweaking the wantsBestResolutionOpenGLSurface is only relevant when using a window for GL rendering the logic has been moved into QCocoaGLContext. Change-Id: I021aaefbb7a9782bc8ee3c9703da246510326d50 Reviewed-by: Timur Pocheptsov --- src/plugins/platforms/cocoa/qcocoaglcontext.h | 4 ++- src/plugins/platforms/cocoa/qcocoaglcontext.mm | 46 ++++++++++++++++---------- src/plugins/platforms/cocoa/qnsview_drawing.mm | 11 ------ 3 files changed, 31 insertions(+), 30 deletions(-) diff --git a/src/plugins/platforms/cocoa/qcocoaglcontext.h b/src/plugins/platforms/cocoa/qcocoaglcontext.h index bb309c0713..4210a4ed3f 100644 --- a/src/plugins/platforms/cocoa/qcocoaglcontext.h +++ b/src/plugins/platforms/cocoa/qcocoaglcontext.h @@ -50,6 +50,8 @@ QT_BEGIN_NAMESPACE +class QCocoaWindow; + class QCocoaGLContext : public QPlatformOpenGLContext { public: @@ -76,12 +78,12 @@ private: static NSOpenGLPixelFormat *pixelFormatForSurfaceFormat(const QSurfaceFormat &format); bool setDrawable(QPlatformSurface *surface); + void prepareDrawable(QCocoaWindow *platformWindow); void updateSurfaceFormat(); NSOpenGLContext *m_context = nil; NSOpenGLContext *m_shareContext = nil; QSurfaceFormat m_format; - bool m_didCheckForSoftwareContext = false; QVarLengthArray m_updateObservers; QAtomicInt m_needsUpdate = false; }; diff --git a/src/plugins/platforms/cocoa/qcocoaglcontext.mm b/src/plugins/platforms/cocoa/qcocoaglcontext.mm index ba7d12ce30..76a210d0b6 100644 --- a/src/plugins/platforms/cocoa/qcocoaglcontext.mm +++ b/src/plugins/platforms/cocoa/qcocoaglcontext.mm @@ -368,23 +368,6 @@ bool QCocoaGLContext::makeCurrent(QPlatformSurface *surface) [m_context makeCurrentContext]; if (surface->surface()->surfaceClass() == QSurface::Window) { - // Disable high-resolution surfaces when using the software renderer, which has the - // problem that the system silently falls back to a to using a low-resolution buffer - // when a high-resolution buffer is requested. This is not detectable using the NSWindow - // convertSizeToBacking and backingScaleFactor APIs. A typical result of this is that Qt - // will display a quarter of the window content when running in a virtual machine. - if (!m_didCheckForSoftwareContext) { - // FIXME: This ensures we check only once per context, - // but the context may be used for multiple surfaces. - m_didCheckForSoftwareContext = true; - - const GLubyte* renderer = glGetString(GL_RENDERER); - if (qstrcmp((const char *)renderer, "Apple Software Renderer") == 0) { - NSView *view = static_cast(surface)->m_view; - [view setWantsBestResolutionOpenGLSurface:NO]; - } - } - if (m_needsUpdate.fetchAndStoreRelaxed(false)) update(); } @@ -413,11 +396,14 @@ bool QCocoaGLContext::setDrawable(QPlatformSurface *surface) } Q_ASSERT(surface->surface()->surfaceClass() == QSurface::Window); - QNSView *view = qnsview_cast(static_cast(surface)->view()); + auto *cocoaWindow = static_cast(surface); + QNSView *view = qnsview_cast(cocoaWindow->view()); if (view == m_context.view) return true; + prepareDrawable(cocoaWindow); + // Setting the drawable may happen on a separate thread as a result of // a call to makeCurrent, so we need to set up the observers before we // associate the view with the context. That way we will guarantee that @@ -460,6 +446,30 @@ bool QCocoaGLContext::setDrawable(QPlatformSurface *surface) return true; } +void QCocoaGLContext::prepareDrawable(QCocoaWindow *platformWindow) +{ + // We generally want high-DPI GL surfaces, unless the user has explicitly disabled them + bool prefersBestResolutionOpenGLSurface = qt_mac_resolveOption(YES, + platformWindow->window(), "_q_mac_wantsBestResolutionOpenGLSurface", + "QT_MAC_WANTS_BEST_RESOLUTION_OPENGL_SURFACE"); + + auto *view = platformWindow->view(); + + // The only case we have to opt out ourselves is when using the Apple software renderer + // in combination with surface-backed views, as these together do not support high-DPI. + if (prefersBestResolutionOpenGLSurface) { + int rendererID = 0; + [m_context getValues:&rendererID forParameter:NSOpenGLContextParameterCurrentRendererID]; + bool isSoftwareRenderer = (rendererID & kCGLRendererIDMatchingMask) == kCGLRendererGenericFloatID; + if (isSoftwareRenderer && !view.layer) { + qCInfo(lcQpaOpenGLContext) << "Disabling high resolution GL surface due to software renderer"; + prefersBestResolutionOpenGLSurface = false; + } + } + + view.wantsBestResolutionOpenGLSurface = prefersBestResolutionOpenGLSurface; +} + // NSOpenGLContext is not re-entrant. Even when using separate contexts per thread, // view, and window, calls into the API will still deadlock. For more information // see https://openradar.appspot.com/37064579 diff --git a/src/plugins/platforms/cocoa/qnsview_drawing.mm b/src/plugins/platforms/cocoa/qnsview_drawing.mm index eb9286519d..2fd63fad67 100644 --- a/src/plugins/platforms/cocoa/qnsview_drawing.mm +++ b/src/plugins/platforms/cocoa/qnsview_drawing.mm @@ -44,17 +44,6 @@ - (void)initDrawing { [self updateLayerBacking]; - - // Enable high-DPI OpenGL for retina displays. Enabling has the side - // effect that Cocoa will start calling glViewport(0, 0, width, height), - // overriding any glViewport calls in application code. This is usually not a - // problem, except if the application wants to have a "custom" viewport. - // (like the hellogl example) - if (m_platformWindow->window()->supportsOpenGL()) { - self.wantsBestResolutionOpenGLSurface = qt_mac_resolveOption(YES, m_platformWindow->window(), - "_q_mac_wantsBestResolutionOpenGLSurface", "QT_MAC_WANTS_BEST_RESOLUTION_OPENGL_SURFACE"); - // See also QCocoaGLContext::makeCurrent for software renderer workarounds. - } } - (BOOL)isOpaque -- cgit v1.2.3 From 7c9ffe3e46bca3cfdec6fd1db3da4c96a6d5acd6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tor=20Arne=20Vestb=C3=B8?= Date: Wed, 23 Oct 2019 14:16:24 +0200 Subject: rhi: Use Q_GLOBAL_STATIC for QRhiGles2 GL program cache MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Defers initialization until actually needed. Change-Id: Idb09dbad0dfa602949d381ee61565d9050e77e7c Reviewed-by: Laszlo Agocs Reviewed-by: Tor Arne Vestbø --- src/gui/rhi/qrhigles2.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/gui/rhi/qrhigles2.cpp b/src/gui/rhi/qrhigles2.cpp index dec28cac9b..abee843a74 100644 --- a/src/gui/rhi/qrhigles2.cpp +++ b/src/gui/rhi/qrhigles2.cpp @@ -2961,7 +2961,7 @@ bool QRhiGles2::isProgramBinaryDiskCacheEnabled() const return checker.get(ctx)->isSupported(); } -static QOpenGLProgramBinaryCache qrhi_programBinaryCache; +Q_GLOBAL_STATIC(QOpenGLProgramBinaryCache, qrhi_programBinaryCache); static inline QShader::Stage toShaderStage(QRhiShaderStage::Type type) { @@ -2995,7 +2995,7 @@ QRhiGles2::DiskCacheResult QRhiGles2::tryLoadFromDiskCache(const QRhiShaderStage } diskCacheKey = binaryProgram.cacheKey(); - if (qrhi_programBinaryCache.load(diskCacheKey, program)) { + if (qrhi_programBinaryCache()->load(diskCacheKey, program)) { qCDebug(lcOpenGLProgramDiskCache, "Program binary received from cache, program %u, key %s", program, diskCacheKey.constData()); result = QRhiGles2::DiskCacheHit; @@ -3013,7 +3013,7 @@ void QRhiGles2::trySaveToDiskCache(GLuint program, const QByteArray &cacheKey) if (isProgramBinaryDiskCacheEnabled()) { qCDebug(lcOpenGLProgramDiskCache, "Saving program binary, program %u, key %s", program, cacheKey.constData()); - qrhi_programBinaryCache.save(cacheKey, program); + qrhi_programBinaryCache()->save(cacheKey, program); } } -- cgit v1.2.3 From f39230fcac4de01f26945bde16c3a10c5ac74afb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tor=20Arne=20Vestb=C3=B8?= Date: Fri, 18 Oct 2019 16:23:48 +0200 Subject: macOS: Skip NSOpenGLContext flush if window exposed size is out of sync MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Some clients such as QOpenGLWidget will end up drawing and flushing during the resize event, which for GL will result in an immediate update on the screen. The problem is that the underlying Core Animation layer, and the window's frame, has not been visually updated yet to the new size, so we end up drawing "ahead" of what the window server is showing the user. Ideally we'd be able to present the GL drawing in a transaction, in sync with the drawing of the window frame, but this API is only available for CAMetalLayer and CAEAGLLayer. As a workaround we detect when the exposed size is out of sync with the window geometry, and skip the flush until the exposed size has caught up. We know this will happen eventually as AppKit will always ask us to display after a resize. Change-Id: I1739ac8878b3fc6820a55dd017ddd170fd5f55d6 Fixes: QTBUG-79139 Reviewed-by: Morten Johan Sørvig --- src/plugins/platforms/cocoa/qcocoaglcontext.mm | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/plugins/platforms/cocoa/qcocoaglcontext.mm b/src/plugins/platforms/cocoa/qcocoaglcontext.mm index 76a210d0b6..7452d53579 100644 --- a/src/plugins/platforms/cocoa/qcocoaglcontext.mm +++ b/src/plugins/platforms/cocoa/qcocoaglcontext.mm @@ -501,6 +501,21 @@ void QCocoaGLContext::swapBuffers(QPlatformSurface *surface) return; } + if (m_context.view.layer) { + // Flushing an NSOpenGLContext will hit the screen immediately, ignoring + // any Core Animation transactions in place. This may result in major + // visual artifacts if the flush happens out of sync with the size + // of the layer, view, and window reflected by other parts of the UI, + // e.g. if the application flushes in the resize event or a timer during + // window resizing, instead of in the expose event. + auto *cocoaWindow = static_cast(surface); + if (cocoaWindow->geometry().size() != cocoaWindow->m_exposedRect.size()) { + qCInfo(lcQpaOpenGLContext) << "Window exposed size does not match geometry (yet)." + << "Skipping flush to avoid visual artifacts."; + return; + } + } + QMutexLocker locker(&s_reentrancyMutex); [m_context flushBuffer]; } -- cgit v1.2.3 From 312793f28eaa1eddd8ffd15f42f51c31ecd1c4ad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tor=20Arne=20Vestb=C3=B8?= Date: Wed, 23 Oct 2019 13:15:07 +0200 Subject: macOS: Respect Qt::AA_UseSoftwareOpenGL MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Change-Id: Ia83e8e9e571e4f46d2a8d810c376015552755457 Reviewed-by: Morten Johan Sørvig --- src/plugins/platforms/cocoa/qcocoaglcontext.mm | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/plugins/platforms/cocoa/qcocoaglcontext.mm b/src/plugins/platforms/cocoa/qcocoaglcontext.mm index 7452d53579..900f0608c8 100644 --- a/src/plugins/platforms/cocoa/qcocoaglcontext.mm +++ b/src/plugins/platforms/cocoa/qcocoaglcontext.mm @@ -223,6 +223,12 @@ NSOpenGLPixelFormat *QCocoaGLContext::pixelFormatForSurfaceFormat(const QSurface attrs << NSOpenGLPFAAllowOfflineRenderers; } + if (qGuiApp->testAttribute(Qt::AA_UseSoftwareOpenGL)) { + // kCGLRendererGenericFloatID is the modern software renderer on macOS, + // as opposed to kCGLRendererGenericID, which is deprecated. + attrs << NSOpenGLPFARendererID << kCGLRendererGenericFloatID; + } + // FIXME: Pull this information out of the NSView QByteArray useLayer = qgetenv("QT_MAC_WANTS_LAYER"); if (!useLayer.isEmpty() && useLayer.toInt() > 0) { -- cgit v1.2.3 From f2edc6cb3a12063005c77aae7946f4a07d3bd30c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tor=20Arne=20Vestb=C3=B8?= Date: Wed, 23 Oct 2019 13:21:32 +0200 Subject: macOS: Don't set NSOpenGLPFANoRecovery for layer-backed views MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Apple software renderer is perfectly capable of being used when compositing CA layers. Change-Id: I3b78ff61a79869ecdb7bd431388041f2c124472e Reviewed-by: Morten Johan Sørvig --- src/plugins/platforms/cocoa/qcocoaglcontext.mm | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/plugins/platforms/cocoa/qcocoaglcontext.mm b/src/plugins/platforms/cocoa/qcocoaglcontext.mm index 900f0608c8..b312e033cd 100644 --- a/src/plugins/platforms/cocoa/qcocoaglcontext.mm +++ b/src/plugins/platforms/cocoa/qcocoaglcontext.mm @@ -229,14 +229,6 @@ NSOpenGLPixelFormat *QCocoaGLContext::pixelFormatForSurfaceFormat(const QSurface attrs << NSOpenGLPFARendererID << kCGLRendererGenericFloatID; } - // FIXME: Pull this information out of the NSView - QByteArray useLayer = qgetenv("QT_MAC_WANTS_LAYER"); - if (!useLayer.isEmpty() && useLayer.toInt() > 0) { - // Disable the software rendering fallback. This makes compositing - // OpenGL and raster NSViews using Core Animation layers possible. - attrs << NSOpenGLPFANoRecovery; - } - attrs << 0; // 0-terminate array return [[NSOpenGLPixelFormat alloc] initWithAttributes:attrs.constData()]; } -- cgit v1.2.3 From 9ab043b6889739ce495c395281774ee2db706dab Mon Sep 17 00:00:00 2001 From: Ulf Hermann Date: Wed, 4 Sep 2019 15:26:10 +0200 Subject: QFileSystemEngine: Consistently check for invalid file names stat() and friends expect a null-terminated C string. There is no way to generate anything useful from a string that has null bytes in the middle. It's important to catch this early, as otherwise, for example, a QDir::exists() on such a path can return true, as the path is silently truncated. Extend the checks for empty file names to windows and add checks for null bytes. Change-Id: Ie9794c3a7c4fd57f9a66bdbbab8b45a08b6f9170 Reviewed-by: Thiago Macieira --- src/corelib/io/qfilesystemengine_p.h | 30 ++++++++++++++++++ src/corelib/io/qfilesystemengine_unix.cpp | 52 +++++++++++-------------------- src/corelib/io/qfilesystemengine_win.cpp | 27 +++++++++++++++- tests/auto/corelib/io/qfile/tst_qfile.cpp | 4 +++ 4 files changed, 79 insertions(+), 34 deletions(-) diff --git a/src/corelib/io/qfilesystemengine_p.h b/src/corelib/io/qfilesystemengine_p.h index e44837747c..ecfdc03743 100644 --- a/src/corelib/io/qfilesystemengine_p.h +++ b/src/corelib/io/qfilesystemengine_p.h @@ -58,6 +58,36 @@ QT_BEGIN_NAMESPACE +#define Q_RETURN_ON_INVALID_FILENAME(message, result) \ + { \ + QMessageLogger(QT_MESSAGELOG_FILE, QT_MESSAGELOG_LINE, QT_MESSAGELOG_FUNC).warning(message); \ + errno = EINVAL; \ + return (result); \ + } + +inline bool qIsFilenameBroken(const QByteArray &name) +{ + return name.contains('\0'); +} + +inline bool qIsFilenameBroken(const QString &name) +{ + return name.contains(QLatin1Char('\0')); +} + +inline bool qIsFilenameBroken(const QFileSystemEntry &entry) +{ + return qIsFilenameBroken(entry.nativeFilePath()); +} + +#define Q_CHECK_FILE_NAME(name, result) \ + do { \ + if (Q_UNLIKELY((name).isEmpty())) \ + Q_RETURN_ON_INVALID_FILENAME("Empty filename passed to function", (result)); \ + if (Q_UNLIKELY(qIsFilenameBroken(name))) \ + Q_RETURN_ON_INVALID_FILENAME("Broken filename passed to function", (result)); \ + } while (false) + class QFileSystemEngine { public: diff --git a/src/corelib/io/qfilesystemengine_unix.cpp b/src/corelib/io/qfilesystemengine_unix.cpp index 74865fe31f..c3abec8989 100644 --- a/src/corelib/io/qfilesystemengine_unix.cpp +++ b/src/corelib/io/qfilesystemengine_unix.cpp @@ -118,13 +118,6 @@ enum { #endif }; -#define emptyFileEntryWarning() emptyFileEntryWarning_(QT_MESSAGELOG_FILE, QT_MESSAGELOG_LINE, QT_MESSAGELOG_FUNC) -static void emptyFileEntryWarning_(const char *file, int line, const char *function) -{ - QMessageLogger(file, line, function).warning("Empty filename passed to function"); - errno = EINVAL; -} - #if defined(Q_OS_DARWIN) static inline bool hasResourcePropertyFlag(const QFileSystemMetaData &data, const QFileSystemEntry &entry, @@ -625,8 +618,7 @@ void QFileSystemMetaData::fillFromDirEnt(const QT_DIRENT &entry) //static QFileSystemEntry QFileSystemEngine::getLinkTarget(const QFileSystemEntry &link, QFileSystemMetaData &data) { - if (Q_UNLIKELY(link.isEmpty())) - return emptyFileEntryWarning(), link; + Q_CHECK_FILE_NAME(link, link); QByteArray s = qt_readlink(link.nativeFilePath().constData()); if (s.length() > 0) { @@ -685,10 +677,7 @@ QFileSystemEntry QFileSystemEngine::getLinkTarget(const QFileSystemEntry &link, //static QFileSystemEntry QFileSystemEngine::canonicalName(const QFileSystemEntry &entry, QFileSystemMetaData &data) { - if (Q_UNLIKELY(entry.isEmpty())) - return emptyFileEntryWarning(), entry; - if (entry.isRoot()) - return entry; + Q_CHECK_FILE_NAME(entry, entry); #if !defined(Q_OS_MAC) && !defined(Q_OS_QNX) && !defined(Q_OS_ANDROID) && !defined(Q_OS_HAIKU) && _POSIX_VERSION < 200809L // realpath(X,0) is not supported @@ -738,8 +727,8 @@ QFileSystemEntry QFileSystemEngine::canonicalName(const QFileSystemEntry &entry, //static QFileSystemEntry QFileSystemEngine::absoluteName(const QFileSystemEntry &entry) { - if (Q_UNLIKELY(entry.isEmpty())) - return emptyFileEntryWarning(), entry; + Q_CHECK_FILE_NAME(entry, entry); + if (entry.isAbsolute() && entry.isClean()) return entry; @@ -773,8 +762,7 @@ QFileSystemEntry QFileSystemEngine::absoluteName(const QFileSystemEntry &entry) //static QByteArray QFileSystemEngine::id(const QFileSystemEntry &entry) { - if (Q_UNLIKELY(entry.isEmpty())) - return emptyFileEntryWarning(), QByteArray(); + Q_CHECK_FILE_NAME(entry, QByteArray()); QT_STATBUF statResult; if (QT_STAT(entry.nativeFilePath().constData(), &statResult)) { @@ -887,8 +875,7 @@ QString QFileSystemEngine::bundleName(const QFileSystemEntry &entry) bool QFileSystemEngine::fillMetaData(const QFileSystemEntry &entry, QFileSystemMetaData &data, QFileSystemMetaData::MetaDataFlags what) { - if (Q_UNLIKELY(entry.isEmpty())) - return emptyFileEntryWarning(), false; + Q_CHECK_FILE_NAME(entry, false); #if defined(Q_OS_DARWIN) if (what & QFileSystemMetaData::BundleType) { @@ -1157,8 +1144,7 @@ static bool createDirectoryWithParents(const QByteArray &nativeName, bool should bool QFileSystemEngine::createDirectory(const QFileSystemEntry &entry, bool createParents) { QString dirName = entry.filePath(); - if (Q_UNLIKELY(dirName.isEmpty())) - return emptyFileEntryWarning(), false; + Q_CHECK_FILE_NAME(dirName, false); // Darwin doesn't support trailing /'s, so remove for everyone while (dirName.size() > 1 && dirName.endsWith(QLatin1Char('/'))) @@ -1177,8 +1163,7 @@ bool QFileSystemEngine::createDirectory(const QFileSystemEntry &entry, bool crea //static bool QFileSystemEngine::removeDirectory(const QFileSystemEntry &entry, bool removeEmptyParents) { - if (Q_UNLIKELY(entry.isEmpty())) - return emptyFileEntryWarning(), false; + Q_CHECK_FILE_NAME(entry, false); if (removeEmptyParents) { QString dirName = QDir::cleanPath(entry.filePath()); @@ -1203,8 +1188,9 @@ bool QFileSystemEngine::removeDirectory(const QFileSystemEntry &entry, bool remo //static bool QFileSystemEngine::createLink(const QFileSystemEntry &source, const QFileSystemEntry &target, QSystemError &error) { - if (Q_UNLIKELY(source.isEmpty() || target.isEmpty())) - return emptyFileEntryWarning(), false; + Q_CHECK_FILE_NAME(source, false); + Q_CHECK_FILE_NAME(target, false); + if (::symlink(source.nativeFilePath().constData(), target.nativeFilePath().constData()) == 0) return true; error = QSystemError(errno, QSystemError::StandardLibraryError); @@ -1233,8 +1219,9 @@ bool QFileSystemEngine::renameFile(const QFileSystemEntry &source, const QFileSy { QFileSystemEntry::NativePath srcPath = source.nativeFilePath(); QFileSystemEntry::NativePath tgtPath = target.nativeFilePath(); - if (Q_UNLIKELY(srcPath.isEmpty() || tgtPath.isEmpty())) - return emptyFileEntryWarning(), false; + + Q_CHECK_FILE_NAME(srcPath, false); + Q_CHECK_FILE_NAME(tgtPath, false); #if defined(RENAME_NOREPLACE) && QT_CONFIG(renameat2) if (renameat2(AT_FDCWD, srcPath, AT_FDCWD, tgtPath, RENAME_NOREPLACE) == 0) @@ -1302,8 +1289,9 @@ bool QFileSystemEngine::renameFile(const QFileSystemEntry &source, const QFileSy //static bool QFileSystemEngine::renameOverwriteFile(const QFileSystemEntry &source, const QFileSystemEntry &target, QSystemError &error) { - if (Q_UNLIKELY(source.isEmpty() || target.isEmpty())) - return emptyFileEntryWarning(), false; + Q_CHECK_FILE_NAME(source, false); + Q_CHECK_FILE_NAME(target, false); + if (::rename(source.nativeFilePath().constData(), target.nativeFilePath().constData()) == 0) return true; error = QSystemError(errno, QSystemError::StandardLibraryError); @@ -1313,8 +1301,7 @@ bool QFileSystemEngine::renameOverwriteFile(const QFileSystemEntry &source, cons //static bool QFileSystemEngine::removeFile(const QFileSystemEntry &entry, QSystemError &error) { - if (Q_UNLIKELY(entry.isEmpty())) - return emptyFileEntryWarning(), false; + Q_CHECK_FILE_NAME(entry, false); if (unlink(entry.nativeFilePath().constData()) == 0) return true; error = QSystemError(errno, QSystemError::StandardLibraryError); @@ -1349,8 +1336,7 @@ static mode_t toMode_t(QFile::Permissions permissions) //static bool QFileSystemEngine::setPermissions(const QFileSystemEntry &entry, QFile::Permissions permissions, QSystemError &error, QFileSystemMetaData *data) { - if (Q_UNLIKELY(entry.isEmpty())) - return emptyFileEntryWarning(), false; + Q_CHECK_FILE_NAME(entry, false); mode_t mode = toMode_t(permissions); bool success = ::chmod(entry.nativeFilePath().constData(), mode) == 0; diff --git a/src/corelib/io/qfilesystemengine_win.cpp b/src/corelib/io/qfilesystemengine_win.cpp index 279918b812..ae29190848 100644 --- a/src/corelib/io/qfilesystemengine_win.cpp +++ b/src/corelib/io/qfilesystemengine_win.cpp @@ -461,7 +461,9 @@ void QFileSystemEngine::clearWinStatData(QFileSystemMetaData &data) QFileSystemEntry QFileSystemEngine::getLinkTarget(const QFileSystemEntry &link, QFileSystemMetaData &data) { - if (data.missingFlags(QFileSystemMetaData::LinkType)) + Q_CHECK_FILE_NAME(link, link); + + if (data.missingFlags(QFileSystemMetaData::LinkType)) QFileSystemEngine::fillMetaData(link, data, QFileSystemMetaData::LinkType); QString target; @@ -480,6 +482,8 @@ QFileSystemEntry QFileSystemEngine::getLinkTarget(const QFileSystemEntry &link, //static QFileSystemEntry QFileSystemEngine::canonicalName(const QFileSystemEntry &entry, QFileSystemMetaData &data) { + Q_CHECK_FILE_NAME(entry, entry); + if (data.missingFlags(QFileSystemMetaData::ExistsAttribute)) QFileSystemEngine::fillMetaData(entry, data, QFileSystemMetaData::ExistsAttribute); @@ -492,6 +496,8 @@ QFileSystemEntry QFileSystemEngine::canonicalName(const QFileSystemEntry &entry, //static QString QFileSystemEngine::nativeAbsoluteFilePath(const QString &path) { + Q_CHECK_FILE_NAME(path, QString()); + // can be //server or //server/share QString absPath; QVarLengthArray buf(qMax(MAX_PATH, path.size() + 1)); @@ -527,6 +533,8 @@ QString QFileSystemEngine::nativeAbsoluteFilePath(const QString &path) //static QFileSystemEntry QFileSystemEngine::absoluteName(const QFileSystemEntry &entry) { + Q_CHECK_FILE_NAME(entry, entry); + QString ret; if (!entry.isRelative()) { @@ -609,6 +617,8 @@ QByteArray fileIdWin8(HANDLE handle) //static QByteArray QFileSystemEngine::id(const QFileSystemEntry &entry) { + Q_CHECK_FILE_NAME(entry, QByteArray()); + QByteArray result; #ifndef Q_OS_WINRT @@ -999,6 +1009,7 @@ static bool isDirPath(const QString &dirPath, bool *existed); bool QFileSystemEngine::fillMetaData(const QFileSystemEntry &entry, QFileSystemMetaData &data, QFileSystemMetaData::MetaDataFlags what) { + Q_CHECK_FILE_NAME(entry, false); what |= QFileSystemMetaData::WinLnkType | QFileSystemMetaData::WinStatFlags; data.entryFlags &= ~what; @@ -1116,6 +1127,8 @@ static bool isDirPath(const QString &dirPath, bool *existed) bool QFileSystemEngine::createDirectory(const QFileSystemEntry &entry, bool createParents) { QString dirName = entry.filePath(); + Q_CHECK_FILE_NAME(dirName, false); + if (createParents) { dirName = QDir::toNativeSeparators(QDir::cleanPath(dirName)); // We spefically search for / so \ would break it.. @@ -1177,6 +1190,8 @@ bool QFileSystemEngine::createDirectory(const QFileSystemEntry &entry, bool crea bool QFileSystemEngine::removeDirectory(const QFileSystemEntry &entry, bool removeEmptyParents) { QString dirName = entry.filePath(); + Q_CHECK_FILE_NAME(dirName, false); + if (removeEmptyParents) { dirName = QDir::toNativeSeparators(QDir::cleanPath(dirName)); for (int oldslash = 0, slash=dirName.length(); slash > 0; oldslash = slash) { @@ -1381,6 +1396,9 @@ bool QFileSystemEngine::copyFile(const QFileSystemEntry &source, const QFileSyst //static bool QFileSystemEngine::renameFile(const QFileSystemEntry &source, const QFileSystemEntry &target, QSystemError &error) { + Q_CHECK_FILE_NAME(source, false); + Q_CHECK_FILE_NAME(target, false); + #ifndef Q_OS_WINRT bool ret = ::MoveFile((wchar_t*)source.nativeFilePath().utf16(), (wchar_t*)target.nativeFilePath().utf16()) != 0; @@ -1396,6 +1414,9 @@ bool QFileSystemEngine::renameFile(const QFileSystemEntry &source, const QFileSy //static bool QFileSystemEngine::renameOverwriteFile(const QFileSystemEntry &source, const QFileSystemEntry &target, QSystemError &error) { + Q_CHECK_FILE_NAME(source, false); + Q_CHECK_FILE_NAME(target, false); + bool ret = ::MoveFileEx(reinterpret_cast(source.nativeFilePath().utf16()), reinterpret_cast(target.nativeFilePath().utf16()), MOVEFILE_REPLACE_EXISTING) != 0; @@ -1407,6 +1428,8 @@ bool QFileSystemEngine::renameOverwriteFile(const QFileSystemEntry &source, cons //static bool QFileSystemEngine::removeFile(const QFileSystemEntry &entry, QSystemError &error) { + Q_CHECK_FILE_NAME(entry, false); + bool ret = ::DeleteFile((wchar_t*)entry.nativeFilePath().utf16()) != 0; if(!ret) error = QSystemError(::GetLastError(), QSystemError::NativeError); @@ -1417,6 +1440,8 @@ bool QFileSystemEngine::removeFile(const QFileSystemEntry &entry, QSystemError & bool QFileSystemEngine::setPermissions(const QFileSystemEntry &entry, QFile::Permissions permissions, QSystemError &error, QFileSystemMetaData *data) { + Q_CHECK_FILE_NAME(entry, false); + Q_UNUSED(data); int mode = 0; diff --git a/tests/auto/corelib/io/qfile/tst_qfile.cpp b/tests/auto/corelib/io/qfile/tst_qfile.cpp index 4f010f37c2..b8ae95dd93 100644 --- a/tests/auto/corelib/io/qfile/tst_qfile.cpp +++ b/tests/auto/corelib/io/qfile/tst_qfile.cpp @@ -550,6 +550,10 @@ void tst_QFile::exists() QFile unc(uncPath); QVERIFY2(unc.exists(), msgFileDoesNotExist(uncPath).constData()); #endif + + QTest::ignoreMessage(QtWarningMsg, "Broken filename passed to function"); + QVERIFY(!QFile::exists(QDir::currentPath() + QLatin1Char('/') + + QChar(QChar::Null) + QLatin1String("x/y"))); } void tst_QFile::open_data() -- cgit v1.2.3 From fcbf15c97bac91a889eccaa0cfad10093fd052f0 Mon Sep 17 00:00:00 2001 From: Alexander Volkov Date: Wed, 23 Oct 2019 16:53:00 +0300 Subject: QKeySequence: Add missing names for multimedia keys Task-number: QTBUG-40030 Change-Id: Ib34bcbf42d6dd1206209c2d76444fd8c777278fe Reviewed-by: Shawn Rutledge --- src/gui/kernel/qkeysequence.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/gui/kernel/qkeysequence.cpp b/src/gui/kernel/qkeysequence.cpp index 2a86b340af..e1244e1006 100644 --- a/src/gui/kernel/qkeysequence.cpp +++ b/src/gui/kernel/qkeysequence.cpp @@ -492,6 +492,8 @@ static const struct { { Qt::Key_LaunchD, QT_TRANSLATE_NOOP("QShortcut", "Launch (D)") }, { Qt::Key_LaunchE, QT_TRANSLATE_NOOP("QShortcut", "Launch (E)") }, { Qt::Key_LaunchF, QT_TRANSLATE_NOOP("QShortcut", "Launch (F)") }, + { Qt::Key_LaunchG, QT_TRANSLATE_NOOP("QShortcut", "Launch (G)") }, + { Qt::Key_LaunchH, QT_TRANSLATE_NOOP("QShortcut", "Launch (H)") }, { Qt::Key_MonBrightnessUp, QT_TRANSLATE_NOOP("QShortcut", "Monitor Brightness Up") }, { Qt::Key_MonBrightnessDown, QT_TRANSLATE_NOOP("QShortcut", "Monitor Brightness Down") }, { Qt::Key_KeyboardLightOnOff, QT_TRANSLATE_NOOP("QShortcut", "Keyboard Light On/Off") }, @@ -518,9 +520,11 @@ static const struct { { Qt::Key_Book, QT_TRANSLATE_NOOP("QShortcut", "Book") }, { Qt::Key_CD, QT_TRANSLATE_NOOP("QShortcut", "CD") }, { Qt::Key_Calculator, QT_TRANSLATE_NOOP("QShortcut", "Calculator") }, + { Qt::Key_Calendar, QT_TRANSLATE_NOOP("QShortcut", "Calendar") }, { Qt::Key_Clear, QT_TRANSLATE_NOOP("QShortcut", "Clear") }, { Qt::Key_ClearGrab, QT_TRANSLATE_NOOP("QShortcut", "Clear Grab") }, { Qt::Key_Close, QT_TRANSLATE_NOOP("QShortcut", "Close") }, + { Qt::Key_ContrastAdjust, QT_TRANSLATE_NOOP("QShortcut", "Adjust contrast") }, { Qt::Key_Copy, QT_TRANSLATE_NOOP("QShortcut", "Copy") }, { Qt::Key_Cut, QT_TRANSLATE_NOOP("QShortcut", "Cut") }, { Qt::Key_Display, QT_TRANSLATE_NOOP("QShortcut", "Display") }, @@ -534,6 +538,7 @@ static const struct { { Qt::Key_LogOff, QT_TRANSLATE_NOOP("QShortcut", "Logoff") }, { Qt::Key_Market, QT_TRANSLATE_NOOP("QShortcut", "Market") }, { Qt::Key_Meeting, QT_TRANSLATE_NOOP("QShortcut", "Meeting") }, + { Qt::Key_Memo, QT_TRANSLATE_NOOP("QShortcut", "Memo") }, { Qt::Key_MenuKB, QT_TRANSLATE_NOOP("QShortcut", "Keyboard Menu") }, { Qt::Key_MenuPB, QT_TRANSLATE_NOOP("QShortcut", "Menu PB") }, { Qt::Key_MySites, QT_TRANSLATE_NOOP("QShortcut", "My Sites") }, @@ -554,6 +559,7 @@ static const struct { { Qt::Key_Support, QT_TRANSLATE_NOOP("QShortcut", "Support") }, { Qt::Key_TaskPane, QT_TRANSLATE_NOOP("QShortcut", "Task Panel") }, { Qt::Key_Terminal, QT_TRANSLATE_NOOP("QShortcut", "Terminal") }, + { Qt::Key_ToDoList, QT_TRANSLATE_NOOP("QShortcut", "To-do list") }, { Qt::Key_Tools, QT_TRANSLATE_NOOP("QShortcut", "Tools") }, { Qt::Key_Travel, QT_TRANSLATE_NOOP("QShortcut", "Travel") }, { Qt::Key_Video, QT_TRANSLATE_NOOP("QShortcut", "Video") }, -- cgit v1.2.3 From 5edf34848a51c7678031aeb9576b8f3b7b5fceab Mon Sep 17 00:00:00 2001 From: Christian Ehrlicher Date: Sat, 19 Oct 2019 19:18:55 +0200 Subject: QTableView: properly deselect row when column 0 is hidden/not visible When the first column is hidden or not visible in the current viewport, it is not possible to deselect the current row. Fix it by passing the correct column to QItemSelectionModel::selectedRows() when testing if the current index is selected. Fixes: QTBUG-79092 Change-Id: I9d8082d2b29ad2f799156aee910c6ff6e3217771 Reviewed-by: David Faure Reviewed-by: Friedemann Kleint --- src/widgets/itemviews/qtableview.cpp | 2 +- .../itemviews/qtableview/tst_qtableview.cpp | 26 ++++++++++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/src/widgets/itemviews/qtableview.cpp b/src/widgets/itemviews/qtableview.cpp index c50156bbce..11c5be10fd 100644 --- a/src/widgets/itemviews/qtableview.cpp +++ b/src/widgets/itemviews/qtableview.cpp @@ -3331,7 +3331,7 @@ void QTableViewPrivate::selectRow(int row, bool anchor) if (q->selectionMode() != QTableView::SingleSelection && command.testFlag(QItemSelectionModel::Toggle)) { if (anchor) - ctrlDragSelectionFlag = verticalHeader->selectionModel()->selectedRows().contains(index) + ctrlDragSelectionFlag = verticalHeader->selectionModel()->selectedRows(column).contains(index) ? QItemSelectionModel::Deselect : QItemSelectionModel::Select; command &= ~QItemSelectionModel::Toggle; command |= ctrlDragSelectionFlag; diff --git a/tests/auto/widgets/itemviews/qtableview/tst_qtableview.cpp b/tests/auto/widgets/itemviews/qtableview/tst_qtableview.cpp index 09990ab70a..3e0d2539b4 100644 --- a/tests/auto/widgets/itemviews/qtableview/tst_qtableview.cpp +++ b/tests/auto/widgets/itemviews/qtableview/tst_qtableview.cpp @@ -418,6 +418,7 @@ private slots: void taskQTBUG_10169_sizeHintForRow(); void taskQTBUG_30653_doItemsLayout(); void taskQTBUG_50171_selectRowAfterSwapColumns(); + void deselectRow(); #if QT_CONFIG(wheelevent) void mouseWheel_data(); @@ -4492,6 +4493,31 @@ void tst_QTableView::taskQTBUG_50171_selectRowAfterSwapColumns() } } +class DeselectTableWidget : public QTableWidget +{ +public: + using QTableWidget::QTableWidget; + QItemSelectionModel::SelectionFlags selectionCommand(const QModelIndex &, + const QEvent * = nullptr) const override + { + return QItemSelectionModel::Toggle; + } +}; + +void tst_QTableView::deselectRow() +{ + DeselectTableWidget tw(20, 20); + tw.show(); + QVERIFY(QTest::qWaitForWindowExposed(&tw)); + tw.hideColumn(0); + QVERIFY(tw.isColumnHidden(0)); + tw.selectRow(1); + QVERIFY(tw.selectionModel()->isRowSelected(1, QModelIndex())); + tw.selectRow(1); + // QTBUG-79092 - deselection was not possible when column 0 was hidden + QVERIFY(!tw.selectionModel()->isRowSelected(1, QModelIndex())); +} + // This has nothing to do with QTableView, but it's convenient to reuse the QtTestTableModel #if QT_CONFIG(textmarkdownwriter) -- cgit v1.2.3