From aa99d4f5cb4f1f512d1db90536752b518dbea449 Mon Sep 17 00:00:00 2001 From: Martin Jones Date: Wed, 27 Jul 2011 14:42:48 +1000 Subject: [PATCH] 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 Reviewed-by: Bea Lam --- src/declarative/items/qsggridview.cpp | 13 +++--- src/declarative/items/qsgitemview.cpp | 54 ++++++++++------------ src/declarative/items/qsglistview.cpp | 39 ++++++++-------- .../declarative/qsglistview/tst_qsglistview.cpp | 2 - 4 files changed, 50 insertions(+), 58 deletions(-) diff --git a/src/declarative/items/qsggridview.cpp b/src/declarative/items/qsggridview.cpp index 8559379..5531c15 100644 --- 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 --- 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 --- 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(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(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(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 --- 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)); -- 2.7.4