From 46162c304195db2376706f2e1a9da2b2c938e97b Mon Sep 17 00:00:00 2001 From: Ulf Hermann Date: Wed, 19 Feb 2020 14:00:46 +0100 Subject: QQuickTableView: Immediately delete delegates when possible In the dtor we don't need to care about any side effects a direct delete may have. Rather, any deleteLater() may not take effect anymore as the event loop may be gone already. Task-number: QTBUG-82000 Change-Id: I97935dc47fbbfd0c050e80c333c36a05f685c45d Reviewed-by: Joni Poikelin Reviewed-by: Simon Hausmann Reviewed-by: Ulf Hermann --- src/qmlmodels/qqmltableinstancemodel.cpp | 19 +++++++++++++++++++ src/qmlmodels/qqmltableinstancemodel_p.h | 1 + src/quick/items/qquicktableview.cpp | 11 ++++++++++- 3 files changed, 30 insertions(+), 1 deletion(-) diff --git a/src/qmlmodels/qqmltableinstancemodel.cpp b/src/qmlmodels/qqmltableinstancemodel.cpp index 9800eb8c72..e2cecfef79 100644 --- a/src/qmlmodels/qqmltableinstancemodel.cpp +++ b/src/qmlmodels/qqmltableinstancemodel.cpp @@ -240,6 +240,25 @@ QQmlInstanceModel::ReleaseFlags QQmlTableInstanceModel::release(QObject *object, return QQmlInstanceModel::Destroyed; } +void QQmlTableInstanceModel::dispose(QObject *object) +{ + Q_ASSERT(object); + auto modelItem = qvariant_cast(object->property(kModelItemTag)); + Q_ASSERT(modelItem); + + modelItem->releaseObject(); + + // The item is not referenced by anyone + Q_ASSERT(!modelItem->isObjectReferenced()); + Q_ASSERT(!modelItem->isReferenced()); + + m_modelItems.remove(modelItem->index); + + emit destroyingItem(object); + delete object; + delete modelItem; +} + void QQmlTableInstanceModel::cancel(int index) { auto modelItem = m_modelItems.value(index); diff --git a/src/qmlmodels/qqmltableinstancemodel_p.h b/src/qmlmodels/qqmltableinstancemodel_p.h index fd5968d872..4ea1d85d16 100644 --- a/src/qmlmodels/qqmltableinstancemodel_p.h +++ b/src/qmlmodels/qqmltableinstancemodel_p.h @@ -117,6 +117,7 @@ public: QObject *object(int index, QQmlIncubator::IncubationMode incubationMode = QQmlIncubator::AsynchronousIfNested) override; ReleaseFlags release(QObject *object) override { return release(object, NotReusable); } ReleaseFlags release(QObject *object, ReusableFlag reusable); + void dispose(QObject *object); void cancel(int) override; void insertIntoReusableItemsPool(QQmlDelegateModelItem *modelItem); diff --git a/src/quick/items/qquicktableview.cpp b/src/quick/items/qquicktableview.cpp index 4b34e3b2c1..4105996b31 100644 --- a/src/quick/items/qquicktableview.cpp +++ b/src/quick/items/qquicktableview.cpp @@ -441,7 +441,16 @@ QQuickTableViewPrivate::QQuickTableViewPrivate() QQuickTableViewPrivate::~QQuickTableViewPrivate() { - releaseLoadedItems(QQmlTableInstanceModel::NotReusable); + for (auto *fxTableItem : loadedItems) { + if (auto item = fxTableItem->item) { + if (fxTableItem->ownItem) + delete item; + else if (tableModel) + tableModel->dispose(item); + } + delete fxTableItem; + } + if (tableModel) delete tableModel; } -- cgit v1.2.3 From 254a56252874b63430701351dcd8c9bef8507353 Mon Sep 17 00:00:00 2001 From: Wang Chuan Date: Wed, 26 Feb 2020 21:14:50 +0800 Subject: QQuickItem: prevent endless loop in focus tab chain Since the commit a18ab2a3822e0d, we promote the [startItem] in focus tab chain when it is invisible to prevent endless loop. However the problem still happen if the [startItem] is equal to [firstFromItem] Fixes it by compare the [current] item with the original start item Fixes: QTBUG-81510 Change-Id: Iae0207f39e2b8c4fc6ed0cf36f0a855668accfba Reviewed-by: Mitch Curtis Reviewed-by: Liang Qi Reviewed-by: Shawn Rutledge --- src/quick/items/qquickitem.cpp | 8 +++++++- .../qquickitem2/data/activeFocusOnTab_infiniteLoop2.qml | 12 ++++++++++++ tests/auto/quick/qquickitem2/tst_qquickitem.cpp | 15 +++++++++++++-- 3 files changed, 32 insertions(+), 3 deletions(-) create mode 100644 tests/auto/quick/qquickitem2/data/activeFocusOnTab_infiniteLoop2.qml diff --git a/src/quick/items/qquickitem.cpp b/src/quick/items/qquickitem.cpp index 3785abc450..11c1f12e75 100644 --- a/src/quick/items/qquickitem.cpp +++ b/src/quick/items/qquickitem.cpp @@ -2568,6 +2568,7 @@ QQuickItem* QQuickItemPrivate::nextPrevItemInTabFocusChain(QQuickItem *item, boo bool skip = false; QQuickItem *startItem = item; + QQuickItem *originalStartItem = startItem; // Protect from endless loop: // If we start on an invisible item we will not find it again. // If there is no other item which can become the focus item, we have a forever loop, @@ -2643,7 +2644,12 @@ QQuickItem* QQuickItemPrivate::nextPrevItemInTabFocusChain(QQuickItem *item, boo } } from = last; - if (current == startItem && from == firstFromItem) { + // if [from] item is equal to [firstFromItem], means we have traversed one path and + // jump back to parent of the chain, and then we have to check whether we have + // traversed all of the chain (by compare the [current] item with [startItem]) + // Since the [startItem] might be promoted to its parent if it is invisible, + // we still have to check [current] item with original start item + if ((current == startItem || current == originalStartItem) && from == firstFromItem) { // wrapped around, avoid endless loops if (item == contentItem) { qCDebug(DBG_FOCUS) << "QQuickItemPrivate::nextPrevItemInTabFocusChain: looped, return contentItem"; diff --git a/tests/auto/quick/qquickitem2/data/activeFocusOnTab_infiniteLoop2.qml b/tests/auto/quick/qquickitem2/data/activeFocusOnTab_infiniteLoop2.qml new file mode 100644 index 0000000000..042f408753 --- /dev/null +++ b/tests/auto/quick/qquickitem2/data/activeFocusOnTab_infiniteLoop2.qml @@ -0,0 +1,12 @@ +import QtQuick 2.14 + +Item { + width: 400 + height: 200 + Item { + objectName: "hiddenChild" + focus: true + activeFocusOnTab: true + visible: false + } +} diff --git a/tests/auto/quick/qquickitem2/tst_qquickitem.cpp b/tests/auto/quick/qquickitem2/tst_qquickitem.cpp index 399535cfa6..767994ec7d 100644 --- a/tests/auto/quick/qquickitem2/tst_qquickitem.cpp +++ b/tests/auto/quick/qquickitem2/tst_qquickitem.cpp @@ -64,6 +64,7 @@ private slots: void activeFocusOnTab8(); void activeFocusOnTab9(); void activeFocusOnTab10(); + void activeFocusOnTab_infiniteLoop_data(); void activeFocusOnTab_infiniteLoop(); void nextItemInFocusChain(); @@ -1025,12 +1026,20 @@ void tst_QQuickItem::activeFocusOnTab10() delete window; } +void tst_QQuickItem::activeFocusOnTab_infiniteLoop_data() +{ + QTest::addColumn("source"); + QTest::newRow("infiniteLoop") << testFileUrl("activeFocusOnTab_infiniteLoop.qml"); // QTBUG-68271 + QTest::newRow("infiniteLoop2") << testFileUrl("activeFocusOnTab_infiniteLoop2.qml");// QTBUG-81510 +} + void tst_QQuickItem::activeFocusOnTab_infiniteLoop() { - // see QTBUG-68271 + QFETCH(QUrl, source); + // create a window where the currently focused item is not visible QScopedPointerwindow(new QQuickView()); - window->setSource(testFileUrl("activeFocusOnTab_infiniteLoop.qml")); + window->setSource(source); window->show(); auto *hiddenChild = findItem(window->rootObject(), "hiddenChild"); QVERIFY(hiddenChild); @@ -1039,6 +1048,8 @@ void tst_QQuickItem::activeFocusOnTab_infiniteLoop() auto *item = hiddenChild->nextItemInFocusChain(); // focus is moved to the root object since there is no other candidate QCOMPARE(item, window->rootObject()); + item = hiddenChild->nextItemInFocusChain(false); + QCOMPARE(item, window->rootObject()); } void tst_QQuickItem::nextItemInFocusChain() -- cgit v1.2.3 From fccffcfabcae4d9e33ce899017073305fdb7d15f Mon Sep 17 00:00:00 2001 From: Shawn Rutledge Date: Sun, 8 Dec 2019 16:11:20 +0100 Subject: Blacklist a few more canvas tests Task-number: QTBUG-41043 Change-Id: I3a48439d30d9ec1cd908197c8d63984c95d336e3 Reviewed-by: Shawn Rutledge --- tests/auto/quick/qquickcanvasitem/BLACKLIST | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/tests/auto/quick/qquickcanvasitem/BLACKLIST b/tests/auto/quick/qquickcanvasitem/BLACKLIST index 21580b6730..a4b25475a4 100644 --- a/tests/auto/quick/qquickcanvasitem/BLACKLIST +++ b/tests/auto/quick/qquickcanvasitem/BLACKLIST @@ -3,12 +3,18 @@ macos [canvas::test_paint] macos [canvas::test_save] +windows macos [canvas::test_implicitlySizedParent] -macos ci +* +# QTBUG-41043 [canvas::test_toDataURL] -macos +* [fillRect::test_fillRect] macos [imagedata::test_rounding] macos ci +[ContextFontValidation::test_pixelSize] +opensuse-leap +[ContextFontValidation::test_valid] +opensuse-leap -- cgit v1.2.3 From 2bb382f9c276a1dc1258148c156222be3576aff5 Mon Sep 17 00:00:00 2001 From: Shawn Rutledge Date: Wed, 25 Sep 2019 11:47:47 +0200 Subject: Make tst_qquickcanvasitem significant again; fix image size rounding - the test items must be visible, so that waitForRendering() works - arcTo::test_paint tried to render an out-of-bounds arc, which resulted in rendering nothing. Now renders within the 100x100 canvas. - painted() is not emitted the first time the Canvas is rendered. - Canvas.save() saves relative to the directory from which the test is run, while Canvas.loadImage() loads relative to the test data directory in this autotest (other tests are loading red.png for example). So we need to use absolute paths to test loading and saving in the directory where the executable is. - canvas.getContext('2d').getImageData(8.5, 8.5, 8.5, 8.5) now triggers different rounding behavior in QRectF::toRect(), after qtbase 88e56d0932a3615231adf40d5ae033e742d72c33: it becomes QRect(9,9 8x8). The assert in qt_create_image_data() needs to accommodate that. - Fixed another pedantic warning in qt_create_image_data a few lines above: if it creates the image itself, it needs to round the qreal width and height values. This reverts commit a23ee5c0de0d91859e1e76e64073861347dd9861 and amends 424cfef3cc3c140df51905713fa3849562bc494d and d142b2d212ea09a7919a0a2761ee9c04d5c9bda8. Task-number: QTBUG-41043 Change-Id: I825c2c5a2bbc8d5324c3ba41a681aa68bc25a159 Reviewed-by: Shawn Rutledge --- src/quick/items/context2d/qquickcontext2d.cpp | 5 +++-- .../quick/qquickcanvasitem/data/CanvasTestCase.qml | 5 +++-- .../auto/quick/qquickcanvasitem/data/tst_arcto.qml | 8 +++---- .../quick/qquickcanvasitem/data/tst_canvas.qml | 25 +++++++++++----------- .../quick/qquickcanvasitem/qquickcanvasitem.pro | 2 -- .../qquickcanvasitem/tst_qquickcanvasitem.cpp | 2 ++ 6 files changed, 24 insertions(+), 23 deletions(-) diff --git a/src/quick/items/context2d/qquickcontext2d.cpp b/src/quick/items/context2d/qquickcontext2d.cpp index 54cda72a36..14fa07099f 100644 --- a/src/quick/items/context2d/qquickcontext2d.cpp +++ b/src/quick/items/context2d/qquickcontext2d.cpp @@ -986,10 +986,11 @@ static QV4::ReturnedValue qt_create_image_data(qreal w, qreal h, QV4::ExecutionE pixelData->setPrototypeOf(p); if (image.isNull()) { - *pixelData->d()->image = QImage(w, h, QImage::Format_ARGB32); + *pixelData->d()->image = QImage(qRound(w), qRound(h), QImage::Format_ARGB32); pixelData->d()->image->fill(0x00000000); } else { - Q_ASSERT(image.width()== qRound(w * image.devicePixelRatioF()) && image.height() == qRound(h * image.devicePixelRatioF())); + // After qtbase 88e56d0932a3615231adf40d5ae033e742d72c33, the image size can be off by one. + Q_ASSERT(qAbs(image.width() - qRound(w * image.devicePixelRatioF())) <= 1 && qAbs(image.height() - qRound(h * image.devicePixelRatioF())) <= 1); *pixelData->d()->image = image.format() == QImage::Format_ARGB32 ? image : image.convertToFormat(QImage::Format_ARGB32); } diff --git a/tests/auto/quick/qquickcanvasitem/data/CanvasTestCase.qml b/tests/auto/quick/qquickcanvasitem/data/CanvasTestCase.qml index b0fb7fcf8c..5e02ca10d0 100644 --- a/tests/auto/quick/qquickcanvasitem/data/CanvasTestCase.qml +++ b/tests/auto/quick/qquickcanvasitem/data/CanvasTestCase.qml @@ -7,6 +7,7 @@ TestCase { when:windowShown width:100 height:100 + visible: true property Component component:CanvasComponent{} function cleanupTestCase() { wait(100) //wait for a short while to make sure no leaked textures @@ -19,8 +20,8 @@ TestCase { // { tag:"image cooperative", properties:{width:100, height:100, renderTarget:Canvas.Image, renderStrategy:Canvas.Cooperative}}, { tag:"image immediate", properties:{width:100, height:100, renderTarget:Canvas.Image, renderStrategy:Canvas.Immediate}}, // { tag:"fbo cooperative", properties:{width:100, height:100, renderTarget:Canvas.FramebufferObject, renderStrategy:Canvas.Cooperative}}, - { tag:"fbo immediate", properties:{width:100, height:100, renderTarget:Canvas.FramebufferObject, renderStrategy:Canvas.Immediate}}, - { tag:"fbo threaded", properties:{width:100, height:100, renderTarget:Canvas.FramebufferObject, renderStrategy:Canvas.Threaded}} + { tag:"fbo immediate", properties:{width:100, height:100, renderTarget:Canvas.FramebufferObject, renderStrategy:Canvas.Immediate}} +// { tag:"fbo threaded", properties:{width:100, height:100, renderTarget:Canvas.FramebufferObject, renderStrategy:Canvas.Threaded}} // QTBUG-82675 ]; return []; } diff --git a/tests/auto/quick/qquickcanvasitem/data/tst_arcto.qml b/tests/auto/quick/qquickcanvasitem/data/tst_arcto.qml index d9017150a4..ef1b7a7b2a 100644 --- a/tests/auto/quick/qquickcanvasitem/data/tst_arcto.qml +++ b/tests/auto/quick/qquickcanvasitem/data/tst_arcto.qml @@ -357,7 +357,7 @@ CanvasTestCase { ctx.fillStyle = '#0f0'; ctx.beginPath(); ctx.moveTo(0, 50); - ctx.translate(100, 0); + ctx.translate(50, 0); ctx.arcTo(50, 50, 50, 0, 50); ctx.lineTo(-100, 0); ctx.fill(); @@ -367,11 +367,11 @@ CanvasTestCase { comparePixel(ctx, 99,0, 0,255,0,255); comparePixel(ctx, 0,25, 0,255,0,255); comparePixel(ctx, 50,25, 0,255,0,255); - comparePixel(ctx, 99,25, 0,255,0,255); + comparePixel(ctx, 99,25, 255,0,0,255); comparePixel(ctx, 0,49, 0,255,0,255); comparePixel(ctx, 50,49, 0,255,0,255); - comparePixel(ctx, 99,49, 0,255,0,255); - } + comparePixel(ctx, 99,49, 255,0,0,255); + } function test_zero(row) { var canvas = createCanvasObject(row); var ctx = canvas.getContext('2d'); diff --git a/tests/auto/quick/qquickcanvasitem/data/tst_canvas.qml b/tests/auto/quick/qquickcanvasitem/data/tst_canvas.qml index 8238d87313..d74df3daa7 100644 --- a/tests/auto/quick/qquickcanvasitem/data/tst_canvas.qml +++ b/tests/auto/quick/qquickcanvasitem/data/tst_canvas.qml @@ -129,12 +129,12 @@ CanvasTestCase { tryCompare(c, "availableChangedCount", 1); c.requestPaint(); - verify(c.save("c.png")); - c.loadImage("c.png"); - wait(200); - verify(c.isImageLoaded("c.png")); - verify(!c.isImageLoading("c.png")); - verify(!c.isImageError("c.png")); + var imagePath = applicationDirPath + "/c.png"; + verify(c.save(imagePath)); + c.loadImage(imagePath); + tryVerify(function() { return c.isImageLoaded(imagePath) }) + verify(!c.isImageLoading(imagePath)); + verify(!c.isImageError(imagePath)); c.destroy(); } @@ -187,28 +187,28 @@ CanvasTestCase { tryCompare(c, "availableChangedCount", 1); //scene graph could be available immediately //in this case, we force waiting a short while until the init paint finished - tryCompare(c, "paintedCount", 1); + tryCompare(c, "paintedCount", 0); ctx.fillRect(0, 0, c.width, c.height); c.toDataURL(); - tryCompare(c, "paintedCount", 2); + tryCompare(c, "paintedCount", 1); tryCompare(c, "paintCount", 1); // implicit repaint when visible and resized testCase.visible = true; c.width += 1; c.height += 1; tryCompare(c, "paintCount", 2); - tryCompare(c, "paintedCount", 2); + tryCompare(c, "paintedCount", 1); // allow explicit repaint even when hidden testCase.visible = false; c.requestPaint(); tryCompare(c, "paintCount", 3); - tryCompare(c, "paintedCount", 2); + tryCompare(c, "paintedCount", 1); // no implicit repaint when resized but hidden c.width += 1; c.height += 1; waitForRendering(c); compare(c.paintCount, 3); - tryCompare(c, "paintedCount", 2); + tryCompare(c, "paintedCount", 1); c.destroy(); } function test_loadImage(row) { @@ -221,8 +221,7 @@ CanvasTestCase { verify(!c.isImageLoaded("red.png")); c.loadImage("red.png"); - wait(200); - verify(c.isImageLoaded("red.png")); + tryVerify(function() { return c.isImageLoaded("red.png") }); verify(!c.isImageLoading("red.png")); verify(!c.isImageError("red.png")); diff --git a/tests/auto/quick/qquickcanvasitem/qquickcanvasitem.pro b/tests/auto/quick/qquickcanvasitem/qquickcanvasitem.pro index 70e5a05f8d..90c7962382 100644 --- a/tests/auto/quick/qquickcanvasitem/qquickcanvasitem.pro +++ b/tests/auto/quick/qquickcanvasitem/qquickcanvasitem.pro @@ -55,5 +55,3 @@ OTHER_FILES += \ data/yellow75.png \ data/tst_invalidContext.qml - -CONFIG += insignificant_test # QTBUG-41043 diff --git a/tests/auto/quick/qquickcanvasitem/tst_qquickcanvasitem.cpp b/tests/auto/quick/qquickcanvasitem/tst_qquickcanvasitem.cpp index dad8df0682..4a83bd6c0d 100644 --- a/tests/auto/quick/qquickcanvasitem/tst_qquickcanvasitem.cpp +++ b/tests/auto/quick/qquickcanvasitem/tst_qquickcanvasitem.cpp @@ -46,6 +46,8 @@ public slots: false #endif )); + engine->rootContext()->setContextProperty("applicationDirPath", + QCoreApplication::applicationDirPath()); } }; -- cgit v1.2.3 From 92daa739ec114a0c9ae841db0459b0eff1f86cef Mon Sep 17 00:00:00 2001 From: Fabian Kosmale Date: Thu, 20 Feb 2020 10:36:37 +0100 Subject: QV4Engine: Avoid memory leak in toVariant conversion We should really backport this to 5.14. Someone will hit it. (cherry-picked from commit 78fd438f158839ffebcd52cc7974eac28489dbdd) Change-Id: I2c713fd759ac40aaaac0c0943edb993d3e27686b Reviewed-by: Fabian Kosmale --- src/qml/jsruntime/qv4engine.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/qml/jsruntime/qv4engine.cpp b/src/qml/jsruntime/qv4engine.cpp index 76fefb767d..df2c46b64a 100644 --- a/src/qml/jsruntime/qv4engine.cpp +++ b/src/qml/jsruntime/qv4engine.cpp @@ -1516,7 +1516,11 @@ static QVariant toVariant(QV4::ExecutionEngine *e, const QV4::Value &value, int return retn; #endif if (typeHint != -1) { - retn = QVariant(typeHint, QMetaType::create(typeHint)); + // the QVariant constructor will create a copy, so we have manually + // destroy the value returned by QMetaType::create + auto temp = QMetaType::create(typeHint); + retn = QVariant(typeHint, temp); + QMetaType::destroy(typeHint, temp); auto retnAsIterable = retn.value(); if (retnAsIterable._iteratorCapabilities & QtMetaTypePrivate::ContainerIsAppendable) { auto const length = a->getLength(); -- cgit v1.2.3