From 896cf6f1523afcf5237efc1ca135672ca3cb8058 Mon Sep 17 00:00:00 2001 From: Aaron Kennedy Date: Thu, 14 Oct 2010 14:41:15 +1000 Subject: Optimization: Don't generate intermediate QDeclarativeComponent's --- src/declarative/qml/qdeclarativecompileddata.cpp | 2 +- src/declarative/qml/qdeclarativecompiler.cpp | 8 ++-- src/declarative/qml/qdeclarativecompiler_p.h | 5 ++- src/declarative/qml/qdeclarativecomponent.cpp | 57 +++++++++++++----------- src/declarative/qml/qdeclarativecomponent_p.h | 7 +-- src/declarative/qml/qdeclarativetypeloader.cpp | 19 +------- src/declarative/qml/qdeclarativetypeloader_p.h | 2 - src/declarative/qml/qdeclarativevme.cpp | 12 +++-- 8 files changed, 48 insertions(+), 64 deletions(-) diff --git a/src/declarative/qml/qdeclarativecompileddata.cpp b/src/declarative/qml/qdeclarativecompileddata.cpp index 5d73d89..a4ecc77 100644 --- a/src/declarative/qml/qdeclarativecompileddata.cpp +++ b/src/declarative/qml/qdeclarativecompileddata.cpp @@ -205,7 +205,7 @@ const QMetaObject *QDeclarativeCompiledData::TypeReference::metaObject() const return type->metaObject(); } else { Q_ASSERT(component); - return static_cast(QObjectPrivate::get(component))->cc->root; + return component->root; } } diff --git a/src/declarative/qml/qdeclarativecompiler.cpp b/src/declarative/qml/qdeclarativecompiler.cpp index 74bc5bd..b2740b8 100644 --- a/src/declarative/qml/qdeclarativecompiler.cpp +++ b/src/declarative/qml/qdeclarativecompiler.cpp @@ -590,7 +590,7 @@ bool QDeclarativeCompiler::compile(QDeclarativeEngine *engine, COMPILE_EXCEPTION(parserRef->refObjects.first(), err); } } else if (tref.typeData) { - ref.component = tref.typeData->component(); + ref.component = tref.typeData->compiledData(); ref.ref = tref.typeData; ref.ref->addref(); } @@ -722,7 +722,7 @@ bool QDeclarativeCompiler::buildObject(Object *obj, const BindingContext &ctxt) obj->metatype = tr.metaObject(); if (tr.component) - obj->url = tr.component->url(); + obj->url = tr.component->url; if (tr.type) obj->typeName = tr.type->qmlTypeName(); obj->className = tr.className; @@ -940,7 +940,7 @@ void QDeclarativeCompiler::genObject(QDeclarativeParser::Object *obj) // ### Surely the creation of this property cache could be more efficient QDeclarativePropertyCache *propertyCache = 0; if (tr.component) - propertyCache = QDeclarativeComponentPrivate::get(tr.component)->cc->rootPropertyCache->copy(); + propertyCache = tr.component->rootPropertyCache->copy(); else propertyCache = enginePrivate->cache(obj->metaObject()->superClass())->copy(); @@ -957,7 +957,7 @@ void QDeclarativeCompiler::genObject(QDeclarativeParser::Object *obj) output->bytecode << meta; } else if (obj == unitRoot) { if (tr.component) - output->rootPropertyCache = QDeclarativeComponentPrivate::get(tr.component)->cc->rootPropertyCache; + output->rootPropertyCache = tr.component->rootPropertyCache; else output->rootPropertyCache = enginePrivate->cache(obj->metaObject()); diff --git a/src/declarative/qml/qdeclarativecompiler_p.h b/src/declarative/qml/qdeclarativecompiler_p.h index 89eef09..43a0901 100644 --- a/src/declarative/qml/qdeclarativecompiler_p.h +++ b/src/declarative/qml/qdeclarativecompiler_p.h @@ -93,10 +93,11 @@ public: QByteArray className; QDeclarativeType *type; - QDeclarativeComponent *component; +// QDeclarativeComponent *component; + QDeclarativeCompiledData *component; QDeclarativeRefCount *ref; - QObject *createInstance(QDeclarativeContextData *, const QBitField &) const; + QObject *createInstance(QDeclarativeContextData *, const QBitField &, QList *) const; const QMetaObject *metaObject() const; }; QList types; diff --git a/src/declarative/qml/qdeclarativecomponent.cpp b/src/declarative/qml/qdeclarativecomponent.cpp index cfef9cf..0a2a6db 100644 --- a/src/declarative/qml/qdeclarativecomponent.cpp +++ b/src/declarative/qml/qdeclarativecomponent.cpp @@ -733,48 +733,45 @@ QDeclarativeComponentPrivate::beginCreate(QDeclarativeContextData *context, cons return 0; } - QDeclarativeEnginePrivate *ep = QDeclarativeEnginePrivate::get(engine); + return begin(context, creationContext, cc, start, count, &state, 0, bindings); +} + +QObject * QDeclarativeComponentPrivate::begin(QDeclarativeContextData *parentContext, + QDeclarativeContextData *componentCreationContext, + QDeclarativeCompiledData *component, int start, int count, + ConstructionState *state, QList *errors, + const QBitField &bindings) +{ + QDeclarativeEnginePrivate *enginePriv = QDeclarativeEnginePrivate::get(parentContext->engine); + bool isRoot = !enginePriv->inBeginCreate; + + Q_ASSERT(!isRoot || state); // Either this isn't a root component, or a state data must be provided + Q_ASSERT((state != 0) ^ (errors != 0)); // One of state or errors (but not both) must be provided - bool isRoot = !ep->inBeginCreate; if (isRoot) QDeclarativeDebugTrace::startRange(QDeclarativeDebugTrace::Creating); - QDeclarativeDebugTrace::rangeData(QDeclarativeDebugTrace::Creating, cc->url); QDeclarativeContextData *ctxt = new QDeclarativeContextData; ctxt->isInternal = true; - ctxt->url = cc->url; - ctxt->imports = cc->importCache; + ctxt->url = component->url; + ctxt->imports = component->importCache; // Nested global imports - if (creationContext && start != -1) - ctxt->importedScripts = creationContext->importedScripts; - - cc->importCache->addref(); - ctxt->setParent(context); + if (componentCreationContext && start != -1) + ctxt->importedScripts = componentCreationContext->importedScripts; - QObject *rv = begin(ctxt, ep, cc, start, count, &state, bindings); + component->importCache->addref(); + ctxt->setParent(parentContext); - if (ep->isDebugging && rv) { - if (!context->isInternal) - context->asQDeclarativeContextPrivate()->instances.append(rv); - QDeclarativeEngineDebugServer::instance()->objectCreated(engine, rv); - } - - return rv; -} - -QObject * QDeclarativeComponentPrivate::begin(QDeclarativeContextData *ctxt, QDeclarativeEnginePrivate *enginePriv, - QDeclarativeCompiledData *component, int start, int count, - ConstructionState *state, const QBitField &bindings) -{ - bool isRoot = !enginePriv->inBeginCreate; enginePriv->inBeginCreate = true; QDeclarativeVME vme; QObject *rv = vme.run(ctxt, component, start, count, bindings); - if (vme.isError()) - state->errors = vme.errors(); + if (vme.isError()) { + if(errors) *errors = vme.errors(); + else state->errors = vme.errors(); + } if (isRoot) { enginePriv->inBeginCreate = false; @@ -794,6 +791,12 @@ QObject * QDeclarativeComponentPrivate::begin(QDeclarativeContextData *ctxt, QDe enginePriv->inProgressCreations++; } + if (enginePriv->isDebugging && rv) { + if (!parentContext->isInternal) + parentContext->asQDeclarativeContextPrivate()->instances.append(rv); + QDeclarativeEngineDebugServer::instance()->objectCreated(parentContext->engine, rv); + } + return rv; } diff --git a/src/declarative/qml/qdeclarativecomponent_p.h b/src/declarative/qml/qdeclarativecomponent_p.h index a551cc8..7b30bad 100644 --- a/src/declarative/qml/qdeclarativecomponent_p.h +++ b/src/declarative/qml/qdeclarativecomponent_p.h @@ -109,9 +109,10 @@ public: }; ConstructionState state; - static QObject *begin(QDeclarativeContextData *ctxt, QDeclarativeEnginePrivate *enginePriv, - QDeclarativeCompiledData *component, int start, int count, - ConstructionState *state, const QBitField &bindings = QBitField()); + static QObject *begin(QDeclarativeContextData *parentContext, QDeclarativeContextData *componentCreationContext, + QDeclarativeCompiledData *component, int start, int count, + ConstructionState *state, QList *errors, + const QBitField &bindings = QBitField()); static void beginDeferred(QDeclarativeEnginePrivate *enginePriv, QObject *object, ConstructionState *state); static void complete(QDeclarativeEnginePrivate *enginePriv, ConstructionState *state); diff --git a/src/declarative/qml/qdeclarativetypeloader.cpp b/src/declarative/qml/qdeclarativetypeloader.cpp index c8e1a07..c015519 100644 --- a/src/declarative/qml/qdeclarativetypeloader.cpp +++ b/src/declarative/qml/qdeclarativetypeloader.cpp @@ -719,7 +719,7 @@ void QDeclarativeTypeLoader::clearCache() QDeclarativeTypeData::QDeclarativeTypeData(const QUrl &url, QDeclarativeTypeLoader::Options options, QDeclarativeTypeLoader *manager) : QDeclarativeDataBlob(url, QmlFile), m_options(options), m_typesResolved(false), - m_compiledData(0), m_component(0), m_typeLoader(manager) + m_compiledData(0), m_typeLoader(manager) { } @@ -768,23 +768,6 @@ QDeclarativeCompiledData *QDeclarativeTypeData::compiledData() const return m_compiledData; } -QDeclarativeComponent *QDeclarativeTypeData::component() const -{ - if (!m_component) { - - if (m_compiledData) { - m_component = new QDeclarativeComponent(typeLoader()->engine(), m_compiledData, -1, -1, 0); - } else { - m_component = new QDeclarativeComponent(typeLoader()->engine()); - QDeclarativeComponentPrivate::get(m_component)->url = finalUrl(); - QDeclarativeComponentPrivate::get(m_component)->state.errors = errors(); - } - - } - - return m_component; -} - void QDeclarativeTypeData::registerCallback(TypeDataCallback *callback) { Q_ASSERT(!m_callbacks.contains(callback)); diff --git a/src/declarative/qml/qdeclarativetypeloader_p.h b/src/declarative/qml/qdeclarativetypeloader_p.h index 7381f28..718537a 100644 --- a/src/declarative/qml/qdeclarativetypeloader_p.h +++ b/src/declarative/qml/qdeclarativetypeloader_p.h @@ -243,7 +243,6 @@ public: const QList &resolvedScripts() const; QDeclarativeCompiledData *compiledData() const; - QDeclarativeComponent *component() const; // Used by QDeclarativeComponent to get notifications struct TypeDataCallback { @@ -278,7 +277,6 @@ private: bool m_typesResolved:1; QDeclarativeCompiledData *m_compiledData; - mutable QDeclarativeComponent *m_component; QList m_callbacks; diff --git a/src/declarative/qml/qdeclarativevme.cpp b/src/declarative/qml/qdeclarativevme.cpp index 360186c..db90aff 100644 --- a/src/declarative/qml/qdeclarativevme.cpp +++ b/src/declarative/qml/qdeclarativevme.cpp @@ -185,12 +185,9 @@ QObject *QDeclarativeVME::run(QDeclarativeVMEStack &stack, bindings = bindings.united(bindingSkipList); QObject *o = - types.at(instr.create.type).createInstance(ctxt, bindings); + types.at(instr.create.type).createInstance(ctxt, bindings, &vmeErrors); if (!o) { - if(types.at(instr.create.type).component) - vmeErrors << types.at(instr.create.type).component->errors(); - VME_EXCEPTION(QCoreApplication::translate("QDeclarativeVME","Unable to create object of type %1").arg(QString::fromLatin1(types.at(instr.create.type).className))); } @@ -933,8 +930,9 @@ QList QDeclarativeVME::errors() const } QObject * -QDeclarativeCompiledData::TypeReference::createInstance(QDeclarativeContextData *ctxt, - const QBitField &bindings) const +QDeclarativeCompiledData::TypeReference::createInstance(QDeclarativeContextData *ctxt, + const QBitField &bindings, + QList *errors) const { if (type) { QObject *rv = 0; @@ -948,7 +946,7 @@ QDeclarativeCompiledData::TypeReference::createInstance(QDeclarativeContextData return rv; } else { Q_ASSERT(component); - return QDeclarativeComponentPrivate::get(component)->create(ctxt, bindings); + return QDeclarativeComponentPrivate::begin(ctxt, 0, component, -1, -1, 0, errors, bindings); } } -- cgit v0.12 From bccb1a82fd796ca31b8b4777c29344bafc6d71d6 Mon Sep 17 00:00:00 2001 From: Martin Jones Date: Thu, 14 Oct 2010 14:54:33 +1000 Subject: Fix consistency of setting currentIndex in ListView and GridView. The behaviour of setting the currentIndex out of bounds, was different depending upon whether it was set -ve, beyond the end, or before component completed. The behaviour is now consistent - setting the currentIndex out of bounds is allowed and will cause the currentItem and highlightItem to become null. Task-number: QTBUG-12571 Reviewed-by: Michael Brasser --- .../graphicsitems/qdeclarativegridview.cpp | 67 ++++++++++++++-------- .../graphicsitems/qdeclarativelistview.cpp | 35 +++++++---- .../data/gridview-noCurrent.qml | 52 +++++++++++++++++ .../tst_qdeclarativegridview.cpp | 43 ++++++++++++++ .../data/listview-noCurrent.qml | 50 ++++++++++++++++ .../tst_qdeclarativelistview.cpp | 44 ++++++++++++++ 6 files changed, 257 insertions(+), 34 deletions(-) create mode 100644 tests/auto/declarative/qdeclarativegridview/data/gridview-noCurrent.qml create mode 100644 tests/auto/declarative/qdeclarativelistview/data/listview-noCurrent.qml diff --git a/src/declarative/graphicsitems/qdeclarativegridview.cpp b/src/declarative/graphicsitems/qdeclarativegridview.cpp index 6ee6b0d..f53625f 100644 --- a/src/declarative/graphicsitems/qdeclarativegridview.cpp +++ b/src/declarative/graphicsitems/qdeclarativegridview.cpp @@ -112,7 +112,7 @@ public: , bufferMode(BufferBefore | BufferAfter), snapMode(QDeclarativeGridView::NoSnap) , ownModel(false), wrap(false), autoHighlight(true) , fixCurrentVisibility(false), lazyRelease(false), layoutScheduled(false) - , deferredRelease(false), haveHighlightRange(false) {} + , deferredRelease(false), haveHighlightRange(false), currentIndexSet(false) {} void init(); void clear(); @@ -392,6 +392,7 @@ public: bool layoutScheduled : 1; bool deferredRelease : 1; bool haveHighlightRange : 1; + bool currentIndexSet : 1; }; void QDeclarativeGridViewPrivate::init() @@ -730,6 +731,8 @@ void QDeclarativeGridViewPrivate::createHighlight() QDeclarative_setParent_noEvent(item, q->contentItem()); item->setParentItem(q->contentItem()); highlight = new FxGridItem(item, q); + if (currentItem) + highlight->setPosition(currentItem->colPos(), currentItem->rowPos()); highlightXAnimator = new QSmoothedAnimation(q); highlightXAnimator->target = QDeclarativeProperty(highlight->item, QLatin1String("x")); highlightXAnimator->userDuration = highlightMoveDuration; @@ -771,8 +774,11 @@ void QDeclarativeGridViewPrivate::updateCurrent(int modelIndex) currentItem->attached->setIsCurrentItem(false); releaseItem(currentItem); currentItem = 0; - currentIndex = -1; + currentIndex = modelIndex; + emit q->currentIndexChanged(); updateHighlight(); + } else if (currentIndex != modelIndex) { + currentIndex = modelIndex; emit q->currentIndexChanged(); } return; @@ -1236,7 +1242,7 @@ void QDeclarativeGridView::setModel(const QVariant &model) d->bufferMode = QDeclarativeGridViewPrivate::BufferBefore | QDeclarativeGridViewPrivate::BufferAfter; if (isComponentComplete()) { refill(); - if (d->currentIndex >= d->model->count() || d->currentIndex < 0) { + if ((d->currentIndex >= d->model->count() || d->currentIndex < 0) && !d->currentIndexSet) { setCurrentIndex(0); } else { d->moveReason = QDeclarativeGridViewPrivate::SetIndex; @@ -1344,10 +1350,13 @@ void QDeclarativeGridView::setCurrentIndex(int index) Q_D(QDeclarativeGridView); if (d->requestedIndex >= 0) // currently creating item return; - if (isComponentComplete() && d->isValid() && index != d->currentIndex && index < d->model->count() && index >= 0) { + d->currentIndexSet = true; + if (index == d->currentIndex) + return; + if (isComponentComplete() && d->isValid()) { d->moveReason = QDeclarativeGridViewPrivate::SetIndex; d->updateCurrent(index); - } else if (index != d->currentIndex) { + } else { d->currentIndex = index; emit currentIndexChanged(); } @@ -1983,22 +1992,25 @@ void QDeclarativeGridView::keyPressEvent(QKeyEvent *event) Move the currentIndex up one item in the view. The current index will wrap if keyNavigationWraps is true and it - is currently at the end. + is currently at the end. This method has no effect if the \l count is zero. \bold Note: methods should only be called after the Component has completed. */ void QDeclarativeGridView::moveCurrentIndexUp() { Q_D(QDeclarativeGridView); + const int count = d->model ? d->model->count() : 0; + if (!count) + return; if (d->flow == QDeclarativeGridView::LeftToRight) { if (currentIndex() >= d->columns || d->wrap) { int index = currentIndex() - d->columns; - setCurrentIndex(index >= 0 ? index : d->model->count()-1); + setCurrentIndex((index >= 0 && index < count) ? index : count-1); } } else { if (currentIndex() > 0 || d->wrap) { int index = currentIndex() - 1; - setCurrentIndex(index >= 0 ? index : d->model->count()-1); + setCurrentIndex((index >= 0 && index < count) ? index : count-1); } } } @@ -2008,22 +2020,25 @@ void QDeclarativeGridView::moveCurrentIndexUp() Move the currentIndex down one item in the view. The current index will wrap if keyNavigationWraps is true and it - is currently at the end. + is currently at the end. This method has no effect if the \l count is zero. \bold Note: methods should only be called after the Component has completed. */ void QDeclarativeGridView::moveCurrentIndexDown() { Q_D(QDeclarativeGridView); + const int count = d->model ? d->model->count() : 0; + if (!count) + return; if (d->flow == QDeclarativeGridView::LeftToRight) { - if (currentIndex() < d->model->count() - d->columns || d->wrap) { + if (currentIndex() < count - d->columns || d->wrap) { int index = currentIndex()+d->columns; - setCurrentIndex(index < d->model->count() ? index : 0); + setCurrentIndex((index >= 0 && index < count) ? index : 0); } } else { - if (currentIndex() < d->model->count() - 1 || d->wrap) { + if (currentIndex() < count - 1 || d->wrap) { int index = currentIndex() + 1; - setCurrentIndex(index < d->model->count() ? index : 0); + setCurrentIndex((index >= 0 && index < count) ? index : 0); } } } @@ -2033,22 +2048,25 @@ void QDeclarativeGridView::moveCurrentIndexDown() Move the currentIndex left one item in the view. The current index will wrap if keyNavigationWraps is true and it - is currently at the end. + is currently at the end. This method has no effect if the \l count is zero. \bold Note: methods should only be called after the Component has completed. */ void QDeclarativeGridView::moveCurrentIndexLeft() { Q_D(QDeclarativeGridView); + const int count = d->model ? d->model->count() : 0; + if (!count) + return; if (d->flow == QDeclarativeGridView::LeftToRight) { if (currentIndex() > 0 || d->wrap) { int index = currentIndex() - 1; - setCurrentIndex(index >= 0 ? index : d->model->count()-1); + setCurrentIndex((index >= 0 && index < count) ? index : count-1); } } else { if (currentIndex() >= d->columns || d->wrap) { int index = currentIndex() - d->columns; - setCurrentIndex(index >= 0 ? index : d->model->count()-1); + setCurrentIndex((index >= 0 && index < count) ? index : count-1); } } } @@ -2058,22 +2076,25 @@ void QDeclarativeGridView::moveCurrentIndexLeft() Move the currentIndex right one item in the view. The current index will wrap if keyNavigationWraps is true and it - is currently at the end. + is currently at the end. This method has no effect if the \l count is zero. \bold Note: methods should only be called after the Component has completed. */ void QDeclarativeGridView::moveCurrentIndexRight() { Q_D(QDeclarativeGridView); + const int count = d->model ? d->model->count() : 0; + if (!count) + return; if (d->flow == QDeclarativeGridView::LeftToRight) { - if (currentIndex() < d->model->count() - 1 || d->wrap) { + if (currentIndex() < count - 1 || d->wrap) { int index = currentIndex() + 1; - setCurrentIndex(index < d->model->count() ? index : 0); + setCurrentIndex((index >= 0 && index < count) ? index : 0); } } else { - if (currentIndex() < d->model->count() - d->columns || d->wrap) { + if (currentIndex() < count - d->columns || d->wrap) { int index = currentIndex()+d->columns; - setCurrentIndex(index < d->model->count() ? index : 0); + setCurrentIndex((index >= 0 && index < count) ? index : 0); } } } @@ -2201,7 +2222,7 @@ void QDeclarativeGridView::componentComplete() if (d->isValid()) { refill(); d->moveReason = QDeclarativeGridViewPrivate::SetIndex; - if (d->currentIndex < 0) + if (d->currentIndex < 0 && !d->currentIndexSet) d->updateCurrent(0); else d->updateCurrent(d->currentIndex); @@ -2282,7 +2303,7 @@ void QDeclarativeGridView::itemsInserted(int modelIndex, int count) if (d->currentItem) d->currentItem->index = d->currentIndex; emit currentIndexChanged(); - } else if (d->currentIndex < 0) { + } else if (d->currentIndex < 0 && !d->currentIndexSet) { d->updateCurrent(0); } d->itemCount += count; diff --git a/src/declarative/graphicsitems/qdeclarativelistview.cpp b/src/declarative/graphicsitems/qdeclarativelistview.cpp index 6fd3b71..7dd5c75 100644 --- a/src/declarative/graphicsitems/qdeclarativelistview.cpp +++ b/src/declarative/graphicsitems/qdeclarativelistview.cpp @@ -182,7 +182,8 @@ public: , bufferMode(BufferBefore | BufferAfter) , ownModel(false), wrap(false), autoHighlight(true), haveHighlightRange(false) , correctFlick(false), inFlickCorrection(false), lazyRelease(false) - , deferredRelease(false), layoutScheduled(false), minExtentDirty(true), maxExtentDirty(true) + , deferredRelease(false), layoutScheduled(false), currentIndexSet(false) + , minExtentDirty(true), maxExtentDirty(true) {} void init(); @@ -544,6 +545,7 @@ public: bool lazyRelease : 1; bool deferredRelease : 1; bool layoutScheduled : 1; + bool currentIndexSet : 1; mutable bool minExtentDirty : 1; mutable bool maxExtentDirty : 1; }; @@ -881,6 +883,7 @@ void QDeclarativeListViewPrivate::createHighlight() } else { highlight->item->setWidth(currentItem->item->width()); } + highlight->setPosition(currentItem->itemPosition()); } QDeclarativeItemPrivate *itemPrivate = static_cast(QGraphicsItemPrivate::get(item)); itemPrivate->addItemChangeListener(this, QDeclarativeItemPrivate::Geometry); @@ -1045,8 +1048,11 @@ void QDeclarativeListViewPrivate::updateCurrent(int modelIndex) currentItem->attached->setIsCurrentItem(false); releaseItem(currentItem); currentItem = 0; - currentIndex = -1; + currentIndex = modelIndex; + emit q->currentIndexChanged(); updateHighlight(); + } else if (currentIndex != modelIndex) { + currentIndex = modelIndex; emit q->currentIndexChanged(); } return; @@ -1598,7 +1604,7 @@ void QDeclarativeListView::setModel(const QVariant &model) if (isComponentComplete()) { updateSections(); refill(); - if (d->currentIndex >= d->model->count() || d->currentIndex < 0) { + if ((d->currentIndex >= d->model->count() || d->currentIndex < 0) && !d->currentIndexSet) { setCurrentIndex(0); } else { d->moveReason = QDeclarativeListViewPrivate::SetIndex; @@ -1708,10 +1714,13 @@ void QDeclarativeListView::setCurrentIndex(int index) Q_D(QDeclarativeListView); if (d->requestedIndex >= 0) // currently creating item return; - if (isComponentComplete() && d->isValid() && index != d->currentIndex && index < d->model->count() && index >= 0) { + d->currentIndexSet = true; + if (index == d->currentIndex) + return; + if (isComponentComplete() && d->isValid()) { d->moveReason = QDeclarativeListViewPrivate::SetIndex; d->updateCurrent(index); - } else if (index != d->currentIndex) { + } else { d->currentIndex = index; emit currentIndexChanged(); } @@ -2523,16 +2532,18 @@ void QDeclarativeListView::geometryChanged(const QRectF &newGeometry, Increments the current index. The current index will wrap if keyNavigationWraps is true and it is currently at the end. + This method has no effect if the \l count is zero. \bold Note: methods should only be called after the Component has completed. */ void QDeclarativeListView::incrementCurrentIndex() { Q_D(QDeclarativeListView); - if (currentIndex() < d->model->count() - 1 || d->wrap) { + int count = d->model ? d->model->count() : 0; + if (count && (currentIndex() < count - 1 || d->wrap)) { d->moveReason = QDeclarativeListViewPrivate::SetIndex; int index = currentIndex()+1; - d->updateCurrent(index < d->model->count() ? index : 0); + d->updateCurrent((index >= 0 && index < count) ? index : 0); } } @@ -2541,16 +2552,18 @@ void QDeclarativeListView::incrementCurrentIndex() Decrements the current index. The current index will wrap if keyNavigationWraps is true and it is currently at the beginning. + This method has no effect if the \l count is zero. \bold Note: methods should only be called after the Component has completed. */ void QDeclarativeListView::decrementCurrentIndex() { Q_D(QDeclarativeListView); - if (currentIndex() > 0 || d->wrap) { + int count = d->model ? d->model->count() : 0; + if (count && (currentIndex() > 0 || d->wrap)) { d->moveReason = QDeclarativeListViewPrivate::SetIndex; int index = currentIndex()-1; - d->updateCurrent(index >= 0 ? index : d->model->count()-1); + d->updateCurrent((index >= 0 && index < count) ? index : count-1); } } @@ -2684,7 +2697,7 @@ void QDeclarativeListView::componentComplete() if (d->isValid()) { refill(); d->moveReason = QDeclarativeListViewPrivate::SetIndex; - if (d->currentIndex < 0) + if (d->currentIndex < 0 && !d->currentIndexSet) d->updateCurrent(0); else d->updateCurrent(d->currentIndex); @@ -2790,7 +2803,7 @@ void QDeclarativeListView::itemsInserted(int modelIndex, int count) if (d->currentItem) d->currentItem->index = d->currentIndex; emit currentIndexChanged(); - } else if (d->currentIndex < 0) { + } else if (d->currentIndex < 0 && !d->currentIndexSet) { d->updateCurrent(0); } d->itemCount += count; diff --git a/tests/auto/declarative/qdeclarativegridview/data/gridview-noCurrent.qml b/tests/auto/declarative/qdeclarativegridview/data/gridview-noCurrent.qml new file mode 100644 index 0000000..1189649 --- /dev/null +++ b/tests/auto/declarative/qdeclarativegridview/data/gridview-noCurrent.qml @@ -0,0 +1,52 @@ +import QtQuick 1.0 + +Rectangle { + property int current: grid.currentIndex + width: 240 + height: 320 + color: "#ffffff" + resources: [ + Component { + id: myDelegate + Rectangle { + id: wrapper + objectName: "wrapper" + width: 80 + height: 60 + border.color: "blue" + Text { + text: index + } + Text { + x: 40 + text: wrapper.x + ", " + wrapper.y + } + Text { + y: 20 + id: textName + objectName: "textName" + text: name + } + Text { + y: 40 + id: textNumber + objectName: "textNumber" + text: number + } + color: GridView.isCurrentItem ? "lightsteelblue" : "white" + } + } + ] + GridView { + id: grid + objectName: "grid" + focus: true + width: 240 + height: 320 + currentIndex: -1 + cellWidth: 80 + cellHeight: 60 + delegate: myDelegate + model: testModel + } +} diff --git a/tests/auto/declarative/qdeclarativegridview/tst_qdeclarativegridview.cpp b/tests/auto/declarative/qdeclarativegridview/tst_qdeclarativegridview.cpp index f7acd87..327bba2 100644 --- a/tests/auto/declarative/qdeclarativegridview/tst_qdeclarativegridview.cpp +++ b/tests/auto/declarative/qdeclarativegridview/tst_qdeclarativegridview.cpp @@ -71,6 +71,7 @@ private slots: void moved(); void changeFlow(); void currentIndex(); + void noCurrentIndex(); void defaultValues(); void properties(); void propertyChanges(); @@ -696,9 +697,51 @@ void tst_QDeclarativeGridView::currentIndex() model.insertItem(0, "Foo", "1111"); QTRY_COMPARE(canvas->rootObject()->property("current").toInt(), 29); + // check removing highlight by setting currentIndex to -1; + gridview->setCurrentIndex(-1); + + QCOMPARE(gridview->currentIndex(), -1); + QVERIFY(!gridview->highlightItem()); + QVERIFY(!gridview->currentItem()); + delete canvas; } +void tst_QDeclarativeGridView::noCurrentIndex() +{ + TestModel model; + for (int i = 0; i < 60; i++) + model.addItem("Item" + QString::number(i), QString::number(i)); + + QDeclarativeView *canvas = new QDeclarativeView(0); + canvas->setFixedSize(240,320); + + QDeclarativeContext *ctxt = canvas->rootContext(); + ctxt->setContextProperty("testModel", &model); + + QString filename(SRCDIR "/data/gridview-noCurrent.qml"); + canvas->setSource(QUrl::fromLocalFile(filename)); + + qApp->processEvents(); + + QDeclarativeGridView *gridview = findItem(canvas->rootObject(), "grid"); + QVERIFY(gridview != 0); + + QDeclarativeItem *contentItem = gridview->contentItem(); + QVERIFY(contentItem != 0); + + // current index should be -1 + QCOMPARE(gridview->currentIndex(), -1); + QVERIFY(!gridview->currentItem()); + QVERIFY(!gridview->highlightItem()); + QCOMPARE(gridview->contentY(), 0.0); + + gridview->setCurrentIndex(5); + QCOMPARE(gridview->currentIndex(), 5); + QVERIFY(gridview->currentItem()); + QVERIFY(gridview->highlightItem()); +} + void tst_QDeclarativeGridView::changeFlow() { QDeclarativeView *canvas = createView(); diff --git a/tests/auto/declarative/qdeclarativelistview/data/listview-noCurrent.qml b/tests/auto/declarative/qdeclarativelistview/data/listview-noCurrent.qml new file mode 100644 index 0000000..1997010 --- /dev/null +++ b/tests/auto/declarative/qdeclarativelistview/data/listview-noCurrent.qml @@ -0,0 +1,50 @@ +import QtQuick 1.0 + +Rectangle { + property int current: list.currentIndex + width: 240 + height: 320 + color: "#ffffff" + resources: [ + Component { + id: myDelegate + Rectangle { + id: wrapper + objectName: "wrapper" + height: 20 + width: 240 + Text { + text: index + } + Text { + x: 30 + id: textName + objectName: "textName" + text: name + } + Text { + x: 120 + id: textNumber + objectName: "textNumber" + text: number + } + Text { + x: 200 + text: wrapper.y + } + color: ListView.isCurrentItem ? "lightsteelblue" : "white" + } + } + ] + ListView { + id: list + objectName: "list" + focus: true + currentIndex: -1 + width: 240 + height: 320 + delegate: myDelegate + highlightMoveSpeed: 1000 + model: testModel + } +} diff --git a/tests/auto/declarative/qdeclarativelistview/tst_qdeclarativelistview.cpp b/tests/auto/declarative/qdeclarativelistview/tst_qdeclarativelistview.cpp index 2649c0d..080631c 100644 --- a/tests/auto/declarative/qdeclarativelistview/tst_qdeclarativelistview.cpp +++ b/tests/auto/declarative/qdeclarativelistview/tst_qdeclarativelistview.cpp @@ -85,6 +85,7 @@ private slots: void itemList(); void currentIndex(); + void noCurrentIndex(); void enforceRange(); void spacing(); void sections(); @@ -1087,9 +1088,52 @@ void tst_QDeclarativeListView::currentIndex() model.insertItem(0, "Foo", "1111"); QTRY_COMPARE(canvas->rootObject()->property("current").toInt(), 29); + // check removing highlight by setting currentIndex to -1; + listview->setCurrentIndex(-1); + + QCOMPARE(listview->currentIndex(), -1); + QVERIFY(!listview->highlightItem()); + QVERIFY(!listview->currentItem()); + delete canvas; } +void tst_QDeclarativeListView::noCurrentIndex() +{ + TestModel model; + for (int i = 0; i < 30; i++) + model.addItem("Item" + QString::number(i), QString::number(i)); + + QDeclarativeView *canvas = new QDeclarativeView(0); + canvas->setFixedSize(240,320); + + QDeclarativeContext *ctxt = canvas->rootContext(); + ctxt->setContextProperty("testModel", &model); + + QString filename(SRCDIR "/data/listview-noCurrent.qml"); + canvas->setSource(QUrl::fromLocalFile(filename)); + + qApp->processEvents(); + + QDeclarativeListView *listview = findItem(canvas->rootObject(), "list"); + QTRY_VERIFY(listview != 0); + + QDeclarativeItem *contentItem = listview->contentItem(); + QTRY_VERIFY(contentItem != 0); + + // current index should be -1 at startup + // and we should not have a currentItem or highlightItem + QCOMPARE(listview->currentIndex(), -1); + QCOMPARE(listview->contentY(), 0.0); + QVERIFY(!listview->highlightItem()); + QVERIFY(!listview->currentItem()); + + listview->setCurrentIndex(2); + QCOMPARE(listview->currentIndex(), 2); + QVERIFY(listview->highlightItem()); + QVERIFY(listview->currentItem()); +} + void tst_QDeclarativeListView::itemList() { QDeclarativeView *canvas = createView(); -- cgit v0.12 From c407d79f79c67f2f2bb84efc93061fd57fe4cf54 Mon Sep 17 00:00:00 2001 From: Aaron Kennedy Date: Thu, 14 Oct 2010 18:22:57 +1000 Subject: Correctly splice properties from derived metaobjects together Task-number: QTBUG-14449 --- src/declarative/qml/qdeclarativepropertycache.cpp | 77 ++++++---------------- src/declarative/qml/qdeclarativepropertycache_p.h | 9 ++- .../data/propertySplicing.qml | 10 +++ .../qdeclarativeecmascript/testtypes.cpp | 1 + .../declarative/qdeclarativeecmascript/testtypes.h | 9 +++ .../tst_qdeclarativeecmascript.cpp | 13 ++++ 6 files changed, 61 insertions(+), 58 deletions(-) create mode 100644 tests/auto/declarative/qdeclarativeecmascript/data/propertySplicing.qml diff --git a/src/declarative/qml/qdeclarativepropertycache.cpp b/src/declarative/qml/qdeclarativepropertycache.cpp index 9e1ceb8..82fa98f 100644 --- a/src/declarative/qml/qdeclarativepropertycache.cpp +++ b/src/declarative/qml/qdeclarativepropertycache.cpp @@ -252,7 +252,8 @@ void QDeclarativePropertyCache::append(QDeclarativeEngine *engine, const QMetaOb } int methodCount = metaObject->methodCount(); - int methodOffset = qMax(2, metaObject->methodOffset()); // 2 to block the destroyed signal + // 3 to block the destroyed signal and the deleteLater() slot + int methodOffset = qMax(3, metaObject->methodOffset()); methodIndexCache.resize(methodCount); for (int ii = methodOffset; ii < methodCount; ++ii) { @@ -268,17 +269,17 @@ void QDeclarativePropertyCache::append(QDeclarativeEngine *engine, const QMetaOb RData *data = new RData; data->identifier = enginePriv->objectClass->createPersistentIdentifier(methodName); - if (stringCache.contains(methodName)) { - stringCache[methodName]->release(); - identifierCache[data->identifier.identifier]->release(); - } - data->load(m); if (m.methodType() == QMetaMethod::Slot || m.methodType() == QMetaMethod::Method) data->flags |= methodFlags; else if (m.methodType() == QMetaMethod::Signal) data->flags |= signalFlags; + if (stringCache.contains(methodName)) { + stringCache[methodName]->release(); + identifierCache[data->identifier.identifier]->release(); + } + methodIndexCache[ii] = data; stringCache.insert(methodName, data); @@ -287,6 +288,16 @@ void QDeclarativePropertyCache::append(QDeclarativeEngine *engine, const QMetaOb } } +void QDeclarativePropertyCache::updateRecur(QDeclarativeEngine *engine, const QMetaObject *metaObject) +{ + if (!metaObject) + return; + + updateRecur(engine, metaObject->superClass()); + + append(engine, metaObject); +} + void QDeclarativePropertyCache::update(QDeclarativeEngine *engine, const QMetaObject *metaObject) { Q_ASSERT(engine); @@ -295,57 +306,11 @@ void QDeclarativePropertyCache::update(QDeclarativeEngine *engine, const QMetaOb clear(); - // ### The properties/methods should probably be spliced on a per-metaobject basis - int propCount = metaObject->propertyCount(); - - indexCache.resize(propCount); - for (int ii = propCount - 1; ii >= 0; --ii) { - QMetaProperty p = metaObject->property(ii); - if (!p.isScriptable()) { - indexCache[ii] = 0; - continue; - } - QString propName = QString::fromUtf8(p.name()); + // Optimization to prevent unnecessary reallocation of lists + indexCache.reserve(metaObject->propertyCount()); + methodIndexCache.reserve(metaObject->methodCount()); - RData *data = new RData; - data->identifier = enginePriv->objectClass->createPersistentIdentifier(propName); - - data->load(p, engine); - - indexCache[ii] = data; - - if (stringCache.contains(propName)) - continue; - - stringCache.insert(propName, data); - identifierCache.insert(data->identifier.identifier, data); - data->addref(); - data->addref(); - } - - int methodCount = metaObject->methodCount(); - for (int ii = methodCount - 1; ii >= 3; --ii) { // >=3 to block the destroyed signal and deleteLater() slot - QMetaMethod m = metaObject->method(ii); - if (m.access() == QMetaMethod::Private) - continue; - QString methodName = QString::fromUtf8(m.signature()); - - int parenIdx = methodName.indexOf(QLatin1Char('(')); - Q_ASSERT(parenIdx != -1); - methodName = methodName.left(parenIdx); - - if (stringCache.contains(methodName)) - continue; - - RData *data = new RData; - data->identifier = enginePriv->objectClass->createPersistentIdentifier(methodName); - - data->load(m); - - stringCache.insert(methodName, data); - identifierCache.insert(data->identifier.identifier, data); - data->addref(); - } + updateRecur(engine,metaObject); } QDeclarativePropertyCache::Data * diff --git a/src/declarative/qml/qdeclarativepropertycache_p.h b/src/declarative/qml/qdeclarativepropertycache_p.h index 79b126d..922010d 100644 --- a/src/declarative/qml/qdeclarativepropertycache_p.h +++ b/src/declarative/qml/qdeclarativepropertycache_p.h @@ -96,7 +96,7 @@ public: IsVMEFunction = 0x00000400, HasArguments = 0x00000800, IsSignal = 0x00001000, - IsVMESignal = 0x00002000, + IsVMESignal = 0x00002000 }; Q_DECLARE_FLAGS(Flags, Flag) @@ -105,7 +105,10 @@ public: Flags flags; int propType; int coreIndex; - int notifyIndex; + union { + int notifyIndex; // When !IsFunction + int relatedIndex; // When IsFunction + }; static Flags flagsForProperty(const QMetaProperty &, QDeclarativeEngine *engine = 0); void load(const QMetaProperty &, QDeclarativeEngine *engine = 0); @@ -152,6 +155,8 @@ private: typedef QHash StringCache; typedef QHash IdentifierCache; + void updateRecur(QDeclarativeEngine *, const QMetaObject *); + QDeclarativeEngine *engine; IndexCache indexCache; IndexCache methodIndexCache; diff --git a/tests/auto/declarative/qdeclarativeecmascript/data/propertySplicing.qml b/tests/auto/declarative/qdeclarativeecmascript/data/propertySplicing.qml new file mode 100644 index 0000000..7deb84a --- /dev/null +++ b/tests/auto/declarative/qdeclarativeecmascript/data/propertySplicing.qml @@ -0,0 +1,10 @@ +import Qt.test 1.0 +import QtQuick 1.0 + +MyDerivedObject { + property bool test: false + + Component.onCompleted: { + test = intProperty() + } +} diff --git a/tests/auto/declarative/qdeclarativeecmascript/testtypes.cpp b/tests/auto/declarative/qdeclarativeecmascript/testtypes.cpp index 810a0f7..94135f9 100644 --- a/tests/auto/declarative/qdeclarativeecmascript/testtypes.cpp +++ b/tests/auto/declarative/qdeclarativeecmascript/testtypes.cpp @@ -109,6 +109,7 @@ void registerTypes() qmlRegisterExtendedType("Qt.test", 1,0, "MyBaseExtendedObject"); qmlRegisterExtendedType("Qt.test", 1,0, "MyExtendedObject"); qmlRegisterType("Qt.test", 1,0, "MyTypeObject"); + qmlRegisterType("Qt.test", 1,0, "MyDerivedObject"); qmlRegisterType("Qt.test", 1,0, "NumberAssignment"); qmlRegisterExtendedType("Qt.test", 1,0, "DefaultPropertyExtendedObject"); qmlRegisterType("Qt.test", 1,0, "OverrideDefaultPropertyObject"); diff --git a/tests/auto/declarative/qdeclarativeecmascript/testtypes.h b/tests/auto/declarative/qdeclarativeecmascript/testtypes.h index 220318d..182c4fa 100644 --- a/tests/auto/declarative/qdeclarativeecmascript/testtypes.h +++ b/tests/auto/declarative/qdeclarativeecmascript/testtypes.h @@ -562,6 +562,15 @@ signals: }; Q_DECLARE_OPERATORS_FOR_FLAGS(MyTypeObject::MyFlags) +class MyDerivedObject : public MyTypeObject +{ + Q_OBJECT +public: + Q_INVOKABLE bool intProperty() const { + return true; + } +}; + Q_DECLARE_METATYPE(QScriptValue); class MyInvokableObject : public QObject { diff --git a/tests/auto/declarative/qdeclarativeecmascript/tst_qdeclarativeecmascript.cpp b/tests/auto/declarative/qdeclarativeecmascript/tst_qdeclarativeecmascript.cpp index 02832f3..66dc160 100644 --- a/tests/auto/declarative/qdeclarativeecmascript/tst_qdeclarativeecmascript.cpp +++ b/tests/auto/declarative/qdeclarativeecmascript/tst_qdeclarativeecmascript.cpp @@ -139,6 +139,7 @@ private slots: void strictlyEquals(); void compiled(); void numberAssignment(); + void propertySplicing(); void bug1(); void bug2(); @@ -2175,6 +2176,18 @@ void tst_qdeclarativeecmascript::numberAssignment() delete object; } +void tst_qdeclarativeecmascript::propertySplicing() +{ + QDeclarativeComponent component(&engine, TEST_FILE("propertySplicing.qml")); + + QObject *object = component.create(); + QVERIFY(object != 0); + + QCOMPARE(object->property("test").toBool(), true); + + delete object; +} + // Test that assigning a null object works // Regressed with: df1788b4dbbb2826ae63f26bdf166342595343f4 void tst_qdeclarativeecmascript::nullObjectBinding() -- cgit v0.12