diff options
author | Volker Hilsheimer <volker.hilsheimer@qt.io> | 2021-06-15 12:45:03 +0200 |
---|---|---|
committer | Qt Cherry-pick Bot <cherrypick_bot@qt-project.org> | 2021-06-16 09:59:50 +0000 |
commit | 9569779c37c2d98f8550a917b7274baffd4ae6c0 (patch) | |
tree | 42a2a2a38db046d1e1b24d2c79cbbb78668f96b5 | |
parent | 3248c0242c43f8e4251013a7a7c2fe39687553e9 (diff) |
Avoid unnecessary palette allocations
The palette provider allocates QQuickPalette instances lazily in the
palette() implementation. Since we only only test here whether a palette
is present, uses the non-mutating providesPalette instead.
As a drive-by, remove the paletteData() check from setCurrentColorGroup;
paletteData() asserts if it would return nullptr, so this check is wrong
and misleading.
Amends 3675f2b235f32e05cf6d754e81e0e8f8ddd59752.
Change-Id: I9701b3520998ec538ef560106a6c6078e7f1c4d8
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>
(cherry picked from commit 7b5c1063de0034d4b9e4345b9493aa3beba62a89)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
-rw-r--r-- | src/quick/items/qquickitem.cpp | 2 | ||||
-rw-r--r-- | tests/auto/quick/qquickitem2/data/paletteAllocate.qml | 35 | ||||
-rw-r--r-- | tests/auto/quick/qquickitem2/tst_qquickitem.cpp | 41 |
3 files changed, 77 insertions, 1 deletions
diff --git a/src/quick/items/qquickitem.cpp b/src/quick/items/qquickitem.cpp index 5b8048c303..ff269363bb 100644 --- a/src/quick/items/qquickitem.cpp +++ b/src/quick/items/qquickitem.cpp @@ -8507,7 +8507,7 @@ bool QQuickItem::event(QEvent *ev) break; case QEvent::WindowActivate: case QEvent::WindowDeactivate: - if (d->palette()) + if (d->providesPalette()) d->setCurrentColorGroup(); for (QQuickItem *item : d->childItems) QCoreApplication::sendEvent(item, ev); diff --git a/tests/auto/quick/qquickitem2/data/paletteAllocate.qml b/tests/auto/quick/qquickitem2/data/paletteAllocate.qml new file mode 100644 index 0000000000..6c959ddbcb --- /dev/null +++ b/tests/auto/quick/qquickitem2/data/paletteAllocate.qml @@ -0,0 +1,35 @@ +import QtQuick + +Item { + id: root + width: 300 + height: 300 + visible: true + + palette.active.base: "blue" + palette.inactive.base: "red" + palette.disabled.base: "gray" + + Rectangle { + id: background + objectName: "background" + + anchors.centerIn: parent + width: parent.width / 2 + height: parent.height / 2 + + color: root.palette.base + + Rectangle { + id: foreground + objectName: "foreground" + + anchors.centerIn: parent + width: parent.width / 2 + height: parent.height / 2 + + color: root.palette.base + border.color: "black" + } + } +} diff --git a/tests/auto/quick/qquickitem2/tst_qquickitem.cpp b/tests/auto/quick/qquickitem2/tst_qquickitem.cpp index 843b916eb1..4d381d59a3 100644 --- a/tests/auto/quick/qquickitem2/tst_qquickitem.cpp +++ b/tests/auto/quick/qquickitem2/tst_qquickitem.cpp @@ -134,7 +134,9 @@ private slots: void isAncestorOf(); void grab(); + void colorGroup(); + void paletteAllocated(); private: QQmlEngine engine; @@ -3770,6 +3772,45 @@ void tst_QQuickItem::colorGroup() QCOMPARE(foreground->property("color").value<QColor>(), palette->inactive()->base()); } +/*! + Verify that items don't allocate their own QQuickPalette instance + unnecessarily. +*/ +void tst_QQuickItem::paletteAllocated() +{ + QQuickView view; + + view.setSource(testFileUrl("paletteAllocate.qml")); + + QQuickItem *root = qobject_cast<QQuickItem *>(view.rootObject()); + QQuickItem *background = root->findChild<QQuickItem *>("background"); + QVERIFY(background); + QQuickItem *foreground = root->findChild<QQuickItem *>("foreground"); + QVERIFY(foreground); + + bool backgroundHasPalette = false; + bool foregroundHasPalette = false; + QObject::connect(background, &QQuickItem::paletteCreated, this, [&]{ backgroundHasPalette = true; }); + QObject::connect(foreground, &QQuickItem::paletteCreated, this, [&]{ foregroundHasPalette = true; }); + + view.show(); + view.requestActivate(); + QVERIFY(QTest::qWaitForWindowActive(&view)); + + QVERIFY(!backgroundHasPalette); + QVERIFY(!foregroundHasPalette); + + view.close(); + + QVERIFY(!backgroundHasPalette); + QVERIFY(!foregroundHasPalette); + + auto quickpalette = foreground->property("palette").value<QQuickPalette*>(); + QVERIFY(!backgroundHasPalette); + QVERIFY(quickpalette); + QVERIFY(foregroundHasPalette); +} + QTEST_MAIN(tst_QQuickItem) #include "tst_qquickitem.moc" |