fix(ui): PR #205 review round — integer px-per-gridunit fit replaces fractional (T-1189, T-1192)
The fractional fit branch is deleted, resolving both review findings at the root: tyre showed its comments cited D-255 for an exception the record does not contain (the language came from the lead's ticket text, not governance), and hoshe showed it returned sub-1x for a canvas exceeding the viewport on one axis. Replacement: fit_scale_ratio() chooses the largest integer pixels-per-gridunit R fitting both legend-reserved axes, floored at 1 (over-viewport draws native and crops like every fixed rung) — the fine-grained integer lattice (GJ1c 1080p -> 9px/gu = 1593x792, ~98% width; 4K -> 20) that makes the fractional hatch unnecessary. center_offset() floors to whole pixels (half-pixel centering would blur the texel grid). Doc comments cite the real sanction (D-255 amendment 2026-07-25, this branch). Legend column constant is now canonical in transport, read directly by the legend (was an independently-typed literal); its test asserts real geometry. New regression test proves _global_body_extent clears on body switch and never caps another body's requests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -12,17 +12,19 @@ extends ImplantPanel
|
||||
## No `class_name` on purpose, matching every other viewer-owned helper in
|
||||
## this cluster.
|
||||
|
||||
const AtlasOverlayColors := preload("res://ui/implant/apps/atlas/atlas_overlay_colors.gd")
|
||||
const StepCanvasTransport := preload("res://ui/implant/apps/atlas/step_canvas/step_canvas_transport.gd")
|
||||
|
||||
const PANEL_MARGIN: float = 16.0
|
||||
const LEGEND_PANEL_WIDTH: float = 260.0
|
||||
|
||||
## T-1192: must stay derivable from PANEL_MARGIN/LEGEND_PANEL_WIDTH above —
|
||||
## StepCanvasTransport.LEGEND_COLUMN_PX mirrors this exact sum so the
|
||||
## Global-rung fit-scale computation reserves precisely this much column,
|
||||
## never more or less than what the legend actually occupies.
|
||||
const RESERVED_COLUMN_PX: float = LEGEND_PANEL_WIDTH + PANEL_MARGIN * 2.0
|
||||
|
||||
const AtlasOverlayColors := preload("res://ui/implant/apps/atlas/atlas_overlay_colors.gd")
|
||||
const StepCanvasTransport := preload("res://ui/implant/apps/atlas/step_canvas/step_canvas_transport.gd")
|
||||
## T-1192 review fix: StepCanvasTransport.LEGEND_COLUMN_PX is now the ONE
|
||||
## canonical source for this width — this file no longer computes its own
|
||||
## independently-typed literal sum. A read of the transport constant, not a
|
||||
## derivation, so the two sides can never drift apart by construction
|
||||
## (transport is static-only, no scene-tree dependency in this direction —
|
||||
## legend already preloads it above for AtlasOverlayColors-style helpers).
|
||||
const RESERVED_COLUMN_PX: float = StepCanvasTransport.LEGEND_COLUMN_PX
|
||||
|
||||
const MORPHOLOGY_FAMILY_ROWS: Array = [
|
||||
{"label": "water", "zones": [0, 1]},
|
||||
|
||||
@@ -39,8 +39,11 @@ extends Node2D
|
||||
##
|
||||
## **T-1192 outer fit scale:** the owning `_canvas` Node2D
|
||||
## (StepCanvasViewer._recompute_canvas_transform()) may additionally carry
|
||||
## its OWN `.scale` — the Global-rung integer-fit multiplier, always 1.0 for
|
||||
## every fixed rung — applied on top of this node's own texel-exact
|
||||
## its OWN `.scale` — for the Global rung, `StepCanvasTransport.
|
||||
## fit_scale_from_ratio()`'s per-body integer pixels-per-gridunit fit
|
||||
## (D-255 amendment 2026-07-25: rung-0's display ratio is viewport-fitted
|
||||
## per body to an INTEGER px/gridunit ratio, not a fixed 5x5); always 1.0
|
||||
## for every fixed rung — applied on top of this node's own texel-exact
|
||||
## `_footprint_px` draw. That is a SECOND texture-to-viewport resize, same
|
||||
## D-255(e) exemption, kept as a parent-transform multiply rather than a
|
||||
## second internal scale field so this node's own footprint math never has
|
||||
|
||||
@@ -85,27 +85,16 @@ const DISPLAY_RATIO_BY_RUNG: Dictionary = {
|
||||
## server-side, per step_canvas_protocol.gd's own doc).
|
||||
const FIXED_CANVAS_MAX_AXIS: int = 3_840
|
||||
|
||||
## T-1192: the on-screen column width reserved for the Atlas legend panel —
|
||||
## mirrors step_canvas_legend.gd's own LEGEND_PANEL_WIDTH + PANEL_MARGIN*2
|
||||
## (panel width plus a margin on each side). Shared here (not duplicated as
|
||||
## a raw literal in step_canvas_viewer.gd) so the Global integer-fit
|
||||
## computation and the legend's own sizing can never silently drift apart —
|
||||
## "legend beside, never over" only holds if both sides agree on the SAME
|
||||
## reserved width.
|
||||
## T-1192: the on-screen column width reserved for the Atlas legend panel
|
||||
## (panel width plus a margin on each side). THIS is the canonical value —
|
||||
## step_canvas_legend.gd's own RESERVED_COLUMN_PX is a direct read of this
|
||||
## constant (review fix: was previously an independently-typed literal sum
|
||||
## on the legend side, which could silently drift from this one), so the
|
||||
## Global integer-fit computation and the legend's own sizing can never
|
||||
## disagree by construction — "legend beside, never over" only holds if
|
||||
## both sides agree on the SAME reserved width.
|
||||
const LEGEND_COLUMN_PX: float = 260.0 + 16.0 * 2.0
|
||||
|
||||
## Below this fraction of the viewport's SMALLER axis covered, an integer
|
||||
## fit "leaves excessive letterboxing" (D-255's own phrase) and the
|
||||
## non-integer escape hatch fires instead. 0.75: the integer candidate must
|
||||
## already cover at least three-quarters of the tighter axis to win outright
|
||||
## — anything looser and the NEXT integer step down/up is visibly a better
|
||||
## use of the frame. Tuned against the GJ1c reference case at 1920x1080
|
||||
## (integer 1x candidate covers ~41% of the tighter available axis after the
|
||||
## legend-column reservation — well under this bar, so the fractional fit
|
||||
## wins there, matching D-255's own "if that leaves excessive letterboxing
|
||||
## on small canvases" framing). See fit_scale() below for the decision.
|
||||
const FIT_MIN_COVERAGE_RATIO: float = 0.75
|
||||
|
||||
|
||||
## The rung name at ladder index `i`, clamped to the legal [0, 5] range —
|
||||
## the one place RUNG_LADDER is indexed into, so a caller passing an
|
||||
@@ -220,49 +209,46 @@ static func canvas_footprint_px(rung: String, extent_cells: Vector2i) -> Vector2
|
||||
## than the viewport on an axis gets a zero/negative offset on that axis (no
|
||||
## letterbox needed there — it already fills or overflows, matching ordinary
|
||||
## pan-and-crop behavior on that axis rather than shrinking the canvas).
|
||||
## FLOORED to a whole pixel on each axis (D-255 texel-exactness): an
|
||||
## integer-px/gridunit canvas (fit_scale_ratio()) still produces an
|
||||
## odd-vs-even remainder half that can land on a .5px boundary — that would
|
||||
## reintroduce a fractional-pixel blur edge on the very rung this exists to
|
||||
## keep texel-exact, so the offset itself is snapped to the pixel grid.
|
||||
static func center_offset(footprint_px: Vector2, viewport_px: Vector2) -> Vector2:
|
||||
return (viewport_px - footprint_px) * 0.5
|
||||
var raw: Vector2 = (viewport_px - footprint_px) * 0.5
|
||||
return Vector2(floorf(raw.x), floorf(raw.y))
|
||||
|
||||
|
||||
## D-255 texel-exactness for the Global opener (T-1192): the largest INTEGER
|
||||
## scale multiple of `canvas_px` that still fits inside `viewport_px` on
|
||||
## BOTH axes, floored at 1 (never downscale below native size — a canvas
|
||||
## larger than the viewport draws at 1x and simply doesn't fit, matching
|
||||
## every fixed rung's own "canvas can exceed the viewport" precedent rather
|
||||
## than introducing a NEW sub-1x shrink path here). Callers needing the
|
||||
## "non-integer fit acceptable on small canvases" escape hatch (D-255's own
|
||||
## exception, nearest-neighbor only) compute their own fractional scale and
|
||||
## skip this function — it only ever returns integers by design, so it
|
||||
## can't accidentally hand back a blurry non-integer multiple.
|
||||
static func integer_fit_scale(canvas_px: Vector2, viewport_px: Vector2) -> int:
|
||||
if canvas_px.x <= 0.0 or canvas_px.y <= 0.0:
|
||||
## D-255 premise (2), texel-exact: the largest INTEGER pixels-per-gridunit
|
||||
## ratio `R` at which a `extent_cells`-gridunit canvas fits inside
|
||||
## `viewport_px` on BOTH axes, floored at 1 (never below native resolution
|
||||
## — an over-viewport canvas draws at 1 px/gridunit and crops/pans, same as
|
||||
## every fixed rung's own "canvas can exceed the viewport" precedent).
|
||||
## `R` is a PIXELS-PER-GRIDUNIT ratio, not a multiple of the whole footprint
|
||||
## — this is the fine-grained lattice D-255's amendment (2026-07-25)
|
||||
## clarifies rung-0's display ratio to be viewport-fitted-per-body against:
|
||||
## GJ1c's 177x88-gridunit canvas at a 1920x1080 viewport (legend column
|
||||
## reserved) lands on R=9 px/gridunit (1593x792, ~98% width), not a coarse
|
||||
## multiple of the BASE 5px/gridunit footprint (885/1770/2655 — the old,
|
||||
## too-coarse lattice that made a fractional escape hatch look necessary).
|
||||
static func fit_scale_ratio(extent_cells: Vector2, viewport_px: Vector2) -> int:
|
||||
if extent_cells.x <= 0.0 or extent_cells.y <= 0.0:
|
||||
return 1
|
||||
var max_x: int = int(floor(viewport_px.x / canvas_px.x))
|
||||
var max_y: int = int(floor(viewport_px.y / canvas_px.y))
|
||||
var max_x: int = int(floor(viewport_px.x / extent_cells.x))
|
||||
var max_y: int = int(floor(viewport_px.y / extent_cells.y))
|
||||
return maxi(1, mini(max_x, max_y))
|
||||
|
||||
|
||||
## The actual scale to draw the Global canvas at (T-1192): the integer fit
|
||||
## if it covers at least FIT_MIN_COVERAGE_RATIO of the viewport's tighter
|
||||
## axis, otherwise a UNIFORM fractional fit (same scale both axes, so the
|
||||
## canvas is never stretched non-uniformly) that fills the tighter axis
|
||||
## exactly. The fractional branch is legal ONLY under D-255's own
|
||||
## nearest-neighbor condition — StepCanvasTerrainLayer._filter_for_rung()
|
||||
## already forces NEAREST for every orbital rung (Global included)
|
||||
## unconditionally, so this function never has to check or set the filter
|
||||
## itself; it only chooses the number.
|
||||
static func fit_scale(canvas_px: Vector2, viewport_px: Vector2) -> float:
|
||||
if canvas_px.x <= 0.0 or canvas_px.y <= 0.0:
|
||||
return 1.0
|
||||
var int_scale: int = integer_fit_scale(canvas_px, viewport_px)
|
||||
var covered: Vector2 = canvas_px * float(int_scale)
|
||||
var coverage_x: float = covered.x / maxf(viewport_px.x, 0.0001)
|
||||
var coverage_y: float = covered.y / maxf(viewport_px.y, 0.0001)
|
||||
if minf(coverage_x, coverage_y) >= FIT_MIN_COVERAGE_RATIO:
|
||||
return float(int_scale)
|
||||
var frac_x: float = viewport_px.x / canvas_px.x
|
||||
var frac_y: float = viewport_px.y / canvas_px.y
|
||||
return maxf(minf(frac_x, frac_y), 0.0001)
|
||||
## Convert a target pixels-per-gridunit ratio `R` (fit_scale_ratio()'s
|
||||
## return) into the `_canvas.scale` multiplier applied ON TOP OF a texture
|
||||
## already rendered at `base_display_ratio` px/gridunit (canvas_footprint_px()
|
||||
## — the rung's own DISPLAY_RATIO_BY_RUNG entry). The multiplier itself may
|
||||
## be fractional (e.g. 9/5 = 1.8) — texel-exactness is NOT about the scale
|
||||
## factor being a whole number, it is about the FINAL on-screen pixel count
|
||||
## per gridunit (`R`) being an exact integer, so every source texel lands on
|
||||
## a whole number of screen pixels with no fractional-pixel blur boundary.
|
||||
static func fit_scale_from_ratio(ratio: int, base_display_ratio: float) -> float:
|
||||
return float(ratio) / maxf(base_display_ratio, 0.0001)
|
||||
|
||||
|
||||
## Fit a fixed-rung request's extent (in gridunits) to the viewport, capped
|
||||
|
||||
@@ -119,12 +119,15 @@ var _app_has_focus: bool = true
|
||||
## T-1192 Global fit scale — the ONE display-time scale this viewer ever
|
||||
## writes (see the class doc). Always 1.0 for a fixed rung (its canvas is
|
||||
## already texel-exact at its own display ratio; T-1189's letterbox only
|
||||
## adds centered margin, never an extra scale). For Global, an INTEGER
|
||||
## multiple when that covers most of the viewport (D-255 texel-exactness),
|
||||
## else a non-integer fractional fit on small canvases — see
|
||||
## StepCanvasTransport.fit_scale()'s own doc for the coverage rule; NEAREST
|
||||
## filtering (required for the non-integer case) is already unconditional
|
||||
## for every orbital rung via StepCanvasTerrainLayer._filter_for_rung().
|
||||
## adds centered margin, never an extra scale). For Global, the `_canvas.scale`
|
||||
## multiplier that puts the FINAL on-screen pixels-per-gridunit at the
|
||||
## largest INTEGER ratio that fits the (legend-reserved) viewport, floored
|
||||
## at 1 (D-255 amendment 2026-07-25 — rung-0's display ratio is
|
||||
## viewport-fitted per body to an integer, never a non-integer/sub-1x
|
||||
## fraction) — see StepCanvasTransport.fit_scale_ratio()'s own doc. The
|
||||
## multiplier itself (`ratio / base_display_ratio`) may be a non-integer
|
||||
## float — that is expected and correct, since texel-exactness is about the
|
||||
## RATIO being integer, not the Node2D scale field's raw value.
|
||||
var _canvas_scale: float = 1.0
|
||||
|
||||
# ── Overlay visibility ─────────────────────────────────────────────────────
|
||||
@@ -361,20 +364,24 @@ func _recompute_canvas_transform() -> void:
|
||||
_apply_transform()
|
||||
|
||||
|
||||
## The fit scale for a given raw (unscaled) footprint at the CURRENTLY held
|
||||
## rung — Global's fit_scale() reserving the legend column, 1.0 for every
|
||||
## fixed rung. Split out from _recompute_canvas_transform() so
|
||||
## _centered_view_offset() (the drift-check baseline) can share the exact
|
||||
## same scale decision without re-deriving it, keeping the two callers
|
||||
## structurally unable to disagree.
|
||||
## The `_canvas.scale` multiplier for a given raw (unscaled) footprint at
|
||||
## the CURRENTLY held rung — Global's per-body integer pixels-per-gridunit
|
||||
## fit (StepCanvasTransport.fit_scale_ratio()/fit_scale_from_ratio(),
|
||||
## reserving the legend column), 1.0 for every fixed rung. Split out from
|
||||
## _recompute_canvas_transform() so _centered_view_offset() (the drift-check
|
||||
## baseline) can share the exact same scale decision without re-deriving it,
|
||||
## keeping the two callers structurally unable to disagree.
|
||||
func _letterbox_scale_for(raw_footprint: Vector2) -> float:
|
||||
if _held_rung != StepCanvasTransport.RUNG_GLOBAL:
|
||||
return 1.0
|
||||
var base_ratio: float = StepCanvasTransport.display_ratio_for_rung(_held_rung)
|
||||
var extent_cells: Vector2 = raw_footprint / maxf(base_ratio, 0.0001)
|
||||
var viewport: Vector2 = get_rect().size
|
||||
var available: Vector2 = Vector2(
|
||||
maxf(viewport.x - StepCanvasTransport.LEGEND_COLUMN_PX, 1.0), viewport.y
|
||||
)
|
||||
return StepCanvasTransport.fit_scale(raw_footprint, available)
|
||||
var ratio: int = StepCanvasTransport.fit_scale_ratio(extent_cells, available)
|
||||
return StepCanvasTransport.fit_scale_from_ratio(ratio, base_ratio)
|
||||
|
||||
|
||||
func _rebuild_terrain_texture(canvas: Variant = null) -> void:
|
||||
|
||||
Reference in New Issue
Block a user