Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix a deadlock with local COPC files in 2D rendering #56432

Merged
merged 1 commit into from
Feb 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 10 additions & 9 deletions src/core/pointcloud/qgscopcpointcloudindex.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -287,10 +287,12 @@ bool QgsCopcPointCloudIndex::fetchNodeHierarchy( const IndexedPointCloudNode &n
if ( nodesCount < 0 )
{
auto hierarchyNodePos = mHierarchyNodePos.constFind( n );
mHierarchyMutex.unlock();
fetchHierarchyPage( hierarchyNodePos->first, hierarchyNodePos->second );
mHierarchyMutex.lock();
}
}
return true;
return mHierarchy.contains( n );
}

void QgsCopcPointCloudIndex::fetchHierarchyPage( uint64_t offset, uint64_t byteSize ) const
Expand All @@ -299,6 +301,11 @@ void QgsCopcPointCloudIndex::fetchHierarchyPage( uint64_t offset, uint64_t byteS
std::unique_ptr<char []> data( new char[ byteSize ] );
mCopcFile.read( data.get(), byteSize );

populateHierarchy( data.get(), byteSize );
}

void QgsCopcPointCloudIndex::populateHierarchy( const char *hierarchyPageData, uint64_t byteSize ) const
{
struct CopcVoxelKey
{
int32_t level;
Expand All @@ -319,7 +326,7 @@ void QgsCopcPointCloudIndex::fetchHierarchyPage( uint64_t offset, uint64_t byteS

for ( uint64_t i = 0; i < byteSize; i += sizeof( CopcEntry ) )
{
CopcEntry *entry = reinterpret_cast<CopcEntry *>( data.get() + i );
const CopcEntry *entry = reinterpret_cast<const CopcEntry *>( hierarchyPageData + i );
const IndexedPointCloudNode nodeId( entry->key.level, entry->key.x, entry->key.y, entry->key.z );
mHierarchy[nodeId] = entry->pointCount;
mHierarchyNodePos.insert( nodeId, QPair<uint64_t, int32_t>( entry->offset, entry->byteSize ) );
Expand All @@ -328,13 +335,7 @@ void QgsCopcPointCloudIndex::fetchHierarchyPage( uint64_t offset, uint64_t byteS

bool QgsCopcPointCloudIndex::hasNode( const IndexedPointCloudNode &n ) const
{
fetchNodeHierarchy( n );
mHierarchyMutex.lock();

auto it = mHierarchy.constFind( n );
const bool found = it != mHierarchy.constEnd() && ( *it ) >= 0;
mHierarchyMutex.unlock();
return found;
return fetchNodeHierarchy( n );
}

QList<IndexedPointCloudNode> QgsCopcPointCloudIndex::nodeChildren( const IndexedPointCloudNode &n ) const
Expand Down
5 changes: 3 additions & 2 deletions src/core/pointcloud/qgscopcpointcloudindex.h
Original file line number Diff line number Diff line change
Expand Up @@ -93,14 +93,15 @@ class CORE_EXPORT QgsCopcPointCloudIndex: public QgsPointCloudIndex
bool loadHierarchy();

//! Fetches all nodes leading to node \a node into memory
virtual bool fetchNodeHierarchy( const IndexedPointCloudNode &n ) const;
bool fetchNodeHierarchy( const IndexedPointCloudNode &n ) const;

/**
* Fetches the COPC hierarchy page at offset \a offset and of size \a byteSize into memory
* \note: This function is NOT thread safe and the mutex mHierarchyMutex needs to be locked before entering
*/
virtual void fetchHierarchyPage( uint64_t offset, uint64_t byteSize ) const;

void populateHierarchy( const char *hierarchyPageData, uint64_t byteSize ) const;

QByteArray fetchCopcStatisticsEvlrData();

bool mIsValid = false;
Expand Down
84 changes: 1 addition & 83 deletions src/core/pointcloud/qgsremotecopcpointcloudindex.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -52,30 +52,6 @@ std::unique_ptr<QgsPointCloudIndex> QgsRemoteCopcPointCloudIndex::clone() const
return std::unique_ptr<QgsPointCloudIndex>( clone );
}

QList<IndexedPointCloudNode> QgsRemoteCopcPointCloudIndex::nodeChildren( const IndexedPointCloudNode &n ) const
{
fetchNodeHierarchy( n );

mHierarchyMutex.lock();
Q_ASSERT( mHierarchy.contains( n ) );
QList<IndexedPointCloudNode> lst;
lst.reserve( 8 );
const int d = n.d() + 1;
const int x = n.x() * 2;
const int y = n.y() * 2;
const int z = n.z() * 2;
mHierarchyMutex.unlock();

for ( int i = 0; i < 8; ++i )
{
int dx = i & 1, dy = !!( i & 2 ), dz = !!( i & 4 );
const IndexedPointCloudNode n2( d, x + dx, y + dy, z + dz );
if ( fetchNodeHierarchy( n2 ) && mHierarchy[n] >= 0 )
lst.append( n2 );
}
return lst;
}

void QgsRemoteCopcPointCloudIndex::load( const QString &uri )
{
mUri = uri;
Expand Down Expand Up @@ -148,40 +124,6 @@ QgsPointCloudBlockRequest *QgsRemoteCopcPointCloudIndex::asyncNodeData( const In
blockOffset, blockSize, pointCount, *mLazInfo.get() );
}

bool QgsRemoteCopcPointCloudIndex::hasNode( const IndexedPointCloudNode &n ) const
{
return fetchNodeHierarchy( n );
}

bool QgsRemoteCopcPointCloudIndex::fetchNodeHierarchy( const IndexedPointCloudNode &n ) const
{
QMutexLocker locker( &mHierarchyMutex );

QVector<IndexedPointCloudNode> ancestors;
IndexedPointCloudNode foundRoot = n;
while ( !mHierarchy.contains( foundRoot ) )
{
ancestors.push_front( foundRoot );
foundRoot = foundRoot.parentNode();
}
ancestors.push_front( foundRoot );
for ( IndexedPointCloudNode n : ancestors )
{
auto hierarchyIt = mHierarchy.constFind( n );
if ( hierarchyIt == mHierarchy.constEnd() )
return false;

int nodesCount = *hierarchyIt;
if ( nodesCount < 0 )
{
auto hierarchyNodePos = mHierarchyNodePos.constFind( n );
locker.unlock();
fetchHierarchyPage( hierarchyNodePos->first, hierarchyNodePos->second );
}
}
return mHierarchy.contains( n );
}

bool QgsRemoteCopcPointCloudIndex::isValid() const
{
return mIsValid;
Expand Down Expand Up @@ -212,31 +154,7 @@ void QgsRemoteCopcPointCloudIndex::fetchHierarchyPage( uint64_t offset, uint64_t

QByteArray data = reply->data();

struct CopcVoxelKey
{
int32_t level;
int32_t x;
int32_t y;
int32_t z;
};

struct CopcEntry
{
CopcVoxelKey key;
uint64_t offset;
int32_t byteSize;
int32_t pointCount;
};

QMutexLocker locker( &mHierarchyMutex );

for ( uint64_t i = 0; i < byteSize; i += sizeof( CopcEntry ) )
{
CopcEntry *entry = reinterpret_cast<CopcEntry *>( data.data() + i );
const IndexedPointCloudNode nodeId( entry->key.level, entry->key.x, entry->key.y, entry->key.z );
mHierarchy[nodeId] = entry->pointCount;
mHierarchyNodePos.insert( nodeId, QPair<uint64_t, int32_t>( entry->offset, entry->byteSize ) );
}
populateHierarchy( data.constData(), byteSize );
}

void QgsRemoteCopcPointCloudIndex::copyCommonProperties( QgsRemoteCopcPointCloudIndex *destination ) const
Expand Down
5 changes: 0 additions & 5 deletions src/core/pointcloud/qgsremotecopcpointcloudindex.h
Original file line number Diff line number Diff line change
Expand Up @@ -54,15 +54,11 @@ class CORE_EXPORT QgsRemoteCopcPointCloudIndex: public QgsCopcPointCloudIndex

std::unique_ptr<QgsPointCloudIndex> clone() const override;

QList<IndexedPointCloudNode> nodeChildren( const IndexedPointCloudNode &n ) const override;

void load( const QString &uri ) override;

std::unique_ptr<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;

bool isValid() const override;

QgsPointCloudIndex::AccessType accessType() const override { return QgsPointCloudIndex::Remote; }
Expand All @@ -74,7 +70,6 @@ class CORE_EXPORT QgsRemoteCopcPointCloudIndex: public QgsCopcPointCloudIndex
void copyCommonProperties( QgsRemoteCopcPointCloudIndex *destination ) const;

protected:
virtual bool fetchNodeHierarchy( const IndexedPointCloudNode &nodeId ) const override;
virtual void fetchHierarchyPage( uint64_t offset, uint64_t byteSize ) const override;

QUrl mUrl;
Expand Down
Loading