review fixes

src/core/pointcloud/qgscopcpointcloudindex.cpp

@@ -524,9 +524,7 @@ void QgsCopcPointCloudIndex::reset()
   mRootBounds = QgsBox3D();
   mAttributes = QgsPointCloudAttributeCollection();
   mSpan = 0;
-  //mFilterExpression
   mError.clear();
-  //mUri
 
   // QgsCopcPointCloudIndex
   mIsValid = false;

src/core/pointcloud/qgscopcupdate.cpp

@@ -82,9 +82,8 @@ HierarchyEntries getHierarchyPage( std::ifstream &file, uint64_t offset, uint64_
 }
 
 
-bool QgsCopcUpdate::write( QString outputFilename, const QHash<QgsPointCloudNodeId, UpdatedChunk> &updatedChunks )
+bool QgsCopcUpdate::write( const QString &outputFilename, const QHash<QgsPointCloudNodeId, UpdatedChunk> &updatedChunks )
 {
-
   std::ofstream m_f;
   m_f.open( QgsLazDecoder::toNativePath( outputFilename ), std::ios::out | std::ios::binary );
 
@@ -113,15 +112,15 @@ bool QgsCopcUpdate::write( QString outputFilename, const QHash<QgsPointCloudNode
   for ( lazperf::chunk ch : mChunks )
   {
     Q_ASSERT( mOffsetToVoxel.contains( currentChunkOffset ) );
-    QgsPointCloudNodeId k = mOffsetToVoxel[currentChunkOffset];
+    QgsPointCloudNodeId n = mOffsetToVoxel[currentChunkOffset];
 
     uint64_t newOffset = m_f.tellp();
-    voxelToNewOffset[k] = newOffset;
+    voxelToNewOffset[n] = newOffset;
 
     // check whether the chunk is modified
-    if ( updatedChunks.contains( k ) )
+    if ( updatedChunks.contains( n ) )
     {
-      const UpdatedChunk &updatedChunk = updatedChunks[k];
+      const UpdatedChunk &updatedChunk = updatedChunks[n];
 
       // use updated one and skip in the original file
       mFile.seekg( static_cast<long>( mFile.tellg() ) + static_cast<long>( ch.offset ) );
@@ -146,10 +145,10 @@ bool QgsCopcUpdate::write( QString outputFilename, const QHash<QgsPointCloudNode
 
   // write chunk table: size in bytes + point count of each chunk
 
-  uint64_t newChunkTableOffset = m_f.tellp();
+  const uint64_t newChunkTableOffset = m_f.tellp();
 
   m_f.write( "\0\0\0\0", 4 ); // chunk table version
-  m_f.write( ( char * )&mChunkCount, sizeof( mChunkCount ) );
+  m_f.write( reinterpret_cast<const char *>( &mChunkCount ), sizeof( mChunkCount ) );
 
   lazperf::OutFileStream outStream( m_f );
   lazperf::compress_chunk_table( outStream.cb(), mChunks, true );
@@ -160,10 +159,10 @@ bool QgsCopcUpdate::write( QString outputFilename, const QHash<QgsPointCloudNode
   // are packed one after another, with no gaps. if that's not the case, things
   // will break apart
 
-  long hierPositionShift = static_cast<long>( m_f.tellp() ) + 60 - static_cast<long>( mHierarchyOffset );
+  const long hierPositionShift = static_cast<long>( m_f.tellp() ) + 60 - static_cast<long>( mHierarchyOffset );
 
-  HierarchyEntry *oldCopcHierarchyBlobEntries = ( HierarchyEntry * ) mHierarchyBlob.data();
-  int nEntries = static_cast<int>( mHierarchyBlob.size() / 32 );
+  HierarchyEntry *oldCopcHierarchyBlobEntries = reinterpret_cast<HierarchyEntry *>( mHierarchyBlob.data() );
+  const int nEntries = static_cast<int>( mHierarchyBlob.size() / 32 );
   for ( int i = 0; i < nEntries; ++i )
   {
     HierarchyEntry &e = oldCopcHierarchyBlobEntries[i];
@@ -193,7 +192,7 @@ bool QgsCopcUpdate::write( QString outputFilename, const QHash<QgsPointCloudNode
 
   // write hierarchy eVLR
 
-  uint64_t newEvlrOffset = m_f.tellp();
+  const uint64_t newEvlrOffset = m_f.tellp();
 
   lazperf::evlr_header outCopcHierEvlr;
   outCopcHierEvlr.reserved = 0;
@@ -219,28 +218,28 @@ bool QgsCopcUpdate::write( QString outputFilename, const QHash<QgsPointCloudNode
   // patch header
 
   m_f.seekp( 235 );
-  m_f.write( ( const char * )&newEvlrOffset, 8 );
+  m_f.write( reinterpret_cast<const char *>( &newEvlrOffset ), 8 );
 
-  uint64_t newRootHierOffset = mCopcVlr.root_hier_offset + hierPositionShift;
+  const uint64_t newRootHierOffset = mCopcVlr.root_hier_offset + hierPositionShift;
   m_f.seekp( 469 );
-  m_f.write( ( const char * )&newRootHierOffset, 8 );
+  m_f.write( reinterpret_cast<const char *>( &newRootHierOffset ), 8 );
 
   m_f.seekp( mHeader.point_offset );
-  m_f.write( ( const char * )&newChunkTableOffset, 8 );
+  m_f.write( reinterpret_cast<const char *>( &newChunkTableOffset ), 8 );
 
   return true;
 }
 
 
 
-bool QgsCopcUpdate::read( QString inputFilename )
+bool QgsCopcUpdate::read( const QString &inputFilename )
 {
   mInputFilename = inputFilename;
 
   mFile.open( QgsLazDecoder::toNativePath( inputFilename ), std::ios::binary | std::ios::in );
   if ( mFile.fail() )
   {
-    mErrorMessage = "Could not open file for reading: " + inputFilename;
+    mErrorMessage = QStringLiteral( "Could not open file for reading: %1" ).arg( inputFilename );
     return false;
   }
 
@@ -260,7 +259,7 @@ bool QgsCopcUpdate::readHeader()
   mHeader = lazperf::header14::create( mFile );
   if ( !mFile )
   {
-    mErrorMessage = "Error reading COPC header";
+    mErrorMessage = QStringLiteral( "Error reading COPC header" );
     return false;
   }
 
@@ -270,7 +269,7 @@ bool QgsCopcUpdate::readHeader()
   int baseCount = lazperf::baseCount( mHeader.point_format_id );
   if ( baseCount == 0 )
   {
-    mErrorMessage = QString( "Bad point record format: %1" ).arg( mHeader.point_format_id );
+    mErrorMessage = QStringLiteral( "Bad point record format: %1" ).arg( mHeader.point_format_id );
     return false;
   }
 
@@ -283,9 +282,9 @@ void QgsCopcUpdate::readChunkTable()
   uint64_t chunkTableOffset;
 
   mFile.seekg( mHeader.point_offset );
-  mFile.read( ( char * )&chunkTableOffset, sizeof( chunkTableOffset ) );
+  mFile.read( reinterpret_cast<char *>( &chunkTableOffset ), sizeof( chunkTableOffset ) );
   mFile.seekg( static_cast<long>( chunkTableOffset ) + 4 ); // The first 4 bytes are the version, then the chunk count.
-  mFile.read( ( char * )&mChunkCount, sizeof( mChunkCount ) );
+  mFile.read( reinterpret_cast<char *>( &mChunkCount ), sizeof( mChunkCount ) );
 
   //
   // read chunk table
@@ -312,11 +311,15 @@ void QgsCopcUpdate::readChunkTable()
 
 void QgsCopcUpdate::readHierarchy()
 {
-
   // get all hierarchy pages
 
   HierarchyEntries childEntriesToProcess;
-  childEntriesToProcess.push_back( HierarchyEntry{ QgsPointCloudNodeId( 0, 0, 0, 0 ), mCopcVlr.root_hier_offset, ( int32_t )mCopcVlr.root_hier_size, -1 } );
+  childEntriesToProcess.push_back( HierarchyEntry
+  {
+    QgsPointCloudNodeId( 0, 0, 0, 0 ),
+    mCopcVlr.root_hier_offset,
+    static_cast<int32_t>( mCopcVlr.root_hier_size ),
+    -1 } );
 
   while ( !childEntriesToProcess.empty() )
   {

src/core/pointcloud/qgscopcupdate.h

@@ -17,12 +17,11 @@
 #define QGSCOPCUPDATE_H
 
 #include "qgis_core.h"
+#include "qgspointcloudindex.h"
 
 #include <lazperf/header.hpp>
 #include <lazperf/vlr.hpp>
 
-#include "qgspointcloudindex.h"
-
 #define SIP_NO_FILE
 
 
@@ -50,10 +49,10 @@ class CORE_EXPORT QgsCopcUpdate
     };
 
     //! Reads input COPC file and initializes all the members
-    bool read( QString inputFilename );
+    bool read( const QString &inputFilename );
 
     //! Writes a COPC file with updated chunks
-    bool write( QString outputFilename, const QHash<QgsPointCloudNodeId, UpdatedChunk> &updatedChunks );
+    bool write( const QString &outputFilename, const QHash<QgsPointCloudNodeId, UpdatedChunk> &updatedChunks );
 
     //! Returns error message
     QString errorMessage() const { return mErrorMessage; }
@@ -81,7 +80,7 @@ class CORE_EXPORT QgsCopcUpdate
     lazperf::header14 mHeader;
     lazperf::copc_info_vlr mCopcVlr;
     std::vector<lazperf::chunk> mChunks;
-    uint32_t mChunkCount;
+    uint32_t mChunkCount = 0;
     uint64_t mHierarchyOffset = 0;
     std::vector<char> mHierarchyBlob;
     std::vector<lazperf::evlr_header> mEvlrHeaders;

src/core/pointcloud/qgspointcloudeditingindex.cpp

@@ -21,7 +21,8 @@
 #include "qgscopcupdate.h"
 #include "qgslazdecoder.h"
 
-#include <QTemporaryDir>
+#include <QDir>
+#include <QFileInfo>
 
 
 QgsPointCloudEditingIndex::QgsPointCloudEditingIndex( QgsPointCloudLayer *layer )
@@ -139,7 +140,7 @@ bool QgsPointCloudEditingIndex::commitChanges( QString *errorMessage )
   }
 
   QFileInfo fileInfo( mUri );
-  QString outputFilename = fileInfo.dir().filePath( fileInfo.baseName() + "-update.copc.laz" );
+  const QString outputFilename = fileInfo.dir().filePath( fileInfo.baseName() + QStringLiteral( "-update.copc.laz" ) );
 
   if ( !QgsCopcUpdate::writeUpdatedFile( mUri, outputFilename, updatedChunks, errorMessage ) )
   {
@@ -150,19 +151,19 @@ bool QgsPointCloudEditingIndex::commitChanges( QString *errorMessage )
   QgsCopcPointCloudIndex *copcIndex = static_cast<QgsCopcPointCloudIndex *>( mIndex.get() );
   copcIndex->reset();
 
-  QString originalFilename = fileInfo.dir().filePath( fileInfo.baseName() + "-original.copc.laz" );
+  const QString originalFilename = fileInfo.dir().filePath( fileInfo.baseName() + QStringLiteral( "-original.copc.laz" ) );
   if ( !QFile::rename( mUri, originalFilename ) )
   {
     if ( errorMessage )
-      *errorMessage = "Rename of the old COPC failed!";
+      *errorMessage = QStringLiteral( "Rename of the old COPC failed!" );
     QFile::remove( outputFilename );
     return false;
   }
 
   if ( !QFile::rename( outputFilename, mUri ) )
   {
     if ( errorMessage )
-      *errorMessage = "Rename of the new COPC failed!";
+      *errorMessage = QStringLiteral( "Rename of the new COPC failed!" );
     QFile::rename( originalFilename, mUri );
     QFile::remove( outputFilename );
     return false;
@@ -171,7 +172,7 @@ bool QgsPointCloudEditingIndex::commitChanges( QString *errorMessage )
   if ( !QFile::remove( originalFilename ) )
   {
     if ( errorMessage )
-      *errorMessage = "Removal of the old COPC failed!";
+      *errorMessage = QStringLiteral( "Removal of the old COPC failed!" );
     // TODO: cleanup here as well?
     return false;
   }
@@ -196,16 +197,13 @@ bool QgsPointCloudEditingIndex::updateNodeData( const QHash<QgsPointCloudNodeId,
     mEditedNodeData[it.key()] = it.value();
   }
 
-  const QList<QgsPointCloudNodeId> modifiedNodesIds = data.keys();
-  QSet<QgsPointCloudNodeId> modifiedNodesSet( modifiedNodesIds.constBegin(), modifiedNodesIds.constEnd() );
-
   // get rid of cached keys that got modified
   {
     QMutexLocker locker( &sBlockCacheMutex );
     const QList<QgsPointCloudCacheKey> cacheKeys = sBlockCache.keys();
-    for ( QgsPointCloudCacheKey cacheKey : cacheKeys )
+    for ( const QgsPointCloudCacheKey &cacheKey : cacheKeys )
     {
-      if ( cacheKey.uri() == mUri && modifiedNodesSet.contains( cacheKey.node() ) )
+      if ( cacheKey.uri() == mUri && data.contains( cacheKey.node() ) )
         sBlockCache.remove( cacheKey );
     }
   }

src/core/pointcloud/qgspointcloudindex.h

@@ -659,8 +659,6 @@ class CORE_EXPORT QgsPointCloudIndex SIP_NODEFAULTCTORS
     std::shared_ptr<QgsAbstractPointCloudIndex> mIndex;
 
     friend class TestQgsPointCloudEditing;
-    friend class QgsPointCloudLayerEditUtils;
-    friend class QgsPointCloudEditingIndex;
 };