From: Bea Lam Date: Mon, 5 Mar 2012 23:39:24 +0000 (+1000) Subject: Fix setting of target lists when target Transition is not set X-Git-Tag: qt-v5.0.0-alpha1~226 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=22a53bcd69d39a5ea128d53231e9e51455a98cc4;p=profile%2Fivi%2Fqtdeclarative.git Fix setting of target lists when target Transition is not set Target items are now set from QQuickViewItem::prepareTransition() instead of QQuickItemView and QQuickPositioner to ensure they are for a displaced transition even if there is no matching target transition. Task-number: QTBUG-24535 Change-Id: I0a6c7e3c6198786527014d421b96fc562c6186dc Reviewed-by: Martin Jones --- diff --git a/src/quick/items/qquickitemview.cpp b/src/quick/items/qquickitemview.cpp index 516cf0c..e368d1a 100644 --- a/src/quick/items/qquickitemview.cpp +++ b/src/quick/items/qquickitemview.cpp @@ -1664,7 +1664,7 @@ void QQuickItemViewPrivate::layout() for (QList::Iterator it = releasePendingTransition.begin(); it != releasePendingTransition.end(); ) { FxViewItem *item = *it; - if (item->transitionRunning() || prepareNonVisibleItemTransition(item, viewBounds)) { + if (prepareNonVisibleItemTransition(item, viewBounds)) { ++it; } else { releaseItem(item); @@ -1676,7 +1676,9 @@ void QQuickItemViewPrivate::layout() visibleItems[i]->startTransition(transitioner); for (int i=0; istartTransition(transitioner); + transitioner->setPopulateTransitionEnabled(false); + transitioner->resetTargetLists(); } runDelayedRemoveTransition = false; @@ -1955,36 +1957,10 @@ void QQuickItemViewPrivate::prepareVisibleItemTransitions() if (!transitioner) return; - transitioner->addTransitionIndexes.clear(); - transitioner->addTransitionTargets.clear(); - transitioner->moveTransitionIndexes.clear(); - transitioner->moveTransitionTargets.clear(); - + // must call for every visible item to init or discard transitions QRectF viewBounds(0, position(), q->width(), q->height()); - for (int i=0; iprepareTransition(viewBounds)) - continue; - if (visibleItems[i]->isTransitionTarget) { - switch (visibleItems[i]->nextTransitionType) { - case QQuickItemViewTransitioner::NoTransition: - break; - case QQuickItemViewTransitioner::PopulateTransition: - case QQuickItemViewTransitioner::AddTransition: - transitioner->addTransitionIndexes.append(visibleItems[i]->index); - transitioner->addTransitionTargets.append(visibleItems[i]->item); - break; - case QQuickItemViewTransitioner::MoveTransition: - transitioner->moveTransitionIndexes.append(visibleItems[i]->index); - transitioner->moveTransitionTargets.append(visibleItems[i]->item); - break; - case QQuickItemViewTransitioner::RemoveTransition: - // removed targets won't be in visibleItems, handle these - // in prepareNonVisibleItemTransition() - break; - } - } - } + for (int i=0; iprepareTransition(transitioner, viewBounds); } void QQuickItemViewPrivate::prepareRemoveTransitions(QHash *removedItems) @@ -1992,10 +1968,8 @@ void QQuickItemViewPrivate::prepareRemoveTransitions(QHashremoveTransitionIndexes.clear(); - transitioner->removeTransitionTargets.clear(); - - if (transitioner->canTransition(QQuickItemViewTransitioner::RemoveTransition, true)) { + if (transitioner->canTransition(QQuickItemViewTransitioner::RemoveTransition, true) + || transitioner->canTransition(QQuickItemViewTransitioner::RemoveTransition, false)) { for (QHash::Iterator it = removedItems->begin(); it != removedItems->end(); ) { bool isRemove = it.key().moveId < 0; @@ -2024,21 +1998,12 @@ bool QQuickItemViewPrivate::prepareNonVisibleItemTransition(FxViewItem *item, co if (item->nextTransitionType == QQuickItemViewTransitioner::MoveTransition) repositionItemAt(item, item->index, 0); - if (!item->prepareTransition(viewBounds)) - return false; - if (item->isTransitionTarget) { - if (item->nextTransitionType == QQuickItemViewTransitioner::MoveTransition) { - transitioner->moveTransitionIndexes.append(item->index); - transitioner->moveTransitionTargets.append(item->item); - } else if (item->nextTransitionType == QQuickItemViewTransitioner::RemoveTransition) { - transitioner->removeTransitionIndexes.append(item->index); - transitioner->removeTransitionTargets.append(item->item); - } + if (item->prepareTransition(transitioner, viewBounds)) { + item->releaseAfterTransition = true; + return true; } - - item->releaseAfterTransition = true; - return true; + return false; } void QQuickItemViewPrivate::viewItemTransitionFinished(QQuickViewItem *i) diff --git a/src/quick/items/qquickitemviewtransition.cpp b/src/quick/items/qquickitemviewtransition.cpp index ac9375b..5669ef9 100644 --- a/src/quick/items/qquickitemviewtransition.cpp +++ b/src/quick/items/qquickitemviewtransition.cpp @@ -165,28 +165,30 @@ QQuickItemViewTransitioner::~QQuickItemViewTransitioner() bool QQuickItemViewTransitioner::canTransition(QQuickItemViewTransitioner::TransitionType type, bool asTarget) const { if (!asTarget - && type != QQuickItemViewTransitioner::NoTransition && type != QQuickItemViewTransitioner::PopulateTransition + && type != NoTransition && type != PopulateTransition && displacedTransition && displacedTransition->enabled()) { return true; } switch (type) { - case QQuickItemViewTransitioner::NoTransition: + case NoTransition: break; - case QQuickItemViewTransitioner::PopulateTransition: + case PopulateTransition: return usePopulateTransition && populateTransition && populateTransition->enabled(); - case QQuickItemViewTransitioner::AddTransition: + case AddTransition: + if (usePopulateTransition) + return false; if (asTarget) return addTransition && addTransition->enabled(); else return addDisplacedTransition && addDisplacedTransition->enabled(); - case QQuickItemViewTransitioner::MoveTransition: + case MoveTransition: if (asTarget) return moveTransition && moveTransition->enabled(); else return moveDisplacedTransition && moveDisplacedTransition->enabled(); - case QQuickItemViewTransitioner::RemoveTransition: + case RemoveTransition: if (asTarget) return removeTransition && removeTransition->enabled(); else @@ -197,27 +199,42 @@ bool QQuickItemViewTransitioner::canTransition(QQuickItemViewTransitioner::Trans void QQuickItemViewTransitioner::transitionNextReposition(QQuickViewItem *item, QQuickItemViewTransitioner::TransitionType type, bool isTarget) { - bool matchedTransition = false; - if (type == QQuickItemViewTransitioner::AddTransition) { - // don't run add transitions for added items while populating - if (usePopulateTransition) - matchedTransition = false; - else - matchedTransition = canTransition(type, isTarget); - } else { - matchedTransition = canTransition(type, isTarget); - } + item->setNextTransition(type, isTarget); +} - if (matchedTransition) { - item->setNextTransition(type, isTarget); - } else { - // the requested transition type is not valid, but the item is scheduled/in another - // transition, so cancel it to allow the item to move directly to the correct pos - if (item->transitionScheduledOrRunning()) - item->stopTransition(); +void QQuickItemViewTransitioner::addToTargetLists(QQuickItemViewTransitioner::TransitionType type, QQuickViewItem *item, int index) +{ + switch (type) { + case NoTransition: + break; + case PopulateTransition: + case AddTransition: + addTransitionIndexes << index; + addTransitionTargets << item->item; + break; + case MoveTransition: + moveTransitionIndexes << index; + moveTransitionTargets << item->item; + break; + case RemoveTransition: + removeTransitionIndexes << index; + removeTransitionTargets << item->item; + break; } } +void QQuickItemViewTransitioner::resetTargetLists() +{ + addTransitionIndexes.clear(); + addTransitionTargets.clear(); + + removeTransitionIndexes.clear(); + removeTransitionTargets.clear(); + + moveTransitionIndexes.clear(); + moveTransitionTargets.clear(); +} + QQuickTransition *QQuickItemViewTransitioner::transitionObject(QQuickItemViewTransitioner::TransitionType type, bool asTarget) { if (type == QQuickItemViewTransitioner::NoTransition) @@ -254,14 +271,14 @@ QQuickTransition *QQuickItemViewTransitioner::transitionObject(QQuickItemViewTra const QList &QQuickItemViewTransitioner::targetIndexes(QQuickItemViewTransitioner::TransitionType type) const { switch (type) { - case QQuickItemViewTransitioner::NoTransition: + case NoTransition: break; - case QQuickItemViewTransitioner::PopulateTransition: - case QQuickItemViewTransitioner::AddTransition: + case PopulateTransition: + case AddTransition: return addTransitionIndexes; - case QQuickItemViewTransitioner::MoveTransition: + case MoveTransition: return moveTransitionIndexes; - case QQuickItemViewTransitioner::RemoveTransition: + case RemoveTransition: return removeTransitionIndexes; } @@ -271,14 +288,14 @@ const QList &QQuickItemViewTransitioner::targetIndexes(QQuickItemViewTransi const QList &QQuickItemViewTransitioner::targetItems(QQuickItemViewTransitioner::TransitionType type) const { switch (type) { - case QQuickItemViewTransitioner::NoTransition: + case NoTransition: break; - case QQuickItemViewTransitioner::PopulateTransition: - case QQuickItemViewTransitioner::AddTransition: + case PopulateTransition: + case AddTransition: return addTransitionTargets; - case QQuickItemViewTransitioner::MoveTransition: + case MoveTransition: return moveTransitionTargets; - case QQuickItemViewTransitioner::RemoveTransition: + case RemoveTransition: return removeTransitionTargets; } @@ -305,6 +322,7 @@ QQuickViewItem::QQuickViewItem(QQuickItem *i) , index(-1) , isTransitionTarget(false) , nextTransitionToSet(false) + , prepared(false) { } @@ -373,7 +391,7 @@ bool QQuickViewItem::isPendingRemoval() const return false; } -bool QQuickViewItem::prepareTransition(const QRectF &viewBounds) +bool QQuickViewItem::prepareTransition(QQuickItemViewTransitioner *transitioner, const QRectF &viewBounds) { bool doTransition = false; @@ -393,7 +411,8 @@ bool QQuickViewItem::prepareTransition(const QRectF &viewBounds) } case QQuickItemViewTransitioner::PopulateTransition: { - return true; + doTransition = true; + break; } case QQuickItemViewTransitioner::AddTransition: case QQuickItemViewTransitioner::RemoveTransition: @@ -426,12 +445,24 @@ bool QQuickViewItem::prepareTransition(const QRectF &viewBounds) break; } + if (doTransition) { + // add item to target lists even if canTransition() is false for a target transition, + // since the target lists still need to be filled for displaced transitions + if (isTransitionTarget) + transitioner->addToTargetLists(nextTransitionType, this, index); + doTransition = transitioner->canTransition(nextTransitionType, isTransitionTarget); + } + if (!doTransition) { + // if transition type is not valid, the previous transition still has to be + // canceled so that the item can move immediately to the right position if (transition) transition->cancel(); item->setPos(nextTransitionTo); resetTransitionData(); } + + prepared = true; return doTransition; } @@ -440,6 +471,11 @@ void QQuickViewItem::startTransition(QQuickItemViewTransitioner *transitioner) if (nextTransitionType == QQuickItemViewTransitioner::NoTransition) return; + if (!prepared) { + qWarning("QQuickViewItem::prepareTransition() not called!"); + return; + } + if (!transition || transition->m_type != nextTransitionType || transition->m_isTarget != isTransitionTarget) { delete transition; transition = new QQuickItemViewTransitionJob; @@ -452,17 +488,7 @@ void QQuickViewItem::startTransition(QQuickItemViewTransitioner *transitioner) transition->startTransition(this, transitioner, nextTransitionType, nextTransitionTo, isTransitionTarget); nextTransitionType = QQuickItemViewTransitioner::NoTransition; -} - -void QQuickViewItem::stopTransition() -{ - if (transition) { - transition->cancel(); - delete transition; - transition = 0; - } - resetTransitionData(); - finishedTransition(); + prepared = false; } void QQuickViewItem::setNextTransition(QQuickItemViewTransitioner::TransitionType type, bool isTargetItem) diff --git a/src/quick/items/qquickitemviewtransition_p.h b/src/quick/items/qquickitemviewtransition_p.h index 3fb43d6..73c238e 100644 --- a/src/quick/items/qquickitemviewtransition_p.h +++ b/src/quick/items/qquickitemviewtransition_p.h @@ -82,6 +82,9 @@ public: bool canTransition(QQuickItemViewTransitioner::TransitionType type, bool asTarget) const; void transitionNextReposition(QQuickViewItem *item, QQuickItemViewTransitioner::TransitionType type, bool isTarget); + void addToTargetLists(QQuickItemViewTransitioner::TransitionType type, QQuickViewItem *item, int index); + void resetTargetLists(); + QQuickTransition *transitionObject(QQuickItemViewTransitioner::TransitionType type, bool asTarget); const QList &targetIndexes(QQuickItemViewTransitioner::TransitionType type) const; const QList &targetItems(QQuickItemViewTransitioner::TransitionType type) const; @@ -136,9 +139,8 @@ public: bool transitionRunning() const; bool isPendingRemoval() const; - bool prepareTransition(const QRectF &viewBounds); + bool prepareTransition(QQuickItemViewTransitioner *transitioner, const QRectF &viewBounds); void startTransition(QQuickItemViewTransitioner *transitioner); - void stopTransition(); QPointF nextTransitionTo; QQuickItem *item; @@ -147,6 +149,7 @@ public: int index; bool isTransitionTarget; bool nextTransitionToSet; + bool prepared; private: friend class QQuickItemViewTransitioner; diff --git a/src/quick/items/qquickpositioners.cpp b/src/quick/items/qquickpositioners.cpp index e1d0457..e9a0c17 100644 --- a/src/quick/items/qquickpositioners.cpp +++ b/src/quick/items/qquickpositioners.cpp @@ -246,11 +246,8 @@ void QQuickBasePositioner::prePositioning() if (addedIndex < 0) addedIndex = posItem.index; PositionedItem *theItem = &positionedItems[positionedItems.count()-1]; - d->transitioner->transitionNextReposition(theItem, QQuickItemViewTransitioner::AddTransition, true); - d->transitioner->addTransitionIndexes << posItem.index; - d->transitioner->addTransitionTargets << posItem.item; } } } else { @@ -273,8 +270,6 @@ void QQuickBasePositioner::prePositioning() addedIndex = item->index; d->transitioner->transitionNextReposition(&positionedItems[positionedItems.count()-1], QQuickItemViewTransitioner::AddTransition, true); - d->transitioner->addTransitionIndexes << item->index; - d->transitioner->addTransitionTargets << item->item; } } else { item->isNew = false; @@ -307,12 +302,11 @@ void QQuickBasePositioner::prePositioning() if (d->transitioner) { QRectF viewBounds; - for (int i=0; itransitioner); - } - d->transitioner->addTransitionIndexes.clear(); - d->transitioner->addTransitionTargets.clear(); + for (int i=0; itransitioner, viewBounds); + for (int i=0; itransitioner); + d->transitioner->resetTargetLists(); } d->doingPositioning = false; diff --git a/tests/auto/quick/qquickgridview/data/displacedTransitions.qml b/tests/auto/quick/qquickgridview/data/displacedTransitions.qml index d9353c0..2db0697 100644 --- a/tests/auto/quick/qquickgridview/data/displacedTransitions.qml +++ b/tests/auto/quick/qquickgridview/data/displacedTransitions.qml @@ -38,6 +38,7 @@ Rectangle { text: number } color: GridView.isCurrentItem ? "lightsteelblue" : "white" + border.width: 1 onXChanged: checkPos() onYChanged: checkPos() @@ -61,6 +62,17 @@ Rectangle { property int targetTransitionsDone property int displaceTransitionsDone + property var displacedTargetIndexes: new Array() + property var displacedTargetItems: new Array() + + // for QDeclarativeListProperty types + function copyList(propList) { + var temp = new Array() + for (var i=0; iproperty("displacedTargetIndexes").toList(); + QVariantList resultTargetItems = gridview->property("displacedTargetItems").toList(); + if ((useDisplaced && displacedEnabled) || (useAddDisplaced && addDisplacedEnabled) || (useMoveDisplaced && moveDisplacedEnabled) || (useRemoveDisplaced && removeDisplacedEnabled)) { QTRY_VERIFY(gridview->property("displaceTransitionsDone").toBool()); + + // check the correct number of target items and indexes were received + QCOMPARE(resultTargetIndexes.count(), expectedDisplacedIndexes.count()); + for (int i=0; i >().count(), change.count); + QCOMPARE(resultTargetItems.count(), expectedDisplacedIndexes.count()); + for (int i=0; iproperty("displacedTargetIndexes").toList(); + QVariantList resultTargetItems = listview->property("displacedTargetItems").toList(); + if ((useDisplaced && displacedEnabled) || (useAddDisplaced && addDisplacedEnabled) || (useMoveDisplaced && moveDisplacedEnabled) || (useRemoveDisplaced && removeDisplacedEnabled)) { QTRY_VERIFY(listview->property("displaceTransitionsDone").toBool()); + + // check the correct number of target items and indexes were received + QCOMPARE(resultTargetIndexes.count(), expectedDisplacedIndexes.count()); + for (int i=0; i >().count(), change.count); + QCOMPARE(resultTargetItems.count(), expectedDisplacedIndexes.count()); + for (int i=0; i