From 0592e17e18e26acc177bc7556a3c9888582f0045 Mon Sep 17 00:00:00 2001 From: Bea Lam Date: Wed, 13 Jul 2011 13:46:57 +1000 Subject: [PATCH] Make lastPosition() result consistent among GridView and ListView lastPosition() returns the end position of the last item so it should always include a -1 calculation for the last item (as FxListItemSG::endPosition() and FxGridItemSG::endPosition() do) to get the last edge pixel of the item. With this fix, both views now calculate startPosition() and endPosition() in the same way. This also fixes positioning of GridView items in TopToBottom+RightToLeft layouts. Incorrect test values in positionViewAtIndex_rightToLeft() test are fixed (the last edge of a 1-pixel border shouldn't be visible, since the bottom edge of a 1-pixel border is actually drawn outside, not inside, the Rectangle). Change-Id: I253c3836f871c61e13c08f67007ebc75e09378d6 Reviewed-on: http://codereview.qt.nokia.com/1547 Reviewed-by: Bea Lam Reviewed-by: Qt Sanity Bot --- src/declarative/items/qsggridview.cpp | 26 ++++++---------------- src/declarative/items/qsgitemview.cpp | 10 +++++++++ src/declarative/items/qsgitemview_p_p.h | 7 +++--- src/declarative/items/qsglistview.cpp | 17 +++----------- .../declarative/qsggridview/tst_qsggridview.cpp | 4 ++-- 5 files changed, 26 insertions(+), 38 deletions(-) diff --git a/src/declarative/items/qsggridview.cpp b/src/declarative/items/qsggridview.cpp index ab02dea..24fc708 100644 --- a/src/declarative/items/qsggridview.cpp +++ b/src/declarative/items/qsggridview.cpp @@ -147,13 +147,11 @@ public: virtual bool isContentFlowReversed() const; bool isRightToLeftTopToBottom() const; - virtual qreal startPosition() const; virtual qreal positionAt(int index) const; - virtual qreal endPosition() const; virtual qreal endPositionAt(int index) const; + virtual qreal originPosition() const; virtual qreal lastPosition() const; - qreal originPosition() const; int rowSize() const; int colSize() const; qreal colPosAt(int modelIndex) const; @@ -255,27 +253,19 @@ qreal QSGGridViewPrivate::originPosition() const qreal QSGGridViewPrivate::lastPosition() const { qreal pos = 0; - if (model && model->count()) - pos = rowPosAt(model->count() - 1) + rowSize(); + 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; + } return pos; } -qreal QSGGridViewPrivate::startPosition() const -{ - return isRightToLeftTopToBottom() ? -lastPosition()+1 : originPosition(); -} - qreal QSGGridViewPrivate::positionAt(int index) const { return rowPosAt(index); } -qreal QSGGridViewPrivate::endPosition() const -{ - return isRightToLeftTopToBottom() ? -originPosition()+1 : lastPosition(); - -} - qreal QSGGridViewPrivate::endPositionAt(int index) const { return rowPosAt(index) + rowSize(); @@ -524,8 +514,6 @@ bool QSGGridViewPrivate::removeNonVisibleItems(int bufferFrom, int bufferTo) void QSGGridViewPrivate::visibleItemsChanged() { - Q_Q(QSGGridView); - updateHeader(); updateFooter(); updateViewport(); @@ -680,7 +668,7 @@ void QSGGridViewPrivate::updateFooter() colOffset = gridItem->item->width()-cellWidth; } if (visibleItems.count()) { - qreal endPos = lastPosition(); + qreal endPos = lastPosition() + 1; if (findLastVisibleIndex() == model->count()-1) { gridItem->setPosition(colOffset, endPos + rowOffset); } else { diff --git a/src/declarative/items/qsgitemview.cpp b/src/declarative/items/qsgitemview.cpp index df242ac..e1788ca 100644 --- a/src/declarative/items/qsgitemview.cpp +++ b/src/declarative/items/qsgitemview.cpp @@ -1012,6 +1012,16 @@ qreal QSGItemViewPrivate::size() const return layoutOrientation() == Qt::Vertical ? q->height() : q->width(); } +qreal QSGItemViewPrivate::startPosition() const +{ + return isContentFlowReversed() ? -lastPosition()-1 : originPosition(); +} + +qreal QSGItemViewPrivate::endPosition() const +{ + return isContentFlowReversed() ? -originPosition()-1 : lastPosition(); +} + int QSGItemViewPrivate::findLastVisibleIndex(int defaultValue) const { if (visibleItems.count()) { diff --git a/src/declarative/items/qsgitemview_p_p.h b/src/declarative/items/qsgitemview_p_p.h index aa1d14d..78f0e14 100644 --- a/src/declarative/items/qsgitemview_p_p.h +++ b/src/declarative/items/qsgitemview_p_p.h @@ -85,6 +85,8 @@ public: bool isValid() const; qreal position() const; qreal size() const; + qreal startPosition() const; + qreal endPosition() const; int findLastVisibleIndex(int defaultValue = -1) const; FxViewItem *visibleItem(int modelIndex) const; FxViewItem *firstVisibleItem() const; @@ -165,11 +167,10 @@ protected: virtual Qt::Orientation layoutOrientation() const = 0; virtual bool isContentFlowReversed() const = 0; - virtual qreal startPosition() const = 0; virtual qreal positionAt(int index) const = 0; - virtual qreal endPosition() const = 0; virtual qreal endPositionAt(int index) const = 0; - virtual qreal lastPosition() const = 0; + virtual qreal originPosition() const = 0; + virtual qreal lastPosition() const = 0; virtual qreal headerSize() const = 0; virtual qreal footerSize() const = 0; diff --git a/src/declarative/items/qsglistview.cpp b/src/declarative/items/qsglistview.cpp index f190e17..e9a5922 100644 --- a/src/declarative/items/qsglistview.cpp +++ b/src/declarative/items/qsglistview.cpp @@ -187,13 +187,11 @@ public: virtual bool isContentFlowReversed() const; bool isRightToLeft() const; - virtual qreal startPosition() const; virtual qreal positionAt(int index) const; - virtual qreal endPosition() const; virtual qreal endPositionAt(int index) const; + virtual qreal originPosition() const; virtual qreal lastPosition() const; - qreal originPosition() const; FxViewItem *nextVisibleItem() const; FxViewItem *itemBefore(int modelIndex) const; QString sectionAt(int modelIndex); @@ -362,21 +360,12 @@ qreal QSGListViewPrivate::lastPosition() const } pos = (*(--visibleItems.constEnd()))->endPosition() + invisibleCount * (averageSize + spacing); } else if (model && model->count()) { - pos = model->count() * averageSize + (model->count()-1) * spacing; + // 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; } return pos; } -qreal QSGListViewPrivate::startPosition() const -{ - return isRightToLeft() ? -lastPosition()-1 : originPosition(); -} - -qreal QSGListViewPrivate::endPosition() const -{ - return isRightToLeft() ? -originPosition()-1 : lastPosition(); -} - qreal QSGListViewPrivate::positionAt(int modelIndex) const { if (FxViewItem *item = visibleItem(modelIndex)) diff --git a/tests/auto/declarative/qsggridview/tst_qsggridview.cpp b/tests/auto/declarative/qsggridview/tst_qsggridview.cpp index 5310632..39359d1 100644 --- a/tests/auto/declarative/qsggridview/tst_qsggridview.cpp +++ b/tests/auto/declarative/qsggridview/tst_qsggridview.cpp @@ -1236,7 +1236,7 @@ void tst_QSGGridView::positionViewAtIndex() // Position on an item that would leave empty space if positioned at the top gridview->positionViewAtIndex(31, QSGGridView::Beginning); QTRY_COMPARE(gridview->indexAt(120, 630), 31); - QTRY_COMPARE(gridview->contentY(), 521.); // 520 then +1 so bottom edge of last item is visible + QTRY_COMPARE(gridview->contentY(), 520.); // Confirm items positioned correctly itemCount = findItems(contentItem, "wrapper").count(); @@ -1528,7 +1528,7 @@ void tst_QSGGridView::positionViewAtIndex_rightToLeft() // Position on an item that would leave empty space if positioned at the top gridview->positionViewAtIndex(31, QSGGridView::Beginning); - QTRY_COMPARE(gridview->contentX(), -639.); + QTRY_COMPARE(gridview->contentX(), -640.); // Confirm items positioned correctly itemCount = findItems(contentItem, "wrapper").count(); -- 2.7.4