From b2e69488625f96edf0f3b623f2e32a727a4ce324 Mon Sep 17 00:00:00 2001 From: Bogdan Kirilenko Date: Thu, 25 Jun 2026 17:10:40 +0200 Subject: [PATCH] [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 --- .../minimap/hover/MinimapHoverController.kt | 85 +++++++--- .../ide/minimap/hover/MinimapHoverHitCheck.kt | 81 ++++------ .../hover/MinimapHoverHitCheckResult.kt | 2 +- .../ide/minimap/MinimapHoverHitCheckTest.kt | 149 ++++++++++++++++++ 4 files changed, 245 insertions(+), 72 deletions(-) create mode 100644 platform/platform-tests/testSrc/com/intellij/ide/minimap/MinimapHoverHitCheckTest.kt diff --git a/platform/platform-impl/src/com/intellij/ide/minimap/hover/MinimapHoverController.kt b/platform/platform-impl/src/com/intellij/ide/minimap/hover/MinimapHoverController.kt index 8970fd3e48c0..1a6b394ba1ad 100644 --- a/platform/platform-impl/src/com/intellij/ide/minimap/hover/MinimapHoverController.kt +++ b/platform/platform-impl/src/com/intellij/ide/minimap/hover/MinimapHoverController.kt @@ -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) { diff --git a/platform/platform-impl/src/com/intellij/ide/minimap/hover/MinimapHoverHitCheck.kt b/platform/platform-impl/src/com/intellij/ide/minimap/hover/MinimapHoverHitCheck.kt index ae272f07d9d5..0389f4954a48 100644 --- a/platform/platform-impl/src/com/intellij/ide/minimap/hover/MinimapHoverHitCheck.kt +++ b/platform/platform-impl/src/com/intellij/ide/minimap/hover/MinimapHoverHitCheck.kt @@ -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 { 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 { - 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 { - 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 { resolveRange(entry) } ?: return context.panelWidth return computeDeclarationWidth(range.startOffset, context, metrics) } diff --git a/platform/platform-impl/src/com/intellij/ide/minimap/hover/MinimapHoverHitCheckResult.kt b/platform/platform-impl/src/com/intellij/ide/minimap/hover/MinimapHoverHitCheckResult.kt index 07f3bf10ed0b..49c06715627d 100644 --- a/platform/platform-impl/src/com/intellij/ide/minimap/hover/MinimapHoverHitCheckResult.kt +++ b/platform/platform-impl/src/com/intellij/ide/minimap/hover/MinimapHoverHitCheckResult.kt @@ -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, ) diff --git a/platform/platform-tests/testSrc/com/intellij/ide/minimap/MinimapHoverHitCheckTest.kt b/platform/platform-tests/testSrc/com/intellij/ide/minimap/MinimapHoverHitCheckTest.kt new file mode 100644 index 000000000000..fe4f8d81b29e --- /dev/null +++ b/platform/platform-tests/testSrc/com/intellij/ide/minimap/MinimapHoverHitCheckTest.kt @@ -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 { + 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 { + ReadAction.computeBlocking { 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.EMPTY_ARRAY + } +}