Fix the use of the same file name in decompressLaz

Fix block ownership issue
qgis · Apr 8, 2021 · 56ed8c1 · 56ed8c1
1 parent 68c5084
commit 56ed8c1
Show file tree

Hide file tree

Showing 7 changed files with 46 additions and 29 deletions.
diff --git a/src/core/pointcloud/qgseptdecoder.cpp b/src/core/pointcloud/qgseptdecoder.cpp
@@ -479,10 +479,11 @@ QgsPointCloudBlock *QgsEptDecoder::decompressLaz( const QString &filename,
 }
 
 QgsPointCloudBlock *QgsEptDecoder::decompressLaz( const QByteArray &byteArrayData,
+    const QString &nodeStr,
     const QgsPointCloudAttributeCollection &attributes,
     const QgsPointCloudAttributeCollection &requestedAttributes )
 {
-  QString filename = QDir::tempPath() + QDir::separator() + QStringLiteral( "node.txt" );
+  QString filename = QDir::tempPath() + QDir::separator() + QStringLiteral( "%1.laz" ).arg( nodeStr );
   std::ofstream file( filename.toStdString(), std::ios::binary | std::ios::out );
   if ( file.is_open() )
   {

diff --git a/src/core/pointcloud/qgseptdecoder.h b/src/core/pointcloud/qgseptdecoder.h
@@ -36,7 +36,7 @@ namespace QgsEptDecoder
   QgsPointCloudBlock *decompressZStandard( const QString &filename, const QgsPointCloudAttributeCollection &attributes, const QgsPointCloudAttributeCollection &requestedAttributes );
   QgsPointCloudBlock *decompressZStandard( const QByteArray &data, const QgsPointCloudAttributeCollection &attributes, const QgsPointCloudAttributeCollection &requestedAttributes );
   QgsPointCloudBlock *decompressLaz( const QString &filename, const QgsPointCloudAttributeCollection &attributes, const QgsPointCloudAttributeCollection &requestedAttributes );
-  QgsPointCloudBlock *decompressLaz( const QByteArray &data, const QgsPointCloudAttributeCollection &attributes, const QgsPointCloudAttributeCollection &requestedAttributes );
+  QgsPointCloudBlock *decompressLaz( const QByteArray &data, const QString &nodeStr, const QgsPointCloudAttributeCollection &attributes, const QgsPointCloudAttributeCollection &requestedAttributes );
 };
 
 ///@endcond

diff --git a/src/core/pointcloud/qgseptpointcloudindex.cpp b/src/core/pointcloud/qgseptpointcloudindex.cpp
@@ -127,6 +127,8 @@ bool QgsEptPointCloudIndex::loadSchema( const QByteArray &dataJson )
 
   mSpan = result.value( QLatin1String( "span" ) ).toInt();
   mPointCount = result.value( QLatin1String( "points" ) ).toInt();
+  if ( mPointCount == 0 )
+    mPointCount = result.value( QLatin1String( "points" ) ).toDouble();
 
   // WKT
   const QJsonObject srs = result.value( QLatin1String( "srs" ) ).toObject();

diff --git a/src/core/pointcloud/qgspointcloudblockrequest.cpp b/src/core/pointcloud/qgspointcloudblockrequest.cpp
@@ -19,22 +19,27 @@
 
 #include "qgstiledownloadmanager.h"
 #include "qgseptdecoder.h"
+#include "qgsapplication.h"
 
 //
 // QgsPointCloudBlockRequest
 //
 
 ///@cond PRIVATE
 
-QgsPointCloudBlockRequest::QgsPointCloudBlockRequest( const QString &dataType, const QgsPointCloudAttributeCollection &attributes, const QgsPointCloudAttributeCollection &requestedAttributes, QgsTileDownloadManagerReply *tileDownloadManagerReply )
-  : mDataType( dataType ), mAttributes( attributes ), mRequestedAttributes( requestedAttributes ), mTileDownloadManagetReply( tileDownloadManagerReply )
+QgsPointCloudBlockRequest::QgsPointCloudBlockRequest( const IndexedPointCloudNode &node, const QString &Uri, const QString &dataType, const QgsPointCloudAttributeCollection &attributes, const QgsPointCloudAttributeCollection &requestedAttributes )
+  : mNode( node ), mDataType( dataType ), mAttributes( attributes ), mRequestedAttributes( requestedAttributes )
 {
-  connect( tileDownloadManagerReply, &QgsTileDownloadManagerReply::finished, this, &QgsPointCloudBlockRequest::blockFinishedLoading );
+  QNetworkRequest nr( Uri );
+  nr.setAttribute( QNetworkRequest::CacheLoadControlAttribute, QNetworkRequest::PreferCache );
+  nr.setAttribute( QNetworkRequest::CacheSaveControlAttribute, true );
+  mTileDownloadManagetReply.reset( QgsApplication::tileDownloadManager()->get( nr ) );
+  connect( mTileDownloadManagetReply.get(), &QgsTileDownloadManagerReply::finished, this, &QgsPointCloudBlockRequest::blockFinishedLoading );
 }
 
 QgsPointCloudBlock *QgsPointCloudBlockRequest::block()
 {
-  return mBlock.get();
+  return mBlock;
 }
 
 QString QgsPointCloudBlockRequest::errorStr()
@@ -44,30 +49,37 @@ QString QgsPointCloudBlockRequest::errorStr()
 
 void QgsPointCloudBlockRequest::blockFinishedLoading()
 {
-  mBlock.reset( nullptr );
+  mBlock = nullptr;
   if ( mTileDownloadManagetReply->error() == QNetworkReply::NetworkError::NoError )
   {
+    bool invalidDataType = false;
     try
     {
+      mBlock = nullptr;
       if ( mDataType == QLatin1String( "binary" ) )
       {
-        mBlock.reset( QgsEptDecoder::decompressBinary( mTileDownloadManagetReply->data(), mAttributes, mRequestedAttributes ) );
+        mBlock = QgsEptDecoder::decompressBinary( mTileDownloadManagetReply->data(), mAttributes, mRequestedAttributes );
       }
       else if ( mDataType == QLatin1String( "zstandard" ) )
       {
-        mBlock.reset( QgsEptDecoder::decompressZStandard( mTileDownloadManagetReply->data(), mAttributes, mRequestedAttributes ) );
+        mBlock = QgsEptDecoder::decompressZStandard( mTileDownloadManagetReply->data(), mAttributes, mRequestedAttributes );
       }
       else if ( mDataType == QLatin1String( "laszip" ) )
       {
-        mBlock.reset( QgsEptDecoder::decompressLaz( mTileDownloadManagetReply->data(), mAttributes, mRequestedAttributes ) );
+        mBlock = QgsEptDecoder::decompressLaz( mTileDownloadManagetReply->data(), mNode.toString(), mAttributes, mRequestedAttributes );
+      }
+      else
+      {
+        mErrorStr = QStringLiteral( "unknown data type %1;" ).arg( mDataType );
+        invalidDataType = true;
       }
     }
-    catch ( std::exception e )
+    catch ( std::exception &e )
     {
-      mErrorStr = QStringLiteral( "Exception while decompressing file: %1" ).arg( e.what() );
+      mErrorStr = QStringLiteral( "Error while decompressing node %1: %2" ).arg( mNode.toString(), e.what() );
     }
-    if ( !mBlock.get() )
-      mErrorStr = QStringLiteral( "unknown data type %1;" ).arg( mDataType ) +  mTileDownloadManagetReply->errorString();
+    if ( invalidDataType && !mBlock )
+      mErrorStr = QStringLiteral( "Error loading point cloud tile: \" %1 \"" ).arg( mTileDownloadManagetReply->errorString() );
   }
   else
   {

diff --git a/src/core/pointcloud/qgspointcloudblockrequest.h b/src/core/pointcloud/qgspointcloudblockrequest.h
@@ -22,6 +22,7 @@
 
 #include "qgspointcloudattribute.h"
 #include "qgstiledownloadmanager.h"
+#include "qgspointcloudindex.h"
 
 #define SIP_NO_FILE
 
@@ -43,13 +44,13 @@ class CORE_EXPORT QgsPointCloudBlockRequest : public QObject
 
     /**
      * QgsPointCloudBlockRequest constructor
-     * Note: the istanced QgsPointCloudBlockRequest object will take ownership over \a tileDownloadManagerReply
+     * Note: It is the responsablitiy of the caller to delete the block if it was loaded correctly
      */
-    QgsPointCloudBlockRequest( const QString &dataType, const QgsPointCloudAttributeCollection &attributes, const QgsPointCloudAttributeCollection &requestedAttributes, QgsTileDownloadManagerReply *tileDownloadManagerReply );
+    QgsPointCloudBlockRequest( const IndexedPointCloudNode &node, const QString &Uri, const QString &dataType, const QgsPointCloudAttributeCollection &attributes, const QgsPointCloudAttributeCollection &requestedAttributes );
 
     /**
      * Returns the requested block. if the returned block is nullptr, that means the data request failed
-     * Note: the returned block is owned by QgsPointCloudBlockRequest and will be deallocated once QgsPointCloudBlockRequest instance is deallocated
+     * Note: It is the responsablitiy of the caller to delete the block if it was loaded correctly
      */
     QgsPointCloudBlock *block();
 
@@ -60,11 +61,12 @@ class CORE_EXPORT QgsPointCloudBlockRequest : public QObject
     //! Emitted when the request processing has finished
     void finished();
   private:
+    IndexedPointCloudNode mNode;
     QString mDataType;
     QgsPointCloudAttributeCollection mAttributes;
     QgsPointCloudAttributeCollection mRequestedAttributes;
     std::unique_ptr<QgsTileDownloadManagerReply> mTileDownloadManagetReply = nullptr;
-    std::unique_ptr<QgsPointCloudBlock> mBlock = nullptr;
+    QgsPointCloudBlock *mBlock = nullptr;
     QString mErrorStr;
   private slots:
     void blockFinishedLoading();

diff --git a/src/core/pointcloud/qgspointcloudlayerrenderer.cpp b/src/core/pointcloud/qgspointcloudlayerrenderer.cpp
@@ -300,7 +300,11 @@ int QgsPointCloudLayerRenderer::renderNodesAsync( const QVector<IndexedPointClou
     for ( int i = 0; i < blockRequests.size(); ++i )
     {
       if ( blockRequests[ i ] )
+      {
+        if ( blockRequests[ i ]->block() )
+          delete blockRequests[ i ]->block();
         blockRequests[ i ]->deleteLater();
+      }
     }
   }
 

diff --git a/src/core/pointcloud/qgsremoteeptpointcloudindex.cpp b/src/core/pointcloud/qgsremoteeptpointcloudindex.cpp
@@ -98,19 +98,18 @@ void QgsRemoteEptPointCloudIndex::load( const QString &url )
 QgsPointCloudBlock *QgsRemoteEptPointCloudIndex::nodeData( const IndexedPointCloudNode &n, const QgsPointCloudRequest &request )
 {
   qDebug() << __PRETTY_FUNCTION__ << " " << n.toString() << " start";
-  QgsPointCloudBlockRequest *blockRequest = asyncNodeData( n, request );
+  std::unique_ptr<QgsPointCloudBlockRequest> blockRequest( asyncNodeData( n, request ) );
   if ( !blockRequest )
     return nullptr;
 
   QEventLoop loop;
-  connect( blockRequest, &QgsPointCloudBlockRequest::finished, &loop, &QEventLoop::quit );
+  connect( blockRequest.get(), &QgsPointCloudBlockRequest::finished, &loop, &QEventLoop::quit );
   loop.exec();
 
   if ( !blockRequest->block() )
   {
     QgsDebugMsg( QStringLiteral( "Error downloading node %1 data, error : %2 " ).arg( n.toString(), blockRequest->errorStr() ) );
   }
-  qDebug() << __PRETTY_FUNCTION__ << " " << n.toString() << " end";
 
   return blockRequest->block();
 }
@@ -120,8 +119,6 @@ QgsPointCloudBlockRequest *QgsRemoteEptPointCloudIndex::asyncNodeData( const Ind
   if ( !loadNodeHierarchy( n ) )
     return nullptr;
 
-  QgsPointCloudBlockRequest *handle = nullptr;
-
   QString fileUrl;
   if ( mDataType == QLatin1String( "binary" ) )
   {
@@ -135,13 +132,12 @@ QgsPointCloudBlockRequest *QgsRemoteEptPointCloudIndex::asyncNodeData( const Ind
   {
     fileUrl = QStringLiteral( "%1/ept-data/%2.laz" ).arg( mUrlDirectoryPart, n.toString() );
   }
-  QNetworkRequest nr( fileUrl );
-  nr.setAttribute( QNetworkRequest::CacheLoadControlAttribute, QNetworkRequest::PreferCache );
-  nr.setAttribute( QNetworkRequest::CacheSaveControlAttribute, true );
+  else
+  {
+    return nullptr;
+  }
 
-  QgsTileDownloadManagerReply *reply = QgsApplication::tileDownloadManager()->get( nr );
-  handle = new QgsPointCloudBlockRequest( mDataType, attributes(), request.attributes(), reply );
-  return handle;
+  return new QgsPointCloudBlockRequest( n, fileUrl, mDataType, attributes(), request.attributes() );
 }
 
 bool QgsRemoteEptPointCloudIndex::hasNode( const IndexedPointCloudNode &n ) const