From 3cb55822d0f407aec6832812bc97786efacffec4 Mon Sep 17 00:00:00 2001 From: uclaros Date: Tue, 7 Jan 2025 13:35:19 +0200 Subject: [PATCH] Added undo command classes for point cloud editing --- src/core/CMakeLists.txt | 2 + src/core/pointcloud/qgspointcloudlayer.cpp | 52 +++++++++--- .../qgspointcloudlayereditutils.cpp | 67 +--------------- .../pointcloud/qgspointcloudlayereditutils.h | 18 +---- .../qgspointcloudlayerundocommand.cpp | 79 +++++++++++++++++++ .../qgspointcloudlayerundocommand.h | 57 +++++++++++++ tests/src/core/testqgspointcloudediting.cpp | 21 ++++- 7 files changed, 202 insertions(+), 94 deletions(-) create mode 100644 src/core/pointcloud/qgspointcloudlayerundocommand.cpp create mode 100644 src/core/pointcloud/qgspointcloudlayerundocommand.h diff --git a/src/core/CMakeLists.txt b/src/core/CMakeLists.txt index c9d7fa662a2d..625f2694beeb 100644 --- a/src/core/CMakeLists.txt +++ b/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 diff --git a/src/core/pointcloud/qgspointcloudlayer.cpp b/src/core/pointcloud/qgspointcloudlayer.cpp index 74da1df3688a..51d86bbd3540 100644 --- a/src/core/pointcloud/qgspointcloudlayer.cpp +++ b/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, [ = ] { triggerRepaint(); } ); } QgsPointCloudLayer::~QgsPointCloudLayer() @@ -1026,11 +1029,7 @@ bool QgsPointCloudLayer::rollBack() if ( !mEditIndex ) return false; - if ( isModified() ) - { - emit layerModified(); - triggerRepaint(); - } + undoStack()->clear(); mEditIndex = QgsPointCloudIndex(); emit editingStopped(); @@ -1062,21 +1061,52 @@ bool QgsPointCloudLayer::isModified() const return mEditIndex.isModified(); } -bool QgsPointCloudLayer::changeAttributeValue( const QgsPointCloudNodeId &n, const QVector &pts, const QgsPointCloudAttribute &attribute, double value ) +bool QgsPointCloudLayer::changeAttributeValue( const QgsPointCloudNodeId &n, const QVector &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(); + return false; + } + + if ( !QgsPointCloudLayerEditUtils::isAttributeValueValid( attribute, value ) ) + { + return false; } - return success; + QVector 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 diff --git a/src/core/pointcloud/qgspointcloudlayereditutils.cpp b/src/core/pointcloud/qgspointcloudlayereditutils.cpp index 28085388aa7a..57eb97e71551 100644 --- a/src/core/pointcloud/qgspointcloudlayereditutils.cpp +++ b/src/core/pointcloud/qgspointcloudlayereditutils.cpp @@ -14,72 +14,10 @@ ***************************************************************************/ #include "qgspointcloudlayereditutils.h" -#include "qgspointcloudlayer.h" -#include "qgslazdecoder.h" +#include "qgspointcloudattribute.h" +#include "qgspointcloudrequest.h" -QgsPointCloudLayerEditUtils::QgsPointCloudLayerEditUtils( QgsPointCloudLayer *layer ) - : mIndex( layer->index() ) -{ -} - -bool QgsPointCloudLayerEditUtils::changeAttributeValue( const QgsPointCloudNodeId &n, const QVector &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 uniquePoints( pts.constBegin(), pts.constEnd() ); - QVector sortedPoints( uniquePoints.constBegin(), uniquePoints.constEnd() ); - std::sort( sortedPoints.begin(), sortedPoints.end() ); - - if ( sortedPoints.constFirst() < 0 || - sortedPoints.constLast() > mIndex.getNode( n ).pointCount() ) - return false; - - QgsPointCloudRequest req; - req.setAttributes( attributeCollection ); - - std::unique_ptr block = mIndex.nodeData( n, req ); - const int count = block->pointCount(); - const int recordSize = attributeCollection.pointRecordSize(); - - // copy data - QByteArray data( block->data(), count * recordSize ); - - char *ptr = data.data(); - - for ( int i : sortedPoints ) - { - // replace attribute for selected point - lazStoreDoubleToStream( ptr, i * recordSize + attributeOffset, attribute.type(), value ); - } - - return mIndex.updateNodeData( {{n, data}} );; -} - QByteArray QgsPointCloudLayerEditUtils::dataForAttributes( const QgsPointCloudAttributeCollection &allAttributes, const QByteArray &data, const QgsPointCloudRequest &request ) { const QVector attributes = allAttributes.attributes(); @@ -99,7 +37,6 @@ QByteArray QgsPointCloudLayerEditUtils::dataForAttributes( const QgsPointCloudAt } } - // Q_ASSERT( nPoints == outData.size() / request.attributes().pointRecordSize() ); return outData; diff --git a/src/core/pointcloud/qgspointcloudlayereditutils.h b/src/core/pointcloud/qgspointcloudlayereditutils.h index 4aabb116f727..7d2822fb6664 100644 --- a/src/core/pointcloud/qgspointcloudlayereditutils.h +++ b/src/core/pointcloud/qgspointcloudlayereditutils.h @@ -17,14 +17,12 @@ #define QGSPOINTCLOUDLAYEREDITUTILS_H #include "qgis_core.h" -#include "qgspointcloudindex.h" #include #include #define SIP_NO_FILE -class QgsPointCloudLayer; class QgsPointCloudNodeId; class QgsPointCloudAttribute; class QgsPointCloudAttributeCollection; @@ -43,27 +41,13 @@ 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 &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: - QgsPointCloudIndex mIndex; }; #endif // QGSPOINTCLOUDLAYEREDITUTILS_H diff --git a/src/core/pointcloud/qgspointcloudlayerundocommand.cpp b/src/core/pointcloud/qgspointcloudlayerundocommand.cpp new file mode 100644 index 000000000000..1a6639a92194 --- /dev/null +++ b/src/core/pointcloud/qgspointcloudlayerundocommand.cpp @@ -0,0 +1,79 @@ +/*************************************************************************** + 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 "qgslazdecoder.h" + + +QgsPointCloudLayerUndoCommand::QgsPointCloudLayerUndoCommand( QgsPointCloudIndex index ) + : mIndex( index ) +{} + +QgsPointCloudLayerUndoCommandChangeAttribute::QgsPointCloudLayerUndoCommandChangeAttribute( QgsPointCloudIndex index, const QgsPointCloudNodeId &n, const QVector &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 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 ); + mPoints.append( qMakePair( point, oldValue ) ); + } +} + +void QgsPointCloudLayerUndoCommandChangeAttribute::undo() +{ + undoRedoPrivate( true ); +} + +void QgsPointCloudLayerUndoCommandChangeAttribute::redo() +{ + undoRedoPrivate( false ); +} + +void QgsPointCloudLayerUndoCommandChangeAttribute::undoRedoPrivate( bool isUndo ) +{ + const QgsPointCloudAttributeCollection allAttributes = mIndex.attributes(); + + QgsPointCloudRequest req; + req.setAttributes( allAttributes ); + + std::unique_ptr block = mIndex.nodeData( mNode, req ); + const int count = block->pointCount(); + const int recordSize = block->pointRecordSize(); + + // copy data + QByteArray data( block->data(), count * recordSize ); + + char *ptr = data.data(); + + for ( const auto &pair : std::as_const( mPoints ) ) + { + // replace attribute for selected point + const double value = isUndo ? pair.second : mNewValue; + lazStoreDoubleToStream( ptr, pair.first * recordSize + mAttributeOffset, mAttribute.type(), value ); + } + + mIndex.updateNodeData( {{mNode, data}} );; +} diff --git a/src/core/pointcloud/qgspointcloudlayerundocommand.h b/src/core/pointcloud/qgspointcloudlayerundocommand.h new file mode 100644 index 000000000000..dd3b15e6ddbf --- /dev/null +++ b/src/core/pointcloud/qgspointcloudlayerundocommand.h @@ -0,0 +1,57 @@ +/*************************************************************************** + qgspointcloudlayerundocommand.h + --------------------- + 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. * + * * + ***************************************************************************/ + +#ifndef QGSPOINTCLOUDLAYERUNDOCOMMAND_H +#define QGSPOINTCLOUDLAYERUNDOCOMMAND_H + +#include "qgis_core.h" +#include "qgspointcloudindex.h" +#include "qgspointcloudattribute.h" + +#include + +class CORE_EXPORT QgsPointCloudLayerUndoCommand : public QUndoCommand +{ + public: + QgsPointCloudLayerUndoCommand( QgsPointCloudIndex index ); + + protected: + QgsPointCloudIndex mIndex; +}; + +class CORE_EXPORT QgsPointCloudLayerUndoCommandChangeAttribute : public QgsPointCloudLayerUndoCommand +{ + public: + + /** + * Constructor for QgsPointCloudLayerUndoCommandChangeAttribute + * \param buffer associated edit buffer + * \param f feature to add to layer + */ + QgsPointCloudLayerUndoCommandChangeAttribute( QgsPointCloudIndex index, const QgsPointCloudNodeId &n, const QVector &points, const QgsPointCloudAttribute &attribute, double value ); + + void undo() override; + void redo() override; + + private: + void undoRedoPrivate( bool isUndo ); + + QgsPointCloudNodeId mNode; + QVector< QPair< int, double > > mPoints; // contains pairs of (point number, old value) + QgsPointCloudAttribute mAttribute; + int mAttributeOffset; + double mNewValue; +}; +#endif // QGSPOINTCLOUDLAYERUNDOCOMMAND_H diff --git a/tests/src/core/testqgspointcloudediting.cpp b/tests/src/core/testqgspointcloudediting.cpp index c160c9951ace..3681c8dc4517 100644 --- a/tests/src/core/testqgspointcloudediting.cpp +++ b/tests/src/core/testqgspointcloudediting.cpp @@ -183,6 +183,7 @@ void TestQgsPointCloudEditing::testModifyAttributeValue() QVERIFY( layer->startEditing() ); QVERIFY( layer->isEditable() ); + QCOMPARE( layer->undoStack()->index(), 0 ); // Change some points, point order should not matter QgsPointCloudAttribute at( QStringLiteral( "Classification" ), QgsPointCloudAttribute::UChar ); @@ -190,19 +191,34 @@ void TestQgsPointCloudEditing::testModifyAttributeValue() QVERIFY( layer->changeAttributeValue( n, { 4, 2, 0, 1, 3, 16, 5, 13, 15, 14 }, at, 1 ) ); QVERIFY( layer->isModified() ); QCOMPARE( spy.size(), 1 ); + QCOMPARE( layer->undoStack()->index(), 1 ); QGSVERIFYRENDERMAPSETTINGSCHECK( "classified_render_edit_1", "classified_render_edit_1", mapSettings ); // Change some more QVERIFY( layer->changeAttributeValue( n, { 42, 82, 62, 52, 72 }, at, 6 ) ); QVERIFY( layer->isModified() ); QCOMPARE( spy.size(), 2 ); + QCOMPARE( layer->undoStack()->index(), 2 ); + QGSVERIFYRENDERMAPSETTINGSCHECK( "classified_render_edit_2", "classified_render_edit_2", mapSettings ); + + // Undo one edit + layer->undoStack()->undo(); + QCOMPARE( spy.size(), 3 ); + QCOMPARE( layer->undoStack()->index(), 1 ); + QGSVERIFYRENDERMAPSETTINGSCHECK( "classified_render_edit_1", "classified_render_edit_1", mapSettings ); + + // Redo edit + layer->undoStack()->redo(); + QCOMPARE( spy.size(), 4 ); + QCOMPARE( layer->undoStack()->index(), 2 ); QGSVERIFYRENDERMAPSETTINGSCHECK( "classified_render_edit_2", "classified_render_edit_2", mapSettings ); // Abort editing, original points should be rendered QVERIFY( layer->rollBack() ); QVERIFY( !layer->isEditable() ); QVERIFY( !layer->isModified() ); - QCOMPARE( spy.size(), 3 ); + QCOMPARE( spy.size(), 5 ); + QCOMPARE( layer->undoStack()->index(), 0 ); QGSVERIFYRENDERMAPSETTINGSCHECK( "classified_render", "classified_render", mapSettings ); } @@ -214,6 +230,7 @@ void TestQgsPointCloudEditing::testModifyAttributeValueInvalid() QVERIFY( layer->isValid() ); QVERIFY( layer->startEditing() ); QVERIFY( layer->isEditable() ); + QCOMPARE( layer->undoStack()->index(), 0 ); QSignalSpy spy( layer.get(), &QgsMapLayer::layerModified ); @@ -407,6 +424,8 @@ void TestQgsPointCloudEditing::testModifyAttributeValueInvalid() QVERIFY( !layer->changeAttributeValue( n, { 42 }, at, 65536 ) ); QVERIFY( !layer->isModified() ); QCOMPARE( spy.size(), 0 ); + + QCOMPARE( layer->undoStack()->index(), 0 ); } QGSTEST_MAIN( TestQgsPointCloudEditing )