diff options
| -rw-r--r-- | docs/GRAPH_VIEW.md | 11 | ||||
| -rw-r--r-- | idatui/app.py | 111 | ||||
| -rw-r--r-- | 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): |