From b2ead4c58908968a7c4885751360fa7ed2b58369 Mon Sep 17 00:00:00 2001 From: Andrew den Exter Date: Fri, 4 Nov 2011 13:43:15 +1000 Subject: [PATCH] Fix invalid remove cache indexes caused by consecutive ranges. listItemsRemoved attempts to merge and consecutive cache only ranges it produces, this same logic would also merge other consecutive ranges incorrectly incrementing the cacheIndex in the process. Since listItemsRemoved won't produce these consecutive ranges itself handle only the cache only ranges there and compress the other consecutive ranges where they originate. Change-Id: If4d95fb741c8e7003ed48bfb2559c30c948c255b Reviewed-by: Martin Jones --- .../util/qdeclarativelistcompositor.cpp | 44 +++++++++-- .../tst_qdeclarativelistcompositor.cpp | 83 ++++++++++++++++++++ 2 files changed, 119 insertions(+), 8 deletions(-) diff --git a/src/declarative/util/qdeclarativelistcompositor.cpp b/src/declarative/util/qdeclarativelistcompositor.cpp index 0618ed2..b8b4dbf 100644 --- a/src/declarative/util/qdeclarativelistcompositor.cpp +++ b/src/declarative/util/qdeclarativelistcompositor.cpp @@ -55,7 +55,7 @@ static bool qt_verifyMinimal( bool minimal = true; int index = 0; - for (const QDeclarativeListCompositor::Range *range = begin->next; range != end; range = range->next, ++index) { + for (const QDeclarativeListCompositor::Range *range = begin->next; range != *end; range = range->next, ++index) { if (range->previous->list == range->list && range->previous->flags == (range->flags & ~QDeclarativeListCompositor::AppendFlag) && range->previous->end() == range->index) { @@ -456,7 +456,12 @@ void QDeclarativeListCompositor::setFlags( from->previous->flags |= AppendFlag; *from = erase(*from)->previous; continue; + } else { + break; } + } else if (!insertFlags) { + from.incrementIndexes(from->count - difference); + continue; } else if (difference < from->count) { *from = insert(*from, from->list, from->index, difference, setFlags)->next; from->index += difference; @@ -674,6 +679,16 @@ void QDeclarativeListCompositor::move( if (fromIt->append()) fromIt->previous->flags |= AppendFlag; *fromIt = erase(*fromIt); + + if (*fromIt != m_ranges.next && fromIt->flags == PrependFlag + && fromIt->previous != &m_ranges + && fromIt->previous->flags == PrependFlag + && fromIt->previous->list == fromIt->list + && fromIt->previous->end() == fromIt->index) { + fromIt.incrementIndexes(fromIt->count); + fromIt->previous->count += fromIt->count; + *fromIt = erase(*fromIt); + } } else if (count > 0) { *fromIt = fromIt->next; } @@ -797,6 +812,14 @@ void QDeclarativeListCompositor::listItemsInserted( } if ((it->flags & ~AppendFlag) == flags) { it->count += insertion.count; + } else if (offset == 0 + && it->previous != &m_ranges + && it->previous->list == list + && it->previous->end() == insertion.index + && it->previous->flags == flags) { + it->previous->count += insertion.count; + it->index += insertion.count; + it.incrementIndexes(insertion.count); } else { if (offset > 0) { it.incrementIndexes(offset); @@ -956,16 +979,22 @@ void QDeclarativeListCompositor::listItemsRemoved( } } else if (relativeIndex < 0) { it->index -= itemsRemoved; + + if (it->previous != &m_ranges + && it->previous->list == it->list + && it->previous->end() == it->index + && it->previous->flags == (it->flags & ~AppendFlag)) { + it->previous->count += it->count; + it->previous->flags = it->flags; + it.incrementIndexes(it->count); + *it = erase(*it); + removed = true; + } } } - if (it->next != &m_ranges - && it->list == it->next->list - && (it->flags == CacheFlag || it->end() == it->next->index) - && it->flags == (it->next->flags & ~AppendFlag)) { - + if (it->flags == CacheFlag && it->next->flags == CacheFlag && it->next->list == it->list) { it.index[Cache] += it->next->count; it->count += it->next->count; - it->flags = it->next->flags; erase(it->next); } else if (!removed) { it.incrementIndexes(it->count); @@ -994,7 +1023,6 @@ void QDeclarativeListCompositor::listItemsMoved( QVector *translatedRemovals, QVector *translatedInsertions) { - QT_DECLARATIVE_TRACE_LISTCOMPOSITOR(<< list << from << to << count) Q_ASSERT(count >= 0); diff --git a/tests/auto/declarative/qdeclarativelistcompositor/tst_qdeclarativelistcompositor.cpp b/tests/auto/declarative/qdeclarativelistcompositor/tst_qdeclarativelistcompositor.cpp index 94bb40e..425f35c 100644 --- a/tests/auto/declarative/qdeclarativelistcompositor/tst_qdeclarativelistcompositor.cpp +++ b/tests/auto/declarative/qdeclarativelistcompositor/tst_qdeclarativelistcompositor.cpp @@ -676,6 +676,27 @@ void tst_qdeclarativelistcompositor::setFlags_data() << IndexArray(defaultIndexes) << ListArray(defaultLists) << IndexArray(visibleIndexes) << ListArray(visibleLists) << IndexArray(selectionIndexes) << ListArray(selectionLists); + } { static const int cacheIndexes[] = {0,1,2,3,4,5,6,7,8,9,10,11}; + static const void *cacheLists[] = {a,a,a,a,a,a,a,a,a,a, a, a}; + static const int defaultIndexes[] = {0,1,2,3,4,5,6,7,8,9,10,11}; + static const void *defaultLists[] = {a,a,a,a,a,a,a,a,a,a, a, a}; + static const int visibleIndexes[] = {0,1,3,4,5,6,7,8,9,10,11}; + static const void *visibleLists[] = {a,a,a,a,a,a,a,a,a, a, a}; + static const int selectionIndexes[] = {2,6,7,8,9}; + static const void *selectionLists[] = {a,a,a,a,a}; + QTest::newRow("Existing flag, sparse selection") + << (RangeList() + << Range(a, 0, 2, C::PrependFlag | VisibleFlag | C::DefaultFlag | C::CacheFlag) + << Range(a, 2, 1, C::PrependFlag | SelectionFlag | C::DefaultFlag | C::CacheFlag) + << Range(a, 3, 3, C::PrependFlag | VisibleFlag | C::DefaultFlag | C::CacheFlag) + << Range(a, 6, 4, C::PrependFlag | SelectionFlag | VisibleFlag | C::DefaultFlag | C::CacheFlag) + << Range(a,10, 2, C::AppendFlag | C::PrependFlag | VisibleFlag | C::DefaultFlag | C::CacheFlag)) + << C::Cache << 3 << 1 << int(VisibleFlag) + << InsertList() + << IndexArray(cacheIndexes) << ListArray(cacheLists) + << IndexArray(defaultIndexes) << ListArray(defaultLists) + << IndexArray(visibleIndexes) << ListArray(visibleLists) + << IndexArray(selectionIndexes) << ListArray(selectionLists); } } @@ -885,6 +906,35 @@ void tst_qdeclarativelistcompositor::move_data() << IndexArray(defaultIndexes) << ListArray(defaultLists) << IndexArray() << ListArray() << IndexArray() << ListArray(); + } { static const int cacheIndexes[] = {8,9,10,4,11,0,1,2,3,5,6,7}; + static const void *cacheLists[] = {a,a, a,a, a,a,a,a,a,a,a,a}; + static const int defaultIndexes[] = {8,9,10,4,11,0,1,2,3,5,6,7}; + static const void *defaultLists[] = {a,a, a,a, a,a,a,a,a,a,a,a}; + static const int visibleIndexes[] = {8,9,10,4,11,0,1,2,3,5,6,7}; + static const void *visibleLists[] = {a,a, a,a, a,a,a,a,a,a,a,a}; + QTest::newRow("3, 4, 5") + << (RangeList() + << Range(a, 8, 4, VisibleFlag | C::DefaultFlag | C::CacheFlag) + << Range(a, 0, 2, C::PrependFlag | VisibleFlag | C::DefaultFlag | C::CacheFlag) + << Range(a, 2, 1, C::PrependFlag) + << Range(a, 2, 1, VisibleFlag | C::DefaultFlag | C::CacheFlag) + << Range(a, 3, 5, C::PrependFlag | VisibleFlag | C::DefaultFlag | C::CacheFlag) + << Range(a, 8, 4, C::AppendFlag | C::PrependFlag)) + << C::Default << 3 << C::Default << 4 << 5 + << (RemoveList() + << Remove(0, 3, 3, 3, 1, VisibleFlag | C::DefaultFlag | C::CacheFlag, 0) + << Remove(0, 3, 3, 3, 2, VisibleFlag | C::DefaultFlag | C::CacheFlag, 1) + << Remove(0, 3, 3, 3, 1, VisibleFlag | C::DefaultFlag | C::CacheFlag, 2) + << Remove(0, 3, 3, 3, 1, VisibleFlag | C::DefaultFlag | C::CacheFlag, 3)) + << (InsertList() + << Insert(0, 4, 4, 4, 1, VisibleFlag | C::DefaultFlag | C::CacheFlag, 0) + << Insert(0, 5, 5, 5, 2, VisibleFlag | C::DefaultFlag | C::CacheFlag, 1) + << Insert(0, 7, 7, 7, 1, VisibleFlag | C::DefaultFlag | C::CacheFlag, 2) + << Insert(0, 8, 8, 8, 1, VisibleFlag | C::DefaultFlag | C::CacheFlag, 3)) + << IndexArray(cacheIndexes) << ListArray(cacheLists) + << IndexArray(defaultIndexes) << ListArray(defaultLists) + << IndexArray(visibleIndexes) << ListArray(visibleLists) + << IndexArray() << ListArray(); } } @@ -1084,6 +1134,21 @@ void tst_qdeclarativelistcompositor::listItemsInserted_data() << IndexArray(defaultIndexes) << IndexArray(visibleIndexes) << IndexArray(); + } { static const int cacheIndexes[] = {/*A*/0,1,2,3,4}; + static const int defaultIndexes[] = {/*A*/0,1,2,3,4,5,6}; + static const int visibleIndexes[] = {/*A*/0,1,2,3,4,5,6}; + QTest::newRow("Consecutive appends") + << (RangeList() + << Range(a, 0, 5, C::PrependFlag | VisibleFlag | C::DefaultFlag | C::CacheFlag) + << Range(a, 5, 1, C::PrependFlag | VisibleFlag | C::DefaultFlag) + << Range(a, 6, 0, C::AppendFlag | VisibleFlag | C::PrependFlag | C::DefaultFlag | C::CacheFlag)) + << a << 6 << 1 + << (InsertList() + << Insert(0, 6, 6, 5, 1, VisibleFlag | C::DefaultFlag)) + << IndexArray(cacheIndexes) + << IndexArray(defaultIndexes) + << IndexArray(visibleIndexes) + << IndexArray(); } } @@ -1358,6 +1423,24 @@ void tst_qdeclarativelistcompositor::listItemsMoved_data() << IndexArray(defaultIndexes) << IndexArray() << IndexArray(); + } { static const int cacheIndexes[] = {/*A*/0,1,5,6,7,8,9,10,11,12}; + static const int defaultIndexes[] = {/*A*/0,1,2,3,4,5,6,7,8,9,10,11,12}; + QTest::newRow("Move merge tail") + << (RangeList() + << Range(a, 0, 10, C::PrependFlag | C::DefaultFlag | C::CacheFlag) + << Range(a, 10, 3, C::PrependFlag | C::DefaultFlag) + << Range(a, 13, 0, C::AppendFlag | C::PrependFlag | C::DefaultFlag | C::CacheFlag)) + << a << 8 << 0 << 5 + << (RemoveList() + << Remove(0, 0, 8, 8, 2, C::DefaultFlag | C::CacheFlag, 0) + << Remove(0, 0, 8, 8, 3, C::DefaultFlag, 1)) + << (InsertList() + << Insert(0, 0, 0, 0, 2, C::DefaultFlag | C::CacheFlag, 0) + << Insert(0, 0, 2, 2, 3, C::DefaultFlag, 1)) + << IndexArray(cacheIndexes) + << IndexArray(defaultIndexes) + << IndexArray() + << IndexArray(); } } -- 1.7.2.5