diff options
22 files changed, 235 insertions, 49 deletions
diff --git a/src/imports/controls/doc/src/includes/customize-button-background.qdocinc b/src/imports/controls/doc/src/includes/customize-button-background.qdocinc new file mode 100644 index 00000000..59df7d8e --- /dev/null +++ b/src/imports/controls/doc/src/includes/customize-button-background.qdocinc @@ -0,0 +1,25 @@ +//! [file] +\qml \QtMinorVersion +import QtQuick 2.\1 +import QtQuick.Controls 2.\1 + +ApplicationWindow { + width: 400 + height: 400 + visible: true + + Button { + id: button + text: "A Special Button" + background: Rectangle { + implicitWidth: 100 + implicitHeight: 40 + color: button.down ? "#d6d6d6" : "#f6f6f6" + border.color: "#26282a" + border.width: 1 + radius: 4 + } + } +} +\endqml +//! [file] diff --git a/src/imports/controls/doc/src/qtquickcontrols2-customize.qdoc b/src/imports/controls/doc/src/qtquickcontrols2-customize.qdoc index cd06a456..857e4e70 100644 --- a/src/imports/controls/doc/src/qtquickcontrols2-customize.qdoc +++ b/src/imports/controls/doc/src/qtquickcontrols2-customize.qdoc @@ -48,29 +48,7 @@ can override the \l {Control::}{background} item and set the radius property of Rectangle: - \qml \QtMinorVersion - import QtQuick 2.\1 - import QtQuick.Controls 2.\1 - - ApplicationWindow { - width: 400 - height: 400 - visible: true - - Button { - id: button - text: "A Special Button" - background: Rectangle { - implicitWidth: 100 - implicitHeight: 40 - color: button.down ? "#d6d6d6" : "#f6f6f6" - border.color: "#26282a" - border.width: 1 - radius: 4 - } - } - } - \endqml + \include customize-button-background.qdocinc file The second way to create the button is good if you plan to use your rounded button in several places. It involves moving the code into its own QML file @@ -213,6 +191,102 @@ files. \endlist + \section3 Considerations for custom styles + + When implementing your own style and customizing controls, there are some + points to keep in mind to ensure that your application is as performant as + possible. + + \section4 Avoid assigning an id to styles' implementations of item delegates + + As explained in \l {Definition of a Style}, when you implement your + own style for a control, you start off with the relevant template for + that control. For example, a style's \c Button.qml will be structured + similarly to this: + + \qml + T.Button { + // ... + + background: Rectangle { + // ... + } + + contentItem: Text { + // ... + } + + // ... + } + \endqml + + When you use a Button in your application, the \c background and + \c contentItem items will be created and parented to the root \c Button + item: + + \qml + // Creates the Button root item, the Rectangle background, + // and the Text contentItem. + Button { + text: qsTr("Confirm") + } + \endqml + + Suppose you then needed to do a one-off customization of the Button (as + explained in \l {Customizing a Control}): + + \include customize-button-background.qdocinc file + + In QML, this would normally result in both the default \c background + implementation and the one-off, custom \c background items being created. + Qt Quick Controls uses a technique that avoids creating both items, and + instead only creates the custom \c background, greatly improving the + creation performance of controls. + + This technique relies on the absence of an \l {The id Attribute}{id} in the + style's implementation of that item. If an id is assigned, the technique + cannot work, and both items will be created. For example, it can be + tempting to assign an id to the \c background or \c contentItem so that + other objects within the file can refer to those items: + + \qml + T.Button { + // ... + + background: Rectangle { + id: backgroundRect + // ... + } + + contentItem: Text { + // Use backgroundRect in some way... + } + + // ... + } + \endqml + + With this code, every time a Button instance with a customized background + is created, both backgrounds will be created, resulting in sub-optimal + creation performance. + + Prior to Qt 5.15, the old, unused background would be deleted to release + the resources associated with it. However, as the control does not own the + items, it should not delete them. As of Qt 5.15, old items are no longer + deleted, and so the \c backgroundRect item will live longer than it needs + to—typically until the application exits. Although the old item will be + hidden, visually unparented from the control, and removed from the + accessibility tree, it is important to keep the creation time and memory + usage of these unused items in mind when assigning an id in this context. + + \section4 Avoid imperative assignments of custom items + + The technique mentioned in the section above only works when an item is + \l {Prefer Declarative Bindings Over Imperative Assignments}{declaratively} + assigned for the first time, and so imperative assignments will result in + orphaned items. Always use declarative bindings to assign custom items + when possible. + \section3 Attached properties It is common for a style to have certain properties or attributes that diff --git a/src/quicktemplates2/qquickabstractbutton.cpp b/src/quicktemplates2/qquickabstractbutton.cpp index bb07d13e..eb592032 100644 --- a/src/quicktemplates2/qquickabstractbutton.cpp +++ b/src/quicktemplates2/qquickabstractbutton.cpp @@ -712,7 +712,7 @@ void QQuickAbstractButton::setIndicator(QQuickItem *indicator) const qreal oldImplicitIndicatorHeight = implicitIndicatorHeight(); d->removeImplicitSizeListener(d->indicator); - delete d->indicator; + QQuickControlPrivate::hideOldItem(d->indicator); d->indicator = indicator; if (indicator) { diff --git a/src/quicktemplates2/qquickapplicationwindow.cpp b/src/quicktemplates2/qquickapplicationwindow.cpp index 19f6f82b..9739650e 100644 --- a/src/quicktemplates2/qquickapplicationwindow.cpp +++ b/src/quicktemplates2/qquickapplicationwindow.cpp @@ -407,7 +407,7 @@ void QQuickApplicationWindow::setBackground(QQuickItem *background) if (!d->background.isExecuting()) d->cancelBackground(); - delete d->background; + QQuickControlPrivate::hideOldItem(d->background); d->background = background; if (background) { background->setParentItem(QQuickWindow::contentItem()); diff --git a/src/quicktemplates2/qquickcombobox.cpp b/src/quicktemplates2/qquickcombobox.cpp index 6b03bbf2..dcd427f3 100644 --- a/src/quicktemplates2/qquickcombobox.cpp +++ b/src/quicktemplates2/qquickcombobox.cpp @@ -51,6 +51,7 @@ #include <QtQml/qqmlcontext.h> #include <QtQml/private/qlazilyallocated_p.h> #include <private/qqmldelegatemodel_p.h> +#include <QtQuick/private/qquickaccessibleattached_p.h> #include <QtQuick/private/qquickevents_p_p.h> #include <QtQuick/private/qquicktextinput_p.h> #include <QtQuick/private/qquickitemview_p.h> @@ -264,6 +265,8 @@ public: void itemImplicitWidthChanged(QQuickItem *item) override; void itemImplicitHeightChanged(QQuickItem *item) override; + static void hideOldPopup(QQuickPopup *popup); + bool flat = false; bool down = false; bool hasDown = false; @@ -774,6 +777,21 @@ void QQuickComboBoxPrivate::itemImplicitHeightChanged(QQuickItem *item) emit q->implicitIndicatorHeightChanged(); } +void QQuickComboBoxPrivate::hideOldPopup(QQuickPopup *popup) +{ + if (!popup) + return; + + qCDebug(lcItemManagement) << "hiding old popup" << popup; + + popup->setVisible(false); + popup->setParentItem(nullptr); + // Remove the item from the accessibility tree. + QQuickAccessibleAttached *accessible = accessibleAttached(popup); + if (accessible) + accessible->setIgnored(true); +} + QQuickComboBox::QQuickComboBox(QQuickItem *parent) : QQuickControl(*(new QQuickComboBoxPrivate), parent) { @@ -794,7 +812,7 @@ QQuickComboBox::~QQuickComboBox() // Disconnect visibleChanged() to avoid a spurious highlightedIndexChanged() signal // emission during the destruction of the (visible) popup. (QTBUG-57650) QObjectPrivate::disconnect(d->popup.data(), &QQuickPopup::visibleChanged, d, &QQuickComboBoxPrivate::popupVisibleChanged); - delete d->popup; + QQuickComboBoxPrivate::hideOldPopup(d->popup); d->popup = nullptr; } } @@ -1136,7 +1154,7 @@ void QQuickComboBox::setIndicator(QQuickItem *indicator) const qreal oldImplicitIndicatorHeight = implicitIndicatorHeight(); d->removeImplicitSizeListener(d->indicator); - delete d->indicator; + QQuickControlPrivate::hideOldItem(d->indicator); d->indicator = indicator; if (indicator) { if (!indicator->parentItem()) @@ -1184,7 +1202,7 @@ void QQuickComboBox::setPopup(QQuickPopup *popup) if (d->popup) { QObjectPrivate::disconnect(d->popup.data(), &QQuickPopup::visibleChanged, d, &QQuickComboBoxPrivate::popupVisibleChanged); - delete d->popup; + QQuickComboBoxPrivate::hideOldPopup(d->popup); } if (popup) { QQuickPopupPrivate::get(popup)->allowVerticalFlip = true; diff --git a/src/quicktemplates2/qquickcombobox_p.h b/src/quicktemplates2/qquickcombobox_p.h index c9063d6a..00bc1242 100644 --- a/src/quicktemplates2/qquickcombobox_p.h +++ b/src/quicktemplates2/qquickcombobox_p.h @@ -48,10 +48,13 @@ // We mean it. // +#include <QtCore/qloggingcategory.h> #include <QtQuickTemplates2/private/qquickcontrol_p.h> QT_BEGIN_NAMESPACE +Q_DECLARE_LOGGING_CATEGORY(lcItemManagement) + class QValidator; class QQuickPopup; class QQmlInstanceModel; diff --git a/src/quicktemplates2/qquickcontainer.cpp b/src/quicktemplates2/qquickcontainer.cpp index 5f38c5b9..08129866 100644 --- a/src/quicktemplates2/qquickcontainer.cpp +++ b/src/quicktemplates2/qquickcontainer.cpp @@ -219,7 +219,7 @@ void QQuickContainerPrivate::cleanup() QQuickWindowPrivate::get(window)->clearFocusInScope(contentItem, focusItem, Qt::OtherFocusReason); q->contentItemChange(nullptr, contentItem); - delete contentItem; + QQuickControlPrivate::hideOldItem(contentItem); } QObject::disconnect(contentModel, &QQmlObjectModel::countChanged, q, &QQuickContainer::countChanged); diff --git a/src/quicktemplates2/qquickcontrol.cpp b/src/quicktemplates2/qquickcontrol.cpp index 0ebaf9a3..3306ff09 100644 --- a/src/quicktemplates2/qquickcontrol.cpp +++ b/src/quicktemplates2/qquickcontrol.cpp @@ -56,6 +56,8 @@ QT_BEGIN_NAMESPACE +Q_LOGGING_CATEGORY(lcItemManagement, "qt.quick.controls.control.itemmanagement") + /*! \qmltype Control \inherits Item @@ -420,7 +422,7 @@ void QQuickControlPrivate::setContentItem_helper(QQuickItem *item, bool notify) contentItem = item; q->contentItemChange(item, oldContentItem); - delete oldContentItem; + QQuickControlPrivate::hideOldItem(oldContentItem); if (item) { connect(contentItem.data(), &QQuickItem::baselineOffsetChanged, this, &QQuickControlPrivate::updateBaselineOffset); @@ -838,6 +840,22 @@ void QQuickControlPrivate::executeBackground(bool complete) quickCompleteDeferred(q, backgroundName(), background); } +void QQuickControlPrivate::hideOldItem(QQuickItem *item) +{ + if (!item) + return; + + qCDebug(lcItemManagement) << "hiding old item" << item; + + item->setVisible(false); + item->setParentItem(nullptr); + + // Remove the item from the accessibility tree. + QQuickAccessibleAttached *accessible = accessibleAttached(item); + if (accessible) + accessible->setIgnored(true); +} + void QQuickControlPrivate::updateBaselineOffset() { Q_Q(QQuickControl); @@ -1590,7 +1608,7 @@ void QQuickControl::setBackground(QQuickItem *background) } d->removeImplicitSizeListener(d->background, QQuickControlPrivate::ImplicitSizeChanges | QQuickItemPrivate::Geometry); - delete d->background; + QQuickControlPrivate::hideOldItem(d->background); d->background = background; if (background) { diff --git a/src/quicktemplates2/qquickcontrol_p_p.h b/src/quicktemplates2/qquickcontrol_p_p.h index a657307b..b649db0a 100644 --- a/src/quicktemplates2/qquickcontrol_p_p.h +++ b/src/quicktemplates2/qquickcontrol_p_p.h @@ -60,8 +60,12 @@ #include <QtGui/qaccessible.h> #endif +#include <QtCore/qloggingcategory.h> + QT_BEGIN_NAMESPACE +Q_DECLARE_LOGGING_CATEGORY(lcItemManagement) + class QQuickAccessibleAttached; class Q_QUICKTEMPLATES2_PRIVATE_EXPORT QQuickControlPrivate : public QQuickItemPrivate, public QQuickItemChangeListener @@ -168,6 +172,8 @@ public: virtual void cancelBackground(); virtual void executeBackground(bool complete = false); + static void hideOldItem(QQuickItem *item); + void updateBaselineOffset(); static const ChangeTypes ImplicitSizeChanges; diff --git a/src/quicktemplates2/qquickdial.cpp b/src/quicktemplates2/qquickdial.cpp index 431b25a8..65fed91c 100644 --- a/src/quicktemplates2/qquickdial.cpp +++ b/src/quicktemplates2/qquickdial.cpp @@ -620,7 +620,7 @@ void QQuickDial::setHandle(QQuickItem *handle) if (!d->handle.isExecuting()) d->cancelHandle(); - delete d->handle; + QQuickControlPrivate::hideOldItem(d->handle); d->handle = handle; if (d->handle && !d->handle->parentItem()) d->handle->setParentItem(this); diff --git a/src/quicktemplates2/qquickgroupbox.cpp b/src/quicktemplates2/qquickgroupbox.cpp index 7f3c7a2d..8f6f8bc4 100644 --- a/src/quicktemplates2/qquickgroupbox.cpp +++ b/src/quicktemplates2/qquickgroupbox.cpp @@ -199,7 +199,7 @@ void QQuickGroupBox::setLabel(QQuickItem *label) const qreal oldImplicitLabelHeight = implicitLabelHeight(); d->removeImplicitSizeListener(d->label); - delete d->label; + QQuickControlPrivate::hideOldItem(d->label); d->label = label; if (label) { diff --git a/src/quicktemplates2/qquicklabel.cpp b/src/quicktemplates2/qquicklabel.cpp index f3e0d512..da9b6985 100644 --- a/src/quicktemplates2/qquicklabel.cpp +++ b/src/quicktemplates2/qquicklabel.cpp @@ -396,7 +396,7 @@ void QQuickLabel::setBackground(QQuickItem *background) } QQuickControlPrivate::removeImplicitSizeListener(d->background, d, QQuickControlPrivate::ImplicitSizeChanges | QQuickItemPrivate::Geometry); - delete d->background; + QQuickControlPrivate::hideOldItem(d->background); d->background = background; if (background) { diff --git a/src/quicktemplates2/qquickmenuitem.cpp b/src/quicktemplates2/qquickmenuitem.cpp index a7fc63e8..118db822 100644 --- a/src/quicktemplates2/qquickmenuitem.cpp +++ b/src/quicktemplates2/qquickmenuitem.cpp @@ -216,7 +216,7 @@ void QQuickMenuItem::setArrow(QQuickItem *arrow) if (!d->arrow.isExecuting()) d->cancelArrow(); - delete d->arrow; + QQuickControlPrivate::hideOldItem(d->arrow); d->arrow = arrow; if (arrow && !arrow->parentItem()) arrow->setParentItem(this); diff --git a/src/quicktemplates2/qquickoverlay.cpp b/src/quicktemplates2/qquickoverlay.cpp index 73a31c84..bf2fc192 100644 --- a/src/quicktemplates2/qquickoverlay.cpp +++ b/src/quicktemplates2/qquickoverlay.cpp @@ -34,6 +34,7 @@ ** ****************************************************************************/ +#include "qquickcontrol_p_p.h" #include "qquickoverlay_p.h" #include "qquickoverlay_p_p.h" #include "qquickpopupitem_p_p.h" @@ -342,7 +343,6 @@ void QQuickOverlay::setModal(QQmlComponent *modal) if (d->modal == modal) return; - delete d->modal; d->modal = modal; emit modalChanged(); } @@ -359,7 +359,6 @@ void QQuickOverlay::setModeless(QQmlComponent *modeless) if (d->modeless == modeless) return; - delete d->modeless; d->modeless = modeless; emit modelessChanged(); } @@ -671,7 +670,6 @@ void QQuickOverlayAttached::setModal(QQmlComponent *modal) if (d->modal == modal) return; - delete d->modal; d->modal = modal; emit modalChanged(); } @@ -704,7 +702,6 @@ void QQuickOverlayAttached::setModeless(QQmlComponent *modeless) if (d->modeless == modeless) return; - delete d->modeless; d->modeless = modeless; emit modelessChanged(); } diff --git a/src/quicktemplates2/qquickrangeslider.cpp b/src/quicktemplates2/qquickrangeslider.cpp index 378ece50..17948987 100644 --- a/src/quicktemplates2/qquickrangeslider.cpp +++ b/src/quicktemplates2/qquickrangeslider.cpp @@ -267,7 +267,7 @@ void QQuickRangeSliderNode::setHandle(QQuickItem *handle) const qreal oldImplicitHandleHeight = implicitHandleHeight(); QQuickControlPrivate::get(d->slider)->removeImplicitSizeListener(d->handle); - delete d->handle; + QQuickControlPrivate::hideOldItem(d->handle); d->handle = handle; if (handle) { diff --git a/src/quicktemplates2/qquickslider.cpp b/src/quicktemplates2/qquickslider.cpp index 054ea502..c7f8a025 100644 --- a/src/quicktemplates2/qquickslider.cpp +++ b/src/quicktemplates2/qquickslider.cpp @@ -579,7 +579,7 @@ void QQuickSlider::setHandle(QQuickItem *handle) const qreal oldImplicitHandleHeight = implicitHandleHeight(); d->removeImplicitSizeListener(d->handle); - delete d->handle; + QQuickControlPrivate::hideOldItem(d->handle); d->handle = handle; if (handle) { diff --git a/src/quicktemplates2/qquickspinbox.cpp b/src/quicktemplates2/qquickspinbox.cpp index 389e5c54..bc8ec809 100644 --- a/src/quicktemplates2/qquickspinbox.cpp +++ b/src/quicktemplates2/qquickspinbox.cpp @@ -1126,7 +1126,7 @@ void QQuickSpinButton::setIndicator(QQuickItem *indicator) QQuickSpinBox *spinBox = static_cast<QQuickSpinBox *>(parent()); QQuickSpinBoxPrivate::get(spinBox)->removeImplicitSizeListener(d->indicator); - delete d->indicator; + QQuickControlPrivate::hideOldItem(d->indicator); d->indicator = indicator; if (indicator) { diff --git a/src/quicktemplates2/qquicktextarea.cpp b/src/quicktemplates2/qquicktextarea.cpp index ef59bd93..ef452fd0 100644 --- a/src/quicktemplates2/qquicktextarea.cpp +++ b/src/quicktemplates2/qquicktextarea.cpp @@ -633,7 +633,7 @@ void QQuickTextArea::setBackground(QQuickItem *background) } QQuickControlPrivate::removeImplicitSizeListener(d->background, d, QQuickControlPrivate::ImplicitSizeChanges | QQuickItemPrivate::Geometry); - delete d->background; + QQuickControlPrivate::hideOldItem(d->background); d->background = background; if (background) { diff --git a/src/quicktemplates2/qquicktextfield.cpp b/src/quicktemplates2/qquicktextfield.cpp index 740edff0..c03b05da 100644 --- a/src/quicktemplates2/qquicktextfield.cpp +++ b/src/quicktemplates2/qquicktextfield.cpp @@ -501,7 +501,7 @@ void QQuickTextField::setBackground(QQuickItem *background) } QQuickControlPrivate::removeImplicitSizeListener(d->background, d, QQuickControlPrivate::ImplicitSizeChanges | QQuickItemPrivate::Geometry); - delete d->background; + QQuickControlPrivate::hideOldItem(d->background); d->background = background; if (background) { diff --git a/tests/auto/controls/data/tst_spinbox.qml b/tests/auto/controls/data/tst_spinbox.qml index d3a0d8bb..22dbb352 100644 --- a/tests/auto/controls/data/tst_spinbox.qml +++ b/tests/auto/controls/data/tst_spinbox.qml @@ -658,4 +658,20 @@ TestCase { compare(control.displayText, data.displayTexts[i]) } } + + Component { + id: overriddenSpinBox + SpinBox { + value: 50 + up.indicator: Rectangle { + property string s: "this is the one" + } + } + } + + function test_indicatorOverridden() { + var control = createTemporaryObject(overriddenSpinBox, testCase) + verify(control) + compare(control.up.indicator.s, "this is the one"); + } } diff --git a/tests/auto/customization/data/styles/identified/AbstractButton.qml b/tests/auto/customization/data/styles/identified/AbstractButton.qml index b9656f7a..cd2b5bdc 100644 --- a/tests/auto/customization/data/styles/identified/AbstractButton.qml +++ b/tests/auto/customization/data/styles/identified/AbstractButton.qml @@ -58,15 +58,18 @@ T.AbstractButton { indicator: Item { id: indicator objectName: "abstractbutton-indicator-identified" + Accessible.name: objectName } contentItem: Item { id: contentItem objectName: "abstractbutton-contentItem-identified" + Accessible.name: objectName } background: Item { id: background objectName: "abstractbutton-background-identified" + Accessible.name: objectName } } diff --git a/tests/auto/customization/tst_customization.cpp b/tests/auto/customization/tst_customization.cpp index 41efc2a6..cce74b41 100644 --- a/tests/auto/customization/tst_customization.cpp +++ b/tests/auto/customization/tst_customization.cpp @@ -42,6 +42,7 @@ #include <QtQuick/qquickitem.h> #include <QtQuick/qquickwindow.h> #include <QtQuickControls2/qquickstyle.h> +#include <QtQuickTemplates2/private/qquickcontrol_p_p.h> #include "../shared/visualtestutil.h" using namespace QQuickVisualTestUtil; @@ -439,11 +440,36 @@ void tst_customization::override() QVERIFY2(qt_createdQObjects()->isEmpty(), qPrintable("unexpectedly created: " + qt_createdQObjects->join(", "))); if (!nonDeferred.isEmpty()) { - for (QString delegate : qAsConst(delegates)) { - if (!delegate.contains("-")) - delegate.append("-" + nonDeferred); - delegate.prepend(type.toLower() + "-"); - QVERIFY2(qt_destroyedQObjects()->removeOne(delegate), qPrintable(delegate + " was not destroyed as expected")); + // There were items for which deferred execution was not possible. + for (QString delegateName : qAsConst(delegates)) { + if (!delegateName.contains("-")) + delegateName.append("-" + nonDeferred); + delegateName.prepend(type.toLower() + "-"); + + const int delegateIndex = qt_destroyedQObjects()->indexOf(delegateName); + QVERIFY2(delegateIndex == -1, qPrintable(delegateName + " was unexpectedly destroyed")); + + const auto controlChildren = control->children(); + const auto childIt = std::find_if(controlChildren.constBegin(), controlChildren.constEnd(), [delegateName](const QObject *child) { + return child->objectName() == delegateName; + }); + // We test other delegates (like the background) here, so make sure we don't end up with XPASSes by using the wrong delegate. + if (delegateName.contains(QLatin1String("handle"))) { + QEXPECT_FAIL("identified:RangeSlider", "For some reason, items that are belong to grouped properties fail here", Abort); + QEXPECT_FAIL("overidentified:RangeSlider", "For some reason, items that are belong to grouped properties fail here", Abort); + } + if (delegateName.contains(QLatin1String("indicator"))) { + QEXPECT_FAIL("identified:SpinBox", "For some reason, items that are belong to grouped properties fail here", Abort); + QEXPECT_FAIL("overidentified:SpinBox", "For some reason, items that are belong to grouped properties fail here", Abort); + } + QVERIFY2(childIt != controlChildren.constEnd(), qPrintable(QString::fromLatin1( + "Expected delegate \"%1\" to still be a QObject child of \"%2\"").arg(delegateName).arg(controlName))); + + const auto *delegate = qobject_cast<QQuickItem*>(*childIt); + // Ensure that the item is hidden, etc. + QVERIFY(delegate); + QCOMPARE(delegate->isVisible(), false); + QCOMPARE(delegate->parentItem(), nullptr); } } |