Only update changed 3D entities when editing point cloud attributes

python/PyQt6/core/auto_additions/qgspointcloudlayer.py

@@ -21,8 +21,8 @@
 except (NameError, AttributeError):
     pass
 try:
-    QgsPointCloudLayer.__attribute_docs__ = {'subsetStringChanged': "Emitted when the layer's subset string has changed.\n\n.. versionadded:: 3.26\n", 'raiseError': 'Signals an error related to this point cloud layer.\n\n.. versionadded:: 3.26\n', 'statisticsCalculationStateChanged': 'Emitted when statistics calculation state has changed\n\n.. versionadded:: 3.26\n'}
-    QgsPointCloudLayer.__signal_arguments__ = {'raiseError': ['msg: str'], 'statisticsCalculationStateChanged': ['state: QgsPointCloudLayer.PointCloudStatisticsCalculationState']}
+    QgsPointCloudLayer.__attribute_docs__ = {'subsetStringChanged': "Emitted when the layer's subset string has changed.\n\n.. versionadded:: 3.26\n", 'raiseError': 'Signals an error related to this point cloud layer.\n\n.. versionadded:: 3.26\n', 'statisticsCalculationStateChanged': 'Emitted when statistics calculation state has changed\n\n.. versionadded:: 3.26\n', 'chunkAttributeValuesChanged': 'Emitted when a node gets some attribute values of a node changed\n\n.. versionadded:: 3.42\n'}
+    QgsPointCloudLayer.__signal_arguments__ = {'raiseError': ['msg: str'], 'statisticsCalculationStateChanged': ['state: QgsPointCloudLayer.PointCloudStatisticsCalculationState'], 'chunkAttributeValuesChanged': ['n: QgsPointCloudNodeId']}
     QgsPointCloudLayer.__group__ = ['pointcloud']
 except (NameError, AttributeError):
     pass

python/PyQt6/core/auto_generated/pointcloud/qgspointcloudindex.sip.in

@@ -368,6 +368,11 @@ an error message in case the call failed.
     bool isModified() const;
 %Docstring
 Returns ``True`` if there are uncommitted changes, ``False`` otherwise
+%End
+
+    QList<QgsPointCloudNodeId> updatedNodes() const;
+%Docstring
+Returns a list of node IDs that have been modified
 %End
 
 };

python/PyQt6/core/auto_generated/pointcloud/qgspointcloudlayer.sip.in

@@ -305,6 +305,13 @@ Signals an error related to this point cloud layer.
 Emitted when statistics calculation state has changed
 
 .. versionadded:: 3.26
+%End
+
+    void chunkAttributeValuesChanged( const QgsPointCloudNodeId &n );
+%Docstring
+Emitted when a node gets some attribute values of a node changed
+
+.. versionadded:: 3.42
 %End
 
   private:

python/core/auto_additions/qgspointcloudlayer.py

@@ -21,8 +21,8 @@
 except (NameError, AttributeError):
     pass
 try:
-    QgsPointCloudLayer.__attribute_docs__ = {'subsetStringChanged': "Emitted when the layer's subset string has changed.\n\n.. versionadded:: 3.26\n", 'raiseError': 'Signals an error related to this point cloud layer.\n\n.. versionadded:: 3.26\n', 'statisticsCalculationStateChanged': 'Emitted when statistics calculation state has changed\n\n.. versionadded:: 3.26\n'}
-    QgsPointCloudLayer.__signal_arguments__ = {'raiseError': ['msg: str'], 'statisticsCalculationStateChanged': ['state: QgsPointCloudLayer.PointCloudStatisticsCalculationState']}
+    QgsPointCloudLayer.__attribute_docs__ = {'subsetStringChanged': "Emitted when the layer's subset string has changed.\n\n.. versionadded:: 3.26\n", 'raiseError': 'Signals an error related to this point cloud layer.\n\n.. versionadded:: 3.26\n', 'statisticsCalculationStateChanged': 'Emitted when statistics calculation state has changed\n\n.. versionadded:: 3.26\n', 'chunkAttributeValuesChanged': 'Emitted when a node gets some attribute values of a node changed\n\n.. versionadded:: 3.42\n'}
+    QgsPointCloudLayer.__signal_arguments__ = {'raiseError': ['msg: str'], 'statisticsCalculationStateChanged': ['state: QgsPointCloudLayer.PointCloudStatisticsCalculationState'], 'chunkAttributeValuesChanged': ['n: QgsPointCloudNodeId']}
     QgsPointCloudLayer.__group__ = ['pointcloud']
 except (NameError, AttributeError):
     pass

python/core/auto_generated/pointcloud/qgspointcloudindex.sip.in

@@ -368,6 +368,11 @@ an error message in case the call failed.
     bool isModified() const;
 %Docstring
 Returns ``True`` if there are uncommitted changes, ``False`` otherwise
+%End
+
+    QList<QgsPointCloudNodeId> updatedNodes() const;
+%Docstring
+Returns a list of node IDs that have been modified
 %End
 
 };

python/core/auto_generated/pointcloud/qgspointcloudlayer.sip.in

@@ -305,6 +305,13 @@ Signals an error related to this point cloud layer.
 Emitted when statistics calculation state has changed
 
 .. versionadded:: 3.26
+%End
+
+    void chunkAttributeValuesChanged( const QgsPointCloudNodeId &n );
+%Docstring
+Emitted when a node gets some attribute values of a node changed
+
+.. versionadded:: 3.42
 %End
 
   private:

src/3d/chunks/qgschunkedentity.cpp

@@ -612,9 +612,10 @@ void QgsChunkedEntity::onActiveJobFinished()
 
   QgsChunkNode *node = job->chunk();
 
-  if ( QgsChunkLoader *loader = qobject_cast<QgsChunkLoader *>( job ) )
+  if ( node->state() == QgsChunkNode::Loading )
   {
-    Q_ASSERT( node->state() == QgsChunkNode::Loading );
+    QgsChunkLoader *loader = qobject_cast<QgsChunkLoader *>( job );
+    Q_ASSERT( loader );
     Q_ASSERT( node->loader() == loader );
 
     QgsEventTracing::addEvent( QgsEventTracing::AsyncEnd, QStringLiteral( "3D" ), QStringLiteral( "Load " ) + node->tileId().text(), node->tileId().text() );
@@ -653,6 +654,18 @@ void QgsChunkedEntity::onActiveJobFinished()
   else
   {
     Q_ASSERT( node->state() == QgsChunkNode::Updating );
+
+    // This is a special case when we're replacing the node's entity
+    // with QgsChunkUpdaterFactory passed to updatedNodes(). The returned
+    // updater is actually a chunk loader that will give us a completely
+    // new QEntity, so we just delete the old one and use the new one
+    if ( QgsChunkLoader *nodeUpdater = qobject_cast<QgsChunkLoader *>( node->updater() ) )
+    {
+      Qt3DCore::QEntity *newEntity = nodeUpdater->createEntity( this );
+      node->replaceEntity( newEntity );
+      emit newEntityCreated( newEntity );
+    }
+
     QgsEventTracing::addEvent( QgsEventTracing::AsyncEnd, QStringLiteral( "3D" ), QStringLiteral( "Update" ), node->tileId().text() );
     node->setUpdated();
   }

src/3d/chunks/qgschunkloader.h

@@ -144,6 +144,31 @@ class _3D_EXPORT QgsQuadtreeChunkLoaderFactory : public QgsChunkLoaderFactory
     int mMaxLevel = 0;
 };
 
+/**
+ * \ingroup 3d
+ * Factory that uses a chunk loader factory for in-place updates
+ * of loaded nodes. Use it with QgsChunkedEntity::updateNodes()
+ * to rebuild entity of an existing node.
+ *
+ * \since QGIS 3.42
+ */
+class QgsChunkUpdaterFactory : public QgsChunkQueueJobFactory
+{
+  public:
+    QgsChunkUpdaterFactory( QgsChunkLoaderFactory *loaderFactory )
+      : mChunkLoaderFactory( loaderFactory )
+    {
+    }
+
+    QgsChunkQueueJob *createJob( QgsChunkNode *chunk ) override
+    {
+      return mChunkLoaderFactory->createChunkLoader( chunk );
+    }
+
+  private:
+    QgsChunkLoaderFactory *mChunkLoaderFactory;
+};
+
 /// @endcond
 
 #endif // QGSCHUNKLOADER_H

src/3d/chunks/qgschunknode.cpp

@@ -238,6 +238,17 @@ void QgsChunkNode::setUpdated()
   mState = QgsChunkNode::Loaded;
 }
 
+void QgsChunkNode::replaceEntity( Qt3DCore::QEntity *newEntity )
+{
+  Q_ASSERT( mState == QgsChunkNode::Updating );
+  Q_ASSERT( mUpdater );
+  Q_ASSERT( mEntity );
+  Q_ASSERT( newEntity );
+
+  mEntity->deleteLater();
+  mEntity = newEntity;
+}
+
 void QgsChunkNode::setExactBox3D( const QgsBox3D &box3D )
 {
   mBox3D = box3D;

src/3d/chunks/qgschunknode.h

@@ -239,6 +239,9 @@ class QgsChunkNode
     //! mark node that it finished updating - back to loaded node
     void setUpdated();
 
+    //! replaces an existing entity with a newly created one (only allowed when updating the node)
+    void replaceEntity( Qt3DCore::QEntity *newEntity );
+
     //! called when the true bounding box is known so that we can use tighter bounding box
     void setExactBox3D( const QgsBox3D &box3D );
 

src/3d/qgs3dmapscene.cpp

@@ -713,7 +713,6 @@ void Qgs3DMapScene::addLayerEntity( QgsMapLayer *layer )
     QgsPointCloudLayer *pclayer = qobject_cast<QgsPointCloudLayer *>( layer );
     connect( pclayer, &QgsPointCloudLayer::renderer3DChanged, this, &Qgs3DMapScene::onLayerRenderer3DChanged );
     connect( pclayer, &QgsPointCloudLayer::subsetStringChanged, this, &Qgs3DMapScene::onLayerRenderer3DChanged );
-    connect( pclayer, &QgsPointCloudLayer::layerModified, this, &Qgs3DMapScene::onLayerRenderer3DChanged );
   }
 }
 

src/3d/qgspointcloudlayer3drenderer.cpp

@@ -162,7 +162,7 @@ Qt3DCore::QEntity *QgsPointCloudLayer3DRenderer::createEntity( Qgs3DMapSettings
   Qt3DCore::QEntity *entity = nullptr;
   if ( pcl->index() )
   {
-    entity = new QgsPointCloudLayerChunkedEntity( map, pcl->index(), coordinateTransform, dynamic_cast<QgsPointCloud3DSymbol *>( mSymbol->clone() ), static_cast<float>( maximumScreenError() ), showBoundingBoxes(), static_cast<const QgsPointCloudLayerElevationProperties *>( pcl->elevationProperties() )->zScale(), static_cast<const QgsPointCloudLayerElevationProperties *>( pcl->elevationProperties() )->zOffset(), mPointBudget );
+    entity = new QgsPointCloudLayerChunkedEntity( map, pcl, pcl->index(), coordinateTransform, dynamic_cast<QgsPointCloud3DSymbol *>( mSymbol->clone() ), static_cast<float>( maximumScreenError() ), showBoundingBoxes(), static_cast<const QgsPointCloudLayerElevationProperties *>( pcl->elevationProperties() )->zScale(), static_cast<const QgsPointCloudLayerElevationProperties *>( pcl->elevationProperties() )->zOffset(), mPointBudget );
   }
   else if ( !pcl->dataProvider()->subIndexes().isEmpty() )
   {

src/3d/qgspointcloudlayerchunkloader_p.cpp

@@ -238,10 +238,60 @@ QVector<QgsChunkNode *> QgsPointCloudLayerChunkLoaderFactory::createChildren( Qg
 ///////////////
 
 
-QgsPointCloudLayerChunkedEntity::QgsPointCloudLayerChunkedEntity( Qgs3DMapSettings *map, QgsPointCloudIndex pc, const QgsCoordinateTransform &coordinateTransform, QgsPointCloud3DSymbol *symbol, float maximumScreenSpaceError, bool showBoundingBoxes, double zValueScale, double zValueOffset, int pointBudget )
-  : QgsChunkedEntity( map, maximumScreenSpaceError, new QgsPointCloudLayerChunkLoaderFactory( Qgs3DRenderContext::fromMapSettings( map ), coordinateTransform, pc, symbol, zValueScale, zValueOffset, pointBudget ), true, pointBudget )
+static QgsChunkNode *findChunkNodeFromNodeId( QgsChunkNode *rootNode, QgsPointCloudNodeId nodeId )
+{
+  // find path from the node to the root
+  QVector<QgsPointCloudNodeId> parentIds;
+  while ( nodeId.d() > 0 )
+  {
+    parentIds << nodeId;
+    nodeId = nodeId.parentNode();
+  }
+
+  // now descend from the root to the node in the QgsChunkNode hierarchy
+  QgsChunkNode *chunk = rootNode;
+  while ( !parentIds.empty() )
+  {
+    QgsPointCloudNodeId p = parentIds.takeLast();
+    QgsChunkNodeId childNodeId( p.d(), p.x(), p.y(), p.z() );
+
+    QgsChunkNode *chunkChild = nullptr;
+    QgsChunkNode *const *children = chunk->children();
+    for ( int i = 0; i < chunk->childCount(); ++i )
+    {
+      if ( children[i]->tileId() == childNodeId )
+      {
+        chunkChild = children[i];
+        break;
+      }
+    }
+    Q_ASSERT( chunkChild );
+    chunk = chunkChild;
+  }
+  return chunk;
+}
+
+
+QgsPointCloudLayerChunkedEntity::QgsPointCloudLayerChunkedEntity( Qgs3DMapSettings *map, QgsPointCloudLayer *pcl, QgsPointCloudIndex index, const QgsCoordinateTransform &coordinateTransform, QgsPointCloud3DSymbol *symbol, float maximumScreenSpaceError, bool showBoundingBoxes, double zValueScale, double zValueOffset, int pointBudget )
+  : QgsChunkedEntity( map, maximumScreenSpaceError, new QgsPointCloudLayerChunkLoaderFactory( Qgs3DRenderContext::fromMapSettings( map ), coordinateTransform, index, symbol, zValueScale, zValueOffset, pointBudget ), true, pointBudget )
+  , mLayer( pcl )
 {
   setShowBoundingBoxes( showBoundingBoxes );
+
+  if ( pcl->supportsEditing() )
+  {
+    // when editing starts or stops, we need to update our index to use the editing index (or not)
+    connect( pcl, &QgsPointCloudLayer::editingStarted, this, &QgsPointCloudLayerChunkedEntity::updateIndex );
+    connect( pcl, &QgsPointCloudLayer::editingStopped, this, &QgsPointCloudLayerChunkedEntity::updateIndex );
+
+    mChunkUpdaterFactory.reset( new QgsChunkUpdaterFactory( mChunkLoaderFactory ) );
+
+    connect( pcl, &QgsPointCloudLayer::chunkAttributeValuesChanged, this, [this]( const QgsPointCloudNodeId &n ) {
+      QList<QgsChunkNode *> nodes;
+      nodes << findChunkNodeFromNodeId( mRootNode, n );
+      updateNodes( nodes, mChunkUpdaterFactory.get() );
+    } );
+  }
 }
 
 QgsPointCloudLayerChunkedEntity::~QgsPointCloudLayerChunkedEntity()
@@ -250,6 +300,11 @@ QgsPointCloudLayerChunkedEntity::~QgsPointCloudLayerChunkedEntity()
   cancelActiveJobs();
 }
 
+void QgsPointCloudLayerChunkedEntity::updateIndex()
+{
+  static_cast<QgsPointCloudLayerChunkLoaderFactory *>( mChunkLoaderFactory )->mPointCloudIndex = mLayer->index();
+}
+
 QVector<QgsRayCastingUtils::RayHit> QgsPointCloudLayerChunkedEntity::rayIntersection( const QgsRayCastingUtils::Ray3D &ray, const QgsRayCastingUtils::RayCastContext &context ) const
 {
   QVector<QgsRayCastingUtils::RayHit> result;

src/3d/qgspointcloudlayerchunkloader_p.h

@@ -127,11 +127,18 @@ class QgsPointCloudLayerChunkedEntity : public QgsChunkedEntity
 {
     Q_OBJECT
   public:
-    explicit QgsPointCloudLayerChunkedEntity( Qgs3DMapSettings *map, QgsPointCloudIndex pc, const QgsCoordinateTransform &coordinateTransform, QgsPointCloud3DSymbol *symbol, float maxScreenError, bool showBoundingBoxes, double zValueScale, double zValueOffset, int pointBudget );
+    explicit QgsPointCloudLayerChunkedEntity( Qgs3DMapSettings *map, QgsPointCloudLayer *pcl, QgsPointCloudIndex index, const QgsCoordinateTransform &coordinateTransform, QgsPointCloud3DSymbol *symbol, float maxScreenError, bool showBoundingBoxes, double zValueScale, double zValueOffset, int pointBudget );
 
     QVector<QgsRayCastingUtils::RayHit> rayIntersection( const QgsRayCastingUtils::Ray3D &ray, const QgsRayCastingUtils::RayCastContext &context ) const override;
 
     ~QgsPointCloudLayerChunkedEntity();
+
+  private slots:
+    void updateIndex();
+
+  private:
+    QgsPointCloudLayer *mLayer = nullptr;
+    std::unique_ptr<QgsChunkUpdaterFactory> mChunkUpdaterFactory;
 };
 
 /// @endcond

src/3d/qgsvirtualpointcloudentity_p.cpp

@@ -61,6 +61,7 @@ QgsVirtualPointCloudEntity::QgsVirtualPointCloudEntity(
   {
     mOverviewEntity = new QgsPointCloudLayerChunkedEntity(
       mapSettings(),
+      mLayer,
       provider()->overview(),
       mCoordinateTransform,
       dynamic_cast<QgsPointCloud3DSymbol *>( mSymbol->clone() ),
@@ -106,6 +107,7 @@ void QgsVirtualPointCloudEntity::createChunkedEntityForSubIndex( int i )
 
   QgsPointCloudLayerChunkedEntity *newChunkedEntity = new QgsPointCloudLayerChunkedEntity(
     mapSettings(),
+    mLayer,
     si.index(),
     mCoordinateTransform,
     static_cast<QgsPointCloud3DSymbol *>( mSymbol->clone() ),

src/core/pointcloud/qgspointcloudeditingindex.cpp

@@ -190,6 +190,11 @@ bool QgsPointCloudEditingIndex::isModified() const
   return !mEditedNodeData.isEmpty();
 }
 
+QList<QgsPointCloudNodeId> QgsPointCloudEditingIndex::updatedNodes() const
+{
+  return mEditedNodeData.keys();
+}
+
 bool QgsPointCloudEditingIndex::updateNodeData( const QHash<QgsPointCloudNodeId, QByteArray> &data )
 {
   for ( auto it = data.constBegin(); it != data.constEnd(); ++it )

src/core/pointcloud/qgspointcloudeditingindex.h

@@ -66,6 +66,9 @@ class CORE_EXPORT QgsPointCloudEditingIndex : public QgsAbstractPointCloudIndex
     //! Returns TRUE if there are uncommitted changes, FALSE otherwise
     bool isModified() const;
 
+    //! Returns a list of node IDs that have been modified
+    QList<QgsPointCloudNodeId> updatedNodes() const;
+
 
   private:
     QgsPointCloudIndex mIndex;

src/core/pointcloud/qgspointcloudindex.cpp

@@ -521,3 +521,10 @@ bool QgsPointCloudIndex::isModified() const
   return false;
 }
 
+QList<QgsPointCloudNodeId> QgsPointCloudIndex::updatedNodes() const
+{
+  if ( QgsPointCloudEditingIndex *index = dynamic_cast<QgsPointCloudEditingIndex *>( mIndex.get() ) )
+    return index->updatedNodes();
+
+  return QList<QgsPointCloudNodeId>();
+}

src/core/pointcloud/qgspointcloudindex.h

@@ -655,6 +655,9 @@ class CORE_EXPORT QgsPointCloudIndex SIP_NODEFAULTCTORS
     //! Returns TRUE if there are uncommitted changes, FALSE otherwise
     bool isModified() const;
 
+    //! Returns a list of node IDs that have been modified
+    QList<QgsPointCloudNodeId> updatedNodes() const;
+
   private:
     std::shared_ptr<QgsAbstractPointCloudIndex> mIndex;
 

src/core/pointcloud/qgspointcloudlayer.cpp

@@ -1012,10 +1012,12 @@ bool QgsPointCloudLayer::commitChanges( bool stopEditing )
     if ( !mEditIndex.commitChanges( &mCommitError ) )
       return false;
 
-    emit layerModified();
-    triggerRepaint();
+    // emitting layerModified() is not required as that's done automatically
+    // when undo stack index changes
   }
 
+  undoStack()->clear();
+
   if ( stopEditing )
   {
     mEditIndex = QgsPointCloudIndex();
@@ -1037,11 +1039,22 @@ bool QgsPointCloudLayer::rollBack()
   if ( !mEditIndex )
     return false;
 
+  const QList<QgsPointCloudNodeId> updatedNodes = mEditIndex.updatedNodes();
+
   undoStack()->clear();
 
   mEditIndex = QgsPointCloudIndex();
   emit editingStopped();
 
+  if ( !updatedNodes.isEmpty() )
+  {
+    for ( const QgsPointCloudNodeId &n : updatedNodes )
+      emit chunkAttributeValuesChanged( n );
+
+    // emitting layerModified() is not required as that's done automatically
+    // when undo stack index changes
+  }
+
   return true;
 }
 
@@ -1112,7 +1125,7 @@ bool QgsPointCloudLayer::changeAttributeValue( const QgsPointCloudNodeId &n, con
        sortedPoints.constLast() >= mEditIndex.getNode( n ).pointCount() )
     return false;
 
-  undoStack()->push( new QgsPointCloudLayerUndoCommandChangeAttribute( mEditIndex, n, sortedPoints, attribute, value ) );
+  undoStack()->push( new QgsPointCloudLayerUndoCommandChangeAttribute( this, n, sortedPoints, attribute, value ) );
 
   return true;
 }

src/core/pointcloud/qgspointcloudlayer.h

@@ -351,6 +351,13 @@ class CORE_EXPORT QgsPointCloudLayer : public QgsMapLayer, public QgsAbstractPro
      */
     void statisticsCalculationStateChanged( QgsPointCloudLayer::PointCloudStatisticsCalculationState state );
 
+    /**
+     * Emitted when a node gets some attribute values of a node changed
+     *
+     * \since QGIS 3.42
+     */
+    void chunkAttributeValuesChanged( const QgsPointCloudNodeId &n );
+
   private slots:
     void onPointCloudIndexGenerationStateChanged( QgsPointCloudDataProvider::PointCloudIndexGenerationState state );
     void setDataSourcePrivate( const QString &dataSource, const QString &baseName, const QString &provider, const QgsDataProvider::ProviderOptions &options, Qgis::DataProviderReadFlags flags ) override;