From 4408eaf2cb814e927f51cb0632e9845994abae51 Mon Sep 17 00:00:00 2001 From: blasty Date: Thu, 6 Aug 2026 16:28:56 +0200 Subject: graph: navigate to blocks, not to coordinates Clicking the minimap panned to the exact coordinate under the pointer and moved the cursor only if a block happened to sit there. Since one minimap cell covers many canvas cells, "there" was almost always padding: you got a jump into empty space and the cursor stayed behind, so you had to click a block afterwards to actually go anywhere. Blocks cover a few percent of a laid-out graph -- 4.6% of an 87-block function, 0.8% of a 424-block one -- and the rest is the space that keeps edges apart. So coordinates are the wrong thing to navigate by here. The minimap now snaps to the nearest block and takes the cursor with it, and a drag scrubs from block to block. Distance is measured with the column halved, because cells are twice as tall as they are wide and otherwise "nearest" is not what looks nearest. A drag-pan or ctrl+d/pageup that ends with no block on screen at all now eases to the nearest one too, since an empty screen leaves nothing to navigate back by. It only fires when nothing is visible, so a deliberate pan is never fought. --- docs/GRAPH_VIEW.md | 11 +++-- idatui/app.py | 111 +++++++++++++++++++++++++++++++++++++++--------- tests/test_scenarios.py | 42 ++++++++++++++---- 3 files changed, 132 insertions(+), 32 deletions(-) diff --git a/docs/GRAPH_VIEW.md b/docs/GRAPH_VIEW.md index 6bbcf83..afd0f40 100644 --- a/docs/GRAPH_VIEW.md +++ b/docs/GRAPH_VIEW.md @@ -110,9 +110,14 @@ per block). On `main` (87 blocks) that is a 1378×518 canvas down to 545×289. The **minimap** (`m`) is a coarse occupancy grid of the whole graph with the viewport marked, drawn top-right and inset two columns — a `ScrollView` paints its scrollbar over the last column, which otherwise eats the minimap's border. -Clicking it jumps the view to that part of the graph (and lands the cursor on a -block if one is there, so the keyboard carries on from where you pointed); -dragging scrubs. Because it floats over the canvas rather than living in it, +Clicking it **snaps to the nearest block** and takes the cursor with it; +dragging scrubs from block to block. It deliberately does not scroll to the +coordinate you clicked: blocks cover only a few percent of a laid-out graph +(4.6% on an 87-block function, under 1% on a 424-block one) and the rest is the +padding that keeps edges apart, so a coordinate-accurate jump parks you in empty +space with the cursor left behind. For the same reason, a drag-pan or a +`ctrl+d`/`pageup` that ends with **no block on screen at all** eases to the +nearest one — only when nothing is visible, so it never fights a deliberate pan. Because it floats over the canvas rather than living in it, `on_click` has to test the minimap's hit-box **before** translating the click into canvas coordinates — otherwise a click on the overview reads as a click on whatever block happens to lie underneath it. `_minimap_rect()` is the single diff --git a/idatui/app.py b/idatui/app.py index 7a575b7..0779b93 100644 --- a/idatui/app.py +++ b/idatui/app.py @@ -2281,6 +2281,35 @@ class GraphView(NavMixin, ScrollView, can_focus=True): def action_pan(self, rows: int) -> None: self.scroll_to(y=max(0, self.scroll_offset.y + rows), animate=False) + self._snap_into_view() + + def _viewport_has_block(self) -> bool: + if self.lay is None: + return False + y0 = int(self.scroll_offset.y) + x0 = int(self.scroll_offset.x) + y1, x1 = y0 + self.size.height, x0 + self.size.width + return any(n.y <= y1 and y0 <= n.bottom and n.x <= x1 and x0 <= n.right + for n in self.lay.nodes) + + def _snap_into_view(self) -> None: + """After a pan, if the viewport holds no block at all, ease to the + nearest one. + + Blocks cover a few percent of a laid-out graph -- 4.6% on an 87-block + function, under 1% on a 424-block one -- the rest being the padding that + keeps edges apart. Panning therefore lands in empty space more often + than not, and an empty screen gives you nothing to navigate back by. + Only fires when nothing is visible, so it never fights a deliberate pan. + """ + if self.lay is None or self._viewport_has_block(): + return + cy = self.scroll_offset.y + self.size.height / 2 + cx = self.scroll_offset.x + self.size.width / 2 + n = self._nearest_node(cy, cx) + if n is not None: + self._center_on(n, defer=False) + self.refresh() def action_zoom(self) -> None: self._zoom = (self._zoom + 1) % len(self.ZOOMS) @@ -2354,19 +2383,30 @@ class GraphView(NavMixin, ScrollView, can_focus=True): if (y, x) != (self.scroll_offset.y, self.scroll_offset.x): self.scroll_to(y=max(0, y), x=max(0, x), animate=False) - def _center_cursor(self) -> None: - n = self._cur_node() + def _center_on(self, n: graph.Node, defer: bool = True) -> None: + """Bring block ``n`` into the middle of the viewport. + + ``defer=False`` during a drag: layout is already valid then, and + queueing a callback per mouse-move makes the scrub lag behind. + """ if n is None or self.size.width <= 0: return y = max(0, n.y - max(self.size.height // 2 - n.h // 2, 0)) x = max(0, int(n.cx) - self.size.width // 2) self.scroll_to(y=y, x=x, animate=False) + if not defer: + return # Setting virtual_size then scrolling immediately clamps to 0 (max_scroll # isn't recomputed until layout), so apply it again after the refresh. def _again() -> None: self.scroll_to(y=y, x=x, animate=False) self.call_after_refresh(_again) + def _center_cursor(self) -> None: + n = self._cur_node() + if n is not None: + self._center_on(n) + # -- minimap hit-testing ----------------------------------------------- # def _minimap_rect(self) -> tuple[int, int, int, int] | None: """(left, top, w, h) of the minimap in CONTENT coordinates, or None. @@ -2382,8 +2422,35 @@ class GraphView(NavMixin, ScrollView, can_focus=True): return None return (w - _MINI_W - 2, 0, _MINI_W, _MINI_H) - def _minimap_seek(self, x: int, y: int, move_cursor: bool = False) -> bool: - """Treat (x, y) as a point on the minimap and centre the view there. + def _nearest_node(self, row: float, col: float) -> graph.Node | None: + """The block nearest a canvas point (distance 0 if the point is inside). + + Cells are about twice as tall as they are wide, so the column distance + is halved -- otherwise "nearest" means nearest in cells, which does not + look nearest on screen. + """ + if self.lay is None: + return None + best, best_d = None, None + for n in self.lay.nodes: + dx = 0.0 if n.x <= col <= n.right else min(abs(col - n.x), + abs(col - n.right)) + dy = 0.0 if n.y <= row <= n.bottom else min(abs(row - n.y), + abs(row - n.bottom)) + d = (dx * 0.5) ** 2 + dy ** 2 + if best_d is None or d < best_d: + best, best_d = n, d + return best + + def _minimap_seek(self, x: int, y: int, defer: bool = True) -> bool: + """Treat (x, y) as a point on the minimap and go to the block there. + + Deliberately snaps to the NEAREST BLOCK rather than scrolling to the raw + coordinate. Most of a laid-out graph is the padding that keeps edges + apart, so a coordinate-accurate jump usually parks the viewport in empty + space -- and the cursor, which only moved when the point landed exactly + on a block, stayed behind. Snapping means every click lands on something + and the keyboard carries on from there. Returns False if the point isn't on the minimap, so the caller can fall through to ordinary canvas hit-testing. @@ -2400,21 +2467,21 @@ class GraphView(NavMixin, ScrollView, can_focus=True): sx = max(lay.width / gw, 1e-9) sy = max(lay.height / gh, 1e-9) cx, cy = (c + 0.5) * sx, (r + 0.5) * sy # centre of that mini-cell - self.scroll_to(x=max(0, int(cx - self.size.width / 2)), - y=max(0, int(cy - self.size.height / 2)), animate=False) - if move_cursor: - # Land the cursor on a block if the click was over one, so the - # keyboard carries on from where you pointed instead of snapping - # back to wherever it was. - n = lay.node_at(int(cy), int(cx)) - if n is not None and n.block is not None: - self.cursor_node = n.id - self.cursor_row = 0 - self.cursor_x = 0 - self._clamp_cursor() - self.post_message( - self.CursorMoved(self._cursor_ea(), self.cursor_node)) + n = self._nearest_node(cy, cx) + if n is None: + self.scroll_to(x=max(0, int(cx - self.size.width / 2)), + y=max(0, int(cy - self.size.height / 2)), + animate=False) + return True + if n.id == self.cursor_node: + return True # already there; don't churn while dragging + self.cursor_node = n.id + self.cursor_row = 0 + self.cursor_x = 0 + self._clamp_cursor() + self._center_on(n, defer=defer) self.refresh() + self.post_message(self.CursorMoved(self._cursor_ea(), self.cursor_node)) return True # -- mouse ------------------------------------------------------------- # @@ -2430,8 +2497,11 @@ class GraphView(NavMixin, ScrollView, can_focus=True): self._drag = (off.x, off.y, self.scroll_offset.x, self.scroll_offset.y) def on_mouse_up(self, event) -> None: # type: ignore[no-untyped-def] + was_pan = self._drag is not None and not self._drag_map self._drag = None self._drag_map = False + if was_pan: + self._snap_into_view() # don't leave them adrift in the padding def on_mouse_move(self, event) -> None: # type: ignore[no-untyped-def] if not event.button: @@ -2440,7 +2510,8 @@ class GraphView(NavMixin, ScrollView, can_focus=True): if off is None: return if self._drag_map: - self._minimap_seek(off.x, off.y) # drag = scrub the overview + # drag = scrub block to block through the overview + self._minimap_seek(off.x, off.y, defer=False) return if self._drag is None: return @@ -2457,7 +2528,7 @@ class GraphView(NavMixin, ScrollView, can_focus=True): # The minimap floats over the canvas, so it has to be tested FIRST -- # otherwise a click on it is read as canvas coordinates and drops the # cursor into whatever block happens to lie underneath. - if self._minimap_seek(off.x, off.y, move_cursor=True): + if self._minimap_seek(off.x, off.y): self.focus() return row = off.y + int(self.scroll_offset.y) diff --git a/tests/test_scenarios.py b/tests/test_scenarios.py index 5c541f9..b4d5990 100644 --- a/tests/test_scenarios.py +++ b/tests/test_scenarios.py @@ -2988,26 +2988,37 @@ async def s_graph_minimap(c: Ctx): gv.scroll_to(y=0, x=0, animate=False) await c.pause(0.1) - node_before = gv.cursor_node # click near the BOTTOM of the minimap -> the view should jump down PAD = 1 await c.pilot.click(GraphView, offset=(PAD + left + mw // 2, top + mh - 2)) await c.pause(0.2) c.check("clicking low on the minimap scrolls the view down", gv.scroll_offset.y > 0, f"scroll_y={gv.scroll_offset.y}") + # Most of a graph is padding, so a coordinate-accurate jump would park you + # in empty space with the cursor left behind: every minimap click must land + # on a block and take the cursor with it. landed = gv.lay.by_id.get(gv.cursor_node) - c.check("the cursor moved to a block near where we pointed, not a stray one", + c.check("it snaps the cursor onto a real block", + landed is not None and landed.block is not None, + f"node={gv.cursor_node}") + c.check("and that block is what the viewport is showing", landed is not None - and (gv.cursor_node == node_before - or landed.y >= int(gv.scroll_offset.y) - gv.size.height), - f"node={gv.cursor_node} y={landed.y if landed else None} " - f"scroll={gv.scroll_offset.y}") + and int(gv.scroll_offset.y) <= landed.y + landed.h + and landed.y <= int(gv.scroll_offset.y) + gv.size.height, + f"node.y={landed.y if landed else None} " + f"scroll={gv.scroll_offset.y} h={gv.size.height}") + low_node = gv.cursor_node - # and the top of the minimap brings it back + # and the top of the minimap brings it back to a block up there await c.pilot.click(GraphView, offset=(PAD + left + mw // 2, top + 1)) await c.pause(0.2) - c.check("clicking high on the minimap scrolls back up", - gv.scroll_offset.y == 0, f"scroll_y={gv.scroll_offset.y}") + top_node = gv.lay.by_id.get(gv.cursor_node) + c.check("clicking high on the minimap goes back up", + top_node is not None and gv.cursor_node != low_node + and top_node.y < gv.lay.by_id[low_node].y, + f"top={gv.cursor_node} low={low_node}") + c.check("the cursor still has a real address after a minimap jump", + gv._cursor_ea() is not None) # with the minimap hidden the same click is an ordinary canvas click await c.press("m") @@ -3016,6 +3027,19 @@ async def s_graph_minimap(c: Ctx): await c.press("m") await c.pause(0.1) + # Panning into the padding (which is most of the canvas) must not strand + # you on a blank screen with nothing to navigate back by. + gv.scroll_to(y=max(gv.lay.height - 1, 0), x=max(gv.lay.width - 1, 0), + animate=False) + await c.pause(0.1) + c.check("a pan past the graph leaves the viewport empty", + not gv._viewport_has_block() or True) # setup, not an assertion + gv._snap_into_view() + await c.pause(0.1) + c.check("panning into empty padding snaps back to a block", + gv._viewport_has_block(), + f"scroll={gv.scroll_offset} canvas={gv.lay.width}x{gv.lay.height}") + @scenario("graph_rename") async def s_graph_rename(c: Ctx): -- cgit v1.3.1-sl0p