aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--src/quick/items/qquickwindow.cpp90
-rw-r--r--tests/auto/quick/qquickitem/tst_qquickitem.cpp82
2 files changed, 165 insertions, 7 deletions
diff --git a/src/quick/items/qquickwindow.cpp b/src/quick/items/qquickwindow.cpp
index 2c89ebd380..3c97475e86 100644
--- a/src/quick/items/qquickwindow.cpp
+++ b/src/quick/items/qquickwindow.cpp
@@ -70,6 +70,8 @@
#include <QtCore/QLibraryInfo>
#include <QtCore/QRunnable>
#include <QtQml/qqmlincubator.h>
+#include <QtQml/qqmlinfo.h>
+#include <QtQml/private/qqmlmetatype_p.h>
#include <QtQuick/private/qquickpixmapcache_p.h>
@@ -293,6 +295,79 @@ static bool transformDirtyOnItemOrAncestor(const QQuickItem *item)
}
#endif
+/*!
+ * \internal
+
+ A "polish loop" can occur inside QQuickWindowPrivate::polishItems(). It is when an item calls
+ polish() on an(other?) item from updatePolish(). If this anomaly happens repeatedly and without
+ interruption (of a well-behaved updatePolish() that doesn't call polish()), it is a strong
+ indication that we are heading towards an infinite polish loop. A polish loop is not a bug in
+ Qt Quick - it is a bug caused by ill-behaved items put in the scene.
+
+ We can detect this sequence of polish loops easily, since the
+ QQuickWindowPrivate::itemsToPolish is basically a stack: polish() will push to it, and
+ polishItems() will pop from it.
+ Therefore if updatePolish() calls polish(), the immediate next item polishItems() processes is
+ the item that was polished by the previous call to updatePolish().
+ We therefore just need to count the number of polish loops we detected in _sequence_.
+*/
+struct PolishLoopDetector
+{
+ PolishLoopDetector(const QVector<QQuickItem*> &itemsToPolish)
+ : itemsToPolish(itemsToPolish)
+ {
+ }
+
+ /*
+ * returns true when it detected a likely infinite loop
+ * (suggests it should abort the polish loop)
+ **/
+ bool check(QQuickItem *item, int itemsRemainingBeforeUpdatePolish)
+ {
+ if (itemsToPolish.count() > itemsRemainingBeforeUpdatePolish) {
+ // Detected potential polish loop.
+ ++numPolishLoopsInSequence;
+ if (numPolishLoopsInSequence >= 1000) {
+ // Start to warn about polish loop after 1000 consecutive polish loops
+ if (numPolishLoopsInSequence == 100000) {
+ // We have looped 100,000 times without actually reducing the list of items to
+ // polish, give up for now.
+ // This is not a fix, just a remedy so that the application can be somewhat
+ // responsive.
+ numPolishLoopsInSequence = 0;
+ return true;
+ } else if (numPolishLoopsInSequence < 1005) {
+ // Show the 5 next items involved in the polish loop.
+ // (most likely they will be the same 5 items...)
+ QQuickItem *guiltyItem = itemsToPolish.last();
+ qmlWarning(item) << "possible QQuickItem::polish() loop";
+
+ auto typeAndObjectName = [](QQuickItem *item) {
+ QString typeName = QQmlMetaType::prettyTypeName(item);
+ QString objName = item->objectName();
+ if (!objName.isNull())
+ return QString::fromLatin1("%1(%2)").arg(typeName, objName);
+ return typeName;
+ };
+
+ qmlWarning(guiltyItem) << typeAndObjectName(guiltyItem)
+ << " called polish() inside updatePolish() of " << typeAndObjectName(item);
+
+ if (numPolishLoopsInSequence == 1004)
+ // Enough warnings. Reset counter in order to speed things up and re-detect
+ // more loops
+ numPolishLoopsInSequence = 0;
+ }
+ }
+ } else {
+ numPolishLoopsInSequence = 0;
+ }
+ return false;
+ }
+ const QVector<QQuickItem*> &itemsToPolish; // Just a ref to the one in polishItems()
+ int numPolishLoopsInSequence = 0;
+};
+
void QQuickWindowPrivate::polishItems()
{
// An item can trigger polish on another item, or itself for that matter,
@@ -300,20 +375,21 @@ void QQuickWindowPrivate::polishItems()
// iterate through the set, we must continue pulling items out until it
// is empty.
// In the case where polish is called from updatePolish() either directly
- // or indirectly, we use a recursionSafeguard to print a warning to
- // the user.
- int recursionSafeguard = INT_MAX;
- while (!itemsToPolish.isEmpty() && --recursionSafeguard > 0) {
+ // or indirectly, we use a PolishLoopDetector to determine if a warning should
+ // be printed to the user.
+
+ PolishLoopDetector polishLoopDetector(itemsToPolish);
+ while (!itemsToPolish.isEmpty()) {
QQuickItem *item = itemsToPolish.takeLast();
QQuickItemPrivate *itemPrivate = QQuickItemPrivate::get(item);
itemPrivate->polishScheduled = false;
+ const int itemsRemaining = itemsToPolish.count();
itemPrivate->updatePolish();
item->updatePolish();
+ if (polishLoopDetector.check(item, itemsRemaining) == true)
+ break;
}
- if (recursionSafeguard == 0)
- qWarning("QQuickWindow: possible QQuickItem::polish() loop");
-
#if QT_CONFIG(im)
if (QQuickItem *focusItem = q_func()->activeFocusItem()) {
// If the current focus item, or any of its anchestors, has changed location
diff --git a/tests/auto/quick/qquickitem/tst_qquickitem.cpp b/tests/auto/quick/qquickitem/tst_qquickitem.cpp
index 7e132f97b6..61b6902b43 100644
--- a/tests/auto/quick/qquickitem/tst_qquickitem.cpp
+++ b/tests/auto/quick/qquickitem/tst_qquickitem.cpp
@@ -40,6 +40,7 @@
#include "../../shared/util.h"
#include "../shared/viewtestutil.h"
#include <QSignalSpy>
+#include <QtCore/qregularexpression.h>
#ifdef TEST_QTBUG_60123
#include <QWidget>
@@ -111,10 +112,15 @@ public:
}
bool wasPolished;
+ int repolishLoopCount = 0;
protected:
virtual void updatePolish() {
wasPolished = true;
+ if (repolishLoopCount > 0) {
+ --repolishLoopCount;
+ polish();
+ }
}
public slots:
@@ -197,6 +203,9 @@ private slots:
void qtBug60123();
#endif
+ void polishLoopDetection_data();
+ void polishLoopDetection();
+
private:
enum PaintOrderOp {
@@ -1427,6 +1436,79 @@ void tst_qquickitem::polishOnCompleted()
QTRY_VERIFY(item->wasPolished);
}
+struct PolishItemSpan {
+ int itemCount; // Number of items...
+ int repolishCount; // ...repolishing 'repolishCount' times
+};
+
+/*
+ * For instance, two consecutive spans {99,0} and {1,2000} } instructs to
+ * construct 99 items with no repolish, and 1 item with 2000 repolishes (in that sibling order)
+ */
+typedef QVector<PolishItemSpan> PolishItemSpans;
+
+Q_DECLARE_METATYPE(PolishItemSpan)
+Q_DECLARE_METATYPE(PolishItemSpans)
+
+void tst_qquickitem::polishLoopDetection_data()
+{
+ QTest::addColumn<PolishItemSpans>("listOfItemsToPolish");
+ QTest::addColumn<int>("expectedNumberOfWarnings");
+
+ QTest::newRow("test1.100") << PolishItemSpans({ {1, 100} }) << 0;
+ QTest::newRow("test1.1002") << PolishItemSpans({ {1, 1002} }) << 3;
+ QTest::newRow("test1.2020") << PolishItemSpans({ {1, 2020} }) << 10;
+
+ QTest::newRow("test5.1") << PolishItemSpans({ {5, 1} }) << 0;
+ QTest::newRow("test5.10") << PolishItemSpans({ {5, 10} }) << 0;
+ QTest::newRow("test5.100") << PolishItemSpans({ {5, 100} }) << 0;
+ QTest::newRow("test5.1000") << PolishItemSpans({ {5, 1000} }) << 5;
+
+ QTest::newRow("test1000.1") << PolishItemSpans({ {1000,1} }) << 0;
+ QTest::newRow("test2000.1") << PolishItemSpans({ {2000,1} }) << 0;
+
+ QTest::newRow("test99.0-1.1100") << PolishItemSpans({ {99,0},{1,1100} }) << 5;
+ QTest::newRow("test98.0-2.1100") << PolishItemSpans({ {98,0},{2,1100} }) << 5+5;
+
+ // reverse the two above
+ QTest::newRow("test1.1100-99.0") << PolishItemSpans({ {1,1100},{99,0} }) << 5;
+ QTest::newRow("test2.1100-98.0") << PolishItemSpans({ {2,1100},{98,0} }) << 5+5;
+}
+
+void tst_qquickitem::polishLoopDetection()
+{
+ QFETCH(PolishItemSpans, listOfItemsToPolish);
+ QFETCH(int, expectedNumberOfWarnings);
+
+ QQuickWindow window;
+ window.resize(200, 200);
+ window.show();
+
+ TestPolishItem *item = nullptr;
+ int count = 0;
+ for (PolishItemSpan s : listOfItemsToPolish) {
+ for (int i = 0; i < s.itemCount; ++i) {
+ item = new TestPolishItem(window.contentItem());
+ item->setSize(QSizeF(200, 100));
+ item->repolishLoopCount = s.repolishCount;
+ item->setObjectName(QString::fromLatin1("obj%1").arg(count++));
+ }
+ }
+
+ for (int i = 0; i < expectedNumberOfWarnings; ++i) {
+ QTest::ignoreMessage(QtWarningMsg, QRegularExpression(".*possible QQuickItem..polish.. loop.*"));
+ QTest::ignoreMessage(QtWarningMsg, QRegularExpression(".*TestPolishItem.* called polish.. inside updatePolish.. of TestPolishItem.*"));
+ }
+
+ QList<QQuickItem*> items = window.contentItem()->childItems();
+ for (int i = 0; i < items.count(); ++i) {
+ static_cast<TestPolishItem*>(items.at(i))->doPolish();
+ }
+ item = static_cast<TestPolishItem*>(items.first());
+ // item is the last item, so we wait until the last item reached 0
+ QVERIFY(QTest::qWaitFor([=](){return item->repolishLoopCount == 0 && item->wasPolished;}));
+}
+
void tst_qquickitem::wheelEvent_data()
{
QTest::addColumn<bool>("visible");