Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Fix remote-ept fast navigation crash (#43057)
* add mutex lock around hierarchy object

* try to fix issue with renderNodesAsync

* add const to nStr string
  • Loading branch information
NEDJIMAbelgacem committed May 3, 2021
1 parent f00f89e commit 8ac7df6
Show file tree
Hide file tree
Showing 5 changed files with 46 additions and 12 deletions.
7 changes: 6 additions & 1 deletion src/core/pointcloud/qgseptpointcloudindex.cpp
Expand Up @@ -273,7 +273,10 @@ bool QgsEptPointCloudIndex::loadSchema( const QByteArray &dataJson )

QgsPointCloudBlock *QgsEptPointCloudIndex::nodeData( const IndexedPointCloudNode &n, const QgsPointCloudRequest &request )
{
if ( !mHierarchy.contains( n ) )
mHierarchyMutex.lock();
bool found = mHierarchy.contains( n );
mHierarchyMutex.unlock();
if ( !found )
return nullptr;

if ( mDataType == QLatin1String( "binary" ) )
Expand Down Expand Up @@ -416,7 +419,9 @@ bool QgsEptPointCloudIndex::loadHierarchy()
else
{
IndexedPointCloudNode nodeId = IndexedPointCloudNode::fromString( nodeIdStr );
mHierarchyMutex.lock();
mHierarchy[nodeId] = nodePointCount;
mHierarchyMutex.unlock();
}
}
}
Expand Down
19 changes: 16 additions & 3 deletions src/core/pointcloud/qgspointcloudindex.cpp
Expand Up @@ -156,8 +156,17 @@ QgsPointCloudIndex::QgsPointCloudIndex() = default;

QgsPointCloudIndex::~QgsPointCloudIndex() = default;

bool QgsPointCloudIndex::hasNode( const IndexedPointCloudNode &n ) const
{
mHierarchyMutex.lock();
bool found = mHierarchy.contains( n );
mHierarchyMutex.unlock();
return found;
}

QList<IndexedPointCloudNode> QgsPointCloudIndex::nodeChildren( const IndexedPointCloudNode &n ) const
{
mHierarchyMutex.lock();
Q_ASSERT( mHierarchy.contains( n ) );
QList<IndexedPointCloudNode> lst;
int d = n.d() + 1;
Expand All @@ -172,6 +181,7 @@ QList<IndexedPointCloudNode> QgsPointCloudIndex::nodeChildren( const IndexedPoin
if ( mHierarchy.contains( n2 ) )
lst.append( n2 );
}
mHierarchyMutex.unlock();
return lst;
}

Expand Down Expand Up @@ -237,7 +247,10 @@ int QgsPointCloudIndex::span() const

int QgsPointCloudIndex::nodePointCount( const IndexedPointCloudNode &n )
{
if ( !mHierarchy.contains( n ) )
return -1;
return mHierarchy[n];
int count = -1;
mHierarchyMutex.lock();
if ( mHierarchy.contains( n ) )
count = mHierarchy.contains( n );
mHierarchyMutex.unlock();
return count;
}
4 changes: 3 additions & 1 deletion src/core/pointcloud/qgspointcloudindex.h
Expand Up @@ -24,6 +24,7 @@
#include <QStringList>
#include <QVector>
#include <QList>
#include <QMutex>

#include "qgis_core.h"
#include "qgsrectangle.h"
Expand Down Expand Up @@ -200,7 +201,7 @@ class CORE_EXPORT QgsPointCloudIndex: public QObject
IndexedPointCloudNode root() { return IndexedPointCloudNode( 0, 0, 0, 0 ); }

//! Returns whether the octree contain given node
virtual bool hasNode( const IndexedPointCloudNode &n ) const { return mHierarchy.contains( n ); }
virtual bool hasNode( const IndexedPointCloudNode &n ) const;

//! Returns all children of node
virtual QList<IndexedPointCloudNode> nodeChildren( const IndexedPointCloudNode &n ) const;
Expand Down Expand Up @@ -283,6 +284,7 @@ class CORE_EXPORT QgsPointCloudIndex: public QObject
QgsRectangle mExtent; //!< 2D extent of data
double mZMin = 0, mZMax = 0; //!< Vertical extent of data

mutable QMutex mHierarchyMutex;
mutable QHash<IndexedPointCloudNode, int> mHierarchy; //!< Data hierarchy
QgsVector3D mScale; //!< Scale of our int32 coordinates compared to CRS coords
QgsVector3D mOffset; //!< Offset of our int32 coordinates compared to CRS coords
Expand Down
6 changes: 3 additions & 3 deletions src/core/pointcloud/qgspointcloudlayerrenderer.cpp
Expand Up @@ -260,13 +260,14 @@ int QgsPointCloudLayerRenderer::renderNodesAsync( const QVector<IndexedPointClou
{
int nodeIndex = groupIndex + i;
const IndexedPointCloudNode &n = nodes[nodeIndex];
const QString nStr = n.toString();
QgsPointCloudBlockRequest *blockRequest = pc->asyncNodeData( n, request );
blockRequests[ i ] = blockRequest;
QObject::connect( blockRequest, &QgsPointCloudBlockRequest::finished, [ &, i, blockRequest ]()
QObject::connect( blockRequest, &QgsPointCloudBlockRequest::finished, &loop, [ &, i, nStr, blockRequest ]()
{
if ( !blockRequest->block() )
{
QgsDebugMsg( QStringLiteral( "Unable to load node %1, error: %2" ).arg( n.toString(), blockRequest->errorStr() ) );
QgsDebugMsg( QStringLiteral( "Unable to load node %1, error: %2" ).arg( nStr, blockRequest->errorStr() ) );
}
finishedLoadingBlock[ i ] = true;
// If all blocks are loaded, exit the event loop
Expand Down Expand Up @@ -371,4 +372,3 @@ QVector<IndexedPointCloudNode> QgsPointCloudLayerRenderer::traverseTree( const Q
}

QgsPointCloudLayerRenderer::~QgsPointCloudLayerRenderer() = default;

22 changes: 18 additions & 4 deletions src/core/pointcloud/qgsremoteeptpointcloudindex.cpp
Expand Up @@ -149,8 +149,12 @@ bool QgsRemoteEptPointCloudIndex::hasNode( const IndexedPointCloudNode &n ) cons

bool QgsRemoteEptPointCloudIndex::loadNodeHierarchy( const IndexedPointCloudNode &nodeId ) const
{
if ( mHierarchy.contains( nodeId ) )
mHierarchyMutex.lock();
bool found = mHierarchy.contains( nodeId );
mHierarchyMutex.unlock();
if ( found )
return true;

QVector<IndexedPointCloudNode> nodePathToRoot;
{
IndexedPointCloudNode currentNode = nodeId;
Expand All @@ -166,10 +170,14 @@ bool QgsRemoteEptPointCloudIndex::loadNodeHierarchy( const IndexedPointCloudNode
{
IndexedPointCloudNode node = nodePathToRoot[i];
//! The hierarchy of the node is found => No need to load its file
if ( mHierarchy.contains( node ) )
mHierarchyMutex.lock();
bool foundInHierarchy = mHierarchy.contains( node );
bool foundInHierarchyNodes = mHierarchyNodes.contains( node );
mHierarchyMutex.unlock();
if ( foundInHierarchy )
continue;

if ( !mHierarchyNodes.contains( node ) )
if ( !foundInHierarchyNodes )
continue;

const QString fileUrl = QStringLiteral( "%1/ept-hierarchy/%2.json" ).arg( mUrlDirectoryPart, node.toString() );
Expand Down Expand Up @@ -203,14 +211,20 @@ bool QgsRemoteEptPointCloudIndex::loadNodeHierarchy( const IndexedPointCloudNode
QString nodeIdStr = it.key();
int nodePointCount = it.value().toInt();
IndexedPointCloudNode nodeId = IndexedPointCloudNode::fromString( nodeIdStr );
mHierarchyMutex.lock();
if ( nodePointCount > 0 )
mHierarchy[nodeId] = nodePointCount;
else if ( nodePointCount == -1 )
mHierarchyNodes.insert( nodeId );
mHierarchyMutex.unlock();
}
}

return mHierarchy.contains( nodeId );
mHierarchyMutex.lock();
found = mHierarchy.contains( nodeId );
mHierarchyMutex.unlock();

return found;
}

bool QgsRemoteEptPointCloudIndex::isValid() const
Expand Down

0 comments on commit 8ac7df6

Please sign in to comment.