From bd20473bf3a032f2b59b9cdbe61b5578c9136b72 Mon Sep 17 00:00:00 2001 From: Martin Dobias Date: Thu, 16 Jan 2025 12:17:17 +0100 Subject: [PATCH] review fixes --- .../pointcloud/qgscopcpointcloudindex.cpp | 2 - src/core/pointcloud/qgscopcupdate.cpp | 51 +++++++------- src/core/pointcloud/qgscopcupdate.h | 9 ++- .../pointcloud/qgspointcloudeditingindex.cpp | 20 +++--- src/core/pointcloud/qgspointcloudindex.h | 2 - .../qgspointcloudlayereditutils.cpp | 68 +++++++++---------- .../pointcloud/qgspointcloudlayereditutils.h | 3 +- 7 files changed, 75 insertions(+), 80 deletions(-) diff --git a/src/core/pointcloud/qgscopcpointcloudindex.cpp b/src/core/pointcloud/qgscopcpointcloudindex.cpp index fcddce4c66f9..24bd1bd60244 100644 --- a/src/core/pointcloud/qgscopcpointcloudindex.cpp +++ b/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; diff --git a/src/core/pointcloud/qgscopcupdate.cpp b/src/core/pointcloud/qgscopcupdate.cpp index 70b6e2eb2fbf..047880aaef75 100644 --- a/src/core/pointcloud/qgscopcupdate.cpp +++ b/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 &updatedChunks ) +bool QgsCopcUpdate::write( const QString &outputFilename, const QHash &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( mFile.tellg() ) + static_cast( ch.offset ) ); @@ -146,10 +145,10 @@ bool QgsCopcUpdate::write( QString outputFilename, const QHash( &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( m_f.tellp() ) + 60 - static_cast( mHierarchyOffset ); + const long hierPositionShift = static_cast( m_f.tellp() ) + 60 - static_cast( mHierarchyOffset ); - HierarchyEntry *oldCopcHierarchyBlobEntries = ( HierarchyEntry * ) mHierarchyBlob.data(); - int nEntries = static_cast( mHierarchyBlob.size() / 32 ); + HierarchyEntry *oldCopcHierarchyBlobEntries = reinterpret_cast( mHierarchyBlob.data() ); + const int nEntries = static_cast( 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( &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( &newRootHierOffset ), 8 ); m_f.seekp( mHeader.point_offset ); - m_f.write( ( const char * )&newChunkTableOffset, 8 ); + m_f.write( reinterpret_cast( &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( &chunkTableOffset ), sizeof( chunkTableOffset ) ); mFile.seekg( static_cast( chunkTableOffset ) + 4 ); // The first 4 bytes are the version, then the chunk count. - mFile.read( ( char * )&mChunkCount, sizeof( mChunkCount ) ); + mFile.read( reinterpret_cast( &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( mCopcVlr.root_hier_size ), + -1 } ); while ( !childEntriesToProcess.empty() ) { diff --git a/src/core/pointcloud/qgscopcupdate.h b/src/core/pointcloud/qgscopcupdate.h index 059fcafb69e7..e0a853022953 100644 --- a/src/core/pointcloud/qgscopcupdate.h +++ b/src/core/pointcloud/qgscopcupdate.h @@ -17,12 +17,11 @@ #define QGSCOPCUPDATE_H #include "qgis_core.h" +#include "qgspointcloudindex.h" #include #include -#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 &updatedChunks ); + bool write( const QString &outputFilename, const QHash &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 mChunks; - uint32_t mChunkCount; + uint32_t mChunkCount = 0; uint64_t mHierarchyOffset = 0; std::vector mHierarchyBlob; std::vector mEvlrHeaders; diff --git a/src/core/pointcloud/qgspointcloudeditingindex.cpp b/src/core/pointcloud/qgspointcloudeditingindex.cpp index 73b4144fa6a9..562d8d0c1119 100644 --- a/src/core/pointcloud/qgspointcloudeditingindex.cpp +++ b/src/core/pointcloud/qgspointcloudeditingindex.cpp @@ -21,7 +21,8 @@ #include "qgscopcupdate.h" #include "qgslazdecoder.h" -#include +#include +#include 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,11 +151,11 @@ bool QgsPointCloudEditingIndex::commitChanges( QString *errorMessage ) QgsCopcPointCloudIndex *copcIndex = static_cast( 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; } @@ -162,7 +163,7 @@ bool QgsPointCloudEditingIndex::commitChanges( QString *errorMessage ) 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 modifiedNodesIds = data.keys(); - QSet modifiedNodesSet( modifiedNodesIds.constBegin(), modifiedNodesIds.constEnd() ); - // get rid of cached keys that got modified { QMutexLocker locker( &sBlockCacheMutex ); const QList 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 ); } } diff --git a/src/core/pointcloud/qgspointcloudindex.h b/src/core/pointcloud/qgspointcloudindex.h index c44d4960961b..531504380e7c 100644 --- a/src/core/pointcloud/qgspointcloudindex.h +++ b/src/core/pointcloud/qgspointcloudindex.h @@ -659,8 +659,6 @@ class CORE_EXPORT QgsPointCloudIndex SIP_NODEFAULTCTORS std::shared_ptr mIndex; friend class TestQgsPointCloudEditing; - friend class QgsPointCloudLayerEditUtils; - friend class QgsPointCloudEditingIndex; }; diff --git a/src/core/pointcloud/qgspointcloudlayereditutils.cpp b/src/core/pointcloud/qgspointcloudlayereditutils.cpp index 251b7a1580ed..e29c19a8db5d 100644 --- a/src/core/pointcloud/qgspointcloudlayereditutils.cpp +++ b/src/core/pointcloud/qgspointcloudlayereditutils.cpp @@ -16,11 +16,11 @@ #include "qgspointcloudlayereditutils.h" #include "qgspointcloudlayer.h" #include "qgslazdecoder.h" - #include "qgscopcpointcloudindex.h" +#include "qgspointcloudeditingindex.h" + #include #include -#include "qgspointcloudeditingindex.h" QgsPointCloudLayerEditUtils::QgsPointCloudLayerEditUtils( QgsPointCloudLayer *layer ) @@ -88,70 +88,70 @@ static void updatePoint( char *pointBuffer, int pointFormat, const QString &attr { if ( attributeName == QLatin1String( "Intensity" ) ) // unsigned short { - quint16 newValueShort = ( quint16 ) newValue; + quint16 newValueShort = static_cast( newValue ); memcpy( pointBuffer + 12, &newValueShort, sizeof( qint16 ) ); } else if ( attributeName == QLatin1String( "ReturnNumber" ) ) // bits 0-3 { - uchar newByteValue = ( ( uchar )newValue ) & 0xf; - pointBuffer[14] = ( char )( ( pointBuffer[14] & 0xf0 ) | newByteValue ); + uchar newByteValue = static_cast( newValue ) & 0xf; + pointBuffer[14] = static_cast( ( pointBuffer[14] & 0xf0 ) | newByteValue ); } else if ( attributeName == QLatin1String( "NumberOfReturns" ) ) // bits 4-7 { - uchar newByteValue = ( ( ( uchar )newValue ) & 0xf ) << 4; - pointBuffer[14] = ( char )( ( pointBuffer[14] & 0xf ) | newByteValue ); + uchar newByteValue = ( static_cast( newValue ) & 0xf ) << 4; + pointBuffer[14] = static_cast( ( pointBuffer[14] & 0xf ) | newByteValue ); } else if ( attributeName == QLatin1String( "Synthetic" ) ) // bit 0 { - uchar newByteValue = ( ( uchar )newValue & 0x1 ); - pointBuffer[15] = ( char )( ( pointBuffer[15] & 0xfe ) | newByteValue ); + uchar newByteValue = ( static_cast( newValue ) & 0x1 ); + pointBuffer[15] = static_cast( ( pointBuffer[15] & 0xfe ) | newByteValue ); } else if ( attributeName == QLatin1String( "KeyPoint" ) ) // bit 1 { - uchar newByteValue = ( ( uchar )newValue & 0x1 ) << 1; - pointBuffer[15] = ( char )( ( pointBuffer[15] & 0xfd ) | newByteValue ); + uchar newByteValue = ( static_cast( newValue ) & 0x1 ) << 1; + pointBuffer[15] = static_cast( ( pointBuffer[15] & 0xfd ) | newByteValue ); } else if ( attributeName == QLatin1String( "Withheld" ) ) // bit 2 { - uchar newByteValue = ( ( uchar )newValue & 0x1 ) << 2; - pointBuffer[15] = ( char )( ( pointBuffer[15] & 0xfb ) | newByteValue ); + uchar newByteValue = ( static_cast( newValue ) & 0x1 ) << 2; + pointBuffer[15] = static_cast( ( pointBuffer[15] & 0xfb ) | newByteValue ); } else if ( attributeName == QLatin1String( "Overlap" ) ) // bit 3 { - uchar newByteValue = ( ( uchar )newValue & 0x1 ) << 3; - pointBuffer[15] = ( char )( ( pointBuffer[15] & 0xf7 ) | newByteValue ); + uchar newByteValue = ( static_cast( newValue ) & 0x1 ) << 3; + pointBuffer[15] = static_cast( ( pointBuffer[15] & 0xf7 ) | newByteValue ); } else if ( attributeName == QLatin1String( "ScannerChannel" ) ) // bits 4-5 { - uchar newByteValue = ( ( uchar )newValue & 0x3 ) << 4; - pointBuffer[15] = ( char )( ( pointBuffer[15] & 0xcf ) | newByteValue ); + uchar newByteValue = ( static_cast( newValue ) & 0x3 ) << 4; + pointBuffer[15] = static_cast( ( pointBuffer[15] & 0xcf ) | newByteValue ); } else if ( attributeName == QLatin1String( "ScanDirectionFlag" ) ) // bit 6 { - uchar newByteValue = ( ( uchar )newValue & 0x1 ) << 6; - pointBuffer[15] = ( char )( ( pointBuffer[15] & 0xbf ) | newByteValue ); + uchar newByteValue = ( static_cast( newValue ) & 0x1 ) << 6; + pointBuffer[15] = static_cast( ( pointBuffer[15] & 0xbf ) | newByteValue ); } else if ( attributeName == QLatin1String( "EdgeOfFlightLine" ) ) // bit 7 { - uchar newByteValue = ( ( uchar )newValue & 0x1 ) << 7; - pointBuffer[15] = ( char )( ( pointBuffer[15] & 0x7f ) | newByteValue ); + uchar newByteValue = ( static_cast( newValue ) & 0x1 ) << 7; + pointBuffer[15] = static_cast( ( pointBuffer[15] & 0x7f ) | newByteValue ); } else if ( attributeName == QLatin1String( "Classification" ) ) // unsigned char { - pointBuffer[16] = ( char )( uchar )newValue; + pointBuffer[16] = static_cast( static_cast( newValue ) ); } else if ( attributeName == QLatin1String( "UserData" ) ) // unsigned char { - pointBuffer[17] = ( char )( uchar )newValue; + pointBuffer[17] = static_cast( static_cast( newValue ) ); } else if ( attributeName == QLatin1String( "ScanAngleRank" ) ) // short { - qint16 newValueShort = ( qint16 ) newValue; + qint16 newValueShort = static_cast( newValue ); memcpy( pointBuffer + 18, &newValueShort, sizeof( qint16 ) ); } else if ( attributeName == QLatin1String( "PointSourceId" ) ) // unsigned short { - quint16 newValueShort = ( quint16 ) newValue; + quint16 newValueShort = static_cast( newValue ); memcpy( pointBuffer + 20, &newValueShort, sizeof( quint16 ) ); } else if ( attributeName == QLatin1String( "GpsTime" ) ) // double @@ -162,24 +162,24 @@ static void updatePoint( char *pointBuffer, int pointFormat, const QString &attr { if ( attributeName == QLatin1String( "Red" ) ) // unsigned short { - quint16 newValueShort = ( quint16 ) newValue; + quint16 newValueShort = static_cast( newValue ); memcpy( pointBuffer + 30, &newValueShort, sizeof( quint16 ) ); } else if ( attributeName == QLatin1String( "Green" ) ) // unsigned short { - quint16 newValueShort = ( quint16 ) newValue; + quint16 newValueShort = static_cast( newValue ); memcpy( pointBuffer + 32, &newValueShort, sizeof( quint16 ) ); } else if ( attributeName == QLatin1String( "Blue" ) ) // unsigned short { - quint16 newValueShort = ( quint16 ) newValue; + quint16 newValueShort = static_cast( newValue ); memcpy( pointBuffer + 34, &newValueShort, sizeof( quint16 ) ); } else if ( pointFormat == 8 ) { if ( attributeName == QLatin1String( "Infrared" ) ) // unsigned short { - quint16 newValueShort = ( quint16 ) newValue; + quint16 newValueShort = static_cast( newValue ); memcpy( pointBuffer + 36, &newValueShort, sizeof( quint16 ) ); } } @@ -187,14 +187,12 @@ static void updatePoint( char *pointBuffer, int pointFormat, const QString &attr } -QByteArray QgsPointCloudLayerEditUtils::updateChunkValues( QgsCopcPointCloudIndex *copcIndex, const QByteArray &chunkData, const QgsPointCloudAttribute &attribute, double newValue, QgsPointCloudNodeId k, QVector pointIndices ) +QByteArray QgsPointCloudLayerEditUtils::updateChunkValues( QgsCopcPointCloudIndex *copcIndex, const QByteArray &chunkData, const QgsPointCloudAttribute &attribute, double newValue, const QgsPointCloudNodeId &n, const QVector &pointIndices ) { - // set new classification value for the given points in voxel and return updated chunk data - - Q_ASSERT( copcIndex->mHierarchy.contains( k ) ); - Q_ASSERT( copcIndex->mHierarchyNodePos.contains( k ) ); + Q_ASSERT( copcIndex->mHierarchy.contains( n ) ); + Q_ASSERT( copcIndex->mHierarchyNodePos.contains( n ) ); - int pointCount = copcIndex->mHierarchy[k]; + int pointCount = copcIndex->mHierarchy[n]; lazperf::header14 header = copcIndex->mLazInfo->header(); diff --git a/src/core/pointcloud/qgspointcloudlayereditutils.h b/src/core/pointcloud/qgspointcloudlayereditutils.h index bd2b158b2e1f..f94ba1720d9d 100644 --- a/src/core/pointcloud/qgspointcloudlayereditutils.h +++ b/src/core/pointcloud/qgspointcloudlayereditutils.h @@ -66,7 +66,8 @@ class CORE_EXPORT QgsPointCloudLayerEditUtils private: - QByteArray updateChunkValues( QgsCopcPointCloudIndex *copcIndex, const QByteArray &chunkData, const QgsPointCloudAttribute &attribute, double newClassValue, QgsPointCloudNodeId k, QVector pointIndices ); + //! 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 &pointIndices ); QgsPointCloudIndex mIndex; };