Dive pictures: Fix crash on picture delete

The recent simplification of the close button code introduced a crash:
Deletion of pictures caused an invalid memory access, because the
CloseButtonItem was deleted with the parent DivePicture item.
For some (not fully understood!) reason, a reference to this button
was stored in the depths of Qt.

Empirically, it was found out that removing the first line of the pair
       QGraphicsItem::mousePressEvent(event);
       emit clicked();
fixed the crash.

It seemed therefore prudent to remove the whole questionable signal/slot
mechanism and directly call the removePicture() function of the parent.
Thus, the intermediate DiveButtonItem class became unnecessary and was
removed, leading to a shallower class hierarchy.

Unfortunately, CloseButtonItem must still be derived from QObject owing
to the Q_PROPERTY machinery, which is in turn needed for animation.

To make this compile on mobile, the conditional compilation of
removePicture() (#ifndef SUBSURFACE_MOBILE) was removed. After all,
if DivePixmapItem is used, there are pictures, so removePicture()
should be functional. Conditional compilation should concern the
whole class, not only this function.

Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
This commit is contained in:
Berthold Stoeger 2018-05-16 22:50:24 +02:00 committed by Dirk Hohndel
parent afe8843509
commit f54268e527
2 changed files with 14 additions and 30 deletions

View file

@ -16,31 +16,26 @@ DivePixmapItem::DivePixmapItem(QGraphicsItem *parent) : QGraphicsPixmapItem(pare
{ {
} }
DiveButtonItem::DiveButtonItem(QGraphicsItem *parent): DivePixmapItem(parent) CloseButtonItem::CloseButtonItem(QGraphicsItem *parent): DivePixmapItem(parent)
{
}
void DiveButtonItem::mousePressEvent(QGraphicsSceneMouseEvent *event)
{
QGraphicsItem::mousePressEvent(event);
emit clicked();
}
CloseButtonItem::CloseButtonItem(QGraphicsItem *parent): DiveButtonItem(parent)
{ {
static QPixmap p = QPixmap(":list-remove-icon"); static QPixmap p = QPixmap(":list-remove-icon");
setPixmap(p); setPixmap(p);
setFlag(ItemIgnoresTransformations); setFlag(ItemIgnoresTransformations);
} }
void CloseButtonItem::mousePressEvent(QGraphicsSceneMouseEvent *event)
{
qgraphicsitem_cast<DivePictureItem*>(parentItem())->removePicture();
}
void CloseButtonItem::hide() void CloseButtonItem::hide()
{ {
DiveButtonItem::hide(); DivePixmapItem::hide();
} }
void CloseButtonItem::show() void CloseButtonItem::show()
{ {
DiveButtonItem::show(); DivePixmapItem::show();
} }
DivePictureItem::DivePictureItem(QGraphicsItem *parent): DivePixmapItem(parent), DivePictureItem::DivePictureItem(QGraphicsItem *parent): DivePixmapItem(parent),
@ -96,7 +91,6 @@ void DivePictureItem::hoverEnterEvent(QGraphicsSceneHoverEvent *event)
button->setOpacity(0); button->setOpacity(0);
button->show(); button->show();
Animations::show(button); Animations::show(button);
connect(button, SIGNAL(clicked()), this, SLOT(removePicture()));
} }
void DivePictureItem::setFileUrl(const QString &s) void DivePictureItem::setFileUrl(const QString &s)
@ -119,9 +113,9 @@ void DivePictureItem::mousePressEvent(QGraphicsSceneMouseEvent *event)
} }
} }
#ifndef SUBSURFACE_MOBILE
void DivePictureItem::removePicture() void DivePictureItem::removePicture()
{ {
#ifndef SUBSURFACE_MOBILE
DivePictureModel::instance()->removePicture(fileUrl, true); DivePictureModel::instance()->removePicture(fileUrl, true);
}
#endif #endif
}

View file

@ -15,20 +15,12 @@ public:
DivePixmapItem(QGraphicsItem *parent = 0); DivePixmapItem(QGraphicsItem *parent = 0);
}; };
class DiveButtonItem : public DivePixmapItem { class CloseButtonItem : public DivePixmapItem {
Q_OBJECT
public:
DiveButtonItem(QGraphicsItem *parent = 0);
protected:
virtual void mousePressEvent(QGraphicsSceneMouseEvent *event);
signals:
void clicked();
};
class CloseButtonItem : public DiveButtonItem {
Q_OBJECT Q_OBJECT
public: public:
CloseButtonItem(QGraphicsItem *parent = 0); CloseButtonItem(QGraphicsItem *parent = 0);
protected:
virtual void mousePressEvent(QGraphicsSceneMouseEvent *event);
public slots: public slots:
void hide(); void hide();
void show(); void show();
@ -42,9 +34,7 @@ public:
void setPixmap(const QPixmap& pix); void setPixmap(const QPixmap& pix);
public slots: public slots:
void settingsChanged(); void settingsChanged();
#ifndef SUBSURFACE_MOBILE
void removePicture(); void removePicture();
#endif
void setFileUrl(const QString& s); void setFileUrl(const QString& s);
protected: protected:
void hoverEnterEvent(QGraphicsSceneHoverEvent *event); void hoverEnterEvent(QGraphicsSceneHoverEvent *event);
@ -54,7 +44,7 @@ private:
QString fileUrl; QString fileUrl;
QGraphicsRectItem *canvas; QGraphicsRectItem *canvas;
QGraphicsRectItem *shadow; QGraphicsRectItem *shadow;
DiveButtonItem *button; CloseButtonItem *button;
}; };
#endif // DIVEPIXMAPITEM_H #endif // DIVEPIXMAPITEM_H