From 1c45cd2ec881866a86fffa76fbde4c167fe14424 Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Sun, 26 Jul 2026 23:43:32 +0200 Subject: [PATCH] fix(client): Global rung derived its canvas from a pre-layout viewport MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Eyeballed on Lendel: the Atlas opened on a Global map that was literally two cells — one green, one blue — stretched across the window, reporting 19,598.512 km/gridunit, which is exactly half the body's circumference. Two bugs, both of which the D-255 extent inversion turned from harmless into fatal. enter() fires its first request BEFORE this Control is laid out, and a not-yet-laid-out size is not always exactly Vector2.ZERO — a few stray pixels sailed past the `== Vector2.ZERO` guard, so the viewer asked for a 2x2 gridunit canvas and the server's 2:1 fit floored it to 2x1. That never mattered while Global discarded the requested extent and took its cell counts from the body's region grid; the moment the request became the canvas size, a transient layout artefact became the map. Any viewport below a plausible panel size is now treated as not-laid-out. And Global was excluded from the refetch settle entirely, so a canvas born at the wrong size could never heal however the window was resized. That exclusion was correct when no viewport could change Global's extent. Global now takes the SIZE refit like every other rung, but still never the pan re-float — its canvas is whole-body and origin-anchored, and the server ignores `center` for it. Both have regression tests. The second asserts on _world_center rather than _view_offset, because _recompute_canvas_transform() legitimately re-centres the offset on any canvas adoption and would have made the test pass for the wrong reason. Worth noting for the class: no test written today could have caught this. Every one supplies an explicit viewport. The bug lived entirely in the gap between "scene loads" and "layout completes" — a seam a live launch exercises and a unit test does not. Also stages governance/README.md's pql-maintained record index (D-258). Pair session with Jeroen, 2026-07-26. Co-Authored-By: Claude --- client/tests/test_step_canvas_viewer.gd | 51 +++++++++++++++++++ .../atlas/step_canvas/step_canvas_viewer.gd | 41 ++++++++++++--- governance/README.md | 1 + 3 files changed, 86 insertions(+), 7 deletions(-) diff --git a/client/tests/test_step_canvas_viewer.gd b/client/tests/test_step_canvas_viewer.gd index b602db169..a70115301 100644 --- a/client/tests/test_step_canvas_viewer.gd +++ b/client/tests/test_step_canvas_viewer.gd @@ -787,6 +787,57 @@ func test_resize_recenters_an_already_held_global_canvas() -> void: ).is_not_equal(offset_before) +## EYEBALL REGRESSION (pair session 2026-07-26, Lendel): the Atlas opened on a +## Global map that was literally two cells — one green, one blue — stretched +## across the window, reporting 19,598 km/gridunit, exactly half the body's +## circumference. Cause: enter() fires its first request BEFORE this Control is +## laid out, and the not-laid-out size is not always exactly ZERO, so a few +## stray pixels sailed past the `== Vector2.ZERO` guard and asked for a 2x2 +## gridunit canvas. Harmless while Global ignored the requested extent and took +## its cell counts from the body's region grid; load-bearing the moment the +## D-255 extent inversion made the request the canvas size. +func test_request_extent_ignores_a_not_yet_laid_out_viewport() -> void: + var v: StepCanvasViewer = _make_viewer() + add_child(v) + v.enter({"body_id": "GJ380c", "body_radius_km": 6238.4}, {}) + for degenerate in [Vector2.ZERO, Vector2(4.0, 4.0), Vector2(1920.0, 2.0)]: + v.size = degenerate + var extent: Vector2i = v._request_extent() + var expected: Vector2i = StepCanvasTransport.viewport_fit_extent( + StepCanvasViewer.FALLBACK_VIEWPORT_PX, v.get_held_rung() + ) + assert_that(extent).override_failure_message( + "a %s viewport must fall back, not be taken literally — got %s" % [degenerate, extent] + ).is_equal(expected) + + +## ...and the second half of the same bug: Global was excluded from the refetch +## settle entirely, so a canvas born at the wrong size could never heal however +## the window was resized. Global must take the SIZE refit (it is viewport-sized +## like every rung now) but never the pan re-float (its canvas is whole-body and +## origin-anchored — the server ignores `center` for Global), which _refloat_now() +## would betray by zeroing _view_offset. +func test_global_takes_the_size_refit_but_never_the_pan_refloat() -> void: + var v: StepCanvasViewer = _make_viewer() + add_child(v) + v.size = Vector2(1920.0, 1080.0) + v.enter({"body_id": "T1189_extent_letterbox_test_body", "body_radius_km": 6371.0}, {}) + TestStepCanvasViewer._land_global_canvas(v, GJ1C_GLOBAL_EXTENT.x, GJ1C_GLOBAL_EXTENT.y) + + # Drift the view far enough that a fixed rung would hard re-float. + v._view_offset = Vector2(-100_000.0, -100_000.0) + v._on_refetch_settle() + + # _refloat_now() would re-centre the request on whatever world point sits + # under the viewport centre; Global's canvas is origin-anchored and the + # server ignores `center` for it, so the world centre must not move. + # (_view_offset is NOT the probe here — _recompute_canvas_transform() + # legitimately re-centres it on any canvas adoption.) + assert_that(v._world_center).override_failure_message( + "Global has no centre to re-float to — _world_center must stay at the origin" + ).is_equal(Vector2.ZERO) + + ## Hoshe (review round 2): a narrow viewport where GJ1c's canvas exceeds ## the available width on the gridunit lattice itself (177 gridunits > ## available px after the legend column is reserved) used to make the OLD diff --git a/client/ui/implant/apps/atlas/step_canvas/step_canvas_viewer.gd b/client/ui/implant/apps/atlas/step_canvas/step_canvas_viewer.gd index 1218465f0..754bc6428 100644 --- a/client/ui/implant/apps/atlas/step_canvas/step_canvas_viewer.gd +++ b/client/ui/implant/apps/atlas/step_canvas/step_canvas_viewer.gd @@ -90,6 +90,17 @@ const REFETCH_SETTLE_S: float = 0.30 ## threshold the mismatch is a couple of screen pixels — not worth a derive. const RESIZE_REFIT_MIN_CELL_DELTA: int = 4 +## Below this many pixels on either axis, the viewport is treated as NOT YET +## LAID OUT rather than as a genuine (tiny) window — see _request_extent(). +## No real Atlas panel is this small, and enter() fires its first request +## before this Control has been sized, so the guard costs nothing and the +## alternative is requesting a canvas sized to a transient layout artefact. +const MIN_LAID_OUT_VIEWPORT_PX: float = 64.0 + +## The size to request at when the viewport isn't trustworthy yet. A resize +## refit corrects it on the next settle if the real panel differs. +const FALLBACK_VIEWPORT_PX: Vector2 = Vector2(1280.0, 720.0) + ## Pan drift thresholds, as a fraction of the held canvas's half-footprint. ## SOFT: the canvas edge is nearing the viewport centre — schedule a settled ## refetch, but keep panning the held canvas smoothly (no snap, no request @@ -479,8 +490,17 @@ func _fire_request() -> void: ## sideways-repeat guard for that one request. func _request_extent() -> Vector2i: var viewport: Vector2 = get_rect().size - if viewport == Vector2.ZERO: - viewport = Vector2(1280.0, 720.0) + # A NOT-YET-LAID-OUT viewport is not always exactly ZERO — enter() fires + # the first request before this Control has been sized, and a few stray + # pixels sail straight past an `== Vector2.ZERO` check. That was harmless + # while Global ignored the requested extent and took its cell counts from + # the body's region grid; since the D-255 extent inversion the request IS + # the canvas size, so a 4x4 viewport asked for a 2x2 gridunit Global and + # got a two-cell map — one green cell, one blue — stretched across the + # window (eyeballed on Lendel, 19,598 km/gridunit: exactly half the body). + # Treat anything below a plausible panel size as "not laid out yet". + if viewport.x < MIN_LAID_OUT_VIEWPORT_PX or viewport.y < MIN_LAID_OUT_VIEWPORT_PX: + viewport = FALLBACK_VIEWPORT_PX var fit: Vector2i = StepCanvasTransport.viewport_fit_extent(viewport, _held_rung) return StepCanvasTransport.cap_extent_to_body(fit, _held_rung, _global_body_extent) @@ -507,13 +527,20 @@ func _schedule_refetch_settle() -> void: ## because a re-float re-requests at the new centre anyway and that request ## already carries the current viewport-fit extent. func _on_refetch_settle() -> void: - if _held_rung == StepCanvasTransport.RUNG_GLOBAL: - return if get_body_id().is_empty() or _held_extent == Vector2i.ZERO: return - if _pan_drift_fraction() >= PAN_REFLOAT_SOFT_FRACTION: - _refloat_now() - return + # Global takes the SIZE refit but never the pan re-float: its canvas is + # whole-body and origin-anchored, so there is no centre to re-float to + # (the server ignores `center` for Global). Before the D-255 extent + # inversion Global was excluded from this callback entirely — correct + # then, because its extent came from the body's region grid and no + # viewport could change it. Now Global is viewport-sized like every other + # rung, and that exclusion was the reason a bad first size could never + # heal: the map stayed as it was born, however the window was resized. + if _held_rung != StepCanvasTransport.RUNG_GLOBAL: + if _pan_drift_fraction() >= PAN_REFLOAT_SOFT_FRACTION: + _refloat_now() + return _maybe_refit_to_viewport() diff --git a/governance/README.md b/governance/README.md index 14e9e6ae1..b3bad2fb1 100644 --- a/governance/README.md +++ b/governance/README.md @@ -323,6 +323,7 @@ line in place — keep the Q-record for the audit trail rather than deleting it. - [D-255: Body Map Viewer — stepped Atlas render architecture (supersedes the T-1143 continuous-ladder mechanism)](decisions/architecture.md#d-255-body-map-viewer--stepped-atlas-render-architecture-supersedes-the-t-1143-continuous-ladder-mechanism) — _architecture_ - [D-256: Canonical sampling convention — one absolute-metre derive core; the batch layer is a survey raster](decisions/architecture.md#d-256-canonical-sampling-convention--one-absolute-metre-derive-core-the-batch-layer-is-a-survey-raster) — _architecture_ - [D-257: Environment props share the character toon shading treatment; minimal-PBR carve-out for glazing](decisions/architecture.md#d-257-environment-props-share-the-character-toon-shading-treatment-minimal-pbr-carve-out-for-glazing) — _architecture_ +- [D-258: Rung-0.5 expanded layer — one derived base for the whole ladder](decisions/architecture.md#d-258-rung-05-expanded-layer--one-derived-base-for-the-whole-ladder) — _architecture_ ## Open questions