make point cloud rendering better and make UI for point budget

qgis · Feb 10, 2021 · 6019168 · 6019168
1 parent 19759d4
commit 6019168
Show file tree

Hide file tree

Showing 14 changed files with 164 additions and 144 deletions.
diff --git a/python/3d/auto_generated/qgspointcloudlayer3drenderer.sip.in b/python/3d/auto_generated/qgspointcloudlayer3drenderer.sip.in
@@ -62,24 +62,6 @@ Returns 3D symbol associated with the renderer
     virtual void resolveReferences( const QgsProject &project );
 
 
-    double maximumScreenError() const;
-%Docstring
-Returns the maximum screen error allowed when rendering the point cloud.
-
-Larger values result in a faster render with less points rendered.
-
-.. seealso:: :py:func:`setMaximumScreenError`
-%End
-
-    void setMaximumScreenError( double error );
-%Docstring
-Sets the maximum screen ``error`` allowed when rendering the point cloud.
-
-Larger values result in a faster render with less points rendered.
-
-.. seealso:: :py:func:`maximumScreenError`
-%End
-
     bool showBoundingBoxes() const;
 %Docstring
 Returns whether bounding boxes will be visible when rendering the point cloud.
@@ -92,6 +74,15 @@ Returns whether bounding boxes will be visible when rendering the point cloud.
 Sets whether bounding boxes will be visible when rendering the point cloud.
 
 .. seealso:: :py:func:`showBoundingBoxes`
+%End
+
+    int pointRenderingBudget() const;
+%Docstring
+Returns the maximum number of points that will be rendered to the scene
+%End
+    void setPointRenderingBudget( int budget );
+%Docstring
+Sets the maximum number of points to be rendered in the scene
 %End
 
   private:

diff --git a/src/3d/chunks/qgschunkedentity_p.cpp b/src/3d/chunks/qgschunkedentity_p.cpp
@@ -72,11 +72,12 @@ static float screenSpaceError( QgsChunkNode *node, const QgsChunkedEntity::Scene
   return sse;
 }
 
-QgsChunkedEntity::QgsChunkedEntity( float tau, QgsChunkLoaderFactory *loaderFactory, bool ownsFactory, Qt3DCore::QNode *parent )
+QgsChunkedEntity::QgsChunkedEntity( float tau, QgsChunkLoaderFactory *loaderFactory, bool ownsFactory, int primitiveBudget, Qt3DCore::QNode *parent )
   : Qt3DCore::QEntity( parent )
   , mTau( tau )
   , mChunkLoaderFactory( loaderFactory )
   , mOwnsFactory( ownsFactory )
+  , mPrimitivesBudget( primitiveBudget )
 {
   mRootNode = loaderFactory->createRootNode();
   mChunkLoaderQueue = new QgsChunkList;
@@ -256,31 +257,26 @@ void QgsChunkedEntity::update( QgsChunkNode *root, const SceneState &state )
   int renderedCount = 0;
   std::priority_queue<slot, std::vector<slot>, decltype( cmp_funct )> pq( cmp_funct );
   pq.push( std::make_pair( root, screenSpaceError( root, state ) ) );
-  int nodes_count = 0;
   while ( !pq.empty() )
   {
     slot s = pq.top();
     pq.pop();
     QgsChunkNode *node = s.first;
-    QgsChunkLoader *nodeLoader = mChunkLoaderFactory->createChunkLoader( node );
     if ( Qgs3DUtils::isCullable( node->bbox(), state.viewProjectionMatrix ) )
     {
       ++mFrustumCulled;
       continue;
     }
-
-    if ( renderedCount + nodeLoader->primitiveCount() >  nodeLoader->primitiveBudget() )
-      continue;
+    // use this instead of renderedCount + mChunkLoaderFactory->primitivesCount( node ) > mPrimitivesBudget to avoid
+    // showing nothing when the user supplies a small value
+    if ( renderedCount > mPrimitivesBudget )
+      break;
 
     QgsChunkNode *const *children = node->children();
     for ( int i = 0; i < node->childCount(); ++i )
-    {
       pq.push( std::make_pair( children[i], screenSpaceError( children[i], state ) ) );
-    }
-    renderedCount += nodeLoader->primitiveCount();
-    nodes_count++;
+    renderedCount += mChunkLoaderFactory->primitivesCount( node );
     nodes.push_back( node );
-    continue;
   }
 
   for ( QgsChunkNode *node : nodes )
@@ -316,7 +312,6 @@ void QgsChunkedEntity::update( QgsChunkNode *root, const SceneState &state )
     else if ( node->allChildChunksResident( mCurrentTime ) )
     {
       // error is not acceptable and children are ready to be used - recursive descent
-
       if ( mAdditiveStrategy )
       {
         // With additive strategy enabled, also all parent nodes are added to active nodes.
@@ -330,10 +325,11 @@ void QgsChunkedEntity::update( QgsChunkNode *root, const SceneState &state )
       // error is not acceptable but children are not ready either - still use parent but request children
       mActiveNodes << node;
 
-      QVector<std::pair<QgsChunkNode *, float>> childrenVector;
       QgsChunkNode *const *children = node->children();
+      // load the closest nodes to the camera first
+      QVector<std::pair<QgsChunkNode *, float>> childrenVector;
       for ( int i = 0; i < node->childCount(); ++i )
-        childrenVector.push_back( std::make_pair( children[i], screenSpaceError( children[i], state ) ) );
+        childrenVector.push_back( std::make_pair( children[i], children[i]->bbox().distanceFromPoint( state.cameraPos ) ) );
       std::sort( childrenVector.begin(), childrenVector.end(), [&]( std::pair<QgsChunkNode *, float> n1, std::pair<QgsChunkNode *, float> n2 )
       {
         return n1.second > n2.second;

diff --git a/src/3d/chunks/qgschunkedentity_p.h b/src/3d/chunks/qgschunkedentity_p.h
@@ -28,6 +28,7 @@
 //
 
 #include <Qt3DCore/QEntity>
+#include <numeric>
 
 #define SIP_NO_FILE
 
@@ -63,7 +64,7 @@ class QgsChunkedEntity : public Qt3DCore::QEntity
     Q_OBJECT
   public:
     //! Constructs a chunked entity
-    QgsChunkedEntity( float tau, QgsChunkLoaderFactory *loaderFactory, bool ownsFactory, Qt3DCore::QNode *parent = nullptr );
+    QgsChunkedEntity( float tau, QgsChunkLoaderFactory *loaderFactory, bool ownsFactory, int primitivesBudget = std::numeric_limits<int>::max(), Qt3DCore::QNode *parent = nullptr );
     ~QgsChunkedEntity() override;
 
     //! Records some bits about the scene (context for update() method)
@@ -191,6 +192,8 @@ class QgsChunkedEntity : public Qt3DCore::QEntity
     bool mAdditiveStrategy = false;
 
     bool mIsValid = true;
+
+    int mPrimitivesBudget = std::numeric_limits<int>::max();
 };
 
 /// @endcond

diff --git a/src/3d/chunks/qgschunkloader_p.h b/src/3d/chunks/qgschunkloader_p.h
@@ -77,6 +77,9 @@ class QgsChunkLoaderFactory  : public QObject
     //! Creates loader for the given chunk node. Ownership of the returned is passed to the caller.
     virtual QgsChunkLoader *createChunkLoader( QgsChunkNode *node ) const = 0;
 
+    //! Returns the primitives count for the chunk \a node
+    virtual int primitivesCount( QgsChunkNode *node ) { return 0; }
+
     //! Creates root node of the hierarchy. Ownership of the returned object is passed to the caller.
     virtual QgsChunkNode *createRootNode() const = 0;
     //! Creates child nodes for the given node. Ownership of the returned objects is passed to the caller.

diff --git a/src/3d/qgs3dmapscene.cpp b/src/3d/qgs3dmapscene.cpp
@@ -383,15 +383,12 @@ void addQLayerComponentsToHierarchy( Qt3DCore::QEntity *entity, const QVector<Qt
 
 void Qgs3DMapScene::updateScene()
 {
-  QElapsedTimer t;
-  t.start();
   QgsEventTracing::addEvent( QgsEventTracing::Instant, QStringLiteral( "3D" ), QStringLiteral( "Update Scene" ) );
   for ( QgsChunkedEntity *entity : qgis::as_const( mChunkEntities ) )
   {
     if ( entity->isEnabled() )
       entity->update( _sceneState( mCameraController ) );
   }
-  qDebug() << "Time taken to update" << t.elapsed();
 #if QT_VERSION < QT_VERSION_CHECK(5, 10, 0)
   QgsWindow3DEngine *windowEngine = qobject_cast<QgsWindow3DEngine *>( mEngine );
   if ( windowEngine != nullptr )

diff --git a/src/3d/qgspointcloudlayer3drenderer.cpp b/src/3d/qgspointcloudlayer3drenderer.cpp
@@ -109,7 +109,6 @@ QgsPointCloudLayer3DRenderer *QgsPointCloudLayer3DRenderer::clone() const
     r->setSymbol( dynamic_cast<QgsPointCloud3DSymbol *>( symbolClone ) );
   }
   r->setShowBoundingBoxes( mShowBoundingBoxes );
-  r->setMaximumScreenError( mMaximumScreenError );
   return r;
 }
 
@@ -121,7 +120,7 @@ Qt3DCore::QEntity *QgsPointCloudLayer3DRenderer::createEntity( const Qgs3DMapSet
   if ( !mSymbol )
     return nullptr;
 
-  return new QgsPointCloudLayerChunkedEntity( pcl->dataProvider()->index(), map, dynamic_cast<QgsPointCloud3DSymbol *>( mSymbol->clone() ), maximumScreenError(), showBoundingBoxes(),
+  return new QgsPointCloudLayerChunkedEntity( pcl->dataProvider()->index(), map, dynamic_cast<QgsPointCloud3DSymbol *>( mSymbol->clone() ), showBoundingBoxes(),
          static_cast< const QgsPointCloudLayerElevationProperties * >( pcl->elevationProperties() )->zScale(),
          static_cast< const QgsPointCloudLayerElevationProperties * >( pcl->elevationProperties() )->zOffset(), mPointBudget );
 }
@@ -138,7 +137,6 @@ void QgsPointCloudLayer3DRenderer::writeXml( QDomElement &elem, const QgsReadWri
   QDomDocument doc = elem.ownerDocument();
 
   elem.setAttribute( QStringLiteral( "layer" ), mLayerRef.layerId );
-  elem.setAttribute( QStringLiteral( "max-screen-error" ), maximumScreenError() );
   elem.setAttribute( QStringLiteral( "show-bounding-boxes" ), showBoundingBoxes() ? QStringLiteral( "1" ) : QStringLiteral( "0" ) );
   elem.setAttribute( QStringLiteral( "point-budget" ), mPointBudget );
 
@@ -158,7 +156,6 @@ void QgsPointCloudLayer3DRenderer::readXml( const QDomElement &elem, const QgsRe
   QDomElement elemSymbol = elem.firstChildElement( QStringLiteral( "symbol" ) );
 
   const QString symbolType = elemSymbol.attribute( QStringLiteral( "type" ) );
-  mMaximumScreenError = elem.attribute( QStringLiteral( "max-screen-error" ), QStringLiteral( "1.0" ) ).toDouble();
   mShowBoundingBoxes = elem.attribute( QStringLiteral( "show-bounding-boxes" ), QStringLiteral( "0" ) ).toInt();
   mPointBudget = elem.attribute( QStringLiteral( "point-budget" ), QStringLiteral( "1000000" ) ).toInt();
 
@@ -182,16 +179,6 @@ void QgsPointCloudLayer3DRenderer::resolveReferences( const QgsProject &project
   mLayerRef.setLayer( project.mapLayer( mLayerRef.layerId ) );
 }
 
-double QgsPointCloudLayer3DRenderer::maximumScreenError() const
-{
-  return mMaximumScreenError;
-}
-
-void QgsPointCloudLayer3DRenderer::setMaximumScreenError( double error )
-{
-  mMaximumScreenError = error;
-}
-
 bool QgsPointCloudLayer3DRenderer::showBoundingBoxes() const
 {
   return mShowBoundingBoxes;

diff --git a/src/3d/qgspointcloudlayer3drenderer.h b/src/3d/qgspointcloudlayer3drenderer.h
@@ -228,24 +228,6 @@ class _3D_EXPORT QgsPointCloudLayer3DRenderer : public QgsAbstract3DRenderer
     void readXml( const QDomElement &elem, const QgsReadWriteContext &context ) override;
     void resolveReferences( const QgsProject &project ) override;
 
-    /**
-     * Returns the maximum screen error allowed when rendering the point cloud.
-     *
-     * Larger values result in a faster render with less points rendered.
-     *
-     * \see setMaximumScreenError()
-     */
-    double maximumScreenError() const;
-
-    /**
-     * Sets the maximum screen \a error allowed when rendering the point cloud.
-     *
-     * Larger values result in a faster render with less points rendered.
-     *
-     * \see maximumScreenError()
-     */
-    void setMaximumScreenError( double error );
-
     /**
      * Returns whether bounding boxes will be visible when rendering the point cloud.
      *
@@ -260,13 +242,19 @@ class _3D_EXPORT QgsPointCloudLayer3DRenderer : public QgsAbstract3DRenderer
      */
     void setShowBoundingBoxes( bool showBoundingBoxes );
 
+    /**
+     * Returns the maximum number of points that will be rendered to the scene
+     */
     int pointRenderingBudget() const { return mPointBudget; };
+
+    /**
+     * Sets the maximum number of points to be rendered in the scene
+     */
     void setPointRenderingBudget( int budget );
 
   private:
     QgsMapLayerRef mLayerRef; //!< Layer used to extract mesh data from
     std::unique_ptr< QgsPointCloud3DSymbol > mSymbol;
-    double mMaximumScreenError = 1.0;
     bool mShowBoundingBoxes = false;
     int mPointBudget = 1000000;
 

diff --git a/src/3d/qgspointcloudlayerchunkloader_p.cpp b/src/3d/qgspointcloudlayerchunkloader_p.cpp
@@ -154,6 +154,14 @@ QgsChunkLoader *QgsPointCloudLayerChunkLoaderFactory::createChunkLoader( QgsChun
   return new QgsPointCloudLayerChunkLoader( this, node, std::unique_ptr< QgsPointCloud3DSymbol >( static_cast< QgsPointCloud3DSymbol * >( mSymbol->clone() ) ), mZValueScale, mZValueOffset, mPointBudget, pointCount );
 }
 
+int QgsPointCloudLayerChunkLoaderFactory::primitivesCount( QgsChunkNode *node )
+{
+  QgsChunkNodeId id = node->tileId();
+  IndexedPointCloudNode n( id.d, id.x, id.y, id.z );
+  Q_ASSERT( mPointCloudIndex->hasNode( n ) );
+  return mPointCloudIndex->nodePointCount( n );
+}
+
 QgsAABB nodeBoundsToAABB( QgsPointCloudDataBounds nodeBounds, QgsVector3D offset, QgsVector3D scale, const Qgs3DMapSettings &map, double zValueOffset );
 
 QgsChunkNode *QgsPointCloudLayerChunkLoaderFactory::createRootNode() const
@@ -210,9 +218,9 @@ QgsAABB nodeBoundsToAABB( QgsPointCloudDataBounds nodeBounds, QgsVector3D offset
 }
 
 
-QgsPointCloudLayerChunkedEntity::QgsPointCloudLayerChunkedEntity( QgsPointCloudIndex *pc, const Qgs3DMapSettings &map, QgsPointCloud3DSymbol *symbol, float maxScreenError, bool showBoundingBoxes, double zValueScale, double zValueOffset, int pointBudget )
-  : QgsChunkedEntity( maxScreenError,
-                      new QgsPointCloudLayerChunkLoaderFactory( map, pc, symbol, zValueScale, zValueOffset, pointBudget ), true )
+QgsPointCloudLayerChunkedEntity::QgsPointCloudLayerChunkedEntity( QgsPointCloudIndex *pc, const Qgs3DMapSettings &map, QgsPointCloud3DSymbol *symbol, bool showBoundingBoxes, double zValueScale, double zValueOffset, int pointBudget )
+  : QgsChunkedEntity( 0.0f,
+                      new QgsPointCloudLayerChunkLoaderFactory( map, pc, symbol, zValueScale, zValueOffset, pointBudget ), true, pointBudget )
 {
   setUsingAdditiveStrategy( true );
   setShowBoundingBoxes( showBoundingBoxes );

diff --git a/src/3d/qgspointcloudlayerchunkloader_p.h b/src/3d/qgspointcloudlayerchunkloader_p.h
@@ -66,6 +66,7 @@ class QgsPointCloudLayerChunkLoaderFactory : public QgsChunkLoaderFactory
     virtual QgsChunkLoader *createChunkLoader( QgsChunkNode *node ) const override;
     virtual QgsChunkNode *createRootNode() const override;
     virtual QVector<QgsChunkNode *> createChildren( QgsChunkNode *node ) const override;
+    virtual int primitivesCount( QgsChunkNode *node ) override;
     const Qgs3DMapSettings &mMap;
     QgsPointCloudIndex *mPointCloudIndex;
     std::unique_ptr< QgsPointCloud3DSymbol > mSymbol;
@@ -123,7 +124,7 @@ class QgsPointCloudLayerChunkedEntity : public QgsChunkedEntity
 {
     Q_OBJECT
   public:
-    explicit QgsPointCloudLayerChunkedEntity( QgsPointCloudIndex *pc, const Qgs3DMapSettings &map, QgsPointCloud3DSymbol *symbol, float maxScreenError, bool showBoundingBoxes,
+    explicit QgsPointCloudLayerChunkedEntity( QgsPointCloudIndex *pc, const Qgs3DMapSettings &map, QgsPointCloud3DSymbol *symbol, bool showBoundingBoxes,
         double zValueScale, double zValueOffset, int pointBudget );
 
     ~QgsPointCloudLayerChunkedEntity();

diff --git a/src/3d/terrain/qgsterrainentity_p.cpp b/src/3d/terrain/qgsterrainentity_p.cpp
@@ -58,7 +58,7 @@ class TerrainMapUpdateJobFactory : public QgsChunkQueueJobFactory
 
 
 QgsTerrainEntity::QgsTerrainEntity( const Qgs3DMapSettings &map, Qt3DCore::QNode *parent )
-  : QgsChunkedEntity( map.maxTerrainScreenError(), map.terrainGenerator(), false, parent )
+  : QgsChunkedEntity( map.maxTerrainScreenError(), map.terrainGenerator(), false, std::numeric_limits<int>::max(), parent )
   , mMap( map )
 {
   map.terrainGenerator()->setTerrain( this );