The views have many +/- 1 which are causing errors.

Remove them all.  endPosition() etc. now refer to the position() +
size() rather than position() + size() - 1.

Change-Id: I6ddf98def39971ee2e2c3c2d0a1df97781290782
Fixes: QTBUG-20020
Reviewed-on: http://codereview.qt.nokia.com/2243
Reviewed-by: Qt Sanity Bot <qt_sanity_bot@ovi.com>
Reviewed-by: Bea Lam <bea.lam@nokia.com>

diff --git a/src/declarative/items/qsggridview.cpp b/src/declarative/items/qsggridview.cpp
index 8559379..5531c15 100644 (file)
--- a/src/declarative/items/qsggridview.cpp
+++ b/src/declarative/items/qsggridview.cpp
@@ -105,12 +105,12 @@ public:
     }
     qreal endRowPos() const {
         if (view->flow() == QSGGridView::LeftToRight) {
-            return item->y() + view->cellHeight() - 1;
+            return item->y() + view->cellHeight();
         } else {
             if (view->effectiveLayoutDirection() == Qt::RightToLeft)
-                return -item->x() - 1;
+                return -item->x();
             else
-                return item->x() + view->cellWidth() - 1;
+                return item->x() + view->cellWidth();
         }
     }
     void setPosition(qreal col, qreal row) {
@@ -255,8 +255,7 @@ qreal QSGGridViewPrivate::lastPosition() const
     qreal pos = 0;
     if (model && model->count()) {
         // get end position of last item
-        // endPosition() of items calculate -1 to get last edge pixel, so do that here as well
-        pos = (rowPosAt(model->count() - 1) + rowSize()) - 1;
+        pos = (rowPosAt(model->count() - 1) + rowSize());
     }
     return pos;
 }
@@ -661,12 +660,12 @@ void QSGGridViewPrivate::updateFooter()
             colOffset = gridItem->item->width() - cellWidth;
     }
     if (visibleItems.count()) {
-        qreal endPos = lastPosition() + 1;
+        qreal endPos = lastPosition();
         if (findLastVisibleIndex() == model->count()-1) {
             gridItem->setPosition(colOffset, endPos + rowOffset);
         } else {
             qreal visiblePos = isRightToLeftTopToBottom() ? -position() : position() + size();
-            if (endPos <= visiblePos || gridItem->endPosition() < endPos + rowOffset)
+            if (endPos <= visiblePos || gridItem->endPosition() <= endPos + rowOffset)
                 gridItem->setPosition(colOffset, endPos + rowOffset);
         }
     } else {

diff --git a/src/declarative/items/qsgitemview.cpp b/src/declarative/items/qsgitemview.cpp
index 0198d58..18015ed 100644 (file)
--- a/src/declarative/items/qsgitemview.cpp
+++ b/src/declarative/items/qsgitemview.cpp
@@ -547,11 +547,11 @@ void QSGItemViewPrivate::positionViewAtIndex(int index, int mode)
         case QSGItemView::Visible:
             if (itemPos > pos + size())
                 pos = itemPos - size() + item->size();
-            else if (item->endPosition() < pos)
+            else if (item->endPosition() <= pos)
                 pos = itemPos;
             break;
         case QSGItemView::Contain:
-            if (item->endPosition() > pos + size())
+            if (item->endPosition() >= pos + size())
                 pos = itemPos - size() + item->size();
             if (itemPos < pos)
                 pos = itemPos;
@@ -746,18 +746,14 @@ void QSGItemView::trackedPositionChanged()
                 if (trackedPos < pos + highlightStart)
                     pos = trackedPos - highlightStart;
             } else {
-                if (trackedPos < d->startPosition() + highlightStart) {
+                if (trackedPos > pos + highlightEnd - trackedSize)
+                    pos = trackedPos - highlightEnd + trackedSize;
+                if (trackedPos < pos + highlightStart)
+                    pos = trackedPos - highlightStart;
+                if (pos > d->endPosition() - d->size())
+                    pos = d->endPosition() - d->size();
+                if (pos < d->startPosition())
                     pos = d->startPosition();
-                } else if (d->trackedItem->endPosition() > d->endPosition() - d->size() + highlightEnd) {
-                    pos = d->endPosition() - d->size() + 1;
-                    if (pos < d->startPosition())
-                        pos = d->startPosition();
-                } else {
-                    if (trackedPos > pos + highlightEnd - trackedSize)
-                        pos = trackedPos - highlightEnd + trackedSize;
-                    if (trackedPos < pos + highlightStart)
-                        pos = trackedPos - highlightStart;
-                }
             }
         } else {
             if (trackedPos < viewPos && d->currentItem->position() < viewPos) {
@@ -765,11 +761,11 @@ void QSGItemView::trackedPositionChanged()
             } else if (d->trackedItem->endPosition() >= viewPos + d->size()
                 && d->currentItem->endPosition() >= viewPos + d->size()) {
                 if (d->trackedItem->endPosition() <= d->currentItem->endPosition()) {
-                    pos = d->trackedItem->endPosition() - d->size() + 1;
+                    pos = d->trackedItem->endPosition() - d->size();
                     if (trackedSize > d->size())
                         pos = trackedPos;
                 } else {
-                    pos = d->currentItem->endPosition() - d->size() + 1;
+                    pos = d->currentItem->endPosition() - d->size();
                     if (d->currentItem->size() > d->size())
                         pos = d->currentItem->position();
                 }
@@ -808,7 +804,7 @@ qreal QSGItemView::minYExtent() const
             d->minExtent += d->highlightRangeStart;
             if (d->visibleItem(0))
                 d->minExtent -= d->visibleItem(0)->sectionSize();
-            d->minExtent = qMax(d->minExtent, -(d->endPositionAt(0) - d->highlightRangeEnd + 1));
+            d->minExtent = qMax(d->minExtent, -(d->endPositionAt(0) - d->highlightRangeEnd));
         }
         d->minExtentDirty = false;
     }
@@ -829,9 +825,9 @@ qreal QSGItemView::maxYExtent() const
         } else if (d->haveHighlightRange && d->highlightRange == StrictlyEnforceRange) {
             d->maxExtent = -(d->positionAt(d->model->count()-1) - d->highlightRangeStart);
             if (d->highlightRangeEnd != d->highlightRangeStart)
-                d->maxExtent = qMin(d->maxExtent, -(d->endPosition() - d->highlightRangeEnd + 1));
+                d->maxExtent = qMin(d->maxExtent, -(d->endPosition() - d->highlightRangeEnd));
         } else {
-            d->maxExtent = -(d->endPosition() - height() + 1);
+            d->maxExtent = -(d->endPosition() - height());
         }
 
         if (d->footer)
@@ -878,7 +874,7 @@ qreal QSGItemView::minXExtent() const
         }
         if (d->haveHighlightRange && d->highlightRange == StrictlyEnforceRange) {
             d->minExtent += highlightStart;
-            d->minExtent = qMax(d->minExtent, -(endPositionFirstItem - highlightEnd + 1));
+            d->minExtent = qMax(d->minExtent, -(endPositionFirstItem - highlightEnd));
         }
         d->minExtentDirty = false;
     }
@@ -915,11 +911,11 @@ qreal QSGItemView::maxXExtent() const
             d->maxExtent = -(lastItemPosition - highlightStart);
             if (highlightEnd != highlightStart) {
                 d->maxExtent = d->isContentFlowReversed()
-                        ? qMax(d->maxExtent, -(d->endPosition() - highlightEnd + 1))
-                        : qMin(d->maxExtent, -(d->endPosition() - highlightEnd + 1));
+                        ? qMax(d->maxExtent, -(d->endPosition() - highlightEnd))
+                        : qMin(d->maxExtent, -(d->endPosition() - highlightEnd));
             }
         } else {
-            d->maxExtent = -(d->endPosition() - width() + 1);
+            d->maxExtent = -(d->endPosition() - width());
         }
         if (d->isContentFlowReversed()) {
             if (d->header && d->visibleItems.count())
@@ -1030,12 +1026,12 @@ qreal QSGItemViewPrivate::size() const
 
 qreal QSGItemViewPrivate::startPosition() const
 {
-    return isContentFlowReversed() ? -lastPosition()-1 : originPosition();
+    return isContentFlowReversed() ? -lastPosition() : originPosition();
 }
 
 qreal QSGItemViewPrivate::endPosition() const
 {
-    return isContentFlowReversed() ? -originPosition()-1 : lastPosition();
+    return isContentFlowReversed() ? -originPosition() : lastPosition();
 }
 
 qreal QSGItemViewPrivate::contentStartPosition() const
@@ -1070,7 +1066,7 @@ FxViewItem *QSGItemViewPrivate::firstVisibleItem() const {
     const qreal pos = isContentFlowReversed() ? -position()-size() : position();
     for (int i = 0; i < visibleItems.count(); ++i) {
         FxViewItem *item = visibleItems.at(i);
-        if (item->index != -1 && item->endPosition() > pos)
+        if (item->index != -1 && item->endPosition() >= pos)
             return item;
     }
     return visibleItems.count() ? visibleItems.first() : 0;
@@ -1172,9 +1168,9 @@ void QSGItemViewPrivate::mirrorChange()
 void QSGItemViewPrivate::refill()
 {
     if (isContentFlowReversed())
-        refill(-position()-size()+1, -position());
+        refill(-position()-size(), -position());
     else
-        refill(position(), position()+size()-1);
+        refill(position(), position()+size());
 }
 
 void QSGItemViewPrivate::refill(qreal from, qreal to, bool doBuffer)
@@ -1250,9 +1246,9 @@ void QSGItemViewPrivate::updateViewport()
     Q_Q(QSGItemView);
     if (isValid()) {
         if (layoutOrientation() == Qt::Vertical)
-            q->setContentHeight(endPosition() - startPosition() + 1);
+            q->setContentHeight(endPosition() - startPosition());
         else
-            q->setContentWidth(endPosition() - startPosition() + 1);
+            q->setContentWidth(endPosition() - startPosition());
     }
 }
 

diff --git a/src/declarative/items/qsglistview.cpp b/src/declarative/items/qsglistview.cpp
index abfb4fe..7fbfe3c 100644 (file)
--- a/src/declarative/items/qsglistview.cpp
+++ b/src/declarative/items/qsglistview.cpp
@@ -132,11 +132,11 @@ public:
     }
     qreal endPosition() const {
         if (view->orientation() == QSGListView::Vertical) {
-            return item->y() + (item->height() >= 1.0 ? item->height() : 1) - 1;
+            return item->y() + item->height();
         } else {
             return (view->effectiveLayoutDirection() == Qt::RightToLeft
-                    ? -item->width()-item->x() + (item->width() >= 1.0 ? item->width() : 1)
-                    : item->x() + (item->width() >= 1.0 ? item->width() : 1)) - 1;
+                    ? -item->x()
+                    : item->x() + item->width());
         }
     }
     void setPosition(qreal pos) {
@@ -295,7 +295,7 @@ FxViewItem *QSGListViewPrivate::nextVisibleItem() const
         if (item->index != -1) {
             if (foundFirst)
                 return item;
-            else if (item->position() < pos && item->endPosition() > pos)
+            else if (item->position() < pos && item->endPosition() >= pos)
                 foundFirst = true;
         }
     }
@@ -360,8 +360,7 @@ qreal QSGListViewPrivate::lastPosition() const
         }
         pos = (*(--visibleItems.constEnd()))->endPosition() + invisibleCount * (averageSize + spacing);
     } else if (model && model->count()) {
-        // endPosition() of items calculate -1 to get last edge pixel, so do that here as well
-        pos = (model->count() * averageSize + (model->count()-1) * spacing) - 1;
+        pos = (model->count() * averageSize + (model->count()-1) * spacing);
     }
     return pos;
 }
@@ -381,7 +380,7 @@ qreal QSGListViewPrivate::positionAt(int modelIndex) const
             return (*visibleItems.constBegin())->position() - count * (averageSize + spacing) - cs;
         } else {
             int count = modelIndex - findLastVisibleIndex(visibleIndex) - 1;
-            return (*(--visibleItems.constEnd()))->endPosition() + spacing + count * (averageSize + spacing) + 1;
+            return (*(--visibleItems.constEnd()))->endPosition() + spacing + count * (averageSize + spacing);
         }
     }
     return 0;
@@ -394,7 +393,7 @@ qreal QSGListViewPrivate::endPositionAt(int modelIndex) const
     if (!visibleItems.isEmpty()) {
         if (modelIndex < visibleIndex) {
             int count = visibleIndex - modelIndex;
-            return (*visibleItems.constBegin())->position() - (count - 1) * (averageSize + spacing) - spacing - 1;
+            return (*visibleItems.constBegin())->position() - (count - 1) * (averageSize + spacing) - spacing;
         } else {
             int count = modelIndex - findLastVisibleIndex(visibleIndex) - 1;
             return (*(--visibleItems.constEnd()))->endPosition() + count * (averageSize + spacing);
@@ -440,7 +439,7 @@ FxViewItem *QSGListViewPrivate::snapItemAt(qreal pos)
         if (item->index == -1)
             continue;
         qreal itemTop = item->position();
-        if (highlight && itemTop >= pos && item->endPosition() <= pos + highlight->size() - 1)
+        if (highlight && itemTop >= pos && item->endPosition() <= pos + highlight->size())
             return item;
         if (itemTop+item->size()/2 >= pos && itemTop-item->size()/2 < pos)
             snapItem = item;
@@ -531,7 +530,7 @@ void QSGListViewPrivate::releaseItem(FxViewItem *item)
 
 bool QSGListViewPrivate::addVisibleItems(int fillFrom, int fillTo, bool doBuffer)
 {
-    qreal itemEnd = visiblePos-1;
+    qreal itemEnd = visiblePos;
     if (visibleItems.count()) {
         visiblePos = (*visibleItems.constBegin())->position();
         itemEnd = (*(--visibleItems.constEnd()))->endPosition() + spacing;
@@ -558,13 +557,13 @@ bool QSGListViewPrivate::addVisibleItems(int fillFrom, int fillTo, bool doBuffer
             modelIndex = 0;
         }
         visibleIndex = modelIndex;
-        visiblePos = itemEnd + count * (averageSize + spacing) + 1;
-        itemEnd = visiblePos-1;
+        visiblePos = itemEnd + count * (averageSize + spacing);
+        itemEnd = visiblePos;
     }
 
     bool changed = false;
     FxListItemSG *item = 0;
-    qreal pos = itemEnd + 1;
+    qreal pos = itemEnd;
     while (modelIndex < model->count() && pos <= fillTo) {
 //        qDebug() << "refill: append item" << modelIndex << "pos" << pos;
         if (!(item = static_cast<FxListItemSG*>(createItem(modelIndex))))
@@ -577,7 +576,7 @@ bool QSGListViewPrivate::addVisibleItems(int fillFrom, int fillTo, bool doBuffer
         if (doBuffer) // never buffer more than one item per frame
             break;
     }
-    while (visibleIndex > 0 && visibleIndex <= model->count() && visiblePos-1 >= fillFrom) {
+    while (visibleIndex > 0 && visibleIndex <= model->count() && visiblePos >= fillFrom) {
 //        qDebug() << "refill: prepend item" << visibleIndex-1 << "current top pos" << visiblePos;
         if (!(item = static_cast<FxListItemSG*>(createItem(visibleIndex-1))))
             break;
@@ -598,7 +597,7 @@ bool QSGListViewPrivate::removeNonVisibleItems(int bufferFrom, int bufferTo)
     FxViewItem *item = 0;
     bool changed = false;
 
-    while (visibleItems.count() > 1 && (item = visibleItems.first()) && item->endPosition() < bufferFrom) {
+    while (visibleItems.count() > 1 && (item = visibleItems.first()) && item->endPosition() <= bufferFrom) {
         if (item->attached->delayRemove())
             break;
 //            qDebug() << "refill: remove first" << visibleIndex << "top end pos" << item->endPosition();
@@ -855,7 +854,7 @@ void QSGListViewPrivate::updateCurrentSection()
         return;
     }
     int index = 0;
-    while (index < visibleItems.count() && visibleItems.at(index)->endPosition() < position())
+    while (index < visibleItems.count() && visibleItems.at(index)->endPosition() <= position())
         ++index;
 
     QString newSection = currentSection;
@@ -929,7 +928,7 @@ void QSGListViewPrivate::updateFooter()
 
     FxListItemSG *listItem = static_cast<FxListItemSG*>(footer);
     if (visibleItems.count()) {
-        qreal endPos = lastPosition() + 1;
+        qreal endPos = lastPosition();
         if (findLastVisibleIndex() == model->count()-1) {
             listItem->setPosition(endPos);
         } else {
@@ -1582,7 +1581,7 @@ void QSGListView::itemsInserted(int modelIndex, int count)
             // there are no visible items except items marked for removal
             index = d->visibleItems.count();
         } else if (d->visibleItems.at(i)->index + 1 == modelIndex
-            && d->visibleItems.at(i)->endPosition() < d->buffer+tempPos+d->size()-1) {
+            && d->visibleItems.at(i)->endPosition() <= d->buffer+tempPos+d->size()) {
             // Special case of appending an item to the model.
             index = d->visibleItems.count();
         } else {
@@ -1613,7 +1612,7 @@ void QSGListView::itemsInserted(int modelIndex, int count)
     int pos = 0;
     if (d->visibleItems.count()) {
         pos = index < d->visibleItems.count() ? d->visibleItems.at(index)->position()
-                                                : d->visibleItems.last()->endPosition()+d->spacing+1;
+                                                : d->visibleItems.last()->endPosition()+d->spacing;
     }
 
     int initialPos = pos;
@@ -1655,7 +1654,7 @@ void QSGListView::itemsInserted(int modelIndex, int count)
         }
     } else {
         int i = 0;
-        int to = d->buffer+tempPos+d->size()-1;
+        int to = d->buffer+tempPos+d->size();
         for (i = 0; i < count && pos <= to; ++i) {
             if (!addedVisible) {
                 d->scheduleLayout();

diff --git a/tests/auto/declarative/qsglistview/tst_qsglistview.cpp b/tests/auto/declarative/qsglistview/tst_qsglistview.cpp
index 30daeb8..a3b5cd6 100644 (file)
--- a/tests/auto/declarative/qsglistview/tst_qsglistview.cpp
+++ b/tests/auto/declarative/qsglistview/tst_qsglistview.cpp
@@ -1297,8 +1297,6 @@ void tst_QSGListView::sectionsDelegate()
 
 void tst_QSGListView::currentIndex()
 {
-    QSKIP("QTBUG-20020", SkipAll);
-
     TestModel model;
     for (int i = 0; i < 30; i++)
         model.addItem("Item" + QString::number(i), QString::number(i));