Crash using QGraphicsEffect with QGraphicsItemGroup

[akiross]

Hello,
I'm experiencing a crash when, after applying a graphic effect on an item group, I try to remove from the scene that item group. The crash doesn't happen if I apply the effect to items of the group or if I don't use the effect at all.

I'm trying to create a scene with some items (with effect), and when a key is pressed, only a part of such items are deleted. I wrote this little demo (see below). Running it, I do some random clicks and press Escape. The application crash (SIGSEGV) on line 135 of qscopedpointer.h, function data.

Please, tell me if I'm doing something wrong or test if it works on your implementation, I'm using Qt 4.7.3 (64 bit).

Here's the code (removed the guards from the headers).

main.cpp

#include <QtGui/QApplication>
#include "mainwindow.hh"
 
int main(int argc, char *argv[]) {
	QApplication a(argc, argv);
	MainWindow w;
	w.show();
	return a.exec();
}

#include <QtGui/QApplication>
#include "mainwindow.hh"

int main(int argc, char *argv[]) {
	QApplication a(argc, argv);
	MainWindow w;
	w.show();
	return a.exec();
}

To copy to clipboard, switch view to plain text mode 

itemfieldeffect.hh

#include <QtGui>
 
class ItemFieldEffect: public QGraphicsEffect {
public:
	ItemFieldEffect(qreal radius);
	void draw(QPainter *painter);
	QRectF boundingRectFor(const QRectF &sourceRect) const;
private:
	qreal _radius;
};

#include <QtGui>

class ItemFieldEffect: public QGraphicsEffect {
public:
	ItemFieldEffect(qreal radius);
	void draw(QPainter *painter);
	QRectF boundingRectFor(const QRectF &sourceRect) const;
private:
	qreal _radius;
};

To copy to clipboard, switch view to plain text mode 

itemfieldeffect.cpp

#include "itemfieldeffect.hh"
 
ItemFieldEffect::ItemFieldEffect(qreal radius):
	_radius(radius) {
}
 
void ItemFieldEffect::draw(QPainter *painter) {
	painter->save();
	QRadialGradient grad(0, 0, _radius);
	grad.setSpread(QGradient::PadSpread);
	grad.setColorAt(0, QColor(0, 0, 0, 200));
	grad.setColorAt(1.0, QColor(0, 0, 0, 30));
	painter->setBrush(QBrush(grad));
	painter->setPen(Qt::NoPen);
	painter->drawEllipse(-_radius, -_radius, _radius * 2, _radius * 2);
	painter->restore();
	drawSource(painter);
}
 
QRectF ItemFieldEffect::boundingRectFor(const QRectF &sourceRect) const {
	return sourceRect.adjusted(-_radius, -_radius, _radius, _radius);
}

#include "itemfieldeffect.hh"

ItemFieldEffect::ItemFieldEffect(qreal radius):
	_radius(radius) {
}

void ItemFieldEffect::draw(QPainter *painter) {
	painter->save();
	QRadialGradient grad(0, 0, _radius);
	grad.setSpread(QGradient::PadSpread);
	grad.setColorAt(0, QColor(0, 0, 0, 200));
	grad.setColorAt(1.0, QColor(0, 0, 0, 30));
	painter->setBrush(QBrush(grad));
	painter->setPen(Qt::NoPen);
	painter->drawEllipse(-_radius, -_radius, _radius * 2, _radius * 2);
	painter->restore();
	drawSource(painter);
}

QRectF ItemFieldEffect::boundingRectFor(const QRectF &sourceRect) const {
	return sourceRect.adjusted(-_radius, -_radius, _radius, _radius);
}

To copy to clipboard, switch view to plain text mode 

mainwindow.hh

#include <QtGui>
 
class MainWindow: public QMainWindow {
	Q_OBJECT
public:
	MainWindow(QWidget *parent = 0);
	bool eventFilter(QObject *target, QEvent *event);
	QGraphicsItem *createPoint(const QPointF &pos, const QString &label);
private:
	QGraphicsView *_gv;
	QGraphicsScene *_gs;
	QMap<QString, QGraphicsItem *> _visiblePoints;
 
};

#include <QtGui>

class MainWindow: public QMainWindow {
	Q_OBJECT
public:
	MainWindow(QWidget *parent = 0);
	bool eventFilter(QObject *target, QEvent *event);
	QGraphicsItem *createPoint(const QPointF &pos, const QString &label);
private:
	QGraphicsView *_gv;
	QGraphicsScene *_gs;
	QMap<QString, QGraphicsItem *> _visiblePoints;

};

To copy to clipboard, switch view to plain text mode 

mainwindow.cpp

#include "mainwindow.hh"
#include "itemfieldeffect.hh"
 
MainWindow::MainWindow(QWidget *parent)
	: QMainWindow(parent) {
	_gv = new QGraphicsView(this);
	setCentralWidget(_gv);
	_gs = new QGraphicsScene(this);
	_gv->setScene(_gs);
	_gv->installEventFilter(this);
}
 
QGraphicsItem *MainWindow::createPoint(const QPointF &pos, const QString &label) {
	QGraphicsItemGroup *grp = new QGraphicsItemGroup;
	QGraphicsEllipseItem *bg = new QGraphicsEllipseItem(grp);
	grp->setFlags(QGraphicsItem::ItemIsSelectable);
	bg->setBrush(QColor::fromRgb(255, 244, 194, 220));
	QGraphicsTextItem *lab = new QGraphicsTextItem(grp);
	lab->setPlainText(label);
	bg->setRect(lab->boundingRect());
	qreal x = bg->rect().width() / -2.0;
	qreal y = bg->rect().height() / -2.0;
	bg->setPos(x, y);
	lab->setPos(x, y);
	grp->setPos(pos);
	grp->setGraphicsEffect(new ItemFieldEffect(50));
	return grp;
}
 
bool MainWindow::eventFilter(QObject *target, QEvent *event) {
	if (target == _gv) {
		switch (event->type()) {
		case QEvent::Resize: {
			QResizeEvent *re = static_cast<QResizeEvent *>(event);
			_gv->setSceneRect(0, 0, re->size().width(), re->size().height());
			break;
		}
		case QEvent::MouseButtonPress: {
			QMouseEvent *me = static_cast<QMouseEvent *>(event);
			QPointF pos = me->posF();
			static int counter = 0;
			QString id = QString("%1").arg(++counter);
			QGraphicsItem *it = createPoint(pos, id);
			_gs->addItem(it);
			if (counter % 2)
				_visiblePoints.insert(id, it);
			break;
		}
		case QEvent::KeyPress: {
			QKeyEvent *ke = static_cast<QKeyEvent *>(event);
			if (ke->key() == Qt::Key_Escape) {
				foreach (QString id, _visiblePoints.keys())
					_gs->removeItem(_visiblePoints.value(id));
				_visiblePoints.clear();
			}
			break;
		}
		default: { }
		}
	}
	return false;
}

#include "mainwindow.hh"
#include "itemfieldeffect.hh"

MainWindow::MainWindow(QWidget *parent)
	: QMainWindow(parent) {
	_gv = new QGraphicsView(this);
	setCentralWidget(_gv);
	_gs = new QGraphicsScene(this);
	_gv->setScene(_gs);
	_gv->installEventFilter(this);
}

QGraphicsItem *MainWindow::createPoint(const QPointF &pos, const QString &label) {
	QGraphicsItemGroup *grp = new QGraphicsItemGroup;
	QGraphicsEllipseItem *bg = new QGraphicsEllipseItem(grp);
	grp->setFlags(QGraphicsItem::ItemIsSelectable);
	bg->setBrush(QColor::fromRgb(255, 244, 194, 220));
	QGraphicsTextItem *lab = new QGraphicsTextItem(grp);
	lab->setPlainText(label);
	bg->setRect(lab->boundingRect());
	qreal x = bg->rect().width() / -2.0;
	qreal y = bg->rect().height() / -2.0;
	bg->setPos(x, y);
	lab->setPos(x, y);
	grp->setPos(pos);
	grp->setGraphicsEffect(new ItemFieldEffect(50));
	return grp;
}

bool MainWindow::eventFilter(QObject *target, QEvent *event) {
	if (target == _gv) {
		switch (event->type()) {
		case QEvent::Resize: {
			QResizeEvent *re = static_cast<QResizeEvent *>(event);
			_gv->setSceneRect(0, 0, re->size().width(), re->size().height());
			break;
		}
		case QEvent::MouseButtonPress: {
			QMouseEvent *me = static_cast<QMouseEvent *>(event);
			QPointF pos = me->posF();
			static int counter = 0;
			QString id = QString("%1").arg(++counter);
			QGraphicsItem *it = createPoint(pos, id);
			_gs->addItem(it);
			if (counter % 2)
				_visiblePoints.insert(id, it);
			break;
		}
		case QEvent::KeyPress: {
			QKeyEvent *ke = static_cast<QKeyEvent *>(event);
			if (ke->key() == Qt::Key_Escape) {
				foreach (QString id, _visiblePoints.keys())
					_gs->removeItem(_visiblePoints.value(id));
				_visiblePoints.clear();
			}
			break;
		}
		default: { }
		}
	}
	return false;
}

To copy to clipboard, switch view to plain text mode 

Any feedback appreciated.
Thanks
~Aki

[totem]

I reproduce on WinXP 32 bits Qt 4.7.0
Problem comes from a call to ItemFieldEffect::draw() from event loop, while associated scene pointer of the effect is null. This triggers a Q_ASSERT() in QGraphicsItemEffectSourcePrivate::draw()

If I unset the graphics effect pointer before removing the item from the scene, crash disappears :

bool MainWindow::eventFilter(QObject *target, QEvent *event) {
if (target == _gv) {
	switch (event->type())
	{
		// ... other events
 
		case QEvent::KeyPress:
		{
			QKeyEvent *ke = static_cast<QKeyEvent *>(event);
			if (ke->key() == Qt::Key_Escape)
			{
				foreach (QGraphicsItem *item, _visiblePoints.values())
				{
					item->setGraphicsEffect(0) ; // HERE
					_gs->removeItem(item);
				}
				_visiblePoints.clear();
			}
			break;
		}
		default: { }
	}
}
return false;
}

bool MainWindow::eventFilter(QObject *target, QEvent *event) {
if (target == _gv) {
	switch (event->type())
	{
		// ... other events

		case QEvent::KeyPress:
		{
			QKeyEvent *ke = static_cast<QKeyEvent *>(event);
			if (ke->key() == Qt::Key_Escape)
			{
				foreach (QGraphicsItem *item, _visiblePoints.values())
				{
					item->setGraphicsEffect(0) ; // HERE
					_gs->removeItem(item);
				}
				_visiblePoints.clear();
			}
			break;
		}
		default: { }
	}
}
return false;
}

To copy to clipboard, switch view to plain text mode 

For me, a test is missing somewhere in Qt code to make sure the scene pointer of the QGraphicsEffect object is reliable (at least not null!) before a call to its draw() method.

[akiross]

Ok, thanks for the feedback. I'm sending a feedback to nokia's bugzilla.

[akiross]

Excuse me if I change sightly the topic, but I've another issue here that may be related to this (bug?):
I did as you suggested, resetting the effect before removal, and now it doesn't crash, but even if effects are removed, some items are still visible on the scene. Note that they are removed, but they seems to be buffered and are still painted.
Invalidating or updating the scene didn't help, but if I add new items after removing them, the repaint seems to work.
In other words, try this sequence (or similar): random clicks -> escape -> random clicks elsewhere -> repaint window -> old items are removed.

I had to hack it setting items invisible before removal:

foreach (QString id, _visiblePoints.keys()) {
	_visiblePoints.value(id)->setGraphicsEffect(0);
	_visiblePoints.value(id)->setVisible(false); // This
	_gs->removeItem(_visiblePoints.value(id));
}

foreach (QString id, _visiblePoints.keys()) {
	_visiblePoints.value(id)->setGraphicsEffect(0);
	_visiblePoints.value(id)->setVisible(false); // This
	_gs->removeItem(_visiblePoints.value(id));
}

To copy to clipboard, switch view to plain text mode 

And seems to clear correctly in every case.
~Aki

[totem]

Isn't it because of this ?

if (counter % 2)
	_visiblePoints.insert(id, it);

if (counter % 2)
	_visiblePoints.insert(id, it);

To copy to clipboard, switch view to plain text mode 

you only save 1 pointer on 2.
try without that if statement; and insert graphicsItem each time you create them ?

[akiross]

No no, I was talking about the removed items... I tried to remove that condition anyway, but the problem persists. Also, I experienced successfully the field effect removal, but the item itself was not correctly cleaned.