[minimap] PY-90535 resolve hover structure presentation off the EDT

Avoids the modal-progress-under-read-lock crash by computing the hover target on a background read action instead of synchronously on the EDT. (cherry picked from commit cb72813cdf9df3f077d6c6a00d3d26b68b5bd4da) IJ-CR-210985 GitOrigin-RevId: d46e641469a24af36779cff8e4f79b146b787998
2026-09-27 10:03:11 +07:00 · 2026-07-07 22:38:05 +00:00
parent c0567c7013
commit b2e6948862
4 changed files with 245 additions and 72 deletions
@@ -6,10 +6,20 @@ import com.intellij.ide.minimap.interaction.MinimapInteractionPolicy
 import com.intellij.ide.minimap.scene.MinimapSnapshot
 import com.intellij.ide.minimap.settings.MinimapSettings
 import com.intellij.openapi.Disposable
+import com.intellij.openapi.application.EDT
+import com.intellij.openapi.application.ModalityState
+import com.intellij.openapi.application.asContextElement
+import com.intellij.openapi.application.readAction
 import com.intellij.openapi.util.Disposer
 import com.intellij.platform.util.coroutines.childScope
+import com.intellij.util.concurrency.annotations.RequiresBackgroundThread
+import com.intellij.util.concurrency.annotations.RequiresEdt
+import com.intellij.util.concurrency.annotations.RequiresReadLock
 import kotlinx.coroutines.CoroutineScope
+import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.Job
 import kotlinx.coroutines.cancel
+import kotlinx.coroutines.launch
 import java.awt.Graphics2D
 import java.awt.Point
 import kotlin.time.Duration
@@ -29,6 +39,7 @@ class MinimapHoverController(
  private var delayNextHover = true
  private var dragging = false
  private var hoverEnabled = true
+  private var hoverComputationJob: Job? = null

  private val hoverStateMachine = MinimapHoverStateMachine(scope, panel) { target ->
    presenter.setTarget(target)
@@ -43,6 +54,7 @@ class MinimapHoverController(
    scope.cancel()
  }

+  @RequiresEdt
  fun onSnapshot(snapshot: MinimapSnapshot) {
    hoverEnabled = settings.state.showHover && hoverPolicy.isHoverEnabled(panel.editor, snapshot)
    if (!hoverEnabled) {
@@ -64,21 +76,26 @@ class MinimapHoverController(
    }
  }

+  @RequiresEdt
  fun paint(graphics: Graphics2D) {
    if (!hoverEnabled) return
    presenter.paint(graphics)
  }

+  @RequiresEdt
  fun hideBalloon() {
+    cancelHoverComputation()
    hoverStateMachine.updateTarget(null)
    hoverStateMachine.syncActiveTarget(null)
    presenter.hide()
  }

+  @RequiresEdt
  fun onMouseEntered() {
    delayNextHover = true
  }

+  @RequiresEdt
  fun onMouseExited() {
    if (dragging) {
      lastMousePoint = null
@@ -88,6 +105,7 @@ class MinimapHoverController(
    updateHover(null)
  }

+  @RequiresEdt
  fun onScroll(point: Point?) {
    point?.let {
      lastMousePoint = Point(it)
@@ -99,17 +117,21 @@ class MinimapHoverController(
    updateActiveTargetForPoint(snapshot, lastPoint)
  }

+  @RequiresEdt
  fun startDragging() {
    dragging = true
  }

+  @RequiresEdt
  fun stopDragging(point: Point?) {
    dragging = false
    updateHover(point)
  }

+  @RequiresEdt
  fun updateHover(point: Point?) {
    if (point == null) {
+      cancelHoverComputation()
      lastMousePoint = null
      delayNextHover = true
      hoverStateMachine.updateTarget(null)
@@ -132,43 +154,58 @@ class MinimapHoverController(

  private fun updateTargetForPoint(snapshot: MinimapSnapshot, point: Point, delay: Duration?) {
    if (!isDocumentCommitted() || snapshot.structureEntries.isEmpty()) {
+      cancelHoverComputation()
      hoverStateMachine.updateTarget(null)
      return
    }

-    val target = computeHoverTarget(snapshot, point) ?: run {
-      hoverStateMachine.updateTarget(null)
-      return
-    }
-
-    if (delay == null) {
-      hoverStateMachine.updateTarget(target)
-    }
-    else {
-      hoverStateMachine.updateTarget(target, delay)
+    computeHoverTargetAsync(snapshot, point) { target ->
+      if (target == null) {
+        hoverStateMachine.updateTarget(null)
+      }
+      else if (delay == null) {
+        hoverStateMachine.updateTarget(target)
+      }
+      else {
+        hoverStateMachine.updateTarget(target, delay)
+      }
    }
  }

-  private fun computeHoverTarget(snapshot: MinimapSnapshot, point: Point): MinimapHoverTarget? {
-    val hit = hitChecker.hitCheck(snapshot, point) ?: return null
-    val text = hit.text ?: return null
+  private fun computeHoverTargetAsync(snapshot: MinimapSnapshot, point: Point, onResult: (MinimapHoverTarget?) -> Unit) {
+    hoverComputationJob?.cancel()
+    hoverComputationJob = scope.launch(Dispatchers.EDT + ModalityState.any().asContextElement()) {
+      // readAction dispatches its body to a background thread, so getPresentation runs off the EDT;
+      // the coroutine then resumes on the EDT to apply the result.
+      val target = readAction { computeHoverTarget(snapshot, point) }
+      if (lastSnapshot !== snapshot || lastMousePoint != point || !isDocumentCommitted()) return@launch
+      onResult(target)
+    }
+  }

-    return MinimapHoverTarget(hit.entry, hit.rect, text, hit.icon, hit.declarationWidth)
+  @RequiresBackgroundThread
+  @RequiresReadLock
+  private fun computeHoverTarget(snapshot: MinimapSnapshot, point: Point): MinimapHoverTarget? {
+    val hit = hitChecker.resolveHit(snapshot, point) ?: return null
+    return MinimapHoverTarget(hit.entry, hit.rect, hit.text, hit.icon, hit.declarationWidth)
+  }
+
+  private fun cancelHoverComputation() {
+    hoverComputationJob?.cancel()
+    hoverComputationJob = null
  }

  private fun updateActiveTargetForPoint(snapshot: MinimapSnapshot, point: Point) {
-    val active = hoverStateMachine.activeTarget() ?: return
-    val target = computeHoverTarget(snapshot, point) ?: run {
-      hoverStateMachine.updateTarget(null)
-      return
-    }
+    if (hoverStateMachine.activeTarget() == null) return
+    computeHoverTargetAsync(snapshot, point) { target ->
+      val active = hoverStateMachine.activeTarget() ?: return@computeHoverTargetAsync
+      if (target == null || !target.entry.isSameEntry(active.entry)) {
+        hoverStateMachine.updateTarget(null)
+        return@computeHoverTargetAsync
+      }

-    if (!target.entry.isSameEntry(active.entry)) {
-      hoverStateMachine.updateTarget(null)
-      return
+      hoverStateMachine.syncActiveTarget(target)
    }
-
-    hoverStateMachine.syncActiveTarget(target)
  }

  private fun updateActiveTargetForSnapshot(snapshot: MinimapSnapshot) {
@@ -14,63 +14,71 @@ import com.intellij.openapi.editor.Editor
 import com.intellij.openapi.util.TextRange
 import com.intellij.psi.PsiElement
 import com.intellij.psi.PsiNameIdentifierOwner
+import com.intellij.util.concurrency.annotations.RequiresBackgroundThread
+import com.intellij.util.concurrency.annotations.RequiresReadLock
 import java.awt.Point
 import java.awt.Rectangle
-import javax.swing.Icon
 import kotlin.math.ceil

 class MinimapHoverHitCheck(private val editor: Editor) {
  private val structureMarkerPolicy = MinimapInteractionPolicy.forEditor(editor)

-  private data class HoverData(
-    val range: TextRange,
-    val text: String?,
-    val icon: Icon?
-  )
-
-  fun hitCheck(snapshot: MinimapSnapshot, point: Point?): MinimapHoverHitCheckResult? {
+  @RequiresBackgroundThread
+  @RequiresReadLock
+  fun resolveHit(snapshot: MinimapSnapshot, point: Point?): MinimapHoverHitCheckResult? {
    if (point == null) return null
    val entries = snapshot.structureEntries
    if (entries.isEmpty()) return null

    val context = snapshot.context
-    var bestResult: MinimapHoverHitCheckResult? = null
+    var bestEntry: MinimapRenderEntry? = null
+    var bestRange: TextRange? = null
+    var bestRect: Rectangle? = null
    var bestArea = Long.MAX_VALUE

    for (entry in entries) {
-      val data = resolveHoverData(entry) ?: continue
-      val rect = computeHoverRect(data.range, context) ?: continue
+      val range = resolveRange(entry) ?: continue
+      val rect = computeHoverRect(range, context) ?: continue
      if (!rect.contains(point)) continue

      val area = rect.width.toLong() * rect.height.toLong()
-
      if (area < bestArea) {
        bestArea = area
-        val declarationWidth = computeDeclarationWidth(data.range.startOffset, context, snapshot.layoutMetrics)
-        bestResult = MinimapHoverHitCheckResult(entry, rect, data.text, data.icon, declarationWidth)
+        bestEntry = entry
+        bestRange = range
+        bestRect = rect
      }
    }

-    return bestResult
+    val entry = bestEntry ?: return null
+    val range = bestRange ?: return null
+    val rect = bestRect ?: return null
+
+    val element = entry.element ?: return null
+    val value = element.value ?: return null
+    val presentation = element.presentation
+    val text = getText(presentation, value) ?: return null
+    val icon = presentation.getIcon(false)
+
+    val declarationWidth = computeDeclarationWidth(range.startOffset, context, snapshot.layoutMetrics)
+    return MinimapHoverHitCheckResult(entry, rect, text, icon, declarationWidth)
  }

  fun computeHoverRect(entry: MinimapRenderEntry, context: MinimapRenderContext): Rectangle? {
-    val range = resolveRange(entry) ?: return null
+    val range = ReadAction.computeBlocking<TextRange?, RuntimeException> { resolveRange(entry) } ?: return null
    return computeHoverRect(range, context)
  }

+  @RequiresReadLock
  private fun resolveRange(entry: MinimapRenderEntry): TextRange? {
    val element = entry.element ?: return null
+    val value = element.value ?: return null
+    if (!structureMarkerPolicy.isRelevantStructureElement(element, value)) return null

-    return ReadAction.computeBlocking<TextRange?, RuntimeException> {
-      val value = element.value ?: return@computeBlocking null
-      if (!structureMarkerPolicy.isRelevantStructureElement(element, value)) return@computeBlocking null
-
-      when (value) {
-        is PsiElement -> value.textRange
-        is TextRange -> value
-        else -> null
-      }
+    return when (value) {
+      is PsiElement -> value.textRange
+      is TextRange -> value
+      else -> null
    }
  }

@@ -104,29 +112,8 @@ class MinimapHoverHitCheck(private val editor: Editor) {
    return Rectangle(0, y, width, heightPx)
  }

-  private fun resolveHoverData(entry: MinimapRenderEntry): HoverData? {
-    val element = entry.element ?: return null
-
-    return ReadAction.computeBlocking<HoverData?, RuntimeException> {
-      val value = element.value ?: return@computeBlocking null
-      if (!structureMarkerPolicy.isRelevantStructureElement(element, value)) return@computeBlocking null
-
-      val range = when (value) {
-        is PsiElement -> value.textRange
-        is TextRange -> value
-        else -> null
-      } ?: return@computeBlocking null
-
-      val presentation = element.presentation
-      val text = getText(presentation, value)
-      val icon = if (text != null) presentation.getIcon(false) else null
-
-      HoverData(range, text, icon)
-    }
-  }
-
  fun computeDeclarationWidth(entry: MinimapRenderEntry, context: MinimapRenderContext, metrics: MinimapLayoutMetrics?): Int {
-    val range = resolveRange(entry) ?: return context.panelWidth
+    val range = ReadAction.computeBlocking<TextRange?, RuntimeException> { resolveRange(entry) } ?: return context.panelWidth
    return computeDeclarationWidth(range.startOffset, context, metrics)
  }

@@ -8,7 +8,7 @@ import javax.swing.Icon
 data class MinimapHoverHitCheckResult(
  val entry: MinimapRenderEntry,
  val rect: Rectangle,
-  val text: String?,
+  val text: String,
  val icon: Icon?,
  val declarationWidth: Int,
 )
@@ -0,0 +1,149 @@
+// Copyright 2000-2026 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license.
+package com.intellij.ide.minimap
+
+import com.intellij.ide.minimap.geometry.MinimapGeometryData
+import com.intellij.ide.minimap.hover.MinimapHoverHitCheck
+import com.intellij.ide.minimap.hover.MinimapHoverHitCheckResult
+import com.intellij.ide.minimap.layout.MinimapLayoutMode
+import com.intellij.ide.minimap.model.MinimapLineProjection
+import com.intellij.ide.minimap.render.MinimapRenderContext
+import com.intellij.ide.minimap.render.MinimapRenderEntry
+import com.intellij.ide.minimap.scene.MinimapSnapshot
+import com.intellij.ide.structureView.StructureViewTreeElement
+import com.intellij.ide.util.treeView.smartTree.TreeElement
+import com.intellij.navigation.ItemPresentation
+import com.intellij.openapi.application.ApplicationManager
+import com.intellij.openapi.application.ReadAction
+import com.intellij.openapi.editor.impl.AbstractEditorTest
+import com.intellij.openapi.util.TextRange
+import com.intellij.util.ui.EmptyIcon
+import java.awt.Point
+import java.awt.geom.Rectangle2D
+import javax.swing.Icon
+
+/**
+ * Guards [MinimapHoverHitCheck.resolveHit]: geometry-first hit detection (smallest matching entry wins) followed
+ * by resolving the presentation of the winning entry only. The presentation call is intentionally kept out of the
+ * per-entry loop and out of the EDT path (see [com.intellij.ide.minimap.hover.MinimapHoverController]) so that a
+ * language's lazy resolve update does not try to run a modal progress while a read lock is held.
+ */
+class MinimapHoverHitCheckTest : AbstractEditorTest() {
+
+  private fun context(): MinimapRenderContext {
+    val editor = editor
+    return MinimapRenderContext(
+      editor = editor,
+      panelWidth = 120,
+      panelHeight = 600,
+      geometry = MinimapGeometryData(minimapHeight = 600, areaStart = 0, areaEnd = 600, thumbStart = 0, thumbHeight = 100),
+      lineProjection = MinimapLineProjection.identity(editor.document.lineCount),
+    )
+  }
+
+  private fun lineEntry(line: Int, text: String): Pair<MinimapRenderEntry, RecordingStructureElement> {
+    val document = editor.document
+    val range = TextRange(document.getLineStartOffset(line), document.getLineEndOffset(line))
+    val element = RecordingStructureElement(range, text)
+    return MinimapRenderEntry(element = element, rect2d = Rectangle2D.Double()) to element
+  }
+
+  /** [MinimapHoverHitCheck.resolveHit] is `@RequiresBackgroundThread @RequiresReadLock`, so call it as production does. */
+  private fun resolveHitOffEdt(hitChecker: MinimapHoverHitCheck, snapshot: MinimapSnapshot, point: Point): MinimapHoverHitCheckResult? {
+    return ApplicationManager.getApplication()
+      .executeOnPooledThread<MinimapHoverHitCheckResult?> {
+        ReadAction.computeBlocking<MinimapHoverHitCheckResult?, RuntimeException> { hitChecker.resolveHit(snapshot, point) }
+      }
+      .get()
+  }
+
+  private fun snapshotOf(context: MinimapRenderContext, vararg entries: MinimapRenderEntry): MinimapSnapshot {
+    return MinimapSnapshot(
+      context = context,
+      geometry = context.geometry,
+      tokenEntries = emptyList(),
+      structureEntries = entries.toList(),
+      diagnosticEntries = emptyList(),
+      breakpointEntries = emptyList(),
+      foldEntries = emptyList(),
+      layoutMetrics = null,
+      layoutMode = MinimapLayoutMode.EXACT,
+    )
+  }
+
+  fun testResolvesPresentationOfEntryUnderPoint() {
+    initText((0 until 20).joinToString("\n") { "line$it" })
+    val context = context()
+    val hitChecker = MinimapHoverHitCheck(editor)
+
+    val (entry, element) = lineEntry(line = 5, text = "fn five")
+    val snapshot = snapshotOf(context, entry)
+
+    val rect = hitChecker.computeHoverRect(entry, context)!!
+    val point = Point(rect.centerX.toInt(), rect.centerY.toInt())
+
+    val result = resolveHitOffEdt(hitChecker, snapshot, point)
+
+    assertNotNull("the entry under the point should produce a hover target", result)
+    assertEquals("fn five", result!!.text)
+    assertSame(EmptyIcon.ICON_16, result.icon)
+    assertTrue("presentation must be resolved for the winning entry", element.presentationResolved)
+  }
+
+  fun testSmallestMatchingEntryWins() {
+    initText((0 until 20).joinToString("\n") { "line$it" })
+    val context = context()
+    val hitChecker = MinimapHoverHitCheck(editor)
+
+    // A wide entry covering many lines and a narrow single-line entry that both contain the point.
+    val (wideEntry, wideElement) = run {
+      val document = editor.document
+      val range = TextRange(document.getLineStartOffset(0), document.getLineEndOffset(15))
+      val element = RecordingStructureElement(range, "outer")
+      MinimapRenderEntry(element = element, rect2d = Rectangle2D.Double()) to element
+    }
+    val (narrowEntry, narrowElement) = lineEntry(line = 5, text = "inner")
+    val snapshot = snapshotOf(context, wideEntry, narrowEntry)
+
+    val narrowRect = hitChecker.computeHoverRect(narrowEntry, context)!!
+    val point = Point(narrowRect.centerX.toInt(), narrowRect.centerY.toInt())
+
+    val result = resolveHitOffEdt(hitChecker, snapshot, point)
+
+    assertNotNull(result)
+    assertEquals("the smallest entry containing the point should win", "inner", result!!.text)
+    // Geometry-first: only the winning entry's presentation is resolved, not every entry's.
+    assertTrue(narrowElement.presentationResolved)
+    assertFalse("the losing entry's presentation must not be resolved", wideElement.presentationResolved)
+  }
+
+  fun testNoTargetWhenPointMissesEntries() {
+    initText((0 until 20).joinToString("\n") { "line$it" })
+    val context = context()
+    val hitChecker = MinimapHoverHitCheck(editor)
+
+    val (entry, _) = lineEntry(line = 5, text = "fn five")
+    val snapshot = snapshotOf(context, entry)
+
+    val result = resolveHitOffEdt(hitChecker, snapshot, Point(0, context.geometry.minimapHeight + 5_000))
+
+    assertNull("a point outside every entry rect must not produce a hover target", result)
+  }
+
+  private class RecordingStructureElement(private val range: TextRange, private val text: String) : StructureViewTreeElement {
+    var presentationResolved: Boolean = false
+      private set
+
+    override fun getValue(): Any = range
+
+    override fun getPresentation(): ItemPresentation {
+      presentationResolved = true
+      return object : ItemPresentation {
+        override fun getPresentableText(): String = text
+        override fun getLocationString(): String? = null
+        override fun getIcon(unused: Boolean): Icon = EmptyIcon.ICON_16
+      }
+    }
+
+    override fun getChildren(): Array<TreeElement> = TreeElement.EMPTY_ARRAY
+  }
+}