From 57e73c58ea66f8611d383954dd3d1fd376ca9e72 Mon Sep 17 00:00:00 2001 From: Dominik Roth Date: Sun, 9 Aug 2026 20:04:55 +0200 Subject: [PATCH] Fix real zoom+square-cells bugs, add drag-to-pan, simplify popover buttons Every fix here was verified directly (numeric checks against the view math, rendered screenshots, GTK smoke tests), not just reasoned about, after the previous zoom/square pass shipped with real problems still in it. Map fixes: - Off-viewport entities bled through onto the canvas when zoomed in: _km_to_px() has no inherent bound, an entity actually elsewhere on the map still got projected and drawn if the result happened to land within the canvas's pixel bounds. Clipped all marker/overlay/arrow/ scout-flight drawing to the grid's own visible rectangle. - Gridlines themselves bled into the label margin above/left of the grid: the line-drawing loop deliberately over-generated a couple of lines past the true viewport edge (meant to cover a trailing partial cell, which doesn't need its own line, just the whole-integer lines already bounding it). Switched to ceil/floor bounds that only ever produce in-range lines, verified directly against the view math for a fractional zoom/pan (no clip needed for this half, the bug was generating the wrong lines in the first place, not failing to hide them). - Square-cell letterbox padding stayed pixel-identical at every zoom level (verified: same padding at 1x/2x/5x), which meant the fixed bands ate a bigger and bigger share of an already-zoomed-in view. Once actually zoomed in with square_cells on, the viewport's own aspect now follows the canvas shape instead of staying locked to the full map's 20:10 ratio, so cells come out square with zero letterboxing rather than fixed padding. The whole-map view (zoom=1) is unchanged, that's the one case that has a real reason to keep the fixed 20:10 shape. - Added drag-to-pan, active once actually zoomed in (at 1x the whole map's already on screen, nothing to pan to). Guards against a drag's release also firing as a click-select via a small pixel threshold. Popover cleanup (Nest/Spotters/Reference Points/Targets): - Removed each popover's own 'Load all from screenshot' bulk button and every row's individual 'set this one from screenshot' button: the universal clipboard button in the header already re-parses a fresh screenshot/paste and merges it into everything it recognizes by name, these were redundant category- and item-specific ways to trigger the exact same merge. Deleted the now-dead _set_*_from_screenshot_info() methods along with them. - The trailing 'Add spotter'/'Add RP'/'Add target'/'Add scout flight' rows are now a single full-width button (new _add_row() helper) instead of the full two-button _row() layout, there's nothing to screenshot into for something that doesn't exist yet. Verified: full pytest suite, direct numeric check that no gridline lands outside the grid rectangle at a fractional zoom/pan, a rendered screenshot at 4x zoom confirming off-viewport entities no longer appear and gridlines terminate cleanly at the edges, and a full-app smoke test exercising drag-pan end to end including the post-drag click-suppression. Co-Authored-By: Claude Sonnet 5 --- src/fenigma/app.py | 157 ++++++++++--------------------------- src/fenigma/grid_widget.py | 135 +++++++++++++++++++++++++++---- 2 files changed, 159 insertions(+), 133 deletions(-) diff --git a/src/fenigma/app.py b/src/fenigma/app.py index c9f4a7c..e0ea7f5 100644 --- a/src/fenigma/app.py +++ b/src/fenigma/app.py @@ -49,7 +49,6 @@ def _row( name: str, *, on_input, - on_screenshot, on_remove=None, hidden=False, on_toggle_hidden=None, @@ -59,8 +58,13 @@ def _row( on_toggle_alive=None, on_convert=None, ) -> Gtk.Widget: - """One list entry: a label plus input/screenshot/geo/hide[/alive] - [/convert][/remove] action buttons.""" + """One list entry: a label plus input/geo/hide[/alive][/convert] + [/remove] action buttons. There used to also be a per-item 'set + from screenshot' button here, dropped: the universal clipboard + button in the header already re-parses a fresh screenshot/paste and + merges it into whatever it recognizes (including this exact item by + name), a second, item-specific way to trigger that same merge was + redundant.""" box = Gtk.Box(orientation=Gtk.Orientation.HORIZONTAL, spacing=6) box.set_margin_top(4) box.set_margin_bottom(4) @@ -80,11 +84,6 @@ def _row( input_btn.connect("clicked", lambda _b: on_input()) box.append(input_btn) - shot_btn = Gtk.Button(icon_name="insert-image-symbolic", - tooltip_text="Set coords from clipboard (screenshot or pasted text)") - shot_btn.connect("clicked", lambda _b: on_screenshot()) - box.append(shot_btn) - if on_toggle_show_geo is not None: geo_btn = Gtk.Button( icon_name="starred-symbolic" if show_geo else "non-starred-symbolic", @@ -131,6 +130,22 @@ def _row( return box +def _add_row(label: str, on_click) -> Gtk.Widget: + """The trailing 'Add ' row in a popover: just one button, not + the full _row() layout (which always pairs a manual-input action + with a per-item screenshot action). There's nothing to screenshot + *into* yet for something that doesn't exist, and the universal + clipboard button in the header already covers 'load everything from + a screenshot' for every category, a second, category-specific + version of that here was redundant.""" + box = Gtk.Box(orientation=Gtk.Orientation.HORIZONTAL, spacing=6, + margin_top=4, margin_bottom=4, margin_start=8, margin_end=8) + btn = Gtk.Button(label=label, hexpand=True) + btn.connect("clicked", lambda _b: on_click()) + box.append(btn) + return box + + def _scout_flight_row(sf, *, on_replot, on_remove, on_toggle_hidden) -> Gtk.Widget: """One scout-flight list entry: unlike _row(), there's no coordinate input/screenshot/geo-overlay, just where it's plotted, a hide toggle, @@ -605,47 +620,6 @@ class MainWindow(Adw.ApplicationWindow): self._refresh() return changed - def _set_nest_from_screenshot_info(self, info: "ocr.ParsedInfo") -> None: - if info.nest_coord is None: - self.toast("Nest position not found in screenshot.") - return - self.board.nest.coord = info.nest_coord - self._refresh() - self.toast(f"Nest set to {info.nest_coord.label()} from screenshot.") - - def _set_spotter_from_screenshot_info(self, info: "ocr.ParsedInfo", spotter) -> None: - coord = info.spotters.get(spotter.id) - if coord is None: - self.toast(f"{spotter.name} not found in screenshot.") - return - spotter.coord = coord - self._refresh() - self.toast(f"{spotter.name} set to {coord.label()} from screenshot.") - - def _set_rp_from_screenshot_info(self, info: "ocr.ParsedInfo", rp) -> None: - result = info.reference_points.get(rp.rp_name) - if result is None: - self.toast(f"{rp.name} not found in screenshot.") - return - raw, clues, coord = result - self._merge_parsed_location(rp, raw, clues, coord) - self._refresh() - self.toast(f"{rp.name} set from screenshot.") - - def _set_target_from_screenshot_info(self, info: "ocr.ParsedInfo", target) -> None: - result = info.targets.get((target.type, target.id)) - if result is None: - self.toast(f"{target.name} not found in screenshot.") - return - raw, clues, coord, shell, requested_time = result - self._merge_parsed_location(target, raw, clues, coord) - if shell is not None: - target.shell = shell - if requested_time is not None: - target.requested_time = requested_time - self._refresh() - self.toast(f"{target.name} set from screenshot.") - def _open_coord_dialog( self, *, title, on_submit, show_id=False, show_type=False, id_placeholder=None, initial_location=None, initial_id=None, initial_type=None, @@ -732,7 +706,6 @@ class MainWindow(Adw.ApplicationWindow): on_submit=lambda loc, _id, _t: self._apply_and_refresh(nest, loc), initial_location=nest.location, ), - on_screenshot=lambda: self._run_ocr_from_clipboard(self._set_nest_from_screenshot_info), hidden=nest.hidden, on_toggle_hidden=lambda: self._toggle_hidden(nest, rebuild), show_geo=nest.show_geo_desc, @@ -746,14 +719,6 @@ class MainWindow(Adw.ApplicationWindow): box.set_margin_top(6) box.set_margin_bottom(6) - load_btn = Gtk.Button(label="Load all from screenshot") - load_btn.set_margin_start(8) - load_btn.set_margin_end(8) - load_btn.set_margin_bottom(4) - load_btn.connect("clicked", lambda _b: self._run_ocr_from_clipboard(self._merge_spotters)) - box.append(load_btn) - box.append(Gtk.Separator()) - for sp in list(self.board.spotters): box.append(_row( f"{sp.name} ({_location_status(sp)})", @@ -762,9 +727,6 @@ class MainWindow(Adw.ApplicationWindow): on_submit=lambda loc, _id, _t, sp=sp: self._apply_and_refresh(sp, loc), initial_location=sp.location, ), - on_screenshot=lambda sp=sp: self._run_ocr_from_clipboard( - lambda info, sp=sp: self._set_spotter_from_screenshot_info(info, sp) - ), on_remove=lambda sp=sp: self._remove_spotter(sp, rebuild), hidden=sp.hidden, on_toggle_hidden=lambda sp=sp: self._toggle_hidden(sp, rebuild), @@ -773,16 +735,12 @@ class MainWindow(Adw.ApplicationWindow): )) box.append(Gtk.Separator()) - box.append(_row( - "Add spotter", - on_input=lambda: self._open_coord_dialog( - title="Add spotter", - on_submit=lambda loc, id_, _t: self._add_spotter(loc, id_), - show_id=True, - id_placeholder=f"ID (blank = auto, next: {self.board.next_spotter_id()})", - ), - on_screenshot=lambda: self._run_ocr_from_clipboard(self._merge_spotters), - )) + box.append(_add_row("Add spotter", lambda: self._open_coord_dialog( + title="Add spotter", + on_submit=lambda loc, id_, _t: self._add_spotter(loc, id_), + show_id=True, + id_placeholder=f"ID (blank = auto, next: {self.board.next_spotter_id()})", + ))) return box def _add_spotter(self, location: Location, id_text: str | None) -> None: @@ -807,14 +765,6 @@ class MainWindow(Adw.ApplicationWindow): box.set_margin_top(6) box.set_margin_bottom(6) - load_btn = Gtk.Button(label="Load all from screenshot") - load_btn.set_margin_start(8) - load_btn.set_margin_end(8) - load_btn.set_margin_bottom(4) - load_btn.connect("clicked", lambda _b: self._run_ocr_from_clipboard(self._merge_reference_points)) - box.append(load_btn) - box.append(Gtk.Separator()) - for rp in list(self.board.reference_points): box.append(_row( f"{rp.name} ({_location_status(rp)})", @@ -823,9 +773,6 @@ class MainWindow(Adw.ApplicationWindow): on_submit=lambda loc, _id, _t, rp=rp: self._apply_and_refresh(rp, loc), initial_location=rp.location, ), - on_screenshot=lambda rp=rp: self._run_ocr_from_clipboard( - lambda info, rp=rp: self._set_rp_from_screenshot_info(info, rp) - ), on_remove=lambda rp=rp: self._remove_rp(rp, rebuild), hidden=rp.hidden, on_toggle_hidden=lambda rp=rp: self._toggle_hidden(rp, rebuild), @@ -835,14 +782,10 @@ class MainWindow(Adw.ApplicationWindow): )) box.append(Gtk.Separator()) - box.append(_row( - "Add RP", - on_input=lambda: self._open_coord_dialog( - title="Add reference point", - on_submit=lambda loc, _id, _t: self._add_rp(loc), - ), - on_screenshot=lambda: self._run_ocr_from_clipboard(self._merge_reference_points), - )) + box.append(_add_row("Add RP", lambda: self._open_coord_dialog( + title="Add reference point", + on_submit=lambda loc, _id, _t: self._add_rp(loc), + ))) return box def _add_rp(self, location: Location) -> None: @@ -875,14 +818,6 @@ class MainWindow(Adw.ApplicationWindow): box.set_margin_top(6) box.set_margin_bottom(6) - load_btn = Gtk.Button(label="Load all from screenshot") - load_btn.set_margin_start(8) - load_btn.set_margin_end(8) - load_btn.set_margin_bottom(4) - load_btn.connect("clicked", lambda _b: self._run_ocr_from_clipboard(self._merge_targets)) - box.append(load_btn) - box.append(Gtk.Separator()) - for t in list(self.board.targets): box.append(_row( f"{t.name} ({_location_status(t)})", @@ -891,9 +826,6 @@ class MainWindow(Adw.ApplicationWindow): on_submit=lambda loc, _id, _t2, t=t: self._apply_and_refresh(t, loc), initial_location=t.location, ), - on_screenshot=lambda t=t: self._run_ocr_from_clipboard( - lambda info, t=t: self._set_target_from_screenshot_info(info, t) - ), on_remove=lambda t=t: self._remove_target(t, rebuild), hidden=t.hidden, on_toggle_hidden=lambda t=t: self._toggle_hidden(t, rebuild), @@ -905,16 +837,12 @@ class MainWindow(Adw.ApplicationWindow): )) box.append(Gtk.Separator()) - box.append(_row( - "Add target", - on_input=lambda: self._open_coord_dialog( - title="Add target", - on_submit=lambda loc, id_, type_: self._add_target(loc, id_, type_), - show_id=True, - show_type=True, - ), - on_screenshot=lambda: self._run_ocr_from_clipboard(self._merge_targets), - )) + box.append(_add_row("Add target", lambda: self._open_coord_dialog( + title="Add target", + on_submit=lambda loc, id_, type_: self._add_target(loc, id_, type_), + show_id=True, + show_type=True, + ))) return box def _add_target(self, location: Location, id_, type_) -> None: @@ -950,12 +878,7 @@ class MainWindow(Adw.ApplicationWindow): )) box.append(Gtk.Separator()) - add_row = Gtk.Box(orientation=Gtk.Orientation.HORIZONTAL, spacing=6, - margin_top=4, margin_bottom=4, margin_start=8, margin_end=8) - add_btn = Gtk.Button(label="Add scout flight") - add_btn.connect("clicked", lambda _b: self._add_scout_flight()) - add_row.append(add_btn) - box.append(add_row) + box.append(_add_row("Add scout flight", self._add_scout_flight)) return box def _add_scout_flight(self) -> None: diff --git a/src/fenigma/grid_widget.py b/src/fenigma/grid_widget.py index 90e3eb0..a8eaf60 100644 --- a/src/fenigma/grid_widget.py +++ b/src/fenigma/grid_widget.py @@ -107,6 +107,15 @@ class GridCanvas(Gtk.DrawingArea): self.pan_km = (COLS / 2.0, ROWS / 2.0) self._last_pointer_px: tuple[float, float] | None = None + # Drag-to-pan state, see _on_drag_begin/_on_drag_update. + # _drag_did_pan tracks whether the in-progress/just-finished + # drag actually moved the view (past a small pixel threshold, + # not just an ordinary click's own tiny jitter), so _on_click + # can skip treating that same gesture as a select/click too. + self._drag_start_pan_km: tuple[float, float] | None = None + self._drag_start_view: _View | None = None + self._drag_did_pan = False + self.set_hexpand(True) self.set_vexpand(True) self.set_draw_func(self._draw) @@ -120,6 +129,15 @@ class GridCanvas(Gtk.DrawingArea): scroll.connect("scroll", self._on_scroll) self.add_controller(scroll) + # Only actually pans once zoomed in (see _on_drag_update), at + # the default zoom the whole map's already on screen, nothing + # to drag to. + drag = Gtk.GestureDrag() + drag.connect("drag-begin", self._on_drag_begin) + drag.connect("drag-update", self._on_drag_update) + drag.connect("drag-end", self._on_drag_end) + self.add_controller(drag) + click = Gtk.GestureClick() click.connect("released", self._on_click) self.add_controller(click) @@ -198,6 +216,29 @@ class GridCanvas(Gtk.DrawingArea): self.square_cells = square self.queue_draw() + def _visible_extent(self, avail_w: float, avail_h: float) -> tuple[float, float]: + """(vis_cols, vis_rows): how much of the 20x10 grid the current + zoom level actually shows. Locked to the grid's own 20:10 shape + unless square_cells is on AND actually zoomed in, in which case + there's no reason a cropped view has to keep the whole map's + fixed shape (only the *full* map has a reason to look like + that), so it follows the canvas's own aspect instead: cells + come out square with zero letterboxing, rather than the same + fixed-size padding band persisting at every zoom level and + eating a bigger and bigger share of an already-zoomed-in view. + Clamped to ROWS/COLS so an extreme canvas aspect can't ask for a + viewport bigger than the whole map itself. Shared by _view() and + _on_scroll(), which both need the exact same numbers, a + mismatch between them would throw off the scroll-to-zoom anchor + math.""" + vis_cols = COLS / self.zoom + if self.square_cells and self.zoom > MIN_ZOOM: + vis_rows = min(vis_cols * avail_h / avail_w, ROWS) + vis_cols = min(vis_rows * avail_w / avail_h, COLS) + else: + vis_rows = ROWS / self.zoom + return vis_cols, vis_rows + def _view(self, width: int, height: int) -> _View: """Everything needed to convert km-space <-> pixels for one frame, see _View's own docstring. zoom==MIN_ZOOM (the default) @@ -206,8 +247,7 @@ class GridCanvas(Gtk.DrawingArea): avail_w = max(width - MARGIN_LEFT - MARGIN_RIGHT, 1) avail_h = max(height - MARGIN_TOP - MARGIN_BOTTOM, 1) - vis_cols = COLS / self.zoom - vis_rows = ROWS / self.zoom + vis_cols, vis_rows = self._visible_extent(avail_w, avail_h) cx, cy = self.pan_km ox = min(max(cx - vis_cols / 2, 0.0), COLS - vis_cols) oy = min(max(cy - vis_rows / 2, 0.0), ROWS - vis_rows) @@ -251,8 +291,9 @@ class GridCanvas(Gtk.DrawingArea): self.zoom = min(max(self.zoom * (ZOOM_STEP ** -dy), MIN_ZOOM), MAX_ZOOM) - vis_cols = COLS / self.zoom - vis_rows = ROWS / self.zoom + avail_w = max(width - MARGIN_LEFT - MARGIN_RIGHT, 1) + avail_h = max(height - MARGIN_TOP - MARGIN_BOTTOM, 1) + vis_cols, vis_rows = self._visible_extent(avail_w, avail_h) frac_x = (px - MARGIN_LEFT - view_before.pad_x) / view_before.grid_w if view_before.grid_w else 0.5 frac_y = 1 - (py - MARGIN_TOP - view_before.pad_y) / view_before.grid_h if view_before.grid_h else 0.5 # _view() clamps this back onto the grid itself if it would @@ -265,6 +306,34 @@ class GridCanvas(Gtk.DrawingArea): self.queue_draw() return True + def _on_drag_begin(self, _gesture, _x, _y) -> None: + if self.zoom <= MIN_ZOOM or self.placement_callback is not None: + return # the whole map's already on screen, nothing to pan to + self._drag_start_pan_km = self.pan_km + self._drag_start_view = self._view(self.get_width(), self.get_height()) + self._drag_did_pan = False + + def _on_drag_update(self, _gesture, offset_x: float, offset_y: float) -> None: + if self._drag_start_view is None: + return + if math.hypot(offset_x, offset_y) > 3: + self._drag_did_pan = True + view = self._drag_start_view + # Dragging pans opposite to how zooming re-centers: the content + # follows the cursor (drag right -> content moves right, drag + # down -> content moves down), like dragging a piece of paper, + # not like moving a camera. Derived directly from _km_to_px()'s + # own formula: solving for how much ox/oy (and so pan_km, which + # is just their re-centered form) has to change for a given + # point_km to land `offset` pixels away from where it started. + cx, cy = self._drag_start_pan_km + self.pan_km = (cx - offset_x / view.cell_w, cy + offset_y / view.cell_h) + self.queue_draw() + + def _on_drag_end(self, _gesture, _offset_x, _offset_y) -> None: + self._drag_start_pan_km = None + self._drag_start_view = None + def _excluded_from_map(self, obj) -> bool: """True if `obj` should be dropped from the map view entirely, it's hidden, or it's a dead target with the map's dead-hiding @@ -345,6 +414,15 @@ class GridCanvas(Gtk.DrawingArea): self.on_hover_change(None, None) def _on_click(self, _gesture, _n_press, x: float, y: float) -> None: + if self._drag_did_pan: + # The GestureDrag that just finished actually panned the + # view (past the jitter threshold), don't also treat its + # release as a click-to-select, that would either select + # whatever ended up under the cursor after the pan or + # deselect the current selection, neither of which is what + # a drag gesture was for. + self._drag_did_pan = False + return view = self._view(self.get_width(), self.get_height()) if self.placement_callback is not None: cursor_km = self._px_to_km(view, x, y) @@ -382,38 +460,61 @@ class GridCanvas(Gtk.DrawingArea): view = self._view(width, height) - # Only the columns/rows actually within the visible viewport, - # not always 0..COLS/0..ROWS, once zoomed in most of the grid - # isn't on screen at all. +2 on the upper bound of range(): one - # to cover the trailing partial cell (int() truncates toward the - # viewport's start), one more because range()'s own upper bound - # is exclusive. - first_col, last_col = int(view.ox), int(view.ox + view.vis_cols) + 2 - first_row, last_row = int(view.oy), int(view.oy + view.vis_rows) + 2 + # Grid lines only for integer boundaries actually within the + # visible viewport, not always 0..COLS/0..ROWS, once zoomed in + # most of the grid isn't on screen at all. A previous version + # over-generated a couple of lines past the true edge (meant to + # cover a trailing partial cell, which doesn't need its own line + # in the first place, just the whole-integer lines bounding it, + # already included here) and those, drawn before anything gets + # clipped, showed up as stray lines bleeding into the label + # margin above/left of the actual grid. ceil/floor here means + # every c/r produced is guaranteed to already land inside the + # grid rectangle, nothing to clip. + first_col, last_col = math.ceil(view.ox), math.floor(view.ox + view.vis_cols) + first_row, last_row = math.ceil(view.oy), math.floor(view.oy + view.vis_rows) cr.set_source_rgba(*GRID_LINE) cr.set_line_width(1) - for c in range(first_col, min(last_col, COLS + 1)): + for c in range(max(first_col, 0), min(last_col, COLS) + 1): x, _ = self._km_to_px(view, (c, 0)) cr.move_to(x, MARGIN_TOP + view.pad_y) cr.line_to(x, MARGIN_TOP + view.pad_y + view.grid_h) - for r in range(first_row, min(last_row, ROWS + 1)): + for r in range(max(first_row, 0), min(last_row, ROWS) + 1): _, y = self._km_to_px(view, (0, r)) cr.move_to(MARGIN_LEFT + view.pad_x, y) cr.line_to(MARGIN_LEFT + view.pad_x + view.grid_w, y) cr.stroke() + # Column/row labels: one per whole large-cell that's at least + # partly visible (its own span overlaps the viewport), not + # pegged to the gridline boundaries above, a partially-visible + # edge cell still gets its letter/number shown. cr.set_source_rgb(*LABEL) cr.set_font_size(11) - for i in range(max(first_col, 0), min(last_col, COLS)): + for i in range(max(math.floor(view.ox), 0), min(math.ceil(view.ox + view.vis_cols), COLS)): x, _ = self._km_to_px(view, (i + 0.5, 0)) cr.move_to(x - 4, MARGIN_TOP + view.pad_y - 10) cr.show_text(LARGE_X[i]) - for r in range(max(first_row, 0), min(last_row, ROWS)): + for r in range(max(math.floor(view.oy), 0), min(math.ceil(view.oy + view.vis_rows), ROWS)): _, y = self._km_to_px(view, (0, r + 0.5)) cr.move_to(4, y + 4) cr.show_text(str(r + 1)) + # Everything below projects a km position to a pixel one with no + # inherent bound, an entity that's genuinely elsewhere on the + # map (outside the current zoomed viewport) would otherwise + # still get drawn whenever its projected pixel position happens + # to land inside the canvas's own bounds (including the + # letterbox padding bands), showing up as markers/lines that + # look like they're floating off the visible grid. Clipping to + # the grid's own drawable rectangle is a single fix for all of + # it (markers, overlays, arrows, scout flights) rather than + # teaching every draw call its own visibility check. + cr.save() + cr.rectangle(MARGIN_LEFT + view.pad_x, MARGIN_TOP + view.pad_y, view.grid_w, view.grid_h) + cr.clip() + self._draw_geo_overlays(cr, view) self._draw_firing_arrows(cr, view) self._draw_blast_radius(cr, view) @@ -459,6 +560,8 @@ class GridCanvas(Gtk.DrawingArea): cr.show_text(f"{grid_name} {sf.bearing_deg:05.1f}°") cr.set_font_size(11) + cr.restore() + def _draw_marker(self, cr, view, point_km, color, label, canvas_width, canvas_height, *, hollow=False, dim=False, selected=False, coord=None, extra_line=None) -> None: