How can I re-parent a QOpenGLWidget ?

elpidiovaldez5

I have a class, GraphicsPane3d that inherits from QOpenGLWidget and QOpenGLFunctions_3_0. I hope that usage is reasonable ! My code crashes deleting an instance of GraphicsPane3d. I have isolated the problem to a fairly minimal sequence that crashes::

GraphicsPane3d *graphics;
void MainWindow::button1_clicked() {
  graphics = new GraphicsPane3d();
  TopFrame->layout()->addWidget(graphics);
}
void MainWindow::button2_clicked() {
    graphics->setParent(NULL);
    delete graphics;
}

If I do not set the parent to NULL, there is no crash. But I WANT to set the parent to NULL. This effectively removes the object from the screen. In my code the object may subsequently be re-parented in another layout or deleted (actually it has been 'cut' in a design view and then pasted (or not) in a different parent frame). My procedure works for a host of other graphical objects, which makes me think it is specifically an open gl issue.

My questions are:

Is there anything I can do to allow me to safely set the parent to NULL ?

Do I need to free resources or set as current somewhere ?
(I have seen the are warnings for destructor in derived classes of QOpenGLWidget).

GraphicsPane3d is defined as follows:


#include <QOpenGLWidget>
#include <QOpenGLFunctions_3_0>
#include <GL/gl.h>
#include <GL/glu.h>
#include <vector>

class GraphicsPane3d : public QOpenGLWidget, public QOpenGLFunctions_3_0 {

  Q_OBJECT

public:

  GraphicsPane3d(QWidget *parent=NULL) : QOpenGLWidget(parent) {
    setFocusPolicy(Qt::StrongFocus); //The pane can receive keyboard events.

    p = NULL;
  }

  ~GraphicsPane3d() {
      delete p;
      p = NULL;
  }

  GLUquadricObj *p;

  //blah, blah.  A bunch more  member functions....
};

The stack shows:

1 ?? 0x7ffff3f2bfbb
2 QOpenGLFunctions_3_0::~QOpenGLFunctions_3_0() 0x7ffff4245b8c
3 GraphicsPane3d::~GraphicsPane3d GraphicsPane3d.h 30 0x49e0e6
4 GraphicsPane3d::~GraphicsPane3d GraphicsPane3d.h 33 0x49e134
5 DynamicDataFrame::holdWidget DynamicDataFrame.cpp 504 0x45a8bd
6 DynamicDataFrame::contextMenuEvent DynamicDataFrame.cpp 460 0x45a455
... <More>

PS sorry to purists for still using NULL :)

mrjj

Hi
I think its a layout issue ?
You put the openGl in
TopFrame->layout()->addWidget(graphics);

Now layout owns it.
So you can try to ask layout to give back ownership and then try the reparent/set NULL

QLayoutItem *child =layout->takeAt(0);
GraphicsPane3d * minegfx=qobject_cast<GraphicsPane3d * > (child->widget() );
or just use the other pointer
Just to show where the widget is.

elpidiovaldez5

Thank you very much for helping.

I think you are totally right that it is to do with layout. I tested idea with this code:

void MainWindow::button1_clicked() {
  graphics = new GraphicsPane3d();
  graphics->setVisible(true);
  //TopFrame->layout()->addWidget(graphics);
  return;
}

void MainWindow::button2_clicked() {
  graphics->setParent(0);
  delete graphics;
  return;

The graphics object appears (in its own window). Deleting the pointer does not crash.

I then did some more testing with the following code:

void MainWindow::button1_clicked() {
  graphics = new GraphicsPane3d();
  graphics->setVisible(true);
  TopFrame->layout()->addWidget(graphics);
  return;
}

void MainWindow::button2_clicked() {
  int         index  = TopFrame->layout()->indexOf(graphics);
  QObject     *owner = graphics->parent();
  QLayoutItem *child = TopFrame->layout()->takeAt(index);

  owner = graphics->parent();

  graphics->setParent(0);

  owner = graphics->parent();

  delete graphics;
  return;

The sigsegv problem came back. 'owner' does not change throughout the test, and remains as TopFrame.

I tried another idea:

void MainWindow::button2_clicked() {
  int         index  = TopFrame->layout()->indexOf(graphics);
  QObject     *owner = graphics->parent();
  TopFrame->layout()->removeWidget(graphics);

  owner = graphics->parent();

  graphics->setParent(0);

  owner = graphics->parent();

  delete graphics;
  return;

The crash remained and 'owner' still remains as TopFrame.

Finally I tried the above tests with another type of widget (not inheriting from QOpenGLWidget or QOpenGLFunctions_3_0). 'owner' is treated identically - it remains as TopFrame, but there is no crash on deletion.

I still have the feeling that the warning about using makeCurrent in destructors of classes derived from QOpenGLWidget may be a clue.

elpidiovaldez5

I have tracked the bug down to a really minimal piece of code.

#include <QOpenGLWidget>
#include <QOpenGLFunctions_3_0>

class TestGraphicsPane : public QOpenGLWidget, public QOpenGLFunctions_3_0 {

  Q_OBJECT

public:

  TestGraphicsPane(QWidget *parent=NULL) : QOpenGLWidget(parent) {
  }

  void initializeGL() {
    initializeOpenGLFunctions();
  }

};

TestGraphicsPane *graphics;

void MainWindow::on_addObjectButton_clicked() {
  graphics = new TestGraphicsPane();
  TopFrame->layout()->addWidget(graphics);
}

void MainWindow::on_loadLayoutButton_clicked() {
  graphics->setParent(0);
  delete graphics;
}

If I comment out the call to InitializeOpenGLFunctions in TestGraphicsPane:: initializeGL, then the crash no longer occurs.

I conclude that it is the destruction of the OpenGL functions that is the root of the problem (to do with calling order of destructors and the current open GL context ?)

I tried a possible to solution - using the alternative (annoying) usage of QOpenGLFunctions which requires all OpenGL calls to be prefixed with 'glf->'. It seems to me that this approach uses a shared object to contain the OpenGL functions and would avoid the need to for the destructor to be called.

I changed the test class as follows:

class TestGraphicsPane : public QOpenGLWidget {

  Q_OBJECT

public:

  TestGraphicsPane(QWidget *parent=NULL) : QOpenGLWidget(parent) {
  }

  void initializeGL() {
    QOpenGLFunctions_3_0 *glf = QOpenGLContext::currentContext()->versionFunctions<QOpenGLFunctions_3_0>();
    glf->glClearColor(1.0, 0.0, 0.0, 0.0);
  }

  QOpenGLFunctions_3_0 *glf;
};

Wonder of wonders ! It no longer crashes on deleting the pointer.

I believe that inheriting from QOpenGLFunctions_3_0 is acceptable usage as is setting 'parent' to NULL. So is this crash a Qt bug ?

Is there some way to safely deconstruct the inherited OpenGL functions base class without crashing ?

I got a compile error when I tried:

  ~TestGraphicsPane() {
    makeCurrent();
    QOpenGLFunctions_3_0::~QOpenGLFunctions_3_0();

    doneCurrent();
  }

mrjj

Hi
Good digging :)

So if we forget the layout for a moment which should be fixed using TakeAt.

You are saying that
class TestGraphicsPane : public QOpenGLWidget, public QOpenGLFunctions_3_0

can crash on delete.

Can we make it do it small small?

The "OpenGL Window Example"
does it the same way.

Can we get this to crash the same way?

Chris Kawa

After you switch parents you need to show the widget again so that it has a surface to re-create the context on:

void MainWindow::on_loadLayoutButton_clicked() {
  graphics->setParent(0);
  graphics->show(); //or showMinimized() if you want to hide it immediately after and avoid the blink
  delete graphics;
}

If you want to just delete the widget don't switch the parent at all to avoid costly context re-creation:

void MainWindow::on_loadLayoutButton_clicked() {
  delete graphics;
}

Btw. You don't need to take a widget out of a layout when you switch parents. The layout is aware of the situation and stops managing the widget automatically.

mrjj

@Chris-Kawa
Thank you for the knowledge.
so if it is inserted into layout, you will need the extra show() to avoid
context conflict ? Its seems that TakeAt do not alter parent so its
the actual graphics->setParent(0); that create the issue?

Chris Kawa

so its the actual graphics->setParent(0); that create the issue?

Depends how you look at it. Setting parent to null hides the widget. To create a context you need a surface. To have a surface a widget needs to be shown. I'd say no single line creates the issue. It's the combination ;)

I haven't looked at the internals, so I speculate here, but It seems that entire context switching (so also de-initializing the old one) is deferred to the time the widget is shown next time, which is kinda... dangerous? So if you delete it without showing it first again it thinks it is still attached to the old surface. I'd say it should de-init properly when it is hidden on parent switch, but apparently that's not happening.

This looks like a bug, but again, I'm just speculating here.

mrjj

@Chris-Kawa

I'd say no single line creates the issue. It's the combination ;)
That would certainly explain testing it gave surprising results.

Your speculating is far more valid than my random guesses ;) and using your show() trick should
solve the OPs issue. \o/

I dont know enough openGl to say its a bug or the use case of reparenting is not very common and
careful managing of surfaces is always needed. However this also explains some cases with docks where we had
crash re-inserting the openglwidget into a dock when it went from full screen back to windowed.

I didn't see this mentions in the Docs even plenty chance its there as did not fine read it yet. :)

Thx for saving my ass :) we could have posted many tests here before we reached the Surface so to speak :)

elpidiovaldez5

@Chris-Kawa

You are right. I just tested and it is true that adding graphics->show() or showMinimized() after setParent(0) stops delete graphics crashing. That solves my problem !

I do not want to just delete the widget without switching parent so the costly context re-creation is something I will have to put up with. Actually it is not an issue because this code is executed as part of user interaction and the delay is imperceptible.

Just to explain my use case: I have a design view of a user interface. The user has a sort-of 'cut and paste' functionality at the widget level. The problem occurred when an open gl graphics pane was 'cut' and held without a parent in a clipboard type thingummy. Pasting would cause the pane to be re-parented somewhere else in the ui, but if the graphics pane was not subesequently pasted, it would be deleted the next time something was put in the clipboard. I will add code to test if the object being deleted is an open gl graphics pane and, if so, call showMinimized.

@Chris-Kawa
@mrjj

Thank you both for helping me with this problem. I have actually been bothered by it for months and am very glad to have a solution.

elpidiovaldez5

I now have a super-simple solution. - I ust put showMinimized at the start of the graphics pane destructor. It is executed in time to avoid the crash.

Chris Kawa

@elpidiovaldez5 said:

I ust put showMinimized at the start of the graphics pane destructor

Might work here and now but that's generally a very bad idea. Destructors should well.. destruct, not initialize stuff. Doing something as heavy as OpenGL context initialization in a destructor smells trouble.
For example later down the line you might subclass that widget and showXXX() is likely calling some virtual functions. If you happen to override one of those then it's gonna be one hell of a debugging day for you or some other poor soul working with that code.

elpidiovaldez5

Initially I started to put a test in the code before the object is deleted. Something like:

QOpenGLWidget *glObj = dynamic_cast<QOpenGLWidget *>(obj);
if (glObj) {
  glObj->showMinimized();
}
delete obj;

I decided I really did not like this, because I had to include the header for QOpenGLWidget in the header for a class that really should not need it. So putting the call in the destructor seemed to keep everything together in the class that was causing the problem.

Taking your objection into consideration I have come up with an alternative which takes the fix into a separate file and leaves everything else relatively unsullied. Cheers.

extern FixQtProblem();
FixQtProblem();
delete obj;

And the provide a self contained .cpp file:

#include <QOpenGLWidget.h>

void FixQtProblem() {
  QOpenGLWidget *glObj = dynamic_cast<QOpenGLWidget *>(obj);
  if (glObj) {
      glObj->showMinimized();   
  }
}