From f7a501515fcf1dafecb88a40e18721ea14fd0a13 Mon Sep 17 00:00:00 2001 From: Olivier Goffart Date: Mon, 16 Aug 2010 13:33:16 +0200 Subject: QtDeclarative debugging: Add an option not to stream the properties of an object. Streaming all the properties is too slow, and we do not need them in the debugger of creator. Reviewed-by: Lasse Holmstedt --- src/declarative/debugger/qdeclarativedebug.cpp | 4 ++-- src/declarative/qml/qdeclarativeenginedebug.cpp | 16 ++++++++++++---- src/declarative/qml/qdeclarativeenginedebug_p.h | 2 +- 3 files changed, 15 insertions(+), 7 deletions(-) diff --git a/src/declarative/debugger/qdeclarativedebug.cpp b/src/declarative/debugger/qdeclarativedebug.cpp index b950aef..154df51 100644 --- a/src/declarative/debugger/qdeclarativedebug.cpp +++ b/src/declarative/debugger/qdeclarativedebug.cpp @@ -507,7 +507,7 @@ QDeclarativeDebugObjectQuery *QDeclarativeEngineDebug::queryObject(const QDeclar QByteArray message; QDataStream ds(&message, QIODevice::WriteOnly); ds << QByteArray("FETCH_OBJECT") << queryId << object.debugId() - << false; + << false << true; d->client->sendMessage(message); } else { query->m_state = QDeclarativeDebugQuery::Error; @@ -530,7 +530,7 @@ QDeclarativeDebugObjectQuery *QDeclarativeEngineDebug::queryObjectRecursive(cons QByteArray message; QDataStream ds(&message, QIODevice::WriteOnly); ds << QByteArray("FETCH_OBJECT") << queryId << object.debugId() - << true; + << true << true; d->client->sendMessage(message); } else { query->m_state = QDeclarativeDebugQuery::Error; diff --git a/src/declarative/qml/qdeclarativeenginedebug.cpp b/src/declarative/qml/qdeclarativeenginedebug.cpp index 1837366..ed98e3c 100644 --- a/src/declarative/qml/qdeclarativeenginedebug.cpp +++ b/src/declarative/qml/qdeclarativeenginedebug.cpp @@ -182,7 +182,7 @@ QVariant QDeclarativeEngineDebugServer::valueContents(const QVariant &value) con } void QDeclarativeEngineDebugServer::buildObjectDump(QDataStream &message, - QObject *object, bool recur) + QObject *object, bool recur, bool dumpProperties) { message << objectData(object); @@ -209,6 +209,8 @@ void QDeclarativeEngineDebugServer::buildObjectDump(QDataStream &message, continue; QDeclarativeBoundSignal *signal = QDeclarativeBoundSignal::cast(child); if (signal) { + if (!dumpProperties) + continue; QDeclarativeObjectProperty prop; prop.type = QDeclarativeObjectProperty::SignalProperty; prop.hasNotifySignal = false; @@ -229,12 +231,17 @@ void QDeclarativeEngineDebugServer::buildObjectDump(QDataStream &message, fakeProperties << prop; } else { if (recur) - buildObjectDump(message, child, recur); + buildObjectDump(message, child, recur, dumpProperties); else message << objectData(child); } } + if (!dumpProperties) { + message << 0; + return; + } + message << (object->metaObject()->propertyCount() + fakeProperties.count()); for (int ii = 0; ii < object->metaObject()->propertyCount(); ++ii) @@ -372,8 +379,9 @@ void QDeclarativeEngineDebugServer::messageReceived(const QByteArray &message) int queryId; int objectId; bool recurse; + bool dumpProperties = true; - ds >> queryId >> objectId >> recurse; + ds >> queryId >> objectId >> recurse >> dumpProperties; QObject *object = QDeclarativeDebugService::objectForId(objectId); @@ -382,7 +390,7 @@ void QDeclarativeEngineDebugServer::messageReceived(const QByteArray &message) rs << QByteArray("FETCH_OBJECT_R") << queryId; if (object) - buildObjectDump(rs, object, recurse); + buildObjectDump(rs, object, recurse, dumpProperties); sendMessage(reply); } else if (type == "WATCH_OBJECT") { diff --git a/src/declarative/qml/qdeclarativeenginedebug_p.h b/src/declarative/qml/qdeclarativeenginedebug_p.h index ea35b40..aa450f3 100644 --- a/src/declarative/qml/qdeclarativeenginedebug_p.h +++ b/src/declarative/qml/qdeclarativeenginedebug_p.h @@ -103,7 +103,7 @@ private Q_SLOTS: private: void buildObjectList(QDataStream &, QDeclarativeContext *); - void buildObjectDump(QDataStream &, QObject *, bool); + void buildObjectDump(QDataStream &, QObject *, bool, bool); QDeclarativeObjectData objectData(QObject *); QDeclarativeObjectProperty propertyData(QObject *, int); QVariant valueContents(const QVariant &defaultValue) const; -- cgit v0.12 From 0fb9e0fff4097bf0b84ff217526b0a9c33b69414 Mon Sep 17 00:00:00 2001 From: Benjamin Poulain Date: Fri, 13 Aug 2010 20:57:07 +0200 Subject: Implement the general blending of ARGB32_pm with SSSE3 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SSSE3 provides two tools to improve the blending speed over SSE2: -palignr -byte permutation The alignement is enforced on src and dst with palignr to always make aligned access. The extraction of the alpha mask is done with a byte permutation in order to save two instructions per cycle. On Atom, this patch gives between 0% (aligned src) to 10% of improvement (unaligned 4 and 12 bytes). On Core 2, this patch gives consistently 8% to 10% of improvement for every miss-alignment. Reviewed-by: Samuel Rødal --- src/gui/painting/painting.pri | 1 + src/gui/painting/qdrawhelper.cpp | 11 ++ src/gui/painting/qdrawhelper_ssse3.cpp | 263 +++++++++++++++++++++++++ tests/benchmarks/gui/image/blendbench/main.cpp | 43 ++++ 4 files changed, 318 insertions(+) create mode 100644 src/gui/painting/qdrawhelper_ssse3.cpp diff --git a/src/gui/painting/painting.pri b/src/gui/painting/painting.pri index dfa4a48..793d380 100644 --- a/src/gui/painting/painting.pri +++ b/src/gui/painting/painting.pri @@ -212,6 +212,7 @@ if(mmx|3dnow|sse|sse2|iwmmxt) { SSE3DNOW_SOURCES += painting/qdrawhelper_sse3dnow.cpp SSE_SOURCES += painting/qdrawhelper_sse.cpp SSE2_SOURCES += painting/qdrawhelper_sse2.cpp + SSSE3_SOURCES += painting/qdrawhelper_ssse3.cpp IWMMXT_SOURCES += painting/qdrawhelper_iwmmxt.cpp } diff --git a/src/gui/painting/qdrawhelper.cpp b/src/gui/painting/qdrawhelper.cpp index 1ff3d7b..276da93 100644 --- a/src/gui/painting/qdrawhelper.cpp +++ b/src/gui/painting/qdrawhelper.cpp @@ -7840,6 +7840,17 @@ void qInitDrawhelperAsm() qBlendFunctions[QImage::Format_ARGB32_Premultiplied][QImage::Format_RGB32] = qt_blend_rgb32_on_rgb32_sse2; qBlendFunctions[QImage::Format_RGB32][QImage::Format_ARGB32_Premultiplied] = qt_blend_argb32_on_argb32_sse2; qBlendFunctions[QImage::Format_ARGB32_Premultiplied][QImage::Format_ARGB32_Premultiplied] = qt_blend_argb32_on_argb32_sse2; + +#if defined(QT_HAVE_SSSE3) + if (features & SSSE3) { + extern void qt_blend_argb32_on_argb32_ssse3(uchar *destPixels, int dbpl, + const uchar *srcPixels, int sbpl, + int w, int h, + int const_alpha); + qBlendFunctions[QImage::Format_RGB32][QImage::Format_ARGB32_Premultiplied] = qt_blend_argb32_on_argb32_ssse3; + qBlendFunctions[QImage::Format_ARGB32_Premultiplied][QImage::Format_ARGB32_Premultiplied] = qt_blend_argb32_on_argb32_ssse3; + } +#endif // QT_HAVE_SSSE3 } else #endif { diff --git a/src/gui/painting/qdrawhelper_ssse3.cpp b/src/gui/painting/qdrawhelper_ssse3.cpp new file mode 100644 index 0000000..bc4a7eb8 --- /dev/null +++ b/src/gui/painting/qdrawhelper_ssse3.cpp @@ -0,0 +1,263 @@ +/**************************************************************************** +** +** Copyright (C) 2010 Nokia Corporation and/or its subsidiary(-ies). +** All rights reserved. +** Contact: Nokia Corporation (qt-info@nokia.com) +** +** This file is part of the QtGui module of the Qt Toolkit. +** +** $QT_BEGIN_LICENSE:LGPL$ +** No Commercial Usage +** This file contains pre-release code and may not be distributed. +** You may use this file in accordance with the terms and conditions +** contained in the Technology Preview License Agreement accompanying +** this package. +** +** GNU Lesser General Public License Usage +** Alternatively, this file may be used under the terms of the GNU Lesser +** General Public License version 2.1 as published by the Free Software +** Foundation and appearing in the file LICENSE.LGPL included in the +** packaging of this file. Please review the following information to +** ensure the GNU Lesser General Public License version 2.1 requirements +** will be met: http://www.gnu.org/licenses/old-licenses/lgpl-2.1.html. +** +** In addition, as a special exception, Nokia gives you certain additional +** rights. These rights are described in the Nokia Qt LGPL Exception +** version 1.1, included in the file LGPL_EXCEPTION.txt in this package. +** +** If you have questions regarding the use of this file, please contact +** Nokia at qt-info@nokia.com. +** +** +** +** +** +** +** +** +** $QT_END_LICENSE$ +** +****************************************************************************/ + + +#ifdef QT_HAVE_SSSE3 + +#include +#include +#include + +QT_BEGIN_NAMESPACE + +inline static void blend_pixel(quint32 &dst, const quint32 src) +{ + if (src >= 0xff000000) + dst = src; + else if (src != 0) + dst = src + BYTE_MUL(dst, qAlpha(~src)); +} + + +/* The instruction palignr uses direct arguments, so we have to generate the code fo the different + shift (4, 8, 12). Checking the alignment inside the loop is unfortunatelly way too slow. + */ +#define BLENDING_LOOP(palignrOffset, length)\ + for (; x < length-3; x += 4) { \ + const __m128i srcVectorLastLoaded = _mm_load_si128((__m128i *)&src[x - minusOffsetToAlignSrcOn16Bytes + 4]);\ + const __m128i srcVector = _mm_alignr_epi8(srcVectorLastLoaded, srcVectorPrevLoaded, palignrOffset); \ + const __m128i srcVectorAlpha = _mm_and_si128(srcVector, alphaMask); \ + if (_mm_movemask_epi8(_mm_cmpeq_epi32(srcVectorAlpha, alphaMask)) == 0xffff) { \ + _mm_store_si128((__m128i *)&dst[x], srcVector); \ + } else if (_mm_movemask_epi8(_mm_cmpeq_epi32(srcVectorAlpha, nullVector)) != 0xffff) { \ + __m128i alphaChannel = _mm_shuffle_epi8(srcVector, alphaShuffleMask); \ + alphaChannel = _mm_sub_epi16(one, alphaChannel); \ + const __m128i dstVector = _mm_load_si128((__m128i *)&dst[x]); \ + __m128i destMultipliedByOneMinusAlpha; \ + BYTE_MUL_SSE2(destMultipliedByOneMinusAlpha, dstVector, alphaChannel, colorMask, half); \ + const __m128i result = _mm_add_epi8(srcVector, destMultipliedByOneMinusAlpha); \ + _mm_store_si128((__m128i *)&dst[x], result); \ + } \ + srcVectorPrevLoaded = srcVectorLastLoaded;\ + } + + +#define BLEND_SOURCE_OVER_ARGB32_FIRST_ROW_SSSE3(dst, src, length, nullVector, half, one, colorMask, alphaMask) { \ + int x = 0; \ +\ + /* First, get dst aligned. */ \ + const int offsetToAlignOn16Bytes = (4 - ((reinterpret_cast(dst) >> 2) & 0x3)) & 0x3;\ + const int prologLength = qMin(length, offsetToAlignOn16Bytes);\ +\ + for (; x < prologLength; ++x) {\ + blend_pixel(dst[x], src[x]); \ + } \ +\ + const int minusOffsetToAlignSrcOn16Bytes = (reinterpret_cast(&(src[x])) >> 2) & 0x3;\ +\ + if (!minusOffsetToAlignSrcOn16Bytes) {\ + /* src is aligned, usual algorithm but with aligned operations.\ + See the SSE2 version for more documentation on the algorithm itself. */\ + const __m128i alphaShuffleMask = _mm_set_epi8(0xff,15,0xff,15,0xff,11,0xff,11,0xff,7,0xff,7,0xff,3,0xff,3);\ + for (; x < length-3; x += 4) { \ + const __m128i srcVector = _mm_load_si128((__m128i *)&src[x]); \ + const __m128i srcVectorAlpha = _mm_and_si128(srcVector, alphaMask); \ + if (_mm_movemask_epi8(_mm_cmpeq_epi32(srcVectorAlpha, alphaMask)) == 0xffff) { \ + _mm_store_si128((__m128i *)&dst[x], srcVector); \ + } else if (_mm_movemask_epi8(_mm_cmpeq_epi32(srcVectorAlpha, nullVector)) != 0xffff) { \ + __m128i alphaChannel = _mm_shuffle_epi8(srcVector, alphaShuffleMask); \ + alphaChannel = _mm_sub_epi16(one, alphaChannel); \ + const __m128i dstVector = _mm_load_si128((__m128i *)&dst[x]); \ + __m128i destMultipliedByOneMinusAlpha; \ + BYTE_MUL_SSE2(destMultipliedByOneMinusAlpha, dstVector, alphaChannel, colorMask, half); \ + const __m128i result = _mm_add_epi8(srcVector, destMultipliedByOneMinusAlpha); \ + _mm_store_si128((__m128i *)&dst[x], result); \ + } \ + } /* end for() */\ + } else if ((length - x) >= 8) {\ + /* We are at the first line, so "x - minusOffsetToAlignSrcOn16Bytes" could go before src, and\ + generate an invalid access. */\ +\ + /* We use two vectors to extract the src: prevLoaded for the first pixels, lastLoaded for the current pixels. */\ + __m128i srcVectorPrevLoaded;\ + if (minusOffsetToAlignSrcOn16Bytes <= prologLength) {\ + srcVectorPrevLoaded = _mm_load_si128((__m128i *)&src[x - minusOffsetToAlignSrcOn16Bytes]);\ + } else {\ + quint32 temp[4] Q_DECL_ALIGN(16);\ + switch (prologLength) {\ + case 3:\ + temp[1] = src[x - 3];\ + case 2:\ + temp[2] = src[x - 2];\ + case 1:\ + temp[3] = src[x - 1];\ + default:\ + break;\ + }\ + srcVectorPrevLoaded = _mm_load_si128((__m128i *)temp);\ + }\ + const int palignrOffset = minusOffsetToAlignSrcOn16Bytes << 2;\ +\ + const __m128i alphaShuffleMask = _mm_set_epi8(0xff,15,0xff,15,0xff,11,0xff,11,0xff,7,0xff,7,0xff,3,0xff,3);\ + switch (palignrOffset) {\ + case 4:\ + BLENDING_LOOP(4, length)\ + break;\ + case 8:\ + BLENDING_LOOP(8, length)\ + break;\ + case 12:\ + BLENDING_LOOP(12, length)\ + break;\ + }\ + }\ + for (; x < length; ++x) \ + blend_pixel(dst[x], src[x]); \ +} + +// Basically blend src over dst with the const alpha defined as constAlphaVector. +// nullVector, half, one, colorMask are constant accross the whole image/texture, and should be defined as: +//const __m128i nullVector = _mm_set1_epi32(0); +//const __m128i half = _mm_set1_epi16(0x80); +//const __m128i one = _mm_set1_epi16(0xff); +//const __m128i colorMask = _mm_set1_epi32(0x00ff00ff); +//const __m128i alphaMask = _mm_set1_epi32(0xff000000); +// +// The computation being done is: +// result = s + d * (1-alpha) +// with shortcuts if fully opaque or fully transparent. +#define BLEND_SOURCE_OVER_ARGB32_MAIN_SSSE3(dst, src, length, nullVector, half, one, colorMask, alphaMask) { \ + int x = 0; \ +\ + /* First, get dst aligned. */ \ + ALIGNMENT_PROLOGUE_16BYTES(dst, x, length) { \ + blend_pixel(dst[x], src[x]); \ + } \ +\ + const int minusOffsetToAlignSrcOn16Bytes = (reinterpret_cast(&(src[x])) >> 2) & 0x3;\ +\ + if (!minusOffsetToAlignSrcOn16Bytes) {\ + /* src is aligned, usual algorithm but with aligned operations.\ + See the SSE2 version for more documentation on the algorithm itself. */\ + const __m128i alphaShuffleMask = _mm_set_epi8(0xff,15,0xff,15,0xff,11,0xff,11,0xff,7,0xff,7,0xff,3,0xff,3);\ + for (; x < length-3; x += 4) { \ + const __m128i srcVector = _mm_load_si128((__m128i *)&src[x]); \ + const __m128i srcVectorAlpha = _mm_and_si128(srcVector, alphaMask); \ + if (_mm_movemask_epi8(_mm_cmpeq_epi32(srcVectorAlpha, alphaMask)) == 0xffff) { \ + _mm_store_si128((__m128i *)&dst[x], srcVector); \ + } else if (_mm_movemask_epi8(_mm_cmpeq_epi32(srcVectorAlpha, nullVector)) != 0xffff) { \ + __m128i alphaChannel = _mm_shuffle_epi8(srcVector, alphaShuffleMask); \ + alphaChannel = _mm_sub_epi16(one, alphaChannel); \ + const __m128i dstVector = _mm_load_si128((__m128i *)&dst[x]); \ + __m128i destMultipliedByOneMinusAlpha; \ + BYTE_MUL_SSE2(destMultipliedByOneMinusAlpha, dstVector, alphaChannel, colorMask, half); \ + const __m128i result = _mm_add_epi8(srcVector, destMultipliedByOneMinusAlpha); \ + _mm_store_si128((__m128i *)&dst[x], result); \ + } \ + } /* end for() */\ + } else if ((length - x) >= 8) {\ + /* We use two vectors to extract the src: prevLoaded for the first pixels, lastLoaded for the current pixels. */\ + __m128i srcVectorPrevLoaded = _mm_load_si128((__m128i *)&src[x - minusOffsetToAlignSrcOn16Bytes]);\ + const int palignrOffset = minusOffsetToAlignSrcOn16Bytes << 2;\ +\ + const __m128i alphaShuffleMask = _mm_set_epi8(0xff,15,0xff,15,0xff,11,0xff,11,0xff,7,0xff,7,0xff,3,0xff,3);\ + switch (palignrOffset) {\ + case 4:\ + BLENDING_LOOP(4, length)\ + break;\ + case 8:\ + BLENDING_LOOP(8, length)\ + break;\ + case 12:\ + BLENDING_LOOP(12, length)\ + break;\ + }\ + }\ + for (; x < length; ++x) \ + blend_pixel(dst[x], src[x]); \ +} + +void qt_blend_argb32_on_argb32_ssse3(uchar *destPixels, int dbpl, + const uchar *srcPixels, int sbpl, + int w, int h, + int const_alpha) +{ + const quint32 *src = (const quint32 *) srcPixels; + quint32 *dst = (quint32 *) destPixels; + if (const_alpha == 256) { + const __m128i alphaMask = _mm_set1_epi32(0xff000000); + const __m128i nullVector = _mm_setzero_si128(); + const __m128i half = _mm_set1_epi16(0x80); + const __m128i one = _mm_set1_epi16(0xff); + const __m128i colorMask = _mm_set1_epi32(0x00ff00ff); + + // We have to unrol the first row in order to deal with the load on unaligned data + // prior to the src pointer. + BLEND_SOURCE_OVER_ARGB32_FIRST_ROW_SSSE3(dst, src, w, nullVector, half, one, colorMask, alphaMask); + dst = (quint32 *)(((uchar *) dst) + dbpl); + src = (const quint32 *)(((const uchar *) src) + sbpl); + + for (int y = 1; y < h; ++y) { + BLEND_SOURCE_OVER_ARGB32_MAIN_SSSE3(dst, src, w, nullVector, half, one, colorMask, alphaMask); + dst = (quint32 *)(((uchar *) dst) + dbpl); + src = (const quint32 *)(((const uchar *) src) + sbpl); + } + } else if (const_alpha != 0) { + // dest = (s + d * sia) * ca + d * cia + // = s * ca + d * (sia * ca + cia) + // = s * ca + d * (1 - sa*ca) + const_alpha = (const_alpha * 255) >> 8; + const __m128i nullVector = _mm_setzero_si128(); + const __m128i half = _mm_set1_epi16(0x80); + const __m128i one = _mm_set1_epi16(0xff); + const __m128i colorMask = _mm_set1_epi32(0x00ff00ff); + const __m128i constAlphaVector = _mm_set1_epi16(const_alpha); + for (int y = 0; y < h; ++y) { + BLEND_SOURCE_OVER_ARGB32_WITH_CONST_ALPHA_SSE2(dst, src, w, nullVector, half, one, colorMask, constAlphaVector) + dst = (quint32 *)(((uchar *) dst) + dbpl); + src = (const quint32 *)(((const uchar *) src) + sbpl); + } + } +} + +QT_END_NAMESPACE + +#endif // QT_HAVE_SSSE3 diff --git a/tests/benchmarks/gui/image/blendbench/main.cpp b/tests/benchmarks/gui/image/blendbench/main.cpp index f53654b..d420d6c 100644 --- a/tests/benchmarks/gui/image/blendbench/main.cpp +++ b/tests/benchmarks/gui/image/blendbench/main.cpp @@ -106,6 +106,9 @@ private slots: void blendBenchAlpha_data(); void blendBenchAlpha(); + + void unalignedBlendArgb32_data(); + void unalignedBlendArgb32(); }; void BlendBench::blendBench_data() @@ -179,6 +182,46 @@ void BlendBench::blendBenchAlpha() } } +void BlendBench::unalignedBlendArgb32_data() +{ + // The performance of blending can depend of the alignment of the data + // on 16 bytes. Some SIMD instruction set have significantly better + // memory access when the memory is aligned on 16 bytes boundary. + + // offset in 32 bits words + QTest::addColumn("offset"); + QTest::newRow("aligned on 16 bytes") << 0; + QTest::newRow("unaligned by 4 bytes") << 1; + QTest::newRow("unaligned by 8 bytes") << 2; + QTest::newRow("unaligned by 12 bytes") << 3; +} + +void BlendBench::unalignedBlendArgb32() +{ + const int dimension = 1024; + + // We use dst aligned by design. We don't want to test all the combination of alignemnt for src and dst. + // Moreover, it make sense for us to align dst in the implementation because it is accessed more often. + uchar *dstMemory = static_cast(qMallocAligned((dimension * dimension * sizeof(quint32)), 16)); + QImage destination(dstMemory, dimension, dimension, QImage::Format_ARGB32_Premultiplied); + destination.fill(0x12345678); // avoid special cases of alpha + + uchar *srcMemory = static_cast(qMallocAligned((dimension * dimension * sizeof(quint32)) + 16, 16)); + QFETCH(int, offset); + srcMemory += (offset * sizeof(quint32)); + + QImage src(srcMemory, dimension, dimension, QImage::Format_ARGB32_Premultiplied); + src.fill(0x87654321); + + QPainter painter(&destination); + QBENCHMARK { + painter.drawImage(QPoint(), src); + } + + qFreeAligned(srcMemory); + qFreeAligned(dstMemory); +} + QTEST_MAIN(BlendBench) #include "main.moc" -- cgit v0.12 From 3e07671a2b043703bc51c2672f16c45ed0c31c42 Mon Sep 17 00:00:00 2001 From: Gabriel de Dietrich Date: Fri, 13 Aug 2010 18:12:30 +0200 Subject: Keyboard navigation regression in QTreeView Ammends commit 8da7252de0badb818302763cbe62c38ad699f1f3 Auto-test included. Reviewed-by: Olivier Task-number: QTBUG-11466 --- src/gui/itemviews/qabstractitemview.cpp | 8 +- tests/auto/qtreeview/tst_qtreeview.cpp | 125 ++++++++++++++++++++++++++++++++ 2 files changed, 130 insertions(+), 3 deletions(-) diff --git a/src/gui/itemviews/qabstractitemview.cpp b/src/gui/itemviews/qabstractitemview.cpp index 97499f3..4ffd284 100644 --- a/src/gui/itemviews/qabstractitemview.cpp +++ b/src/gui/itemviews/qabstractitemview.cpp @@ -2270,9 +2270,11 @@ void QAbstractItemView::keyPressEvent(QKeyEvent *event) } else { d->selectionModel->setCurrentIndex(newCurrent, command); d->pressedPosition = visualRect(newCurrent).center() + d->offset(); - // We copy the same behaviour as for mousePressEvent(). - QRect rect(d->pressedPosition - d->offset(), QSize(1, 1)); - setSelection(rect, command); + if (newCurrent.isValid()) { + // We copy the same behaviour as for mousePressEvent(). + QRect rect(d->pressedPosition - d->offset(), QSize(1, 1)); + setSelection(rect, command); + } } event->accept(); return; diff --git a/tests/auto/qtreeview/tst_qtreeview.cpp b/tests/auto/qtreeview/tst_qtreeview.cpp index 75a4c62..7e2e800 100644 --- a/tests/auto/qtreeview/tst_qtreeview.cpp +++ b/tests/auto/qtreeview/tst_qtreeview.cpp @@ -239,6 +239,7 @@ private slots: void doubleClickedWithSpans(); void taskQTBUG_6450_selectAllWith1stColumnHidden(); void taskQTBUG_9216_setSizeAndUniformRowHeightsWrongRepaint(); + void taskQTBUG_11466_keyboardNavigationRegression(); }; class QtTestModel: public QAbstractItemModel @@ -3785,5 +3786,129 @@ void tst_QTreeView::keyboardNavigationWithDisabled() QCOMPARE(view.currentIndex(), model.index(6, 0)); } +class Model_11466 : public QAbstractItemModel +{ + Q_OBJECT +public: + Model_11466(QObject *parent) : + m_block(false) + { + // set up the model to have two top level items and a few others + m_selectionModel = new QItemSelectionModel(this, this); // owned by this + + connect(m_selectionModel, SIGNAL(currentChanged(const QModelIndex &,const QModelIndex &)), + this, SLOT(slotCurrentChanged(const QModelIndex &,const QModelIndex &))); + }; + + int rowCount(const QModelIndex &parent) const + { + if (parent.isValid()) + return (parent.internalId() == 0) ? 4 : 0; + return 2; // two top level items + } + + int columnCount(const QModelIndex &parent) const + { + return 2; + } + + QVariant data(const QModelIndex &index, int role) const + { + if (role == Qt::DisplayRole && index.isValid()) { + qint64 parentRowPlusOne = index.internalId(); + QString str; + QTextStream stream(&str); + if (parentRowPlusOne > 0) + stream << parentRowPlusOne << " -> " << index.row() << " : " << index.column(); + else + stream << index.row() << " : " << index.column(); + return QVariant(str); + } + return QVariant(); + } + + QModelIndex parent(const QModelIndex &index) const + { + if (index.isValid()) { + qint64 parentRowPlusOne = index.internalId(); + if (parentRowPlusOne > 0) { + int row = static_cast(parentRowPlusOne - 1); + return createIndex(row, 0, (quint32)0); + } + } + return QModelIndex(); + } + + void bindView(QTreeView *view) + { + // sets the view to this model with a shared selection model + QItemSelectionModel *oldModel = view->selectionModel(); + if (oldModel != m_selectionModel) + delete oldModel; + view->setModel(this); // this creates a new selection model for the view, but we dont want it either ... + oldModel = view->selectionModel(); + view->setSelectionModel(m_selectionModel); + delete oldModel; + } + + QModelIndex index(int row, int column, const QModelIndex &parent) const + { + return createIndex(row, column, parent.isValid() ? (quint32)(parent.row() + 1) : (quint32)0); + } + +public slots: + void slotCurrentChanged(const QModelIndex ¤t,const QModelIndex &) + { + if (m_block) + return; + + if (current.isValid()) { + int selectedRow = current.row(); + quint32 parentRowPlusOne = static_cast(current.internalId()); + + for (int i = 0; i < 2; ++i) { + // announce the removal of all non top level items + beginRemoveRows(createIndex(i, 0, 0), 0, 3); + // nothing to actually do for the removal + endRemoveRows(); + + // put them back in again + beginInsertRows(createIndex(i, 0, 0), 0, 3); + // nothing to actually do for the insertion + endInsertRows(); + } + // reselect the current item ... + QModelIndex selectedIndex = createIndex(selectedRow, 0, parentRowPlusOne); + + m_block = true; // recursion block + m_selectionModel->select(selectedIndex, QItemSelectionModel::ClearAndSelect|QItemSelectionModel::Current|QItemSelectionModel::Rows); + m_selectionModel->setCurrentIndex(selectedIndex, QItemSelectionModel::NoUpdate); + m_block = false; + } else { + m_selectionModel->clear(); + } + } + +private: + bool m_block; + QItemSelectionModel *m_selectionModel; +}; + +void tst_QTreeView::taskQTBUG_11466_keyboardNavigationRegression() +{ + QTreeView treeView; + treeView.setSelectionBehavior(QAbstractItemView::SelectRows); + treeView.setSelectionMode(QAbstractItemView::SingleSelection); + Model_11466 model(&treeView); + model.bindView(&treeView); + treeView.expandAll(); + treeView.show(); + QTest::qWaitForWindowShown(&treeView); + + QTest::keyPress(treeView.viewport(), Qt::Key_Down); + QTest::qWait(10); + QTRY_COMPARE(treeView.currentIndex(), treeView.selectionModel()->selection().indexes().first()); +} + QTEST_MAIN(tst_QTreeView) #include "tst_qtreeview.moc" -- cgit v0.12