diff options
author | J-P Nurmi <jpnurmi@theqtcompany.com> | 2016-02-16 11:32:33 +0100 |
---|---|---|
committer | J-P Nurmi <jpnurmi@theqtcompany.com> | 2016-03-03 18:05:14 +0000 |
commit | 0e30dc40df70cef2cd3f31b913bf867a620327cb (patch) | |
tree | 971afe01ee262838247feb4ca776016327e6587d /src | |
parent | 0e0c351fff752ceadc99eef5fbcf5a180f453efc (diff) |
AnimatedSprite: don't access deleted scene graph nodes
It’s a bad idea to store a scene graph paint node as a member variable.
First of all, it should not access the node outside updatePaintNode(),
that is, outside the render thread. Secondly, the node is owned by the
scene graph and may be nuked whenever the scene graph feels so. Some
creative re-parenting easily triggers a case where AnimatedSprite ends
up accessing a node that was already deleted by the scene graph.
Change-Id: I89205ac36333a2fcb094121afa61b6409fda5883
Task-number: QTBUG-51162
Reviewed-by: Gunnar Sletta <gunnar@sletta.org>
Diffstat (limited to 'src')
-rw-r--r-- | src/quick/items/qquickanimatedsprite.cpp | 92 | ||||
-rw-r--r-- | src/quick/items/qquickanimatedsprite_p.h | 10 |
2 files changed, 49 insertions, 53 deletions
diff --git a/src/quick/items/qquickanimatedsprite.cpp b/src/quick/items/qquickanimatedsprite.cpp index 5f77c6461f..d7f3f4f83b 100644 --- a/src/quick/items/qquickanimatedsprite.cpp +++ b/src/quick/items/qquickanimatedsprite.cpp @@ -310,8 +310,6 @@ struct AnimatedSpriteVertices { //TODO: Implicitly size element to size of sprite QQuickAnimatedSprite::QQuickAnimatedSprite(QQuickItem *parent) : QQuickItem(parent) - , m_node(0) - , m_material(0) , m_sprite(new QQuickSprite(this)) , m_spriteEngine(0) , m_curFrame(0) @@ -325,9 +323,9 @@ QQuickAnimatedSprite::QQuickAnimatedSprite(QQuickItem *parent) : { setFlag(ItemHasContents); connect(this, SIGNAL(widthChanged()), - this, SLOT(sizeVertices())); + this, SLOT(reset())); connect(this, SIGNAL(heightChanged()), - this, SLOT(sizeVertices())); + this, SLOT(reset())); } bool QQuickAnimatedSprite::isCurrentFrameChangedConnected() @@ -455,12 +453,9 @@ static QSGGeometry::AttributeSet AnimatedSprite_AttributeSet = AnimatedSprite_Attributes }; -void QQuickAnimatedSprite::sizeVertices() +void QQuickAnimatedSprite::sizeVertices(QSGGeometryNode *node) { - if (!m_node) - return; - - AnimatedSpriteVertices *p = (AnimatedSpriteVertices *) m_node->geometry()->vertexData(); + AnimatedSpriteVertices *p = (AnimatedSpriteVertices *) node->geometry()->vertexData(); p->v1.x = 0; p->v1.y = 0; @@ -488,21 +483,21 @@ QSGGeometryNode* QQuickAnimatedSprite::buildNode() return 0; } - m_material = new QQuickAnimatedSpriteMaterial(); + QQuickAnimatedSpriteMaterial *material = new QQuickAnimatedSpriteMaterial(); QImage image = m_spriteEngine->assembledImage(); //Engine prints errors if there are any if (image.isNull()) return 0; m_sheetSize = QSizeF(image.size()); - m_material->texture = window()->createTextureFromImage(image); + material->texture = window()->createTextureFromImage(image); m_spriteEngine->start(0); - m_material->animT = 0; - m_material->animX1 = m_spriteEngine->spriteX() / m_sheetSize.width(); - m_material->animY1 = m_spriteEngine->spriteY() / m_sheetSize.height(); - m_material->animX2 = m_material->animX1; - m_material->animY2 = m_material->animY1; - m_material->animW = m_spriteEngine->spriteWidth() / m_sheetSize.width(); - m_material->animH = m_spriteEngine->spriteHeight() / m_sheetSize.height(); + material->animT = 0; + material->animX1 = m_spriteEngine->spriteX() / m_sheetSize.width(); + material->animY1 = m_spriteEngine->spriteY() / m_sheetSize.height(); + material->animX2 = material->animX1; + material->animY2 = material->animY1; + material->animW = m_spriteEngine->spriteWidth() / m_sheetSize.width(); + material->animH = m_spriteEngine->spriteHeight() / m_sheetSize.height(); int vCount = 4; int iCount = 6; @@ -511,7 +506,7 @@ QSGGeometryNode* QQuickAnimatedSprite::buildNode() AnimatedSpriteVertices *p = (AnimatedSpriteVertices *) g->vertexData(); - QRectF texRect = m_material->texture->normalizedTextureSubRect(); + QRectF texRect = material->texture->normalizedTextureSubRect(); p->v1.tx = texRect.topLeft().x(); p->v1.ty = texRect.topLeft().y(); @@ -534,51 +529,51 @@ QSGGeometryNode* QQuickAnimatedSprite::buildNode() indices[5] = 2; - m_node = new QSGGeometryNode(); - m_node->setGeometry(g); - m_node->setMaterial(m_material); - m_node->setFlag(QSGGeometryNode::OwnsMaterial); - m_node->setFlag(QSGGeometryNode::OwnsGeometry); - sizeVertices(); - return m_node; + QSGGeometryNode *node = new QSGGeometryNode(); + node->setGeometry(g); + node->setMaterial(material); + node->setFlag(QSGGeometryNode::OwnsMaterial); + node->setFlag(QSGGeometryNode::OwnsGeometry); + sizeVertices(node); + return node; } void QQuickAnimatedSprite::reset() { m_pleaseReset = true; + update(); } -QSGNode *QQuickAnimatedSprite::updatePaintNode(QSGNode *, UpdatePaintNodeData *) +QSGNode *QQuickAnimatedSprite::updatePaintNode(QSGNode *oldNode, UpdatePaintNodeData *) { if (m_pleaseReset) { - delete m_node; + delete oldNode; - m_node = 0; - m_material = 0; + oldNode = 0; m_pleaseReset = false; } - prepareNextFrame(); + QSGGeometryNode *node = static_cast<QSGGeometryNode *>(oldNode); + if (!node) + node = buildNode(); + + if (node) + prepareNextFrame(node); if (m_running) { if (!m_paused) update(); - if (m_node) { - m_node->markDirty(QSGNode::DirtyMaterial); + if (node) { + node->markDirty(QSGNode::DirtyMaterial); } } - return m_node; + return node; } -void QQuickAnimatedSprite::prepareNextFrame() +void QQuickAnimatedSprite::prepareNextFrame(QSGGeometryNode *node) { - if (m_node == 0) - m_node = buildNode(); - if (m_node == 0) //error creating node - return; - int timeInt = m_timestamp.elapsed() + m_pauseOffset; qreal time = timeInt / 1000.; @@ -694,14 +689,15 @@ void QQuickAnimatedSprite::prepareNextFrame() } } - m_material->animX1 = x1; - m_material->animY1 = y1; - m_material->animX2 = x2; - m_material->animY2 = y2; - m_material->animW = w; - m_material->animH = h; - m_material->animT = m_interpolate ? progress : 0.0; - m_material->texture->setFiltering(smooth() ? QSGTexture::Linear : QSGTexture::Nearest); + QQuickAnimatedSpriteMaterial *material = static_cast<QQuickAnimatedSpriteMaterial *>(node->material()); + material->animX1 = x1; + material->animY1 = y1; + material->animX2 = x2; + material->animY2 = y2; + material->animW = w; + material->animH = h; + material->animT = m_interpolate ? progress : 0.0; + material->texture->setFiltering(smooth() ? QSGTexture::Linear : QSGTexture::Nearest); } QT_END_NAMESPACE diff --git a/src/quick/items/qquickanimatedsprite_p.h b/src/quick/items/qquickanimatedsprite_p.h index ffaddefb47..5b181640f9 100644 --- a/src/quick/items/qquickanimatedsprite_p.h +++ b/src/quick/items/qquickanimatedsprite_p.h @@ -351,19 +351,19 @@ public Q_SLOTS: private Q_SLOTS: void createEngine(); - void sizeVertices(); + void sizeVertices(QSGGeometryNode *node); -protected: +protected Q_SLOTS: void reset(); + +protected: void componentComplete() Q_DECL_OVERRIDE; QSGNode *updatePaintNode(QSGNode *, UpdatePaintNodeData *) Q_DECL_OVERRIDE; private: bool isCurrentFrameChangedConnected(); - void prepareNextFrame(); + void prepareNextFrame(QSGGeometryNode *node); void reloadImage(); QSGGeometryNode* buildNode(); - QSGGeometryNode *m_node; - QQuickAnimatedSpriteMaterial *m_material; QQuickSprite* m_sprite; QQuickSpriteEngine* m_spriteEngine; QElapsedTimer m_timestamp; |