Fix #1214 - coord_polar: geom_point tooltips should take in account p…

…oint size
JetBrains · Oct 21, 2024 · 5d06cb0 · 5d06cb0
1 parent 342ae5e
commit 5d06cb0
Show file tree

Hide file tree

Showing 6 changed files with 97 additions and 15 deletions.
diff --git a/...lot-common/src/commonMain/kotlin/demo/plot/common/model/plotConfig/CoordPolarPlotSpecs.kt b/...lot-common/src/commonMain/kotlin/demo/plot/common/model/plotConfig/CoordPolarPlotSpecs.kt
@@ -17,9 +17,66 @@ class CoordPolarPlotSpecs {
             geomTile(),
             geomSegment(),
             windRose(),
+            pointTooltip(),
         )
     }
 
+    private fun pointTooltip(): MutableMap<String, Any> {
+        val spec = """
+            |{
+            |  "kind": "subplots",
+            |  "layout": { "ncol": 2.0, "nrow": 1.0, "name": "grid" },
+            |  "figures": [
+            |    {
+            |      "data": {
+            |        "x": [ 1.0, 2.0, 3.0, 4.0, 5.0 ],
+            |        "y": [ 1.0, 2.0, 3.0, 4.0, 5.0 ],
+            |        "s": [ 5.0, 4.0, 3.0, 2.0, 1.0 ]
+            |      },
+            |      "data_meta": {
+            |        "series_annotations": [
+            |          { "type": "int", "column": "x" },
+            |          { "type": "int", "column": "y" },
+            |          { "type": "int", "column": "s" }
+            |        ]
+            |      },
+            |      "kind": "plot",
+            |      "layers": [
+            |        {
+            |          "geom": "point",
+            |          "mapping": { "x": "x", "y": "y", "size": "s" }
+            |        }
+            |      ]
+            |    },
+            |    {
+            |      "data": {
+            |        "x": [ 1.0, 2.0, 3.0, 4.0, 5.0 ],
+            |        "y": [ 1.0, 2.0, 3.0, 4.0, 5.0 ],
+            |        "s": [ 5.0, 4.0, 3.0, 2.0, 1.0 ]
+            |      },
+            |      "data_meta": {
+            |        "series_annotations": [
+            |          { "type": "int", "column": "x" },
+            |          { "type": "int", "column": "y" },
+            |          { "type": "int", "column": "s" }
+            |        ]
+            |      },
+            |      "coord": { "name": "polar" },
+            |      "kind": "plot",
+            |      "layers": [
+            |        {
+            |          "geom": "point",
+            |          "mapping": { "x": "x", "y": "y", "size": "s" }
+            |        }
+            |      ]
+            |    }
+            |  ]
+            |}
+        """.trimMargin()
+
+        return parsePlotSpec(spec)
+    }
+
     private fun windRose(): MutableMap<String, Any> {
         val spec = """
             {

diff --git a/...monMain/kotlin/org/jetbrains/letsPlot/core/plot/builder/tooltip/loc/LayerTargetLocator.kt b/...monMain/kotlin/org/jetbrains/letsPlot/core/plot/builder/tooltip/loc/LayerTargetLocator.kt
@@ -59,6 +59,7 @@ internal class LayerTargetLocator(
             return when (prototype.hitShape.kind) {
                 POINT -> PointTargetProjection.create(
                     prototype.hitShape.point.center,
+                    prototype.hitShape.point.radius,
                     lookupSpec.lookupSpace
                 )
 

diff --git a/.../commonMain/kotlin/org/jetbrains/letsPlot/core/plot/builder/tooltip/loc/TargetDetector.kt b/.../commonMain/kotlin/org/jetbrains/letsPlot/core/plot/builder/tooltip/loc/TargetDetector.kt
@@ -104,19 +104,19 @@ internal class TargetDetector(
             LookupSpace.NONE -> false
             LookupSpace.X -> when (locatorLookupStrategy) {
                 LookupStrategy.NONE -> false
-                LookupStrategy.HOVER -> MathUtil.areEqual(pointProjection.x(), cursorCoord.x, POINT_AREA_EPSILON)
+                LookupStrategy.HOVER -> MathUtil.areEqual(pointProjection.x(), cursorCoord.x, pointProjection.radius + POINT_AREA_EPSILON)
                 LookupStrategy.NEAREST -> closestPointChecker.check(DoubleVector(pointProjection.x(), 0.0))
             }
 
             LookupSpace.Y -> when (locatorLookupStrategy) {
                 LookupStrategy.NONE -> false
-                LookupStrategy.HOVER -> MathUtil.areEqual(pointProjection.y(), cursorCoord.y, POINT_AREA_EPSILON)
+                LookupStrategy.HOVER -> MathUtil.areEqual(pointProjection.y(), cursorCoord.y, pointProjection.radius + POINT_AREA_EPSILON)
                 LookupStrategy.NEAREST -> closestPointChecker.check(DoubleVector(0.0, pointProjection.y()))
             }
 
             LookupSpace.XY -> when (locatorLookupStrategy) {
                 LookupStrategy.NONE -> false
-                LookupStrategy.HOVER -> MathUtil.areEqual(pointProjection.xy(), cursorCoord, POINT_AREA_EPSILON)
+                LookupStrategy.HOVER -> MathUtil.areEqual(pointProjection.xy(), cursorCoord, pointProjection.radius + POINT_AREA_EPSILON)
                 LookupStrategy.NEAREST -> closestPointChecker.check(pointProjection.xy())
             }
         }

diff --git a/...ommonMain/kotlin/org/jetbrains/letsPlot/core/plot/builder/tooltip/loc/TargetProjection.kt b/...ommonMain/kotlin/org/jetbrains/letsPlot/core/plot/builder/tooltip/loc/TargetProjection.kt
@@ -21,17 +21,20 @@ import kotlin.math.min
 
 internal open class TargetProjection
 
-internal class PointTargetProjection private constructor(val data: Any) : TargetProjection() {
+internal class PointTargetProjection private constructor(
+    val data: Any,
+    val radius: Double
+) : TargetProjection() {
     fun x() = data as Double
     fun y() = data as Double
     fun xy() = data as DoubleVector
 
     companion object {
-        fun create(p: DoubleVector, lookupSpace: LookupSpace): PointTargetProjection {
+        fun create(p: DoubleVector, radius: Double, lookupSpace: LookupSpace): PointTargetProjection {
             return when (lookupSpace) {
-                X -> PointTargetProjection(p.x)
-                Y -> PointTargetProjection(p.y)
-                XY -> PointTargetProjection(p)
+                X -> PointTargetProjection(p.x, radius)
+                Y -> PointTargetProjection(p.y, radius)
+                XY -> PointTargetProjection(p, radius)
                 NONE -> undefinedLookupSpaceError()
             }
         }
@@ -165,9 +168,9 @@ internal class PathTargetProjection(val data: List<PathPoint>) : TargetProjectio
         companion object {
             internal fun create(p: DoubleVector, index: Int, lookupSpace: LookupSpace): PathPoint {
                 return when (lookupSpace) {
-                    X -> PathPoint(PointTargetProjection.create(p, lookupSpace), p, index)
-                    Y -> PathPoint(PointTargetProjection.create(p, lookupSpace), p, index)
-                    XY -> PathPoint(PointTargetProjection.create(p, lookupSpace), p, index)
+                    X -> PathPoint(PointTargetProjection.create(p, radius = 0.0, lookupSpace), p, index)
+                    Y -> PathPoint(PointTargetProjection.create(p, radius = 0.0, lookupSpace), p, index)
+                    XY -> PathPoint(PointTargetProjection.create(p, radius = 0.0, lookupSpace), p, index)
                     NONE -> undefinedLookupSpaceError()
                 }
             }

diff --git a/plot-builder/src/jvmTest/kotlin/org/jetbrains/letsPlot/core/plot/builder/tooltip/TestUtil.kt b/plot-builder/src/jvmTest/kotlin/org/jetbrains/letsPlot/core/plot/builder/tooltip/TestUtil.kt
@@ -324,8 +324,8 @@ object TestUtil {
         )
     }
 
-    internal fun pointTarget(key: Any, p: DoubleVector): TargetPrototype {
-        val pointShape = HitShape.point(p, 0.0)
+    internal fun pointTarget(key: Any, p: DoubleVector, radius: Double = 0.0): TargetPrototype {
+        val pointShape = HitShape.point(p, radius)
         return TargetPrototype(
             pointShape, { key as Int },
             defaultTooltipParams, TipLayoutHint.Kind.VERTICAL_TOOLTIP

diff --git a/...org/jetbrains/letsPlot/core/plot/builder/tooltip/loc/LayerTargetLocatorSinglePointTest.kt b/...org/jetbrains/letsPlot/core/plot/builder/tooltip/loc/LayerTargetLocatorSinglePointTest.kt
@@ -8,9 +8,9 @@ package org.jetbrains.letsPlot.core.plot.builder.tooltip.loc
 import org.jetbrains.letsPlot.core.plot.base.tooltip.GeomTargetLocator
 import org.jetbrains.letsPlot.core.plot.base.tooltip.GeomTargetLocator.LookupSpace
 import org.jetbrains.letsPlot.core.plot.base.tooltip.GeomTargetLocator.LookupStrategy
-import org.jetbrains.letsPlot.core.plot.builder.tooltip.TestUtil
 import org.jetbrains.letsPlot.core.plot.builder.tooltip.TestUtil.assertEmpty
 import org.jetbrains.letsPlot.core.plot.builder.tooltip.TestUtil.assertObjects
+import org.jetbrains.letsPlot.core.plot.builder.tooltip.TestUtil.createLocator
 import org.jetbrains.letsPlot.core.plot.builder.tooltip.TestUtil.offsetX
 import org.jetbrains.letsPlot.core.plot.builder.tooltip.TestUtil.offsetY
 import org.jetbrains.letsPlot.core.plot.builder.tooltip.TestUtil.point
@@ -19,6 +19,27 @@ import kotlin.test.Test
 
 class LayerTargetLocatorSinglePointTest {
 
+    @Test
+    fun `issue1214 - coord_polar geom_point tooltips should take in account point size`() {
+        val pointKey = 1
+        val pointCenter = 50.0
+        val pointRadius = 20.0
+
+        val locator = createLocator(
+            LookupStrategy.HOVER,
+            LookupSpace.XY,
+            pointTarget(pointKey, point(pointCenter, pointCenter), pointRadius)
+        )
+
+        val locatorExtraDistance = 5.1 // see POINT_AREA_EPSILON
+
+        // exceeds the radius
+        assertEmpty(locator, point(pointCenter - pointRadius - locatorExtraDistance - 1.0, pointCenter))
+
+        // within the radius
+        assertObjects(locator, point(pointCenter - pointRadius - locatorExtraDistance + 1.0, pointCenter), pointKey)
+    }
+
     @Test
     fun hoverXy() {
         val locator = createLocator(LookupStrategy.HOVER, LookupSpace.XY)
@@ -55,7 +76,7 @@ class LayerTargetLocatorSinglePointTest {
     }
 
     private fun createLocator(strategy: LookupStrategy, space: LookupSpace): GeomTargetLocator {
-        return TestUtil.createLocator(
+        return createLocator(
             strategy, space,
             TARGET
         )