Fix search tolerance when doing identification in 3D map view

wonder-sk · wonder-sk · commit 2da705864bf5 · 2018-09-19T13:20:31.000+02:00
Until now the identification from 3D map view used tolerance based
on the current view of the main 2D map canvas - that was giving often
unexpected results if the 2D map canvas had significantly different
zoom level from the 3D map view.
diff --git a/python/gui/auto_generated/qgsmaptoolidentify.sip.in b/python/gui/auto_generated/qgsmaptoolidentify.sip.in
@@ -161,6 +161,31 @@ Call the right method depending on layer type
     QMap< QString, QString > derivedAttributesForPoint( const QgsPoint &point );
 %Docstring
 Returns derived attributes map for a clicked point in map coordinates. May be 2D or 3D point.
+%End
+
+    void setCanvasPropertiesOverrides( double searchRadiusMapUnits );
+%Docstring
+Overrides some map canvas properties inside the map tool for the upcoming identify requests.
+
+This is useful when the identification is triggered by some other piece of GUI like a 3D map view
+and some properties like search radius need to be adjusted so that identification returns correct
+results. Currently only search radius may be overridden.
+
+When the custom identification has finished, restoreCanvasPropertiesOverrides() should
+be called to erase any overrides.
+
+.. seealso:: :py:func:`restoreCanvasProperties`
+
+.. versionadded:: 3.4
+%End
+
+    void restoreCanvasPropertiesOverrides();
+%Docstring
+Clears canvas properties overrides previously set with setCanvasPropertiesOverrides()
+
+.. seealso:: :py:func:`setCanvasPropertiesOverrides`
+
+.. versionadded:: 3.4
 %End
 
 };
diff --git a/src/3d/qgs3dmapscene.cpp b/src/3d/qgs3dmapscene.cpp
@@ -210,6 +210,20 @@ void Qgs3DMapScene::unregisterPickHandler( Qgs3DMapScenePickHandler *pickHandler
   }
 }
 
+float Qgs3DMapScene::worldSpaceError( float epsilon, float distance )
+{
+  Qt3DRender::QCamera *camera = mCameraController->camera();
+  float fov = camera->fieldOfView();
+  QRect rect = mCameraController->viewport();
+  float screenSizePx = std::max( rect.width(), rect.height() ); // TODO: is this correct?
+
+  // in qgschunkedentity_p.cpp there is inverse calculation (world space error to screen space error)
+  // with explanation of the math.
+  float frustumWidthAtDistance = 2 * distance * tan( fov / 2 );
+  float err = frustumWidthAtDistance * epsilon / screenSizePx;
+  return err;
+}
+
 QgsChunkedEntity::SceneState _sceneState( QgsCameraController *cameraController )
 {
   Qt3DRender::QCamera *camera = cameraController->camera();
diff --git a/src/3d/qgs3dmapscene.h b/src/3d/qgs3dmapscene.h
@@ -83,6 +83,12 @@ class _3D_EXPORT Qgs3DMapScene : public Qt3DCore::QEntity
     //! Unregisters previously registered pick handler. Pick handler is not deleted. Also removes object picker components from 3D entities.
     void unregisterPickHandler( Qgs3DMapScenePickHandler *pickHandler );
 
+    /**
+     * Given screen error (in pixels) and distance from camera (in 3D world coordinates), this function
+     * estimates the error in world space. Takes into account camera's field of view and the screen (3D view) size.
+     */
+    float worldSpaceError( float epsilon, float distance );
+
   signals:
     //! Emitted when the current terrain entity is replaced by a new one
     void terrainEntityChanged();
diff --git a/src/app/3d/qgs3dmaptoolidentify.cpp b/src/app/3d/qgs3dmaptoolidentify.cpp
@@ -96,9 +96,16 @@ void Qgs3DMapToolIdentify::onTerrainPicked( Qt3DRender::QPickEvent *event )
 
   QgsGeometry geom = QgsGeometry::fromPointXY( QgsPointXY( mapCoords.x(), mapCoords.y() ) );
 
+  // estimate search radius
+  Qgs3DMapScene *scene = mCanvas->scene();
+  double searchRadiusMM = QgsMapTool::searchRadiusMM();
+  double pixelsPerMM = mCanvas->logicalDpiX() / 25.4;
+  double searchRadiusPx = searchRadiusMM * pixelsPerMM;
+  double searchRadiusMapUnits = scene->worldSpaceError( searchRadiusPx, event->distance() );
+
   QgsMapToolIdentifyAction *identifyTool2D = QgisApp::instance()->identifyMapTool();
 
-  identifyTool2D->identifyAndShowResults( geom );
+  identifyTool2D->identifyAndShowResults( geom, searchRadiusMapUnits );
 }
 
 void Qgs3DMapToolIdentify::onTerrainEntityChanged()
diff --git a/src/app/qgsmaptoolidentifyaction.cpp b/src/app/qgsmaptoolidentifyaction.cpp
@@ -212,9 +212,11 @@ void QgsMapToolIdentifyAction::deactivate()
   QgsMapToolIdentify::deactivate();
 }
 
-void QgsMapToolIdentifyAction::identifyAndShowResults( const QgsGeometry &geom )
+void QgsMapToolIdentifyAction::identifyAndShowResults( const QgsGeometry &geom, double searchRadiusMapUnits )
 {
+  setCanvasPropertiesOverrides( searchRadiusMapUnits );
   mSelectionHandler->setSelectedGeometry( geom );
+  restoreCanvasPropertiesOverrides();
 }
 
 void QgsMapToolIdentifyAction::clearResults()
diff --git a/src/app/qgsmaptoolidentifyaction.h b/src/app/qgsmaptoolidentifyaction.h
@@ -62,7 +62,7 @@ class APP_EXPORT QgsMapToolIdentifyAction : public QgsMapToolIdentify
     void deactivate() override;
 
     //! Triggers map identification of at the given location and outputs results in GUI
-    void identifyAndShowResults( const QgsGeometry &geom );
+    void identifyAndShowResults( const QgsGeometry &geom, double searchRadiusMapUnits );
     //! Clears any previous results from the GUI
     void clearResults();
     //! Looks up feature by its ID and outputs the result in GUI
diff --git a/src/gui/qgsmaptoolidentify.cpp b/src/gui/qgsmaptoolidentify.cpp
@@ -179,6 +179,16 @@ QList<QgsMapToolIdentify::IdentifyResult> QgsMapToolIdentify::identify( const Qg
   return results;
 }
 
+void QgsMapToolIdentify::setCanvasPropertiesOverrides( double searchRadiusMapUnits )
+{
+  mOverrideCanvasSearchRadius = searchRadiusMapUnits;
+}
+
+void QgsMapToolIdentify::restoreCanvasPropertiesOverrides()
+{
+  mOverrideCanvasSearchRadius = -1;
+}
+
 void QgsMapToolIdentify::activate()
 {
   QgsMapTool::activate();
@@ -270,7 +280,7 @@ bool QgsMapToolIdentify::identifyVectorLayer( QList<QgsMapToolIdentify::Identify
     QgsRectangle r;
     if ( isSingleClick )
     {
-      double sr = searchRadiusMU( mCanvas );
+      double sr = mOverrideCanvasSearchRadius < 0 ? searchRadiusMU( mCanvas ) : mOverrideCanvasSearchRadius;
       r = toLayerCoordinates( layer, QgsRectangle( point.x() - sr, point.y() - sr, point.x() + sr, point.y() + sr ) );
     }
     else
diff --git a/src/gui/qgsmaptoolidentify.h b/src/gui/qgsmaptoolidentify.h
@@ -168,6 +168,27 @@ class GUI_EXPORT QgsMapToolIdentify : public QgsMapTool
     //! Returns derived attributes map for a clicked point in map coordinates. May be 2D or 3D point.
     QMap< QString, QString > derivedAttributesForPoint( const QgsPoint &point );
 
+    /**
+     * Overrides some map canvas properties inside the map tool for the upcoming identify requests.
+     *
+     * This is useful when the identification is triggered by some other piece of GUI like a 3D map view
+     * and some properties like search radius need to be adjusted so that identification returns correct
+     * results. Currently only search radius may be overridden.
+     *
+     * When the custom identification has finished, restoreCanvasPropertiesOverrides() should
+     * be called to erase any overrides.
+     * \see restoreCanvasProperties()
+     * \since QGIS 3.4
+     */
+    void setCanvasPropertiesOverrides( double searchRadiusMapUnits );
+
+    /**
+     * Clears canvas properties overrides previously set with setCanvasPropertiesOverrides()
+     * \see setCanvasPropertiesOverrides()
+     * \since QGIS 3.4
+     */
+    void restoreCanvasPropertiesOverrides();
+
   private:
 
     bool identifyLayer( QList<QgsMapToolIdentify::IdentifyResult> *results, QgsMapLayer *layer, const QgsGeometry &geometry, const QgsRectangle &viewExtent, double mapUnitsPerPixel, QgsMapToolIdentify::LayerType layerType = AllLayers );
@@ -238,6 +259,8 @@ class GUI_EXPORT QgsMapToolIdentify : public QgsMapTool
     QgsRectangle mLastExtent;
 
     int mCoordinatePrecision;
+
+    double mOverrideCanvasSearchRadius = -1;
 };
 
 Q_DECLARE_OPERATORS_FOR_FLAGS( QgsMapToolIdentify::LayerType )

Original file line number	Diff line number	Diff line change
`@@ -212,9 +212,11 @@ void QgsMapToolIdentifyAction::deactivate()`
`212`	`212`	`QgsMapToolIdentify::deactivate();`
`213`	`213`	`}`
`214`	`214`
`215`		`-void QgsMapToolIdentifyAction::identifyAndShowResults( const QgsGeometry &geom )`
	`215`	`+void QgsMapToolIdentifyAction::identifyAndShowResults( const QgsGeometry &geom, double searchRadiusMapUnits )`
`216`	`216`	`{`
	`217`	`+ setCanvasPropertiesOverrides( searchRadiusMapUnits );`
`217`	`218`	`mSelectionHandler->setSelectedGeometry( geom );`
	`219`	`+ restoreCanvasPropertiesOverrides();`
`218`	`220`	`}`
`219`	`221`
`220`	`222`	`void QgsMapToolIdentifyAction::clearResults()`
Original file line number	Diff line number	Diff line change
`@@ -179,6 +179,16 @@ QList<QgsMapToolIdentify::IdentifyResult> QgsMapToolIdentify::identify( const Qg`
`179`	`179`	`return results;`
`180`	`180`	`}`
`181`	`181`
	`182`	`+void QgsMapToolIdentify::setCanvasPropertiesOverrides( double searchRadiusMapUnits )`
	`183`	`+{`
	`184`	`+ mOverrideCanvasSearchRadius = searchRadiusMapUnits;`
	`185`	`+}`
	`186`	`+`
	`187`	`+void QgsMapToolIdentify::restoreCanvasPropertiesOverrides()`
	`188`	`+{`
	`189`	`+ mOverrideCanvasSearchRadius = -1;`
	`190`	`+}`
	`191`	`+`
`182`	`192`	`void QgsMapToolIdentify::activate()`
`183`	`193`	`{`
`184`	`194`	`QgsMapTool::activate();`
`@@ -270,7 +280,7 @@ bool QgsMapToolIdentify::identifyVectorLayer( QList<QgsMapToolIdentify::Identify`
`270`	`280`	`QgsRectangle r;`
`271`	`281`	`if ( isSingleClick )`
`272`	`282`	`{`
`273`		`- double sr = searchRadiusMU( mCanvas );`
	`283`	`+ double sr = mOverrideCanvasSearchRadius < 0 ? searchRadiusMU( mCanvas ) : mOverrideCanvasSearchRadius;`
`274`	`284`	`r = toLayerCoordinates( layer, QgsRectangle( point.x() - sr, point.y() - sr, point.x() + sr, point.y() + sr ) );`
`275`	`285`	`}`
`276`	`286`	`else`