Search code examples
c++qtqmlscenegraphqsgclipnode

How to use QSGGeometryNode without causing a memory leak and ensure correct clean up


I using a QSGGeometry, QSGVertexColorMaterial & a QSGGeometryNode to draw something real time on my QQuickItem derived class that is MyQuickItem here.

Following is my updatePaintNode method where the crux of the repaint logic lies.

QSGNode * MyQuickItem::updatePaintNode(QSGNode * oldNode, UpdatePaintNodeData * updatePaintNodeData) {

  if (!oldNode) {
    oldNode = new QSGNode;
  }

  oldNode->removeAllChildNodes();

  QSGGeometry * geometry = GetMyGeometry();
  QSGVertexColorMaterial * material = new QSGVertexColorMaterial;
  QSGGeometryNode * child_node = new QSGGeometryNode;

  child_node->setGeometry(geometry);
  child_node->setMaterial(material);
  child_node->setFlag(QSGNode::OwnsMaterial);
  oldNode->appendChildNode(child_node);

  return oldNode;
}

Issue:
Above logic works great. No problem with functionality at all. No performance issues as well. But I worried that I am causing memory leaks. Look at following two lines in the above method updatePaintNode where I allocate raw pointers.

QSGVertexColorMaterial * material = new QSGVertexColorMaterial;
QSGGeometryNode * child_node = new QSGGeometryNode;

I allocate them & I do not delete them. This is because the point where they should be deleted is after updatePaintNode finishes. And thats not under my control.

Question:
How can I ensure that the 2 pointers material & child_node are cleared from memory correctly?
Does doing a child_node->setFlag(QSGNode::OwnsMaterial) like I am doing above sets the ownership of the pointers to QtSceneGraph & relieve me of the burden of deleting the pointers?

Secondary Question:
I am using oldNode->removeAllChildNodes() to clear the data drawn in the previous frame. Is this a good way to clear previous data on screen before painting new data?

PS:
I reiterate: There are no performance issues with this implementation. I just want to be sure that I am not causing any memory leaks. I have tried using material & child_node as smart pointers like so:

auto material = std::make_shared<QSGVertexColorMaterial>();
auto child_node = new std::make_shared<QSGGeometryNode>();

But this causes a crash when material & child_node are auto-cleared from memory at a later point.


Solution

  • Yes, in your example code you can rely on the automatic cleanup of nodes. You are not leaking memory from updatePaintNode.

    oldnode and child_node

    oldnode returned from QQuickItem::updatePaintNode() is automatically deleted on the right thread at the right time. Trees of QSGNode instances are managed through the use of QSGNode::OwnedByParent, which is set by default.

    material

    Because you have set the flag QSGNode::OwnsMaterial for your child_node material is deleted when child_node is deleted.

    Second question: Is this a good way?

    The answer is no. There is no point of creating and deleting nodes every time the scene is rendered. Instead, you should reuse the node/nodes. In the below example code I'm assuming that geometry changes but material doesn't change during the lifetime of the QQuickItem. If the material changes you may need to call node->markDirty(QSGNode::DirtyMaterial). Note that only one node is created, and it's created only once (unless e.g. window hidden and then brought back to fg or something else).

    QSGNode * MyQuickItem::updatePaintNode(QSGNode * oldNode, UpdatePaintNodeData * updatePaintNodeData) {
    
        QSGGeometryNode *node = static_cast<QSGGeometryNode *>(oldNode);
        if (!node) {
            node = new QSGGeometryNode;
    
            QSGVertexColorMaterial * material = new QSGVertexColorMaterial;
            node->setMaterial(material);
            node->setFlag(QSGNode::OwnsMaterial);
        }
    
        // if GetMyGeometry returns every time a new dynamically allocated object then you should
        // call node->setFlag(QSGNode::OwnsGeometry) to not leak memory here:
        QSGGeometry * geometry = GetMyGeometry(); 
        node->setGeometry(geometry);
        // No need to call node->markDirty(QSGNode::DirtyGeometry) because setGeometry is called.
    
        return node;
    }