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

Question

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.

Bynum

Answer 1

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;
}

Condensate