perf(linux): paint the grid subgrid and its pointer in one repaint (#1510)

Author: y3owk1n

docs/adr/0003-overlay-frame-port.md

@@ -81,6 +81,23 @@ are declarative; updates — hot, already narrow, already correct — are not.
   measurable time cost. That is the price this ADR said a fully declarative
   port would charge on every keystroke, paid only where the surface was
   already being repainted anyway.
+- **The update half widens; it does not become a frame.** #1492 is the first
+  time the hybrid's imperative side hit the problem the declarative side solves
+  by construction: grid mode's selection keystroke opens a subgrid *and* moves
+  the pointer, and on Linux both are painted into one Cairo target, so two
+  calls were two full repaints of it per key — held arrow keys inside a subgrid
+  included. The frame path would have fixed it and is exactly what this ADR
+  rules out here, so `ShowGridSubgrid` took the pointer as an argument instead,
+  the way `RecursiveGridFrame` already carried it. Measured the way this ADR's
+  previous entry was, interleaved over the simulation harness (n=8, Apple M3):
+  the selection keystroke's grid-surface updates went 2 → 1 (p=0.000) with time
+  and allocations unchanged (~4.74µs → ~4.79µs, p=0.65; 18 allocations either
+  way), and grid narrowing — the keystroke this ADR exists to protect — stayed
+  unchanged again (~5.25µs both ways, p=0.80). Widening a hot call is therefore
+  the move when a keystroke changes two things on one surface, and the frame
+  path is still not. The leaving half took the matching share: `ClearFrame`
+  drops what the update calls left, because a mode resetting it by hand was a
+  mode repainting a surface it was about to clear.
 - **The last two surfaces converted, and the mode handler kept one overlay
   reference.** Done in #1212: `MonitorSelectFrame` carries the displays on
   offer, and `ScrollFrame` carries nothing at all — scroll is a mode the

internal/adapter/overlay/AGENTS.md

@@ -127,6 +127,21 @@ func (a *Adapter) RedrawFrame(ctx context.Context, frame ports.Frame) error {
 }
 
 // ClearFrame takes the frame on screen off it and returns the overlay to idle.
+//
+// It also drops what the grid's incremental calls left behind, which grid mode
+// used to reset for itself on the way out (#1492). Both run after the surface
+// has been cleared, which is what makes them cost nothing where the reset used
+// to cost a repaint: the hide-unmatched flag is a flag, and a backend that
+// repaints on a pointer change has already forgotten the pointer, so the hide
+// is the statement without the repaint. What each backend does beyond that is
+// its own — macOS marks its emptied view for redisplay either way — and this is
+// teardown, not a keystroke. The match prefix needs no reset at all: a grid
+// coming back up is a transition, which clears and redraws in full.
+//
+// The pair is unconditional rather than gated on grid mode having been the one
+// on screen. The leaving half does not ask which mode it is leaving — that is
+// what stops a caller from having to remember — and neither call means anything
+// to a surface no grid was drawn on.
 func (a *Adapter) ClearFrame(ctx context.Context) error {
 	err := contextAlive(ctx)
 	if err != nil {
@@ -138,6 +153,8 @@ func (a *Adapter) ClearFrame(ctx context.Context) error {
 	a.manager.Clear()
 	a.manager.ClearCache()
 	a.hideMonitorSelect()
+	a.manager.SetHideUnmatched(false)
+	a.manager.HideGridPointer(ModeGrid)
 	a.manager.Hide()
 	a.manager.SwitchTo(ModeIdle)
 	a.subgridDrawn.Store(false)
@@ -235,9 +252,18 @@ func (a *Adapter) SetGridHideUnmatched(hide bool) {
 }
 
 // ShowGridSubgrid opens the finer grid inside one cell, with the grid Style
-// the overlay already resolved.
-func (a *Adapter) ShowGridSubgrid(cell *domainGrid.Cell) {
-	a.manager.ShowSubgrid(cell, ResolvedStyle(a.styles).Grid)
+// the overlay already resolved and the pointer stand-in that belongs on the
+// same surface.
+//
+// The pointer rides the open rather than following it in a call of its own
+// (#1492), which is what makes the keystroke that picks a cell cost one repaint
+// on a backend that paints the two into one surface. Meeting it with the
+// resolved Style happens here, as it does for the pointer a recursive-grid
+// frame carries — appearance never travels from a mode.
+func (a *Adapter) ShowGridSubgrid(cell *domainGrid.Cell, pointer ports.GridPointer) {
+	style := ResolvedStyle(a.styles)
+
+	a.manager.ShowSubgrid(cell, style.Grid, gridSurfacePointer(pointer, style.VirtualPointer))
 	a.subgridDrawn.Store(true)
 }
 
@@ -532,7 +558,7 @@ func (a *Adapter) drawRecursiveGrid(frame ports.RecursiveGridFrame, kind drawKin
 		frame.NextLayout.Keys,
 		frame.NextLayout.Dimensions,
 		style.RecursiveGrid,
-		recursiveGridPointer(frame.Pointer, style.VirtualPointer),
+		gridSurfacePointer(frame.Pointer, style.VirtualPointer),
 	)
 	if drawErr != nil {
 		if derrors.IsNotSupported(drawErr) {
@@ -637,10 +663,10 @@ func (a *Adapter) drawScroll(kind drawKind) error {
 	return nil
 }
 
-// recursiveGridPointer meets the pointer a frame describes with the Style the
-// overlay resolved. Position is the caller's; everything else is appearance,
-// and appearance never travels on a frame.
-func recursiveGridPointer(
+// gridSurfacePointer meets the pointer a frame or a subgrid open describes with
+// the Style the overlay resolved. Position is the caller's; everything else is
+// appearance, and appearance never travels from a mode.
+func gridSurfacePointer(
 	pointer ports.GridPointer,
 	style VirtualPointerStyle,
 ) overlayRecursiveGrid.VirtualPointerState {

internal/adapter/overlay/adapter.go

@@ -116,6 +116,8 @@ type screenManager struct {
 	matchPrefix       string
 	hideUnmatched     bool
 	subgridCell       *domainGrid.Cell
+	subgridPointer    renderrecursivegrid.VirtualPointerState
+	subgridOpens      int
 
 	recursiveGrid      recursiveGridDraw
 	recursiveGridDraws int
@@ -203,8 +205,14 @@ func (m *screenManager) UpdateGridMatches(prefix string) {
 
 func (m *screenManager) SetHideUnmatched(hide bool) { m.hideUnmatched = hide }
 
-func (m *screenManager) ShowSubgrid(cell *domainGrid.Cell, _ rendergrid.Style) {
+func (m *screenManager) ShowSubgrid(
+	cell *domainGrid.Cell,
+	_ rendergrid.Style,
+	virtualPointer renderrecursivegrid.VirtualPointerState,
+) {
 	m.subgridCell = cell
+	m.subgridPointer = virtualPointer
+	m.subgridOpens++
 }
 
 func (m *screenManager) DrawRecursiveGrid(
@@ -499,7 +507,7 @@ func TestAdapterRedrawFrame_ErasesASubgridTheGridReplaces(t *testing.T) {
 		t.Fatal("fixture grid has no cells")
 	}
 
-	adapter.ShowGridSubgrid(cells[0])
+	adapter.ShowGridSubgrid(cells[0], ports.GridPointer{})
 
 	err := adapter.RedrawFrame(context.Background(), gridFrame(t, ""))
 	if err != nil {
@@ -651,13 +659,59 @@ func TestAdapterGridUpdates_ReachTheOverlayWithoutAFrame(t *testing.T) {
 		t.Fatal("fixture grid has no cells")
 	}
 
-	adapter.ShowGridSubgrid(cells[0])
+	adapter.ShowGridSubgrid(cells[0], ports.GridPointer{})
 
 	if manager.subgridCell != cells[0] {
 		t.Error("the subgrid's cell never reached the overlay")
 	}
 }
 
+// TestAdapterShowGridSubgrid_CarriesThePointerIntoTheSameCall is what #1492
+// asks of this seam: the keystroke that picks a cell moves the selection too,
+// and a backend that paints both into one surface must be told both at once or
+// it repaints that surface twice for one key.
+//
+// The appearance is asserted with the cell, because the pointer travels as
+// position alone from a mode: a subgrid open that carried the position and left
+// the Style behind would draw the default glyph rather than the user's.
+func TestAdapterShowGridSubgrid_CarriesThePointerIntoTheSameCall(t *testing.T) {
+	t.Parallel()
+
+	manager := newScreenManager()
+	adapter := overlay.NewAdapter(manager, pointerStyles{}, zap.NewNop())
+
+	cells := gridFrame(t, "").Grid.Cells()
+	if len(cells) == 0 {
+		t.Fatal("fixture grid has no cells")
+	}
+
+	adapter.ShowGridSubgrid(cells[0], ports.GridPointer{
+		Visible:  true,
+		Position: image.Pt(120, 240),
+	})
+
+	if manager.subgridOpens != 1 {
+		t.Fatalf("subgrid opened %d times, want 1", manager.subgridOpens)
+	}
+
+	want := renderrecursivegrid.VirtualPointerState{
+		Visible:   true,
+		Position:  image.Pt(120, 240),
+		Size:      pointerSize,
+		FillColor: pointerFill,
+		Char:      pointerChar,
+		FontName:  pointerFontFamily,
+	}
+	if manager.subgridPointer != want {
+		t.Errorf("subgrid opened with pointer %+v, want %+v", manager.subgridPointer, want)
+	}
+
+	if manager.pointer.mode != "" {
+		t.Errorf("the open also made a pointer call of its own (%+v); one call paints both",
+			manager.pointer)
+	}
+}
+
 // TestAdapterUpdateGridPointer_CarriesTheWholeResolvedAppearance pins what a
 // backend that paints the glyph itself needs and used to be denied: the char
 // and the font family travel with the size and the fill, so a backend with no
@@ -790,6 +844,47 @@ func TestAdapterClearFrame_TakesTheFrameOffScreen(t *testing.T) {
 	}
 }
 
+// TestAdapterClearFrame_DropsWhatTheGridUpdatesLeft is the other half of "content
+// and all" (#1492): the hide-unmatched flag and the pointer stand-in outlive a
+// clear unless this call drops them, and grid mode used to drop them itself —
+// which meant repainting a grid on the way to taking it off the screen.
+//
+// It is also what makes that free: both run after the surface is cleared, so a
+// backend that repaints on a pointer change has nothing left to repaint and its
+// own record is already empty. Losing this call would put the flag back on the
+// next session's first draw.
+func TestAdapterClearFrame_DropsWhatTheGridUpdatesLeft(t *testing.T) {
+	t.Parallel()
+
+	manager := newScreenManager()
+	adapter := overlay.NewAdapter(manager, pointerStyles{}, zap.NewNop())
+
+	adapter.SetGridHideUnmatched(true)
+	adapter.UpdateGridPointer(
+		domain.ModeGrid,
+		ports.GridPointer{Visible: true, Position: image.Pt(12, 34)},
+	)
+
+	clearErr := adapter.ClearFrame(context.Background())
+	if clearErr != nil {
+		t.Fatalf("ClearFrame() error = %v", clearErr)
+	}
+
+	if manager.hideUnmatched {
+		t.Error("unmatched cells are still asked to hide after the frame was cleared")
+	}
+
+	if manager.pointer.mode != overlay.ModeGrid || manager.pointer.visible {
+		t.Errorf("grid pointer left as %+v, want it taken off the grid surface",
+			manager.pointer)
+	}
+
+	// The clear comes first, so neither reset had a surface to paint on.
+	if manager.cleared == 0 {
+		t.Error("the surface was never cleared, so the resets ran against a live one")
+	}
+}
+
 // TestAdapterFrame_ReportsACanceledContext keeps a canceled activation from
 // drawing over whatever replaced it.
 func TestAdapterFrame_ReportsACanceledContext(t *testing.T) {

internal/adapter/overlay/adapter_test.go

@@ -451,12 +451,24 @@ func (m *Manager) UpdateGridMatches(prefix string) {
 	m.GridOverlay().UpdateMatches(prefix)
 }
 
-// ShowSubgrid shows a subgrid for the specified cell.
-func (m *Manager) ShowSubgrid(cell *domainGrid.Cell, style grid.Style) {
+// ShowSubgrid shows a subgrid for the specified cell, with the pointer
+// stand-in the selection that opened it asks for.
+//
+// The pointer is a layer of the overlay window here rather than something
+// painted into the cells, so applying it after the subgrid costs a layer
+// update and no redraw — which is why this platform never paid the double
+// repaint #1492 is about, and why taking the pointer as an argument changes
+// nothing it does.
+func (m *Manager) ShowSubgrid(
+	cell *domainGrid.Cell,
+	style grid.Style,
+	virtualPointer recursivegrid.VirtualPointerState,
+) {
 	if m.GridOverlay() == nil {
 		return
 	}
 	m.GridOverlay().ShowSubgrid(cell, style)
+	m.ApplyGridPointer(manager.ModeGrid, virtualPointer)
 }
 
 // SetHideUnmatched sets whether to hide unmatched cells.

internal/adapter/overlay/darwin/manager.go

@@ -159,7 +159,12 @@ func (n *headlessManager) DrawRecursiveGrid(
 func (n *headlessManager) UpdateGridMatches(prefix string) {}
 
 // ShowSubgrid is a no-op implementation.
-func (n *headlessManager) ShowSubgrid(cell *domainGrid.Cell, style rendergrid.Style) {}
+func (n *headlessManager) ShowSubgrid(
+	cell *domainGrid.Cell,
+	style rendergrid.Style,
+	virtualPointer renderrecursivegrid.VirtualPointerState,
+) {
+}
 
 // SetHideUnmatched is a no-op implementation.
 func (n *headlessManager) SetHideUnmatched(hide bool) {}