Add undo command classes for point cloud editing (#60111)

* Added undo command for point cloud editing change attribute

* review fixes

src/app/3d/qgs3dmaptoolpointcloudchangeattribute.cpp

@@ -140,10 +140,12 @@ void Qgs3DMapToolPointCloudChangeAttribute::run()
   int offset;
   const QgsPointCloudAttribute *attribute = pcLayer->attributes().find( mAttributeName, offset );
 
+  pcLayer->undoStack()->beginMacro( tr( "Change attribute values" ) );
   for ( auto it = sel.begin(); it != sel.end(); ++it )
   {
     pcLayer->changeAttributeValue( it.key(), it.value(), *attribute, mNewValue );
   }
+  pcLayer->undoStack()->endMacro();
 }
 
 void Qgs3DMapToolPointCloudChangeAttribute::restart()

src/core/CMakeLists.txt

@@ -875,6 +875,7 @@ set(QGIS_CORE_SRCS
   pointcloud/qgspointcloudrendererregistry.cpp
   pointcloud/qgspointcloudrgbrenderer.cpp
   pointcloud/qgspointcloudlayerexporter.cpp
+  pointcloud/qgspointcloudlayerundocommand.cpp
 
   pointcloud/expression/qgspointcloudexpression.cpp
   pointcloud/expression/qgspointcloudexpressionnode.cpp
@@ -1742,6 +1743,7 @@ set(QGIS_CORE_HDRS
   pointcloud/qgspointcloudrendererregistry.h
   pointcloud/qgspointcloudrgbrenderer.h
   pointcloud/qgspointcloudlayerexporter.h
+  pointcloud/qgspointcloudlayerundocommand.h
 
   pointcloud/expression/qgspointcloudexpression.h
   pointcloud/expression/qgspointcloudexpressionnode.h

src/core/pointcloud/qgscopcpointcloudindex.h

@@ -124,6 +124,7 @@ class CORE_EXPORT QgsCopcPointCloudIndex: public QgsAbstractPointCloudIndex
 
     friend class QgsPointCloudLayerEditUtils;
     friend class QgsPointCloudEditingIndex;
+    friend class QgsPointCloudLayerUndoCommandChangeAttribute;
 };
 
 ///@endcond

src/core/pointcloud/qgslazdecoder.cpp

@@ -186,11 +186,6 @@ bool lazSerialize_( char *data, size_t outputPosition, QgsPointCloudAttribute::D
   return true;
 }
 
-bool lazStoreDoubleToStream( char *s, size_t position, QgsPointCloudAttribute::DataType type, double value )
-{
-  return lazStoreToStream_<double>( s, position, type, value );
-}
-
 // //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
 
 std::vector< QgsLazDecoder::RequestedAttributeDetails > prepareRequestedAttributeDetails_( const QgsPointCloudAttributeCollection &requestedAttributes, QVector<QgsLazInfo::ExtraBytesAttributeDetails> &extrabytesAttr )

src/core/pointcloud/qgslazdecoder.h

@@ -37,7 +37,6 @@ template <typename T>
 bool lazStoreToStream_( char *s, size_t position, QgsPointCloudAttribute::DataType type, T value );
 bool lazSerialize_( char *data, size_t outputPosition, QgsPointCloudAttribute::DataType outputType,
                     const char *input, QgsPointCloudAttribute::DataType inputType, int inputSize, size_t inputPosition );
-bool lazStoreDoubleToStream( char *s, size_t position, QgsPointCloudAttribute::DataType type, double value );
 
 class QgsLazDecoder
 {

src/core/pointcloud/qgspointcloudeditingindex.h

@@ -73,6 +73,7 @@ class CORE_EXPORT QgsPointCloudEditingIndex : public QgsAbstractPointCloudIndex
     QHash<QgsPointCloudNodeId, QByteArray> mEditedNodeData;
 
     friend class QgsPointCloudLayerEditUtils;
+    friend class QgsPointCloudLayerUndoCommandChangeAttribute;
 };
 
 #endif // QGSPOINTCLOUDEDITINGINDEX_H

src/core/pointcloud/qgspointcloudlayer.cpp

@@ -43,6 +43,7 @@
 #include "qgstaskmanager.h"
 #include "qgsthreadingutils.h"
 #include "qgspointcloudlayerprofilegenerator.h"
+#include "qgspointcloudlayerundocommand.h"
 #ifdef HAVE_COPC
 #include "qgscopcpointcloudindex.h"
 #endif
@@ -73,6 +74,8 @@ QgsPointCloudLayer::QgsPointCloudLayer( const QString &uri,
 
   setLegend( QgsMapLayerLegend::defaultPointCloudLegend( this ) );
   connect( this, &QgsPointCloudLayer::subsetStringChanged, this, &QgsMapLayer::configChanged );
+  connect( undoStack(), &QUndoStack::indexChanged, this, &QgsMapLayer::layerModified );
+  connect( this, &QgsMapLayer::layerModified, this, [this] { triggerRepaint(); } );
 }
 
 QgsPointCloudLayer::~QgsPointCloudLayer()
@@ -1034,11 +1037,7 @@ bool QgsPointCloudLayer::rollBack()
   if ( !mEditIndex )
     return false;
 
-  if ( isModified() )
-  {
-    emit layerModified();
-    triggerRepaint();
-  }
+  undoStack()->clear();
 
   mEditIndex = QgsPointCloudIndex();
   emit editingStopped();
@@ -1070,23 +1069,52 @@ bool QgsPointCloudLayer::isModified() const
   return mEditIndex.isModified();
 }
 
-bool QgsPointCloudLayer::changeAttributeValue( const QgsPointCloudNodeId &n, const QVector<int> &pts, const QgsPointCloudAttribute &attribute, double value )
+bool QgsPointCloudLayer::changeAttributeValue( const QgsPointCloudNodeId &n, const QVector<int> &points, const QgsPointCloudAttribute &attribute, double value )
 {
   QGIS_PROTECT_QOBJECT_THREAD_ACCESS
   if ( !mEditIndex )
     return false;
 
-  QgsPointCloudLayerEditUtils utils( this );
+  // Cannot allow x,y,z editing as points may get moved outside the node extents
+  if ( attribute.name().compare( QLatin1String( "X" ), Qt::CaseInsensitive ) == 0 ||
+       attribute.name().compare( QLatin1String( "Y" ), Qt::CaseInsensitive ) == 0 ||
+       attribute.name().compare( QLatin1String( "Z" ), Qt::CaseInsensitive ) == 0 )
+    return false;
+
+  if ( !n.isValid() || !mEditIndex.hasNode( n ) ) // todo: should not have to check if n.isValid
+    return false;
+
+  if ( points.isEmpty() )
+    return false;
+
+  const QgsPointCloudAttributeCollection attributeCollection = mEditIndex.attributes();
+
+  int attributeOffset;
+  const QgsPointCloudAttribute *at = attributeCollection.find( attribute.name(), attributeOffset );
 
-  const bool success = utils.changeAttributeValue( n, pts, attribute, value );
-  if ( success )
+  if ( !at ||
+       at->size() != attribute.size() ||
+       at->type() != attribute.type() )
   {
-    emit layerModified();
-    emit triggerRepaint();
-    emit trigger3DUpdate();
+    return false;
+  }
+
+  if ( !QgsPointCloudLayerEditUtils::isAttributeValueValid( attribute, value ) )
+  {
+    return false;
   }
 
-  return success;
+  QVector<int> sortedPoints( points.constBegin(), points.constEnd() );
+  std::sort( sortedPoints.begin(), sortedPoints.end() );
+  sortedPoints.erase( std::unique( sortedPoints.begin(), sortedPoints.end() ), sortedPoints.end() );
+
+  if ( sortedPoints.constFirst() < 0 ||
+       sortedPoints.constLast() >= mEditIndex.getNode( n ).pointCount() )
+    return false;
+
+  undoStack()->push( new QgsPointCloudLayerUndoCommandChangeAttribute( mEditIndex, n, sortedPoints, attribute, value ) );
+
+  return true;
 }
 
 QgsPointCloudIndex QgsPointCloudLayer::index() const

src/core/pointcloud/qgspointcloudlayereditutils.cpp

@@ -14,76 +14,13 @@
  ***************************************************************************/
 
 #include "qgspointcloudlayereditutils.h"
-#include "qgspointcloudlayer.h"
 #include "qgslazdecoder.h"
 #include "qgscopcpointcloudindex.h"
-#include "qgspointcloudeditingindex.h"
 
 #include <lazperf/readers.hpp>
 #include <lazperf/writers.hpp>
 
 
-QgsPointCloudLayerEditUtils::QgsPointCloudLayerEditUtils( QgsPointCloudLayer *layer )
-  : mIndex( layer->index() )
-{
-}
-
-bool QgsPointCloudLayerEditUtils::changeAttributeValue( const QgsPointCloudNodeId &n, const QVector<int> &pts, const QgsPointCloudAttribute &attribute, double value )
-{
-  // Cannot allow x,y,z editing as points may get moved outside the node extents
-  if ( attribute.name().compare( QLatin1String( "X" ), Qt::CaseInsensitive ) == 0 ||
-       attribute.name().compare( QLatin1String( "Y" ), Qt::CaseInsensitive ) == 0 ||
-       attribute.name().compare( QLatin1String( "Z" ), Qt::CaseInsensitive ) == 0 )
-    return false;
-
-  if ( !n.isValid() || !mIndex.hasNode( n ) ) // todo: should not have to check if n.isValid
-    return false;
-
-  const QgsPointCloudAttributeCollection attributeCollection = mIndex.attributes();
-
-  int attributeOffset;
-  const QgsPointCloudAttribute *at = attributeCollection.find( attribute.name(), attributeOffset );
-
-  if ( !at ||
-       at->size() != attribute.size() ||
-       at->type() != attribute.type() )
-  {
-    return false;
-  }
-
-  if ( !isAttributeValueValid( attribute, value ) )
-  {
-    return false;
-  }
-
-  const QSet<int> uniquePoints( pts.constBegin(), pts.constEnd() );
-  QVector<int> sortedPoints( uniquePoints.constBegin(), uniquePoints.constEnd() );
-  std::sort( sortedPoints.begin(), sortedPoints.end() );
-
-  if ( sortedPoints.constFirst() < 0 ||
-       sortedPoints.constLast() > mIndex.getNode( n ).pointCount() )
-    return false;
-
-  QgsPointCloudEditingIndex *editIndex = static_cast<QgsPointCloudEditingIndex *>( mIndex.get() );
-  QgsCopcPointCloudIndex *copcIndex = static_cast<QgsCopcPointCloudIndex *>( editIndex->mIndex.get() );
-
-  QByteArray chunkData;
-  if ( editIndex->mEditedNodeData.contains( n ) )
-  {
-    chunkData = editIndex->mEditedNodeData[n];
-  }
-  else
-  {
-    QPair<uint64_t, int32_t> offsetSizePair = copcIndex->mHierarchyNodePos[n];
-    chunkData = copcIndex->readRange( offsetSizePair.first, offsetSizePair.second );
-  }
-
-  QByteArray data = updateChunkValues( copcIndex, chunkData, *at, value, n, pts );
-
-  return mIndex.updateNodeData( {{n, data}} );
-}
-
-
 static void updatePoint( char *pointBuffer, int pointFormat, const QString &attributeName, double newValue )
 {
   if ( attributeName == QLatin1String( "Intensity" ) )  // unsigned short
@@ -187,7 +124,7 @@ static void updatePoint( char *pointBuffer, int pointFormat, const QString &attr
 }
 
 
-QByteArray QgsPointCloudLayerEditUtils::updateChunkValues( QgsCopcPointCloudIndex *copcIndex, const QByteArray &chunkData, const QgsPointCloudAttribute &attribute, double newValue, const QgsPointCloudNodeId &n, const QVector<int> &pointIndices )
+QByteArray QgsPointCloudLayerEditUtils::updateChunkValues( QgsCopcPointCloudIndex *copcIndex, const QByteArray &chunkData, const QgsPointCloudAttribute &attribute, const QgsPointCloudNodeId &n, const QHash<int, double> &pointValues, std::optional<double> newValue )
 {
   Q_ASSERT( copcIndex->mHierarchy.contains( n ) );
   Q_ASSERT( copcIndex->mHierarchyNodePos.contains( n ) );
@@ -204,19 +141,17 @@ QByteArray QgsPointCloudLayerEditUtils::updateChunkValues( QgsCopcPointCloudInde
   // only PDRF 6/7/8 is allowed by COPC
   Q_ASSERT( header.pointFormat() == 6 || header.pointFormat() == 7 || header.pointFormat() == 8 );
 
-  QSet<int> pointIndicesSet( pointIndices.constBegin(), pointIndices.constEnd() );
-
   QString attributeName = attribute.name();
 
   for ( int i = 0 ; i < pointCount; ++i )
   {
     decompressor.decompress( decodedData.get() );
     char *buf = decodedData.get();
 
-    if ( pointIndicesSet.contains( i ) )
+    if ( pointValues.contains( i ) )
     {
       // TODO: support for extrabytes attributes
-      updatePoint( buf, header.point_format_id, attributeName, newValue );
+      updatePoint( buf, header.point_format_id, attributeName, newValue ? *newValue : pointValues[i] );
     }
 
     compressor.compress( decodedData.get() );

src/core/pointcloud/qgspointcloudlayereditutils.h

@@ -19,6 +19,8 @@
 #include "qgis_core.h"
 #include "qgspointcloudindex.h"
 
+#include <optional>
+
 #include <QVector>
 #include <QByteArray>
 
@@ -45,31 +47,17 @@ class CORE_EXPORT QgsPointCloudLayerEditUtils
 {
   public:
     //! Ctor
-    QgsPointCloudLayerEditUtils( QgsPointCloudLayer *layer );
-
-    /**
-     * Attempts to modify attribute values for specific points in the editing buffer.
-     *
-     * \param n The point cloud node containing the points
-     * \param points The point ids of the points to be modified
-     * \param attribute The attribute whose value will be updated
-     * \param value The new value to set to the attribute
-     * \return TRUE if the editing buffer was updated successfully, FALSE otherwise
-     */
-    bool changeAttributeValue( const QgsPointCloudNodeId &n, const QVector<int> &points, const QgsPointCloudAttribute &attribute, double value );
+    QgsPointCloudLayerEditUtils() = delete;
 
     //! Takes \a data comprising of \a allAttributes and returns a QByteArray with data only for the attributes included in the \a request
     static QByteArray dataForAttributes( const QgsPointCloudAttributeCollection &allAttributes, const QByteArray &data, const QgsPointCloudRequest &request );
 
     //! Check if \a value is within proper range for the \a attribute
     static bool isAttributeValueValid( const QgsPointCloudAttribute &attribute, double value );
 
-  private:
-
     //! Sets new classification value for the given points in voxel and return updated chunk data
-    QByteArray updateChunkValues( QgsCopcPointCloudIndex *copcIndex, const QByteArray &chunkData, const QgsPointCloudAttribute &attribute, double newClassValue, const QgsPointCloudNodeId &n, const QVector<int> &pointIndices );
+    static QByteArray updateChunkValues( QgsCopcPointCloudIndex *copcIndex, const QByteArray &chunkData, const QgsPointCloudAttribute &attribute, const QgsPointCloudNodeId &n, const QHash<int, double> &pointValues, std::optional<double> newValue = std::nullopt );
 
-    QgsPointCloudIndex mIndex;
 };
 
 #endif // QGSPOINTCLOUDLAYEREDITUTILS_H

src/core/pointcloud/qgspointcloudlayerundocommand.cpp

@@ -0,0 +1,84 @@
+/***************************************************************************
+    qgspointcloudlayerundocommand.cpp
+    ---------------------
+    begin                : January 2025
+    copyright            : (C) 2025 by Stefanos Natsis
+    email                : uclaros at gmail dot com
+ ***************************************************************************
+ *                                                                         *
+ *   This program is free software; you can redistribute it and/or modify  *
+ *   it under the terms of the GNU General Public License as published by  *
+ *   the Free Software Foundation; either version 2 of the License, or     *
+ *   (at your option) any later version.                                   *
+ *                                                                         *
+ ***************************************************************************/
+
+#include "qgspointcloudlayerundocommand.h"
+#include "qgspointcloudeditingindex.h"
+#include "qgscopcpointcloudindex.h"
+#include "qgspointcloudlayereditutils.h"
+
+
+QgsPointCloudLayerUndoCommand::QgsPointCloudLayerUndoCommand( QgsPointCloudIndex index )
+  : mIndex( index )
+{}
+
+QgsPointCloudLayerUndoCommandChangeAttribute::QgsPointCloudLayerUndoCommandChangeAttribute( QgsPointCloudIndex index, const QgsPointCloudNodeId &n, const QVector<int> &points, const QgsPointCloudAttribute &attribute, double value )
+  : QgsPointCloudLayerUndoCommand( index )
+  , mNode( n )
+  , mAttribute( attribute )
+  , mNewValue( value )
+{
+  const QgsPointCloudAttributeCollection allAttributes = mIndex.attributes();
+  QgsPointCloudRequest req;
+  req.setAttributes( allAttributes );
+  std::unique_ptr<QgsPointCloudBlock> block = mIndex.nodeData( n, req );
+  const char *ptr = block->data();
+  block->attributes().find( attribute.name(), mAttributeOffset );
+  const int size = block->pointRecordSize();
+  for ( const int point : points )
+  {
+    const int offset = point * size + mAttributeOffset;
+    const double oldValue = attribute.convertValueToDouble( ptr + offset );
+    mPointValues[point] = oldValue;
+  }
+}
+
+void QgsPointCloudLayerUndoCommandChangeAttribute::undo()
+{
+  undoRedoPrivate( true );
+}
+
+void QgsPointCloudLayerUndoCommandChangeAttribute::redo()
+{
+  undoRedoPrivate( false );
+}
+
+void QgsPointCloudLayerUndoCommandChangeAttribute::undoRedoPrivate( bool isUndo )
+{
+  QgsPointCloudEditingIndex *editIndex = static_cast<QgsPointCloudEditingIndex *>( mIndex.get() );
+  QgsCopcPointCloudIndex *copcIndex = static_cast<QgsCopcPointCloudIndex *>( editIndex->mIndex.get() );
+
+  QByteArray chunkData;
+  if ( editIndex->mEditedNodeData.contains( mNode ) )
+  {
+    chunkData = editIndex->mEditedNodeData[mNode];
+  }
+  else
+  {
+    QPair<uint64_t, int32_t> offsetSizePair = copcIndex->mHierarchyNodePos[mNode];
+    chunkData = copcIndex->readRange( offsetSizePair.first, offsetSizePair.second );
+  }
+
+  QByteArray data;
+  if ( isUndo )
+  {
+    data = QgsPointCloudLayerEditUtils::updateChunkValues( copcIndex, chunkData, mAttribute, mNode, mPointValues );
+  }
+  else
+  {
+    data = QgsPointCloudLayerEditUtils::updateChunkValues( copcIndex, chunkData, mAttribute, mNode, mPointValues, mNewValue );
+  }
+
+  mIndex.updateNodeData( {{mNode, data}} );
+}