fix: hide cursor natively, keep OSD on top, pin codec extension contract (#3)

internal/display/x11.go

@@ -117,21 +117,27 @@ func stackModeForVideoAppear(role LayerRole) uint32 {
 	return xproto.StackModeBelow
 }
 
-// stackReassertTarget reports which video window the OSD overlay must sit
-// above after an external restack, and whether one exists. Only the OSD
-// overlay re-asserts its place: window managers promote fullscreened video
-// to the top of their stacking order after the video-appear restack, which
-// would bury the overlay — and its text — for the rest of the flight. The
-// fallback canvas keeps its event-driven lifecycle, and the video window is
-// never moved.
-func stackReassertTarget(role LayerRole, videoActive bool, topVideo xproto.Window, videoWindows map[xproto.Window]struct{}) (xproto.Window, bool) {
-	if role != RoleAbove || !videoActive || topVideo == 0 {
-		return 0, false
+// stackReassert decides the OSD overlay's stacking correction on its
+// refresh: above the tracked video window while video is live, and raised
+// to the top while video is absent — the fallback canvas raises itself to
+// the top when video ends and would otherwise bury the overlay's text under
+// the fullscreen presentation indefinitely. Only the OSD overlay ever
+// re-asserts; the canvas and video windows are never moved by this.
+func stackReassert(role LayerRole, videoActive bool, topVideo xproto.Window, videoWindows map[xproto.Window]struct{}) (sibling xproto.Window, mode uint32, ok bool) {
+	if role != RoleAbove {
+		return 0, 0, false
+	}
+	if !videoActive {
+		// No sibling: raise the overlay to the top of the stack.
+		return 0, xproto.StackModeAbove, true
+	}
+	if topVideo == 0 {
+		return 0, 0, false
 	}
 	if _, tracked := videoWindows[topVideo]; !tracked {
-		return 0, false
+		return 0, 0, false
 	}
-	return topVideo, true
+	return topVideo, xproto.StackModeAbove, true
 }
 
 // raisesOnVideoEnd reports whether a layer raises itself when video ends:
@@ -180,6 +186,43 @@ func osdFallbackFontMetrics() (ascent, descent, cell int16) {
 	return 11, 3, 6
 }
 
+// createWindowValues builds the CreateWindow value mask and values; the
+// order must follow the mask bit order (back pixel, event mask, cursor) so
+// the server reads each value correctly. The blank cursor hides the pointer
+// over the layer from its first frame, independently of video, wfb-ng or
+// any cursor-hiding helper process.
+func createWindowValues(backPixel uint32, eventMask uint32, cursor xproto.Cursor) (uint32, []uint32) {
+	values := []uint32{backPixel, eventMask, uint32(cursor)}
+	return xproto.CwBackPixel | xproto.CwEventMask | xproto.CwCursor, values
+}
+
+// newBlankCursor builds the invisible cursor from zeroed 1x1 source and
+// mask pixmaps: an all-zero mask displays nothing regardless of the source
+// contents, so the pointer disappears over the layer without any shell
+// polling or helper process. The pixmaps and graphics context are freed
+// immediately; the cursor itself lives until the session closes.
+func newBlankCursor(conn *xgb.Conn, screen *xproto.ScreenInfo) xproto.Cursor {
+	pixmap, _ := xproto.NewPixmapId(conn)
+	xproto.CreatePixmap(conn, screen.RootDepth, pixmap,
+		xproto.Drawable(screen.Root), 1, 1)
+	mask, _ := xproto.NewPixmapId(conn)
+	xproto.CreatePixmap(conn, screen.RootDepth, mask,
+		xproto.Drawable(screen.Root), 1, 1)
+	gc, _ := xproto.NewGcontextId(conn)
+	xproto.CreateGC(conn, gc, xproto.Drawable(pixmap), 0, nil)
+	unit := []xproto.Rectangle{{Width: 1, Height: 1}}
+	xproto.PolyFillRectangle(conn, xproto.Drawable(pixmap), gc, unit)
+	xproto.PolyFillRectangle(conn, xproto.Drawable(mask), gc, unit)
+	xproto.FreeGC(conn, gc)
+	cursor, _ := xproto.NewCursorId(conn)
+	// Zeroed colors and hotspot: with the all-zero mask nothing is ever
+	// displayed, so the fore/back colors never reach the screen.
+	xproto.CreateCursor(conn, cursor, pixmap, mask, 0, 0, 0, 0, 0, 0, 0, 0)
+	xproto.FreePixmap(conn, pixmap)
+	xproto.FreePixmap(conn, mask)
+	return cursor
+}
+
 // osdTextRect is the output region covering one drawn text: its glyph cell
 // around the baseline, widened by the shape padding.
 func osdTextRect(x, y, units int, cell, ascent, descent int16) xproto.Rectangle {
@@ -302,11 +345,15 @@ func (o X11Opener) Open(ctx context.Context) (Session, error) {
 	if o.Role == RoleAbove {
 		eventMask = osdEventMask
 	}
-	values := uint32(xproto.CwBackPixel | xproto.CwEventMask)
+	// The blank cursor hides the pointer over this layer from its first
+	// frame and stays on the layer across no-video/video transitions; the
+	// server reclaims it when the session's connection closes.
+	cursor := newBlankCursor(conn, screen)
+	valueMask, values := createWindowValues(screen.BlackPixel, eventMask, cursor)
 	xproto.CreateWindow(conn, screen.RootDepth, window, screen.Root,
 		0, 0, screen.WidthInPixels, screen.HeightInPixels, 0,
-		xproto.WindowClassInputOutput, screen.RootVisual, values,
-		[]uint32{screen.BlackPixel, eventMask})
+		xproto.WindowClassInputOutput, screen.RootVisual, valueMask,
+		values)
 	// Tag the window as a Ground Station layer and announce its EWMH role
 	// before mapping, so no observer or window manager ever sees an
 	// untagged frame of our own layers.
@@ -442,6 +489,7 @@ func (o X11Opener) Open(ctx context.Context) (Session, error) {
 		markerAtom:   markerAtom,
 		role:         o.Role,
 		unicodeFont:  unicodeFont,
+		cursor:       cursor,
 		fontAscent:   ascent,
 		fontDescent:  descent,
 		fontCell:     cell,
@@ -711,6 +759,9 @@ type x11Session struct {
 	markerAtom  xproto.Atom
 	role        LayerRole
 	unicodeFont bool
+	// cursor is the blank cursor applied to the layer window at creation;
+	// it is freed when the session closes.
+	cursor xproto.Cursor
 	// fontAscent, fontDescent and fontCell describe the drawn font so the
 	// OSD overlay can punch its output shape around each text's glyph cell.
 	fontAscent  int16
@@ -849,17 +900,22 @@ func (s *x11Session) WaitEvent(ctx context.Context) (bool, error) {
 	}
 }
 
-// reassertStacking re-claims the overlay's place above the presented video
-// window; a no-op for the canvas and whenever video is absent.
+// reassertStacking re-claims the overlay's stacking place on the OSD's
+// slower refresh cadence: above the live video window, or on top of the
+// stack while video is absent so the fallback canvas can never bury it. A
+// no-op for the canvas layer.
 func (s *x11Session) reassertStacking() {
-	if s.video == nil || !s.video.Active() {
-		return
-	}
-	target, ok := stackReassertTarget(s.role, s.video.Active(), s.topVideo, s.videoWindows)
+	videoActive := s.video != nil && s.video.Active()
+	sibling, mode, ok := stackReassert(s.role, videoActive, s.topVideo, s.videoWindows)
 	if !ok {
 		return
 	}
-	stackWindowRelative(s.conn, s.window, target, xproto.StackModeAbove)
+	if sibling != 0 {
+		stackWindowRelative(s.conn, s.window, sibling, mode)
+		return
+	}
+	xproto.ConfigureWindow(s.conn, s.window,
+		xproto.ConfigWindowStackMode, []uint32{mode})
 }
 
 // applyVideoChange folds one root structure event into the shared video
@@ -983,7 +1039,9 @@ func foldToASCII(message string) string {
 }
 
 func (s *x11Session) Close() error {
-	// Closing the connection makes the X server destroy the kiosk window.
+	// Free the blank cursor explicitly; closing the connection then makes
+	// the X server reclaim everything else, including the kiosk window.
+	xproto.FreeCursor(s.conn, s.cursor)
 	s.conn.Close()
 	return nil
 }

internal/display/x11_test.go

@@ -286,27 +286,41 @@ func TestOSDEventMaskNeverSelectsDeviceInput(t *testing.T) {
 	}
 }
 
-func TestStackReassertTargetOnlyAboveLiveTrackedVideo(t *testing.T) {
-	// Window managers promote fullscreened video to the top of their
-	// stacking order after the video-appear restack, burying the OSD
-	// overlay; the overlay's refresh must re-assert its place above the
-	// tracked video window — and only the OSD overlay may do so.
+func TestStackReassertKeepsTheOSDOnTop(t *testing.T) {
+	// While video is live the overlay must sit above the tracked video
+	// window; while video is absent the overlay must rise to the top,
+	// because the fallback canvas raises itself to the top when video ends
+	// and would otherwise bury the overlay's text. Only the OSD overlay
+	// re-asserts; the canvas and video windows are never moved by this.
 	windows := map[xproto.Window]struct{}{0x20: {}}
 
-	if target, ok := stackReassertTarget(RoleAbove, true, 0x20, windows); !ok || target != 0x20 {
-		t.Fatalf("target=%v ok=%v want the tracked video window", target, ok)
+	if sibling, mode, ok := stackReassert(RoleAbove, true, 0x20, windows); !ok || sibling != 0x20 || mode != xproto.StackModeAbove {
+		t.Fatalf("sibling=%v mode=%v ok=%v want above the tracked video window", sibling, mode, ok)
 	}
-	if _, ok := stackReassertTarget(RoleBelow, true, 0x20, windows); ok {
+	if sibling, mode, ok := stackReassert(RoleAbove, false, 0, nil); !ok || sibling != 0 || mode != xproto.StackModeAbove {
+		t.Fatalf("sibling=%v mode=%v ok=%v want a top raise while video is absent", sibling, mode, ok)
+	}
+	if _, _, ok := stackReassert(RoleBelow, true, 0x20, windows); ok {
 		t.Fatal("the fallback canvas must never re-assert stacking")
 	}
-	if _, ok := stackReassertTarget(RoleAbove, false, 0x20, windows); ok {
-		t.Fatal("no re-assert while video is absent")
-	}
-	if _, ok := stackReassertTarget(RoleAbove, true, 0x30, windows); ok {
+	if _, _, ok := stackReassert(RoleAbove, true, 0x30, windows); ok {
 		t.Fatal("a stale top video window must not be restacked against")
 	}
-	if _, ok := stackReassertTarget(RoleAbove, true, 0, windows); ok {
-		t.Fatal("no tracked video window means nothing to re-assert")
+	if _, _, ok := stackReassert(RoleAbove, true, 0, windows); ok {
+		t.Fatal("no tracked video window means no video-relative restack")
+	}
+}
+
+func TestCreateWindowValuesOrderMatchesTheValueMask(t *testing.T) {
+	// The CreateWindow values must follow the mask bit order (back pixel,
+	// event mask, cursor) so the server reads each value correctly; the
+	// blank cursor hides the pointer over the layer from its first frame.
+	mask, values := createWindowValues(0x100, 0x200, 0xc0de)
+	if mask != xproto.CwBackPixel|xproto.CwEventMask|xproto.CwCursor {
+		t.Fatalf("mask=%b want back pixel, event mask and cursor bits", mask)
+	}
+	if len(values) != 3 || values[0] != 0x100 || values[1] != 0x200 || values[2] != 0xc0de {
+		t.Fatalf("values=%v want [backPixel eventMask cursor]", values)
 	}
 }
 

internal/osd/contract_test.go

@@ -1,7 +1,12 @@
 package osd
 
 import (
+	"os"
+	"path/filepath"
 	"testing"
+	"time"
+
+	"git.piksar.eu/piotr/groundstation/internal/protocol"
 )
 
 func TestTableSelectionRequiresAnIdentifier(t *testing.T) {
@@ -129,3 +134,142 @@ func TestVTXLifecycleStatesAreDistinct(t *testing.T) {
 		t.Fatal("pending, applied and error must be distinct lifecycle states")
 	}
 }
+
+func TestSnapshotDomainsAreEncodedInDisjointByteRanges(t *testing.T) {
+	// The four contract domains — operator configuration (table identity
+	// and ceiling), the requested policy level, the applied acknowledgment
+	// level and the observed link — must occupy disjoint byte ranges: a
+	// change to one domain may never alias another domain's bytes, so
+	// future extensions land behind a version bump instead of colliding.
+	base := Snapshot{
+		Sequence:    1,
+		TimestampMS: 2,
+		Channels:    [protocol.Channels]int16{},
+		Armed:       false,
+		Video:       false,
+		OpConfig: OperatorConfig{
+			Table:    Table{Gen: 1, Hash: 1},
+			MaxPower: 1,
+		},
+		Policy: GSPolicy{Level: 1},
+		Ack: VTXAck{
+			Table:     Table{Gen: 1},
+			Level:     1,
+			Lifecycle: VTXLifecycleApplied,
+		},
+		Link: GSObservedLink{Known: false},
+	}
+
+	deltas := map[string]struct {
+		rangeStart, rangeEnd int
+		mutate               func(*Snapshot)
+	}{
+		"operator-config": {opMaxPowerOffset, policyLevelOffset, func(s *Snapshot) { s.OpConfig.MaxPower = 2 }},
+		"policy-level":    {policyLevelOffset, ackTableGenOffset, func(s *Snapshot) { s.Policy.Level = 2 }},
+		"ack-level":       {ackTableGenOffset, linkAgeOffset, func(s *Snapshot) { s.Ack.Level = 2 }},
+		"observed-link":   {linkAgeOffset, crcOffset, func(s *Snapshot) { s.Link.AgeMS = 2 }},
+	}
+	reference := Encode(base)
+	for name, delta := range deltas {
+		modified := base
+		delta.mutate(&modified)
+		encoded := Encode(modified)
+		if len(encoded) != len(reference) {
+			t.Fatalf("%s: encoded length changed", name)
+		}
+		for i := range encoded {
+			if encoded[i] == reference[i] {
+				continue
+			}
+			// The CRC covers the whole record and necessarily changes with
+			// any domain change; every payload byte must stay inside the
+			// domain's own range.
+			if i >= crcOffset {
+				continue
+			}
+			if i < delta.rangeStart || i >= delta.rangeEnd {
+				t.Fatalf("%s: byte %d changed outside the domain range [%d,%d)",
+					name, i, delta.rangeStart, delta.rangeEnd)
+			}
+		}
+		if _, err := Decode(encoded); err != nil {
+			t.Fatalf("%s: modified snapshot must still decode: %v", name, err)
+		}
+	}
+}
+
+func TestDecodeRejectsCorruptionAtomically(t *testing.T) {
+	// Malformed input must never yield a partially decoded snapshot that
+	// could clear or corrupt one domain while keeping others: any flipped
+	// byte anywhere in the record rejects the whole snapshot, and the
+	// publisher keeps its last valid state until a new valid one arrives.
+	encoded := Encode(Snapshot{
+		OpConfig: OperatorConfig{Table: Table{Gen: 5, Hash: 5}, MaxPower: 5},
+		Policy:   GSPolicy{Level: 5},
+		Ack:      VTXAck{Level: 5, Lifecycle: VTXLifecycleApplied},
+		Link:     GSObservedLink{Known: true, AgeMS: 5},
+	})
+	// Sample corruption positions across the whole record — magic, body
+	// regions and CRC — derived from the actual encoded length, so the
+	// atomicity contract holds across future version bumps and layout
+	// extensions without coupling to today's offsets.
+	positions := []int{
+		0,
+		len(encoded) / 4,
+		len(encoded) / 2,
+		3 * len(encoded) / 4,
+		len(encoded) - 1,
+	}
+	for _, offset := range positions {
+		corrupt := append([]byte(nil), encoded...)
+		corrupt[offset] ^= 0xff
+		got, err := Decode(corrupt)
+		if err == nil {
+			t.Fatalf("flipped byte at %d must reject the record", offset)
+		}
+		if got != (Snapshot{}) {
+			t.Fatalf("flipped byte at %d must not emit a partial snapshot", offset)
+		}
+	}
+}
+
+func TestFileCorruptionDoesNotClearLastValidSnapshot(t *testing.T) {
+	// A corrupted state file reports no snapshot for the moment, but the
+	// publisher retains its last valid snapshot and the next heartbeat
+	// rewrite restores exactly those values: corruption never clears
+	// last valid data, and nothing is fabricated in between.
+	path := filepath.Join(t.TempDir(), "osd-state.bin")
+	now := time.UnixMilli(10_000_000)
+	source := NewFileSource(path)
+	source.now = func() time.Time { return now }
+
+	want := testSnapshot()
+	want.PublishedMS = now.UnixMilli()
+	if err := os.WriteFile(path, Encode(want), 0644); err != nil {
+		t.Fatal(err)
+	}
+	if _, state := source.Latest(); state != SnapshotFresh {
+		t.Fatalf("state=%v want fresh before corruption", state)
+	}
+
+	corrupt := append([]byte(nil), Encode(want)...)
+	corrupt[len(corrupt)/2] ^= 0xff
+	if err := os.WriteFile(path, corrupt, 0644); err != nil {
+		t.Fatal(err)
+	}
+	if _, state := source.Latest(); state != SnapshotNone {
+		t.Fatalf("state=%v want none while the file is corrupt", state)
+	}
+
+	// The publisher's heartbeat rewrites its retained last valid snapshot.
+	if err := os.WriteFile(path, Encode(want), 0644); err != nil {
+		t.Fatal(err)
+	}
+	got, state := source.Latest()
+	if state != SnapshotFresh {
+		t.Fatalf("state=%v want fresh after the heartbeat restore", state)
+	}
+	if got != want {
+		t.Fatalf("restored snapshot=%+v want the untouched last valid %+v", got, want)
+	}
+}

internal/osd/osd.go

@@ -16,6 +16,24 @@
 // intent only: it constrains what may be requested and is never proof of
 // applied VTX power, and static configuration is never rendered or serialized
 // as applied VTX state.
+//
+// Extension contract for future domains: the snapshot is an explicit,
+// extensible foundation of independent domains — operator table identity,
+// requested level, applied level, observed link, and flight telemetry
+// (versioned WFB-derived battery/GPS/arm/flight-mode/attitude with source
+// and freshness, future work). Telemetry domains land as new typed nested
+// structs with their own explicit pending/unknown states, appended at fixed
+// offsets before the CRC behind a version bump; the typed fields plus the
+// versioned all-or-nothing codec are the extension seam, and no runtime
+// placeholder fields exist before a domain is actually modeled. Domains are
+// independent by construction: each occupies a disjoint byte range (pinned
+// by contract tests), decoding is atomic — malformed or damaged records are
+// rejected whole and can never clear one domain while keeping others — and
+// the publisher retains its last valid snapshot, republishing it until a
+// new valid snapshot replaces it. Flight telemetry stays independent of
+// video and policy; battery, GPS and similar aircraft state must never be
+// inferred from RC channels, and the only RC-derived flight state remains
+// the already-defined CH5/AUX1 arm indication.
 package osd
 
 import (