From: Andrew den Exter Date: Wed, 28 Mar 2012 07:42:32 +0000 (+1000) Subject: Don't construct VisualDataModel attached properties unless requested. X-Git-Url: http://git.silmor.de/gitweb/?a=commitdiff_plain;h=afbde67c01baad017cf7ed385a6415e137269e9e;p=konrad%2Fqtdeclarative.git Don't construct VisualDataModel attached properties unless requested. This saves allocating a QObject per item model in the common case. Change-Id: I0e77e6c6c0c64ac6c5e482ef55e194c68e778b32 Reviewed-by: Bea Lam --- diff --git a/src/quick/items/qquickvisualdatamodel.cpp b/src/quick/items/qquickvisualdatamodel.cpp index 7f80fa0..a1da179 100644 --- a/src/quick/items/qquickvisualdatamodel.cpp +++ b/src/quick/items/qquickvisualdatamodel.cpp @@ -74,8 +74,6 @@ QQuickVisualDataModelParts::QQuickVisualDataModelParts(QQuickVisualDataModel *pa //--------------------------------------------------------------------------- -QHash QQuickVisualDataModelAttached::attachedProperties; - /*! \qmlclass VisualDataModel QQuickVisualDataModel \inqmlmodule QtQuick 2 @@ -418,29 +416,17 @@ int QQuickVisualDataModel::count() const return d->m_compositor.count(d->m_compositorGroup); } -void QQuickVisualDataModelPrivate::destroy(QObject *object) -{ - QObjectPrivate *p = QObjectPrivate::get(object); - Q_ASSERT(p->declarativeData); - QQmlData *data = static_cast(p->declarativeData); - if (data->ownContext && data->context) - data->context->clearContext(); - object->deleteLater(); -} - QQuickVisualDataModel::ReleaseFlags QQuickVisualDataModelPrivate::release(QObject *object) { QQuickVisualDataModel::ReleaseFlags stat = 0; if (!object) return stat; - if (QQuickVisualDataModelAttached *attached = QQuickVisualDataModelAttached::properties(object)) { - QQuickVisualDataModelItem *cacheItem = attached->m_cacheItem; + if (QQuickVisualDataModelItem *cacheItem = QQuickVisualDataModelItem::dataForObject(object)) { if (cacheItem->releaseObject()) { - destroy(object); + cacheItem->destroyObject(); if (QQuickItem *item = qobject_cast(object)) emitDestroyingItem(item); - cacheItem->setObject(0); if (cacheItem->incubationTask) { releaseIncubator(cacheItem->incubationTask); cacheItem->incubationTask = 0; @@ -486,13 +472,13 @@ void QQuickVisualDataModel::cancel(int index) cacheItem->incubationTask = 0; } if (cacheItem->object() && !cacheItem->isObjectReferenced()) { - d->destroy(cacheItem->object()); - if (QQuickPackage *package = qobject_cast(cacheItem->object())) + QObject *object = cacheItem->object(); + cacheItem->destroyObject(); + if (QQuickPackage *package = qobject_cast(object)) d->emitDestroyingPackage(package); - else if (QQuickItem *item = qobject_cast(cacheItem->object())) + else if (QQuickItem *item = qobject_cast(object)) d->emitDestroyingItem(item); - cacheItem->setObject(0); - cacheItem->Dispose(); + cacheItem->scriptRef -= 1; } if (!cacheItem->isReferenced()) { d->m_compositor.clearFlags(Compositor::Cache, it.cacheIndex, 1, Compositor::CacheFlag); @@ -786,10 +772,6 @@ void QQuickVisualDataModelPrivate::setInitialState(QVDMIncubationTask *incubatio QQml_setParent_noEvent(incubationTask->incubatingContext, cacheItem->object()); incubationTask->incubatingContext = 0; - cacheItem->attached = QQuickVisualDataModelAttached::properties(cacheItem->object()); - cacheItem->attached->setCacheItem(cacheItem); - new QQuickVisualDataModelAttachedMetaObject(cacheItem->attached, m_cacheMetaType); - cacheItem->attached->emitChanges(); if (QQuickPackage *package = qobject_cast(cacheItem->object())) emitInitPackage(cacheItem, package); @@ -908,8 +890,8 @@ QString QQuickVisualDataModel::stringValue(int index, const QString &name) int QQuickVisualDataModel::indexOf(QQuickItem *item, QObject *) const { Q_D(const QQuickVisualDataModel); - if (QQuickVisualDataModelAttached *attached = QQuickVisualDataModelAttached::properties(item)) - return attached->m_cacheItem->index[d->m_compositorGroup]; + if (QQuickVisualDataModelItem *cacheItem = QQuickVisualDataModelItem::dataForObject(item)) + return cacheItem->index[d->m_compositorGroup]; return -1; } @@ -1125,10 +1107,11 @@ void QQuickVisualDataModelPrivate::itemsRemoved( for (; cacheIndex < remove.cacheIndex + remove.count - removedCache; ++cacheIndex) { QQuickVisualDataModelItem *cacheItem = m_cache.at(cacheIndex); if (remove.inGroup(Compositor::Persisted) && cacheItem->objectRef == 0 && cacheItem->object()) { - destroy(cacheItem->object()); - if (QQuickPackage *package = qobject_cast(cacheItem->object())) + QObject *object = cacheItem->object(); + cacheItem->destroyObject(); + if (QQuickPackage *package = qobject_cast(object)) emitDestroyingPackage(package); - else if (QQuickItem *item = qobject_cast(cacheItem->object())) + else if (QQuickItem *item = qobject_cast(object)) emitDestroyingItem(item); cacheItem->setObject(0); cacheItem->scriptRef -= 1; @@ -1371,7 +1354,13 @@ void QQuickVisualDataModel::_q_layoutChanged() QQuickVisualDataModelAttached *QQuickVisualDataModel::qmlAttachedProperties(QObject *obj) { - return QQuickVisualDataModelAttached::properties(obj); + if (QQuickVisualDataModelItem *cacheItem = QQuickVisualDataModelItem::dataForObject(obj)) { + if (cacheItem->object() == obj) { // Don't create attached item for child objects. + cacheItem->attached = new QQuickVisualDataModelAttached(cacheItem, obj); + return cacheItem->attached; + } + } + return new QQuickVisualDataModelAttached(obj); } bool QQuickVisualDataModelPrivate::insert( @@ -1628,6 +1617,8 @@ v8::Handle QQuickVisualDataModelItemMetaType::get_index( //--------------------------------------------------------------------------- +QHash QQuickVisualDataModelItem::contextData; + QQuickVisualDataModelItem::QQuickVisualDataModelItem( QQuickVisualDataModelItemMetaType *metaType, int modelIndex) : QV8ObjectResource(metaType->v8Engine) @@ -1675,8 +1666,40 @@ void QQuickVisualDataModelItem::Dispose() delete this; } -void QQuickVisualDataModelItem::objectDestroyed(QObject *) +void QQuickVisualDataModelItem::setObject(QObject *g) +{ + if (QObject *previous = object()) + contextData.remove(previous); + if (g) + contextData.insert(g, this); + + QQmlGuard::setObject(g); +} + +void QQuickVisualDataModelItem::destroyObject() +{ + QObject * const obj = object(); + setObject(0); + + Q_ASSERT(obj); + + QObjectPrivate *p = QObjectPrivate::get(obj); + Q_ASSERT(p->declarativeData); + QQmlData *data = static_cast(p->declarativeData); + if (data->ownContext && data->context) + data->context->clearContext(); + obj->deleteLater(); + + if (attached) { + attached->m_cacheItem = 0; + attached = 0; + } +} + +void QQuickVisualDataModelItem::objectDestroyed(QObject *object) { + contextData.remove(object); + attached = 0; Dispose(); } @@ -1688,6 +1711,9 @@ QQuickVisualDataModelAttachedMetaObject::QQuickVisualDataModelAttachedMetaObject : attached(attached) , metaType(metaType) { + if (!metaType->metaObject) + metaType->initializeMetaObject(); + metaType->addref(); *static_cast(this) = *metaType->metaObject; QObjectPrivate::get(attached)->metaObject = this; @@ -1738,11 +1764,25 @@ int QQuickVisualDataModelAttachedMetaObject::metaCall(QMetaObject::Call call, in return attached->qt_metacall(call, _id, arguments); } -void QQuickVisualDataModelAttached::setCacheItem(QQuickVisualDataModelItem *item) +QQuickVisualDataModelAttached::QQuickVisualDataModelAttached(QObject *parent) + : m_cacheItem(0) + , m_previousGroups(0) + , m_modelChanged(false) { - m_cacheItem = item; + QQml_setParent_noEvent(this, parent); +} + +QQuickVisualDataModelAttached::QQuickVisualDataModelAttached( + QQuickVisualDataModelItem *cacheItem, QObject *parent) + : m_cacheItem(cacheItem) + , m_previousGroups(cacheItem->groups) + , m_modelChanged(false) +{ + QQml_setParent_noEvent(this, parent); for (int i = 1; i < m_cacheItem->metaType->groupCount; ++i) m_previousIndex[i] = m_cacheItem->index[i]; + + new QQuickVisualDataModelAttachedMetaObject(this, cacheItem->metaType); } /*! @@ -2299,7 +2339,7 @@ void QQuickVisualDataGroup::resolve(QQmlV8Function *args) Q_ASSERT(model->m_cache.count() == model->m_compositor.count(Compositor::Cache)); } else { cacheItem->resolveIndex(model->m_adaptorModel, resolvedIndex); - if (cacheItem->object()) + if (cacheItem->attached) cacheItem->attached->emitUnresolvedChanged(); } @@ -2729,8 +2769,8 @@ int QQuickVisualPartsModel::indexOf(QQuickItem *item, QObject *) const { QHash::const_iterator it = m_packaged.find(item); if (it != m_packaged.end()) { - if (QQuickVisualDataModelAttached *attached = QQuickVisualDataModelAttached::properties(*it)) - return attached->m_cacheItem->index[m_compositorGroup]; + if (QQuickVisualDataModelItem *cacheItem = QQuickVisualDataModelItem::dataForObject(*it)) + return cacheItem->index[m_compositorGroup]; } return -1; } diff --git a/src/quick/items/qquickvisualdatamodel_p.h b/src/quick/items/qquickvisualdatamodel_p.h index 1752e40..114d394 100644 --- a/src/quick/items/qquickvisualdatamodel_p.h +++ b/src/quick/items/qquickvisualdatamodel_p.h @@ -196,14 +196,9 @@ class QQuickVisualDataModelAttached : public QObject Q_PROPERTY(QStringList groups READ groups WRITE setGroups NOTIFY groupsChanged) Q_PROPERTY(bool isUnresolved READ isUnresolved NOTIFY unresolvedChanged) public: - QQuickVisualDataModelAttached(QObject *parent) - : m_cacheItem(0) - , m_previousGroups(0) - , m_modelChanged(false) - { - QQml_setParent_noEvent(this, parent); - } - ~QQuickVisualDataModelAttached() { attachedProperties.remove(parent()); } + QQuickVisualDataModelAttached(QObject *parent); + QQuickVisualDataModelAttached(QQuickVisualDataModelItem *cacheItem, QObject *parent); + ~QQuickVisualDataModelAttached() {} void setCacheItem(QQuickVisualDataModelItem *item); @@ -218,16 +213,6 @@ public: void emitUnresolvedChanged() { emit unresolvedChanged(); } - static QQuickVisualDataModelAttached *properties(QObject *obj) - { - QQuickVisualDataModelAttached *rv = attachedProperties.value(obj); - if (!rv) { - rv = new QQuickVisualDataModelAttached(obj); - attachedProperties.insert(obj, rv); - } - return rv; - } - Q_SIGNALS: void modelChanged(); void groupsChanged(); @@ -239,8 +224,6 @@ public: int m_previousIndex[QQuickListCompositor::MaximumGroupCount]; bool m_modelChanged; - static QHash attachedProperties; - friend class QQuickVisualDataModelAttachedMetaObject; }; diff --git a/src/quick/items/qquickvisualdatamodel_p_p.h b/src/quick/items/qquickvisualdatamodel_p_p.h index 5fd99d0..7dd6603 100644 --- a/src/quick/items/qquickvisualdatamodel_p_p.h +++ b/src/quick/items/qquickvisualdatamodel_p_p.h @@ -103,7 +103,7 @@ public: class QQuickVisualAdaptorModel; class QVDMIncubationTask; -class QQuickVisualDataModelItem : public QObject, public QV8ObjectResource, public QQmlGuard +class QQuickVisualDataModelItem : public QObject, public QV8ObjectResource, private QQmlGuard { Q_OBJECT Q_PROPERTY(int index READ modelIndex NOTIFY modelIndexChanged) @@ -127,6 +127,12 @@ public: QObject *modelObject() { return this; } + inline QObject *object() const { return QQmlGuard::object(); } + void setObject(QObject *g); + void destroyObject(); + + static QQuickVisualDataModelItem *dataForObject(QObject *object) { return contextData.value(object, 0); } + int modelIndex() const { return index[0]; } void setModelIndex(int idx) { index[0] = idx; emit modelIndexChanged(); } @@ -151,6 +157,12 @@ Q_SIGNALS: protected: void objectDestroyed(QObject *); + +private: + // A static hash of context data for all delegate object isn't ideal, but it provides a way + // to get the context data when constructing attached objects rather than constructing the + // attached objects on suspicion. + static QHash contextData; }; @@ -236,7 +248,6 @@ public: void connectModel(QQuickVisualAdaptorModel *model); QObject *object(Compositor::Group group, int index, bool asynchronous, bool reference); - void destroy(QObject *object); QQuickVisualDataModel::ReleaseFlags release(QObject *object); QString stringValue(Compositor::Group group, int index, const QString &name); void emitCreatedPackage(QQuickVisualDataModelItem *cacheItem, QQuickPackage *package); @@ -295,7 +306,6 @@ public: QString m_filterGroup; - int m_count; int m_groupCount; diff --git a/tests/auto/quick/qquickvisualdatamodel/data/invalidAttachment.qml b/tests/auto/quick/qquickvisualdatamodel/data/invalidAttachment.qml new file mode 100644 index 0000000..2758f56 --- /dev/null +++ b/tests/auto/quick/qquickvisualdatamodel/data/invalidAttachment.qml @@ -0,0 +1,19 @@ +import QtQuick 2.0 + +Item { + property VisualDataModel invalidVdm: VisualDataModel.model + Repeater { + model: 1 + delegate: Item { + id: outer + objectName: "delegate" + + property VisualDataModel validVdm: outer.VisualDataModel.model + property VisualDataModel invalidVdm: inner.VisualDataModel.model + + Item { + id: inner + } + } + } +} diff --git a/tests/auto/quick/qquickvisualdatamodel/tst_qquickvisualdatamodel.cpp b/tests/auto/quick/qquickvisualdatamodel/tst_qquickvisualdatamodel.cpp index b8d46ac..9047dcc 100644 --- a/tests/auto/quick/qquickvisualdatamodel/tst_qquickvisualdatamodel.cpp +++ b/tests/auto/quick/qquickvisualdatamodel/tst_qquickvisualdatamodel.cpp @@ -229,6 +229,7 @@ private slots: void resolve(); void warnings_data(); void warnings(); + void invalidAttachment(); private: template void groups_verify( @@ -3439,6 +3440,31 @@ void tst_qquickvisualdatamodel::warnings() QCOMPARE(evaluate(listView, "count"), count); } +void tst_qquickvisualdatamodel::invalidAttachment() +{ + QQmlComponent component(&engine); + component.loadUrl(testFileUrl("invalidAttachment.qml")); + + QScopedPointer object(component.create()); + QVERIFY(object); + QCOMPARE(component.errors().count(), 0); + + QVariant property = object->property("invalidVdm"); + QCOMPARE(property.userType(), qMetaTypeId()); + QVERIFY(!property.value()); + + QQuickItem *item = object->findChild("delegate"); + QVERIFY(item); + + property = item->property("validVdm"); + QCOMPARE(property.userType(), qMetaTypeId()); + QVERIFY(property.value()); + + property = item->property("invalidVdm"); + QCOMPARE(property.userType(), qMetaTypeId()); + QVERIFY(!property.value()); +} + QTEST_MAIN(tst_qquickvisualdatamodel)