Deleting QGraphicsItem with QGraphicsEffect leads to segfault

kshegunov

Hello,
A curious conundrum. The problem is you have events for the graphics effect that are pending in the event queue, when you delete the graphics item. When the events are ultimately processed you get a crash as Qt's referencing a dangling pointer, at least that's my theory. To fix it first I'd try calling:

global_rect->setGraphicsEffect(Q_NULLPTR);

just before deleting the actual graphics item.

Kind regards.

tfm123

Thanks for your feedback.

Unfortunately I already had tried that and it does not change anything.

I'd generally like to know whether or not I am using QGraphicsEffect right.

kshegunov

Okay, can you at least confirm or deny that this is the actual problem - comment out the delete and see if a crash occurs?

tfm123

It does not happen when you do one of the following:

do not add a child to MyRect
do not add a QGraphicsEffect
comment out the delete as you suggested

When commenting out the delete I now noticed something else: The white rectangle of MyRect sometimes remains on the screen although I pressed right mouse button. This is curious because it still is removed from the scene.

My guess is, these are the cases that lead to segfault when the delete is in place. The scene somehow thinks MyRect is still present. If it is actually still in memory it's okay and just an artefact on the screen. If it is actually deleted it results in segfault.

kshegunov

@tfm123 said in Deleting QGraphicsItem with QGraphicsEffect leads to segfault:

When commenting out the delete I now noticed something else: The white rectangle of MyRect sometimes remains on the screen although I pressed right mouse button. This is curious because it still is removed from the scene.

Emit the changed() signal after removing the item to notify the view of the change. I think this should fix it. It may even fix the original problem, although it's a very long shot.

tfm123

I'm not quite sure how to do that. I have not been using signals and slots; I'm only using QGraphics. Is there a method on the view or the scene I can call to do this?

Edit: I have tried QGraphicsView::update(), QGraphicsView::invalidateScene(), QGraphicsScene::update() and QGraphicsScene::invalidate(). Nothing changes anything.

kshegunov

class MyScene: public QGraphicsScene
{
    Q_OBJECT //< You need also to add this

public:
    void mouseReleaseEvent(QGraphicsSceneMouseEvent* me) override
    {
        if (me->button()==Qt::LeftButton)  {
            createRect(this);
            emit changed(); //< Emit changed()
        }
        else if (me->button()==Qt::RightButton)  {
            destroyRect(this);
            emit changed();
        }
        // ...
    }
};

PS.
Sorry, the example won't work like this, one needs to pass a number of regions to the signal, but I'm not exactly sure how to obtain them in this case.

Perhaps something like this:

class MyScene: public QGraphicsScene
{
    Q_OBJECT

public:
    void mouseReleaseEvent(QGraphicsSceneMouseEvent* me) override
    {
        // ...
        else if (me->button()==Qt::RightButton)  {
            if (global_rect)  {
                QRectF rect = global_rect->rect();
                scene->removeItem(global_rect);

                emit changed(QList<QRectF>() << rect);

                delete global_rect; //< Still might crash, it's a long shot
                global_rect=nullptr;
            }
        }
        // ...
    }
};

tfm123

Unfortunately it changes nothing. Without delete there are still the artifects, With delete there still is segfault.

Thanks for your help though.

But I'd like to come back to a more basic question: Do you think that my code is correct this way? Or do I use QT in a wrong way?

kshegunov

I have no way to answer that, as I neither have any idea what you're trying to accomplish, nor can I tell if this code is a stripped down example that should reproduce the problem, or is an actual snippet from your project ... or something in between.

tfm123

It is a minimal example written to reproduce the error. It is fully compilable, I even provided the compiler call.

I was hoping someone with experience in QT would be able to tell me whether I am using QT the right way.

Or, to put it in other words, is this kind of error expected behaviour for the example program?

kshegunov

Well I had to run it through the debugger to figure out what's going on. The problem is you add child items in the constructor of the parent item. At that point the scene the item holds a reference to isn't initialized (apparently), so you get a weird event-race of some kind. Anyway, moving new QGraphicsRectItem{this}; away from the constructor of MyRect and into createRect() should fix the issue, at least it did for me.

EDIT: Well it didn't as it appears. Okay back to the debugger.

As for your code (the one posted), there's a few things I don't like:

You use global functions to do internal bookkeeping
You use global variables for no good reason
You use a heap allocation (i.e. in main()) when that's not really warranted, declaring the view and scene on the stack works just as well.

I don't know if that was the feedback you were looking for, but I hope it helps.

EDIT 2:

Well, apparently it some kind of double-deletion problem. Moving all things as members with auto-storage fixes it. Here's the code I tested with:
main.cpp

#include<QApplication>
#include<QGraphicsView>
#include<QScreen>

#include "myscene.h"

int main(int argc, char ** argv)
{
    QApplication qapp(argc, argv);

    QGraphicsView view;
    MyScene scene;

    QRect rect = QGuiApplication::primaryScreen()->geometry();
    scene.setSceneRect(0.0f, 0.0f, rect.width(), rect.height());
    view.setVerticalScrollBarPolicy(Qt::ScrollBarAlwaysOff);
    view.setHorizontalScrollBarPolicy(Qt::ScrollBarAlwaysOff);
    view.setFrameShape(QFrame::NoFrame);
    view.setBackgroundBrush(QBrush(Qt::black, Qt::SolidPattern));
    view.setScene(&scene);
    view.showFullScreen();

    return qapp.exec();
}

myscene.h

#ifndef MYSCENE_H
#define MYSCENE_H

#include <QGraphicsScene>
#include <QGraphicsSceneMouseEvent>
#include <QGraphicsRectItem>
#include <QGraphicsDropShadowEffect>

class MyRect : public QGraphicsRectItem
{
public:
    MyRect(QGraphicsItem * parent = nullptr)
        : QGraphicsRectItem(QRectF{0.0f, 0.0f, 100.0f, 100.0f}, parent)
    {
        setPen(QPen(Qt::white));
        setBrush(QBrush(Qt::white, Qt::SolidPattern));
    }
};

class MyScene: public QGraphicsScene
{
    Q_OBJECT

public:
    MyScene(QObject * parent = Q_NULLPTR)
        : QGraphicsScene(parent), background(0.0f, 0.0f, 500.0f, 500.0f), global_rect(Q_NULLPTR)
    {
        background.setPen(QPen(Qt::white));
        background.setBrush(QBrush(Qt::red, Qt::SolidPattern));
        addItem(&background);

        QGraphicsDropShadowEffect * shadow=new QGraphicsDropShadowEffect();
        shadow->setBlurRadius(15.0f);

        global_rect.setGraphicsEffect(shadow);


        addItem(new QGraphicsRectItem(&global_rect));

    }

    void mouseReleaseEvent(QGraphicsSceneMouseEvent* me) override
    {
        if (me->button()==Qt::LeftButton)
        {
            addItem(&global_rect);
            global_rect.setPos(me->scenePos());
        }
        else if (me->button()==Qt::RightButton)
        {
            removeItem(&global_rect);
        }

        QGraphicsScene::mouseReleaseEvent(me);
    }

    QGraphicsRectItem background;
    MyRect global_rect;
};

#endif // MYSCENE_H

Kind regards.

Chris Kawa

I've seen this problem recently in my colleague's code, and a (curious enough) workaround we found was calling prepareGeometryChange() before the item was removed from the scene. Unfortunately it's protected so you need to expose it e.g.

class MyRect: public QGraphicsRectItem {
public:
    void ouch() { prepareGeometryChange(); }
    ...
};

//and then
    if (global_rect!=nullptr)
    {
        global_rect->ouch(); //call it
        scene->removeItem(global_rect);
        delete global_rect;
        global_rect=nullptr;
    }

Unfortunately I never got to investigate what's going on in depth but it looks like a Qt bug.
A working theory based on the callstack we got was that the crash occurs when the area of the effect crosses the bounds of the internal BSP structure, but the item itself does not. The idea was that the item is removed and that part of the tree it occupied is invalidated but the part that only displayed the effect is not and somehow still somewhere holds a pointer to now deleted object. Calling prepareGeometryChange() would invalidate the whole area (including the area of the effect) and thus prevented the crash.
As I said - it's just a shot in the dark and can be totally off track, so take it with a grain of salt, but it's definitely not an expected behavior and I'd consider reporting it.

tfm123

@Chris-Kawa Thank you a lot for your assessment. Also your suggestion fixes the problem.

I have put the prepareGeometryChange() into MyRect's destructor. I now just delete the item and do not explicitly remove it from the scene. ~QGraphicsItem() does remove itself from the scene and it is called after ~MyRect(). So this is basicly equivalent to what you wrote, but saves the exposure of prepareGeometryChange().

The only thing you cannot do now, is to remove the item from the scene without deleting it. I'm going to file a bug report when i have some time.

Thanks again!

Oleksandr Malyushytskyy

Check if item bounding rectangle increases if QGraphicsDropShadowEffect is on.
If not this is a problem.
This would be also a reason why calling prepareGeometryChange() helped.
I had similar problem for my custom items if bounding rectangle was not set properly.

Oleksandr Malyushytskyy

@tfm123 As a workaround you could call setGraphicsEffect(0). before removing item., but this does not guarantee to fix all problems, the same as approach you used.
Problem may still occur in other usage cases which were not found by you yet. Drawing of the item should be always inside the bounding box it reports.

jsulm

@tfm123 Why do you create a QGraphicsRectItem instance in the body of the constructor?

MyRect(QGraphicsItem* parent = nullptr):
		QGraphicsRectItem{QRectF{0.0f, 0.0f, 100.0f, 100.0f}, parent}
{
    setPen(QPen{Qt::white});
    setBrush(QBrush{Qt::white, Qt::SolidPattern});
    new QGraphicsRectItem{this}; // <-- why this?
}

tfm123

@Oleksandr-Malyushytskyy The boundingRect() of the rectangle does not change when the effect is applied to it. But the effect itself has a boundingRect() which changes when the effect is applied. I don't know if this is how it is supposed to be.

tfm123

@Oleksandr-Malyushytskyy This is one of the first things I tried and unfortunately it does not help.

tfm123

@jsulm Like I already wrote, the issue only triggers when MyRect has a child. This is an example. The only reason for things to be like they are is that they are required to be this way to trigger the issue.

aziesemer

@tfm123

I think this bug is described here:
https://bugreports.qt.io/browse/QTBUG-18021

My workaround was to setVisible(false) before removing the item.