From 1ae19b476f4675dff954ce8dac71c1625a96c0c0 Mon Sep 17 00:00:00 2001 From: Alan Alpert Date: Tue, 1 May 2012 16:08:54 +1000 Subject: [PATCH] Avoid accessing sprites until they finish loading Because sprite properties are not valid until image assembly finishes, some features of the sprite engine cannot work until sprite assembly is finished. In these circumstances, we shouldn't crash. Task-number: QTBUG-24797 Change-Id: I2701f9302620c2cfd5b7bc5ce1cb9f31be4d4e19 Reviewed-by: Alan Alpert --- src/quick/items/qquickspriteengine.cpp | 26 ++++++-- src/quick/items/qquickspriteengine_p.h | 1 + .../qquickspritesequence/data/crashonstart.qml | 74 ++++++++++++++++++++ .../tst_qquickspritesequence.cpp | 13 ++++ 4 files changed, 109 insertions(+), 5 deletions(-) create mode 100644 tests/auto/quick/qquickspritesequence/data/crashonstart.qml diff --git a/src/quick/items/qquickspriteengine.cpp b/src/quick/items/qquickspriteengine.cpp index 60d6edd..1763d7a 100644 --- a/src/quick/items/qquickspriteengine.cpp +++ b/src/quick/items/qquickspriteengine.cpp @@ -107,12 +107,12 @@ QQuickStochasticEngine::~QQuickStochasticEngine() } QQuickSpriteEngine::QQuickSpriteEngine(QObject *parent) - : QQuickStochasticEngine(parent), m_startedImageAssembly(false) + : QQuickStochasticEngine(parent), m_startedImageAssembly(false), m_loaded(false) { } QQuickSpriteEngine::QQuickSpriteEngine(QList sprites, QObject *parent) - : QQuickStochasticEngine(parent), m_startedImageAssembly(false) + : QQuickStochasticEngine(parent), m_startedImageAssembly(false), m_loaded(false) { foreach (QQuickSprite* sprite, sprites) m_states << (QQuickStochasticState*)sprite; @@ -156,6 +156,8 @@ int QQuickSpriteEngine::pseudospriteProgress(int sprite, int state, int* rowDura int QQuickSpriteEngine::spriteState(int sprite) { + if (!m_loaded) + return 0; int state = m_things[sprite]; if (!m_sprites[state]->m_generatedCount) return state; @@ -175,7 +177,7 @@ int QQuickSpriteEngine::spriteState(int sprite) int QQuickSpriteEngine::spriteStart(int sprite) { - if (!m_duration[sprite]) + if (!m_duration[sprite] || !m_loaded) return m_timeOffset; int state = m_things[sprite]; if (!m_sprites[state]->m_generatedCount) @@ -189,6 +191,8 @@ int QQuickSpriteEngine::spriteStart(int sprite) int QQuickSpriteEngine::spriteFrames(int sprite) { + if (!m_loaded) + return 1; int state = m_things[sprite]; if (!m_sprites[state]->m_generatedCount) return m_sprites[state]->frames(); @@ -212,7 +216,7 @@ int QQuickSpriteEngine::spriteFrames(int sprite) int QQuickSpriteEngine::spriteDuration(int sprite)//Full duration, not per frame { - if (!m_duration[sprite]) + if (!m_duration[sprite] || !m_loaded) return m_duration[sprite]; int state = m_things[sprite]; if (!m_sprites[state]->m_generatedCount) @@ -230,6 +234,8 @@ int QQuickSpriteEngine::spriteDuration(int sprite)//Full duration, not per frame int QQuickSpriteEngine::spriteY(int sprite) { + if (!m_loaded) + return 0; int state = m_things[sprite]; if (!m_sprites[state]->m_generatedCount) return m_sprites[state]->m_rowY; @@ -250,6 +256,8 @@ int QQuickSpriteEngine::spriteY(int sprite) int QQuickSpriteEngine::spriteX(int sprite) { + if (!m_loaded) + return 0; int state = m_things[sprite]; if (!m_sprites[state]->m_generatedCount) return m_sprites[state]->m_rowStartX; @@ -339,6 +347,7 @@ void QQuickSpriteEngine::startAssemblingImage() { if (m_startedImageAssembly) return; + m_loaded = false; //This could also trigger the start of the image loading in Sprites, however that currently happens in Sprite::setSource @@ -477,6 +486,8 @@ QImage QQuickSpriteEngine::assembledImage() qDebug() << "Assembled image output to: " << fPath.arg(acc); #endif + m_loaded = true; + m_startedImageAssembly = false; return image; } @@ -530,7 +541,7 @@ void QQuickStochasticEngine::restart(int index) void QQuickSpriteEngine::restart(int index) //Reimplemented to recognize and handle pseudostates { bool randomStart = (m_startTimes[index] == NINF); - if (m_sprites[m_things[index]]->frameSync()) {//Manually advanced + if (m_loaded && m_sprites[m_things[index]]->frameSync()) {//Manually advanced m_startTimes[index] = 0; if (randomStart && m_sprites[m_things[index]]->m_generatedCount) m_startTimes[index] += qrand() % m_sprites[m_things[index]]->m_generatedCount; @@ -567,6 +578,11 @@ void QQuickStochasticEngine::advance(int idx) void QQuickSpriteEngine::advance(int idx) //Reimplemented to recognize and handle pseudostates { + if (!m_loaded) { + qWarning() << QLatin1String("QQuickSpriteEngine: Trying to advance sprites before sprites finish loading. Ignoring directive"); + return; + } + if (idx >= m_things.count()) return;//TODO: Proper fix(because this has happened and I just ignored it) if (m_duration[idx] == 0) { diff --git a/src/quick/items/qquickspriteengine_p.h b/src/quick/items/qquickspriteengine_p.h index f6ef79c..d78a103 100644 --- a/src/quick/items/qquickspriteengine_p.h +++ b/src/quick/items/qquickspriteengine_p.h @@ -294,6 +294,7 @@ private: int pseudospriteProgress(int,int,int*rd=0); QList m_sprites; bool m_startedImageAssembly; + bool m_loaded; }; //Common use is to have your own list property which is transparently an engine diff --git a/tests/auto/quick/qquickspritesequence/data/crashonstart.qml b/tests/auto/quick/qquickspritesequence/data/crashonstart.qml new file mode 100644 index 0000000..67e09e4 --- /dev/null +++ b/tests/auto/quick/qquickspritesequence/data/crashonstart.qml @@ -0,0 +1,74 @@ +/**************************************************************************** +** +** Copyright (C) 2012 Nokia Corporation and/or its subsidiary(-ies). +** Contact: http://www.qt-project.org/ +** +** This file is part of the test suite of the Qt Toolkit. +** +** $QT_BEGIN_LICENSE:LGPL$ +** GNU Lesser General Public License Usage +** 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. +** +** GNU General Public License Usage +** Alternatively, this file may be used under the terms of the GNU General +** Public License version 3.0 as published by the Free Software Foundation +** and appearing in the file LICENSE.GPL included in the packaging of this +** file. Please review the following information to ensure the GNU General +** Public License version 3.0 requirements will be met: +** http://www.gnu.org/copyleft/gpl.html. +** +** Other Usage +** Alternatively, this file may be used in accordance with the terms and +** conditions contained in a signed written agreement between you and Nokia. +** +** +** +** +** +** +** $QT_END_LICENSE$ +** +****************************************************************************/ + +//QTBUG-24797 +import QtQuick 2.0 + +Rectangle { + width: 800 + height: 800 + + SpriteSequence { + id: mysprite + sprites: [s1,s2] + scale: 2 + height: 200 + width: 200 + anchors.centerIn: parent + } + + Component.onCompleted: mysprite.jumpTo("running") + Sprite { + id: s1 + name: "standing" + frameCount: 12 + frameDuration: 80 + source: "squarefacesprite.png" + } + + Sprite { + id: s2 + name: "running" + frameCount: 6 + frameDuration: 80 + source: "squarefacesprite.png" + } +} diff --git a/tests/auto/quick/qquickspritesequence/tst_qquickspritesequence.cpp b/tests/auto/quick/qquickspritesequence/tst_qquickspritesequence.cpp index 8469a94..7a82cbb 100644 --- a/tests/auto/quick/qquickspritesequence/tst_qquickspritesequence.cpp +++ b/tests/auto/quick/qquickspritesequence/tst_qquickspritesequence.cpp @@ -52,6 +52,7 @@ public: private slots: void test_properties(); void test_framerateAdvance();//Separate codepath for QQuickSpriteEngine + void test_jumpToCrash(); }; void tst_qquickspritesequence::test_properties() @@ -93,6 +94,18 @@ void tst_qquickspritesequence::test_framerateAdvance() delete canvas; } +void tst_qquickspritesequence::test_jumpToCrash() +{ + QQuickView *canvas = new QQuickView(0); + + canvas->setSource(testFileUrl("crashonstart.qml")); + canvas->show(); + QTest::qWaitForWindowShown(canvas); + //verify: Don't crash + + delete canvas; +} + QTEST_MAIN(tst_qquickspritesequence) #include "tst_qquickspritesequence.moc" -- 1.7.2.5