Fix node children and hasNode bug

Add caching for network requests Fix requesting a file that is clearly not in the server
qgis · Apr 8, 2021 · 68c5084 · 68c5084
1 parent d3e32dc
commit 68c5084
Show file tree

Hide file tree

Showing 5 changed files with 57 additions and 20 deletions.
diff --git a/src/core/pointcloud/qgspointcloudblockrequest.cpp b/src/core/pointcloud/qgspointcloudblockrequest.cpp
@@ -47,19 +47,25 @@ void QgsPointCloudBlockRequest::blockFinishedLoading()
   mBlock.reset( nullptr );
   if ( mTileDownloadManagetReply->error() == QNetworkReply::NetworkError::NoError )
   {
-    if ( mDataType == QLatin1String( "binary" ) )
+    try
     {
-      mBlock.reset( QgsEptDecoder::decompressBinary( mTileDownloadManagetReply->data(), mAttributes, mRequestedAttributes ) );
+      if ( mDataType == QLatin1String( "binary" ) )
+      {
+        mBlock.reset( QgsEptDecoder::decompressBinary( mTileDownloadManagetReply->data(), mAttributes, mRequestedAttributes ) );
+      }
+      else if ( mDataType == QLatin1String( "zstandard" ) )
+      {
+        mBlock.reset( QgsEptDecoder::decompressZStandard( mTileDownloadManagetReply->data(), mAttributes, mRequestedAttributes ) );
+      }
+      else if ( mDataType == QLatin1String( "laszip" ) )
+      {
+        mBlock.reset( QgsEptDecoder::decompressLaz( mTileDownloadManagetReply->data(), mAttributes, mRequestedAttributes ) );
+      }
     }
-    else if ( mDataType == QLatin1String( "zstandard" ) )
+    catch ( std::exception e )
     {
-      mBlock.reset( QgsEptDecoder::decompressZStandard( mTileDownloadManagetReply->data(), mAttributes, mRequestedAttributes ) );
+      mErrorStr = QStringLiteral( "Exception while decompressing file: %1" ).arg( e.what() );
     }
-    else if ( mDataType == QLatin1String( "laszip" ) )
-    {
-      mBlock.reset( QgsEptDecoder::decompressLaz( mTileDownloadManagetReply->data(), mAttributes, mRequestedAttributes ) );
-    }
-
     if ( !mBlock.get() )
       mErrorStr = QStringLiteral( "unknown data type %1;" ).arg( mDataType ) +  mTileDownloadManagetReply->errorString();
   }

diff --git a/src/core/pointcloud/qgspointcloudblockrequest.h b/src/core/pointcloud/qgspointcloudblockrequest.h
@@ -43,7 +43,7 @@ class CORE_EXPORT QgsPointCloudBlockRequest : public QObject
 
     /**
      * QgsPointCloudBlockRequest constructor
-     * Note: the istanced object will take ownership over \a tileDownloadManagerReply
+     * Note: the istanced QgsPointCloudBlockRequest object will take ownership over \a tileDownloadManagerReply
      */
     QgsPointCloudBlockRequest( const QString &dataType, const QgsPointCloudAttributeCollection &attributes, const QgsPointCloudAttributeCollection &requestedAttributes, QgsTileDownloadManagerReply *tileDownloadManagerReply );
 

diff --git a/src/core/pointcloud/qgspointcloudindex.h b/src/core/pointcloud/qgspointcloudindex.h
@@ -200,7 +200,7 @@ class CORE_EXPORT QgsPointCloudIndex: public QObject
     IndexedPointCloudNode root() { return IndexedPointCloudNode( 0, 0, 0, 0 ); }
 
     //! Returns whether the octree contain given node
-    bool hasNode( const IndexedPointCloudNode &n ) const { return mHierarchy.contains( n ); }
+    virtual bool hasNode( const IndexedPointCloudNode &n ) const { return mHierarchy.contains( n ); }
 
     //! Returns all children of node
     virtual QList<IndexedPointCloudNode> nodeChildren( const IndexedPointCloudNode &n ) const;

diff --git a/src/core/pointcloud/qgsremoteeptpointcloudindex.cpp b/src/core/pointcloud/qgsremoteeptpointcloudindex.cpp
@@ -52,13 +52,22 @@ QgsRemoteEptPointCloudIndex::~QgsRemoteEptPointCloudIndex() = default;
 
 QList<IndexedPointCloudNode> QgsRemoteEptPointCloudIndex::nodeChildren( const IndexedPointCloudNode &n ) const
 {
-  // Potential bug: what if loadNodeHierarchy fails and returns false
+  QList<IndexedPointCloudNode> lst;
   if ( !loadNodeHierarchy( n ) )
+    return lst;
+  int d = n.d() + 1;
+  int x = n.x() * 2;
+  int y = n.y() * 2;
+  int z = n.z() * 2;
+
+  for ( int i = 0; i < 8; ++i )
   {
-    return QList<IndexedPointCloudNode>();
+    int dx = i & 1, dy = !!( i & 2 ), dz = !!( i & 4 );
+    IndexedPointCloudNode n2( d, x + dx, y + dy, z + dz );
+    if ( loadNodeHierarchy( n2 ) )
+      lst.append( n2 );
   }
-
-  return QgsPointCloudIndex::nodeChildren( n );
+  return lst;
 }
 
 void QgsRemoteEptPointCloudIndex::load( const QString &url )
@@ -88,6 +97,7 @@ 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 );
   if ( !blockRequest )
     return nullptr;
@@ -100,6 +110,7 @@ QgsPointCloudBlock *QgsRemoteEptPointCloudIndex::nodeData( const IndexedPointClo
   {
     QgsDebugMsg( QStringLiteral( "Error downloading node %1 data, error : %2 " ).arg( n.toString(), blockRequest->errorStr() ) );
   }
+  qDebug() << __PRETTY_FUNCTION__ << " " << n.toString() << " end";
 
   return blockRequest->block();
 }
@@ -125,11 +136,19 @@ 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 );
+
   QgsTileDownloadManagerReply *reply = QgsApplication::tileDownloadManager()->get( nr );
   handle = new QgsPointCloudBlockRequest( mDataType, attributes(), request.attributes(), reply );
   return handle;
 }
 
+bool QgsRemoteEptPointCloudIndex::hasNode( const IndexedPointCloudNode &n ) const
+{
+  return loadNodeHierarchy( n );
+}
+
 int QgsRemoteEptPointCloudIndex::pointCount() const
 {
   return mPointCount;
@@ -141,20 +160,26 @@ bool QgsRemoteEptPointCloudIndex::loadNodeHierarchy( const IndexedPointCloudNode
     return true;
   QVector<IndexedPointCloudNode> nodePathToRoot;
   {
-    nodePathToRoot.push_back( nodeId );
     IndexedPointCloudNode currentNode = nodeId;
-    while ( currentNode.d() != 0 )
+    do
     {
-      currentNode = currentNode.parentNode();
       nodePathToRoot.push_back( currentNode );
+      currentNode = currentNode.parentNode();
     }
+    while ( currentNode.d() >= 0 );
   }
 
-  for ( int i = nodePathToRoot.size() - 1; i >= 0 && mHierarchy.find( nodeId ) == mHierarchy.end(); --i )
+  QSet<IndexedPointCloudNode> toBeLoaded;
+  toBeLoaded.insert( IndexedPointCloudNode::fromString( QStringLiteral( "0-0-0-0" ) ) );
+
+  for ( int i = nodePathToRoot.size() - 1; i >= 0 && !mHierarchy.contains( nodeId ); --i )
   {
     IndexedPointCloudNode node = nodePathToRoot[i];
     //! The hierarchy of the node is found => No need to load its file
-    if ( mHierarchy.find( node ) != mHierarchy.end() )
+    if ( mHierarchy.contains( node ) )
+      continue;
+
+    if ( !toBeLoaded.contains( node ) )
       continue;
 
     const QString fileUrl = QStringLiteral( "%1/ept-hierarchy/%2.json" ).arg( mUrlDirectoryPart, node.toString() );
@@ -192,6 +217,10 @@ bool QgsRemoteEptPointCloudIndex::loadNodeHierarchy( const IndexedPointCloudNode
         IndexedPointCloudNode nodeId = IndexedPointCloudNode::fromString( nodeIdStr );
         mHierarchy[nodeId] = nodePointCount;
       }
+      else if ( nodePointCount == -1 )
+      {
+        toBeLoaded.insert( IndexedPointCloudNode::fromString( nodeIdStr ) );
+      }
     }
   }
 

diff --git a/src/core/pointcloud/qgsremoteeptpointcloudindex.h b/src/core/pointcloud/qgsremoteeptpointcloudindex.h
@@ -54,6 +54,8 @@ class CORE_EXPORT QgsRemoteEptPointCloudIndex: public QgsEptPointCloudIndex
     QgsPointCloudBlock *nodeData( const IndexedPointCloudNode &n, const QgsPointCloudRequest &request ) override;
     QgsPointCloudBlockRequest *asyncNodeData( const IndexedPointCloudNode &n, const QgsPointCloudRequest &request ) override;
 
+    bool hasNode( const IndexedPointCloudNode &n ) const override;
+
     int pointCount() const override;
 
     bool isValid() const override;