[3d] fix crash when switching terrain generator (fixes #21538)

wonder-sk · wonder-sk · commit 56e98f2b0b39 · 2019-03-11T00:01:44.000+01:00
The problem was introduced in PR #8828 when fixing issue #20963 This fix immediately deletes terrain entity on generator change and informs other code about that + identify map tool is now aware of the fact that terrain entity may be temporarily null.
diff --git a/src/3d/qgs3dmapscene.cpp b/src/3d/qgs3dmapscene.cpp
@@ -351,6 +351,16 @@ void Qgs3DMapScene::onFrameTriggered( float dt )
 
 void Qgs3DMapScene::createTerrain()
 {
+  if ( mTerrain )
+  {
+    mChunkEntities.removeOne( mTerrain );
+
+    mTerrain->deleteLater();
+    mTerrain = nullptr;
+
+    emit terrainEntityChanged();
+  }
+
   if ( !mTerrainUpdateScheduled )
   {
     // defer re-creation of terrain: there may be multiple invocations of this slot, so create the new entity just once
@@ -362,14 +372,6 @@ void Qgs3DMapScene::createTerrain()
 
 void Qgs3DMapScene::createTerrainDeferred()
 {
-  if ( mTerrain )
-  {
-    mChunkEntities.removeOne( mTerrain );
-
-    mTerrain->deleteLater();
-    mTerrain = nullptr;
-  }
-
   double tile0width = mMap.terrainGenerator()->extent().width();
   int maxZoomLevel = Qgs3DUtils::maxZoomLevel( tile0width, mMap.mapTileResolution(), mMap.maxTerrainGroundError() );
 
diff --git a/src/3d/qgs3dmapscene.h b/src/3d/qgs3dmapscene.h
@@ -59,7 +59,7 @@ class _3D_EXPORT Qgs3DMapScene : public Qt3DCore::QEntity
 
     //! Returns camera controller
     QgsCameraController *cameraController() { return mCameraController; }
-    //! Returns terrain entity
+    //! Returns terrain entity (may be temporarily null)
     QgsTerrainEntity *terrainEntity() { return mTerrain; }
 
     //! Resets camera view to show the whole scene (top view)
diff --git a/src/app/3d/qgs3dmaptoolidentify.cpp b/src/app/3d/qgs3dmaptoolidentify.cpp
@@ -77,16 +77,20 @@ void Qgs3DMapToolIdentify::mousePressEvent( QMouseEvent *event )
 
 void Qgs3DMapToolIdentify::activate()
 {
-  Qt3DRender::QObjectPicker *picker = mCanvas->scene()->terrainEntity()->terrainPicker();
-  connect( picker, &Qt3DRender::QObjectPicker::clicked, this, &Qgs3DMapToolIdentify::onTerrainPicked );
+  if ( QgsTerrainEntity *terrainEntity = mCanvas->scene()->terrainEntity() )
+  {
+    connect( terrainEntity->terrainPicker(), &Qt3DRender::QObjectPicker::clicked, this, &Qgs3DMapToolIdentify::onTerrainPicked );
+  }
 
   mCanvas->scene()->registerPickHandler( mPickHandler.get() );
 }
 
 void Qgs3DMapToolIdentify::deactivate()
 {
-  Qt3DRender::QObjectPicker *picker = mCanvas->scene()->terrainEntity()->terrainPicker();
-  disconnect( picker, &Qt3DRender::QObjectPicker::clicked, this, &Qgs3DMapToolIdentify::onTerrainPicked );
+  if ( QgsTerrainEntity *terrainEntity = mCanvas->scene()->terrainEntity() )
+  {
+    disconnect( terrainEntity->terrainPicker(), &Qt3DRender::QObjectPicker::clicked, this, &Qgs3DMapToolIdentify::onTerrainPicked );
+  }
 
   mCanvas->scene()->unregisterPickHandler( mPickHandler.get() );
 }
@@ -137,6 +141,8 @@ void Qgs3DMapToolIdentify::onTerrainEntityChanged()
 {
   // no need to disconnect from the previous entity: it has been destroyed
   // start listening to the new terrain entity
-  Qt3DRender::QObjectPicker *picker = mCanvas->scene()->terrainEntity()->terrainPicker();
-  connect( picker, &Qt3DRender::QObjectPicker::clicked, this, &Qgs3DMapToolIdentify::onTerrainPicked );
+  if ( QgsTerrainEntity *terrainEntity = mCanvas->scene()->terrainEntity() )
+  {
+    connect( terrainEntity->terrainPicker(), &Qt3DRender::QObjectPicker::clicked, this, &Qgs3DMapToolIdentify::onTerrainPicked );
+  }
 }

Original file line number	Diff line number	Diff line change
`@@ -77,16 +77,20 @@ void Qgs3DMapToolIdentify::mousePressEvent( QMouseEvent *event )`
`77`	`77`
`78`	`78`	`void Qgs3DMapToolIdentify::activate()`
`79`	`79`	`{`
`80`		`- Qt3DRender::QObjectPicker *picker = mCanvas->scene()->terrainEntity()->terrainPicker();`
`81`		`- connect( picker, &Qt3DRender::QObjectPicker::clicked, this, &Qgs3DMapToolIdentify::onTerrainPicked );`
	`80`	`+ if ( QgsTerrainEntity *terrainEntity = mCanvas->scene()->terrainEntity() )`
	`81`	`+ {`
	`82`	`+ connect( terrainEntity->terrainPicker(), &Qt3DRender::QObjectPicker::clicked, this, &Qgs3DMapToolIdentify::onTerrainPicked );`
	`83`	`+ }`
`82`	`84`
`83`	`85`	`mCanvas->scene()->registerPickHandler( mPickHandler.get() );`
`84`	`86`	`}`
`85`	`87`
`86`	`88`	`void Qgs3DMapToolIdentify::deactivate()`
`87`	`89`	`{`
`88`		`- Qt3DRender::QObjectPicker *picker = mCanvas->scene()->terrainEntity()->terrainPicker();`
`89`		`- disconnect( picker, &Qt3DRender::QObjectPicker::clicked, this, &Qgs3DMapToolIdentify::onTerrainPicked );`
	`90`	`+ if ( QgsTerrainEntity *terrainEntity = mCanvas->scene()->terrainEntity() )`
	`91`	`+ {`
	`92`	`+ disconnect( terrainEntity->terrainPicker(), &Qt3DRender::QObjectPicker::clicked, this, &Qgs3DMapToolIdentify::onTerrainPicked );`
	`93`	`+ }`
`90`	`94`
`91`	`95`	`mCanvas->scene()->unregisterPickHandler( mPickHandler.get() );`
`92`	`96`	`}`
`@@ -137,6 +141,8 @@ void Qgs3DMapToolIdentify::onTerrainEntityChanged()`
`137`	`141`	`{`
`138`	`142`	`// no need to disconnect from the previous entity: it has been destroyed`
`139`	`143`	`// start listening to the new terrain entity`
`140`		`- Qt3DRender::QObjectPicker *picker = mCanvas->scene()->terrainEntity()->terrainPicker();`
`141`		`- connect( picker, &Qt3DRender::QObjectPicker::clicked, this, &Qgs3DMapToolIdentify::onTerrainPicked );`
	`144`	`+ if ( QgsTerrainEntity *terrainEntity = mCanvas->scene()->terrainEntity() )`
	`145`	`+ {`
	`146`	`+ connect( terrainEntity->terrainPicker(), &Qt3DRender::QObjectPicker::clicked, this, &Qgs3DMapToolIdentify::onTerrainPicked );`
	`147`	`+ }`
`142`	`148`	`}`