diff --git a/client/tests/test_step_canvas_annotation_layer.gd b/client/tests/test_step_canvas_annotation_layer.gd index c4d71ffc4..cb9739f05 100644 --- a/client/tests/test_step_canvas_annotation_layer.gd +++ b/client/tests/test_step_canvas_annotation_layer.gd @@ -94,104 +94,292 @@ func _straight_course_points(spacing_px: float = 10.0) -> PackedVector2Array: ## Vertex 0 (the source, cumulative length 0) gets the hairline minimum ## width, never the class's full width — this is the taper's whole point. -func test_course_widths_by_arc_length_starts_at_the_taper_minimum() -> void: +## _head_widths_by_arc_length() is handed the HEAD span only (post PR #207 +## finding 4's ribbon/polyline split) — this test exercises it directly on +## a short head span (the first two points), which is what +## _split_course_at_arc_length() would hand it for this same course. +func test_head_widths_by_arc_length_starts_at_the_taper_minimum() -> void: var layer: StepCanvasAnnotationLayer = auto_free(StepCanvasAnnotationLayer.new()) add_child(layer) - var pts := _straight_course_points() - var widths: PackedFloat32Array = layer._course_widths_by_arc_length(pts, 2.4) - assert_float(widths[0]).is_equal_approx( - StepCanvasAnnotationLayer.TAPER_MIN_WIDTH_PX, 0.001 - ) + var head := PackedVector2Array([Vector2(0.0, 0.0), Vector2(6.0, 0.0)]) + var widths: PackedFloat32Array = layer._head_widths_by_arc_length(head, 2.4) + assert_float(widths[0]).is_equal_approx(StepCanvasAnnotationLayer.TAPER_MIN_WIDTH_PX, 0.001) -## Vertices beyond TAPER_ARC_FRACTION of the total run hold at the class's -## own full width — the taper does not run the whole length of the course, -## only its own leading fraction (ticket instruction: "not the whole run"). -func test_course_widths_by_arc_length_holds_full_width_past_the_taper_fraction() -> void: +## The head's own LAST vertex always ramps to exactly full_width — that's +## the butt-joint contract _draw_tapered_course() relies on to hand off to +## the AA polyline tail at identical width. +func test_head_widths_by_arc_length_ends_at_full_width() -> void: var layer: StepCanvasAnnotationLayer = auto_free(StepCanvasAnnotationLayer.new()) add_child(layer) - # Total length 40; taper window = 0.15 * 40 = 6px. Vertex 1 (cumulative - # 10px) is already past that window. - var pts := _straight_course_points() - var widths: PackedFloat32Array = layer._course_widths_by_arc_length(pts, 2.4) - assert_float(widths[1]).is_equal_approx(2.4, 0.001) - assert_float(widths[4]).is_equal_approx(2.4, 0.001) + var head := PackedVector2Array([Vector2(0.0, 0.0), Vector2(3.0, 0.0), Vector2(6.0, 0.0)]) + var widths: PackedFloat32Array = layer._head_widths_by_arc_length(head, 2.4) + assert_float(widths[2]).is_equal_approx(2.4, 0.001) -## The ramp is monotonically non-decreasing from source to mouth — no -## "wobble" where a later vertex is narrower than an earlier one within the -## taper window. -func test_course_widths_by_arc_length_is_monotonic_within_the_taper_window() -> void: +## The ramp is monotonically non-decreasing from source to the head's last +## vertex — no "wobble" where a later vertex is narrower than an earlier one. +func test_head_widths_by_arc_length_is_monotonic() -> void: var layer: StepCanvasAnnotationLayer = auto_free(StepCanvasAnnotationLayer.new()) add_child(layer) - # Denser spacing (1px) so several vertices fall inside the 6px taper - # window (total length 20, taper window 3px). - var pts := _straight_course_points(1.0) - var widths: PackedFloat32Array = layer._course_widths_by_arc_length(pts, 2.4) + var head := _straight_course_points(1.0) + var widths: PackedFloat32Array = layer._head_widths_by_arc_length(head, 2.4) for i in range(1, widths.size()): assert_float(widths[i]).is_greater_equal(widths[i - 1]) -## A degenerate two-point course where both points coincide (zero-length) +## A degenerate two-point head where both points coincide (zero-length) ## must not divide by zero — every vertex falls back to full width rather ## than crashing or producing NaN. -func test_course_widths_by_arc_length_handles_a_degenerate_zero_length_course() -> void: +func test_head_widths_by_arc_length_handles_a_degenerate_zero_length_span() -> void: var layer: StepCanvasAnnotationLayer = auto_free(StepCanvasAnnotationLayer.new()) add_child(layer) - var pts := PackedVector2Array([Vector2(5.0, 5.0), Vector2(5.0, 5.0)]) - var widths: PackedFloat32Array = layer._course_widths_by_arc_length(pts, 2.4) + var head := PackedVector2Array([Vector2(5.0, 5.0), Vector2(5.0, 5.0)]) + var widths: PackedFloat32Array = layer._head_widths_by_arc_length(head, 2.4) assert_float(widths[0]).is_equal_approx(2.4, 0.001) assert_float(widths[1]).is_equal_approx(2.4, 0.001) -## A legal but minimal two-point course (source directly connected to -## mouth, no interior vertices) still tapers at the source end. -func test_course_widths_by_arc_length_tapers_a_two_point_course() -> void: - var layer: StepCanvasAnnotationLayer = auto_free(StepCanvasAnnotationLayer.new()) - add_child(layer) - var pts := PackedVector2Array([Vector2(0.0, 0.0), Vector2(100.0, 0.0)]) - var widths: PackedFloat32Array = layer._course_widths_by_arc_length(pts, 2.4) - assert_float(widths[0]).is_equal_approx( - StepCanvasAnnotationLayer.TAPER_MIN_WIDTH_PX, 0.001 - ) - # Vertex 1 (the mouth) is at cumulative length 100, far past the - # TAPER_ARC_FRACTION * 100 = 15px taper window — full width. - assert_float(widths[1]).is_equal_approx(2.4, 0.001) +# ----------------------------------------------------------------------- +# PR #207 finding 4 — head/tail split (the AA-hybrid seam) +# ----------------------------------------------------------------------- -## The ribbon polygon for an n-point course has exactly 2n vertices (n on -## each side) — this pins the "side-A then side-B reversed" construction -## produces a closed strip outline with no dropped or duplicated vertex. -func test_draw_tapered_course_ribbon_vertex_count_matches_two_times_point_count() -> void: +## The split point lands EXACTLY at TAPER_ARC_FRACTION of the total arc +## length, interpolated within the straddling segment — not snapped to the +## nearest existing vertex (see _split_course_at_arc_length()'s own doc for +## why interpolation, not snapping, is required). +func test_split_course_at_arc_length_interpolates_the_exact_fraction() -> void: var layer: StepCanvasAnnotationLayer = auto_free(StepCanvasAnnotationLayer.new()) add_child(layer) - layer.set_frame( - { - "width": 4, - "height": 4, - "courses": [{"class": 2, "points": [[0, 0], [10, 0], [20, 0], [30, 0]], "terminus": ""}], - }, - Vector2.ZERO, - "Chunk", - Vector2i(4, 4) - ) - # Draw-call correctness needs a live render pass (this suite's own header - # note); what's pinned here is that _course_widths_by_arc_length()'s - # output size always matches the input point count, which - # _draw_tapered_course() relies on 1:1 to build its 2n-vertex ribbon — - # see the width tests above for the per-vertex ramp itself. - var pts := PackedVector2Array([Vector2(0, 0), Vector2(10, 0), Vector2(20, 0), Vector2(30, 0)]) - var widths: PackedFloat32Array = layer._course_widths_by_arc_length(pts, 2.4) - assert_int(widths.size()).is_equal(pts.size()) + # Total length 40 (4 segments of 10px); taper fraction 0.15 -> split at + # arc-length 6, which is 60% of the way through the FIRST segment + # (0 -> 10), i.e. at x=6. + var pts := _straight_course_points() + var split: Array = layer._split_course_at_arc_length(pts, StepCanvasAnnotationLayer.TAPER_ARC_FRACTION) + var head: PackedVector2Array = split[0] + var tail: PackedVector2Array = split[1] + assert_vector(head[head.size() - 1]).is_equal_approx(Vector2(6.0, 0.0), Vector2(0.001, 0.001)) + assert_vector(tail[0]).is_equal_approx(Vector2(6.0, 0.0), Vector2(0.001, 0.001)) + + +## The head and tail share their boundary point EXACTLY (the butt-joint +## contract) — no gap, no overlap. +func test_split_course_at_arc_length_head_and_tail_share_the_boundary_point() -> void: + var layer: StepCanvasAnnotationLayer = auto_free(StepCanvasAnnotationLayer.new()) + add_child(layer) + var pts := _straight_course_points() + var split: Array = layer._split_course_at_arc_length(pts, StepCanvasAnnotationLayer.TAPER_ARC_FRACTION) + var head: PackedVector2Array = split[0] + var tail: PackedVector2Array = split[1] + assert_vector(head[head.size() - 1]).is_equal(tail[0]) + + +## `_split_course_at_arc_length()` is a generic arc-length splitter (the +## `t_fraction` parameter is not hardwired to TAPER_ARC_FRACTION) — when the +## requested fraction covers the WHOLE course (t_fraction >= 1.0, "the taper +## window would run past the mouth"), there is no meaningful post-split +## span: the whole course is the head, tail is empty. TAPER_ARC_FRACTION +## itself (0.15) can never trigger this branch for a real course (any +## positive-length course has SOME arc beyond 15% of itself) — this pins +## the branch directly via an out-of-the-ordinary fraction, the same way a +## unit test for a generic clamp function exercises both ends of its range +## regardless of what the one real call site happens to pass. +func test_split_course_at_arc_length_returns_empty_tail_when_fraction_covers_the_whole_course() -> void: + var layer: StepCanvasAnnotationLayer = auto_free(StepCanvasAnnotationLayer.new()) + add_child(layer) + var pts := PackedVector2Array([Vector2(0.0, 0.0), Vector2(1.0, 0.0)]) + var split: Array = layer._split_course_at_arc_length(pts, 1.0) + var head: PackedVector2Array = split[0] + var tail: PackedVector2Array = split[1] + assert_int(tail.size()).is_equal(0) + assert_int(head.size()).is_equal(pts.size()) + + +## The real call site's fraction (TAPER_ARC_FRACTION, 0.15) DOES still split +## even a very short two-point course — the split point just lands close to +## the source rather than at the mouth, and both head and tail are +## non-empty. This is the behavior _draw_tapered_course() actually relies +## on for a minimal two-point interior-source course. +func test_split_course_at_arc_length_still_splits_a_short_two_point_course() -> void: + var layer: StepCanvasAnnotationLayer = auto_free(StepCanvasAnnotationLayer.new()) + add_child(layer) + var pts := PackedVector2Array([Vector2(0.0, 0.0), Vector2(1.0, 0.0)]) + var split: Array = layer._split_course_at_arc_length(pts, StepCanvasAnnotationLayer.TAPER_ARC_FRACTION) + var head: PackedVector2Array = split[0] + var tail: PackedVector2Array = split[1] + assert_int(head.size()).is_equal(2) + assert_int(tail.size()).is_equal(2) + assert_vector(head[head.size() - 1]).is_equal_approx(Vector2(0.15, 0.0), Vector2(0.001, 0.001)) + + +## A degenerate (zero-length, coincident-point) course must not divide by +## zero in the split math — falls back to "whole course is the head". +func test_split_course_at_arc_length_handles_a_degenerate_zero_length_course() -> void: + var layer: StepCanvasAnnotationLayer = auto_free(StepCanvasAnnotationLayer.new()) + add_child(layer) + var pts := PackedVector2Array([Vector2(5.0, 5.0), Vector2(5.0, 5.0)]) + var split: Array = layer._split_course_at_arc_length(pts, StepCanvasAnnotationLayer.TAPER_ARC_FRACTION) + var tail: PackedVector2Array = split[1] + assert_int(tail.size()).is_equal(0) + + +# ----------------------------------------------------------------------- +# PR #207 findings 2/3 — mitred offset (perpendicular width at bends, +# clamped against self-intersection at hairpins) +# ----------------------------------------------------------------------- ## A perpendicular offset at any point along a straight horizontal course ## points along +/-Y, never +/-X — the ribbon must widen ACROSS the flow -## direction, not along it. -func test_segment_normal_is_perpendicular_to_a_straight_horizontal_course() -> void: +## direction, not along it. On a straight run theta=0, so the mitred offset +## reduces to the plain half-width (no widening). +func test_mitred_offset_is_perpendicular_on_a_straight_course() -> void: var layer: StepCanvasAnnotationLayer = auto_free(StepCanvasAnnotationLayer.new()) add_child(layer) var pts := _straight_course_points() - var normal: Vector2 = layer._segment_normal(pts, 2) - assert_float(normal.x).is_equal_approx(0.0, 0.001) - assert_float(absf(normal.y)).is_equal_approx(1.0, 0.001) + var offset: Vector2 = layer._mitred_offset(pts, 2, 1.0) + assert_float(offset.x).is_equal_approx(0.0, 0.001) + assert_float(absf(offset.y)).is_equal_approx(1.0, 0.001) + + +## Finding 3 (Hoshe) — at a 90-degree bend, the mitred offset LENGTH is +## half_w / cos(45deg) = half_w * sqrt(2) ~= 1.414 * half_w, which projects +## back to exactly half_w perpendicular to EACH adjacent segment (the true +## width the old averaged-unit-normal joint under-widened by cos(theta/2), +## a 29% pinch). Course: (0,0) -> (10,0) -> (10,10) — a clean right-angle +## turn at the middle vertex. +func test_mitred_offset_at_a_90_degree_bend_restores_perpendicular_width() -> void: + var layer: StepCanvasAnnotationLayer = auto_free(StepCanvasAnnotationLayer.new()) + add_child(layer) + var pts := PackedVector2Array([Vector2(0.0, 0.0), Vector2(10.0, 0.0), Vector2(10.0, 10.0)]) + var half_w := 1.0 + var offset: Vector2 = layer._mitred_offset(pts, 1, half_w) + # The offset's projection onto EITHER adjacent segment's own unit + # normal must equal half_w (the true perpendicular width on both + # faces of the bend) — not the offset's raw length (which is longer, + # by design, along the bisector). + var incoming_normal := Vector2(0.0, 1.0) # normal to the (0,0)->(10,0) segment + var outgoing_normal := Vector2(1.0, 0.0) # normal to the (10,0)->(10,10) segment + assert_float(absf(offset.dot(incoming_normal))).is_equal_approx(half_w, 0.01) + assert_float(absf(offset.dot(outgoing_normal))).is_equal_approx(half_w, 0.01) + + +## Finding 2 (Hoshe) — a tight hairpin (turn radius below half-width) must +## not produce a self-intersecting bowtie: the mitre offset is clamped to +## HAIRPIN_SEGMENT_FACTOR of the SHORTER adjacent segment length. Course +## with a very short middle segment (length 1) and a near-180-degree turn +## back on itself — an unclamped mitre would blow the offset length far +## past that short segment. +func test_mitred_offset_clamps_at_a_tight_hairpin() -> void: + var layer: StepCanvasAnnotationLayer = auto_free(StepCanvasAnnotationLayer.new()) + add_child(layer) + # (0,0) -> (1,0) -> (0, 0.01): a near-reversal at vertex 1, short + # adjacent segments (length 1 and ~1). + var pts := PackedVector2Array([Vector2(0.0, 0.0), Vector2(1.0, 0.0), Vector2(0.0, 0.01)]) + var half_w := 1.0 + var offset: Vector2 = layer._mitred_offset(pts, 1, half_w) + var shortest_segment := minf(pts[1].distance_to(pts[0]), pts[2].distance_to(pts[1])) + assert_float(offset.length()).is_less_equal( + shortest_segment * StepCanvasAnnotationLayer.HAIRPIN_SEGMENT_FACTOR + 0.001 + ) + + +## The ribbon polygon for an n-point head span has exactly 2n vertices (n on +## each side) — this pins the "side-A then side-B reversed" construction +## produces a closed strip outline with no dropped or duplicated vertex. +func test_head_widths_output_size_matches_head_point_count() -> void: + var layer: StepCanvasAnnotationLayer = auto_free(StepCanvasAnnotationLayer.new()) + add_child(layer) + var head := PackedVector2Array([Vector2(0, 0), Vector2(2, 0), Vector2(4, 0), Vector2(6, 0)]) + var widths: PackedFloat32Array = layer._head_widths_by_arc_length(head, 2.4) + assert_int(widths.size()).is_equal(head.size()) + + +# ----------------------------------------------------------------------- +# PR #207 finding 1 — crop-edge false-headwater detection gate +# ----------------------------------------------------------------------- + + +## An interior source (well inside the canvas bounds) IS a true source — +## tapering fires. +func test_is_true_source_in_canvas_true_for_an_interior_point() -> void: + var layer: StepCanvasAnnotationLayer = auto_free(StepCanvasAnnotationLayer.new()) + add_child(layer) + # District spacing 2048m, extent 4x4 -> half-extent 4096m on each axis. + layer.set_frame({"width": 4, "height": 4, "courses": []}, Vector2(1000.0, 2000.0), "District", Vector2i(4, 4)) + assert_bool(layer._is_true_source_in_canvas(Vector2(1000.0, 2000.0))).is_true() + + +## A point beyond the canvas's own declared bounds is the one-station crop +## overhang (`crop_course_to_window`'s `lo = first_in.saturating_sub(1)`), +## not a true source — no taper. +func test_is_true_source_in_canvas_false_for_a_point_outside_the_bounds() -> void: + var layer: StepCanvasAnnotationLayer = auto_free(StepCanvasAnnotationLayer.new()) + add_child(layer) + layer.set_frame({"width": 4, "height": 4, "courses": []}, Vector2(1000.0, 2000.0), "District", Vector2i(4, 4)) + # Half-extent is 4096m; world center + 5000m on X is well outside. + assert_bool(layer._is_true_source_in_canvas(Vector2(1000.0 + 5000.0, 2000.0))).is_false() + + +## A source sitting exactly at the boundary (within CROP_EDGE_EPSILON_M) +## behaves conservatively — treated as OUTSIDE (no taper), per the ruling. +func test_is_true_source_in_canvas_is_conservative_at_the_exact_boundary() -> void: + var layer: StepCanvasAnnotationLayer = auto_free(StepCanvasAnnotationLayer.new()) + add_child(layer) + layer.set_frame({"width": 4, "height": 4, "courses": []}, Vector2.ZERO, "District", Vector2i(4, 4)) + # Half-extent is 4096m exactly. A point AT the boundary (x=4096) is + # within epsilon of the edge -> conservatively NOT a true source. + assert_bool(layer._is_true_source_in_canvas(Vector2(4096.0, 0.0))).is_false() + + +## A null/malformed point (defensive — the caller already guards this via +## screen_pts.size() < 2) is conservatively NOT a true source. +func test_is_true_source_in_canvas_false_for_null() -> void: + var layer: StepCanvasAnnotationLayer = auto_free(StepCanvasAnnotationLayer.new()) + add_child(layer) + layer.set_frame({"width": 4, "height": 4, "courses": []}, Vector2.ZERO, "District", Vector2i(4, 4)) + assert_bool(layer._is_true_source_in_canvas(null)).is_false() + + +## Godot only allows draw_*() calls INSIDE an active `_draw()`/NOTIFICATION_ +## DRAW context (calling `_draw_tapered_course()` directly, outside that +## context, is a Godot Runtime Error, not a code bug) — so the "does not +## crash" smoke check for the taper=false/true routing goes through the SAME +## public entry every other "no crash" test in this suite already uses: +## `set_frame()` + `queue_redraw()` (matches +## `test_set_frame_stores_the_frame_and_triggers_no_crash_on_draw`'s own +## established pattern). This end-to-end path exercises +## `_draw_one_course()`'s routing decision (`_is_true_source_in_canvas()` -> +## `_draw_tapered_course()`'s `taper` argument) for real, without requiring +## a SubViewport or an explicit live-render await — matching this suite's +## own stated "pin the frame state, not pixels" scope. A crop-passthrough +## course (source point OUTSIDE the canvas bounds) exercises the taper=false +## flat-polyline path. +func test_set_frame_with_a_crop_passthrough_course_does_not_crash() -> void: + var layer: StepCanvasAnnotationLayer = auto_free(StepCanvasAnnotationLayer.new()) + add_child(layer) + var canvas := { + "width": 4, + "height": 4, + # world_center (0,0), District extent 4x4 -> half-extent 4096m. A + # source at x=-9000 is well outside the canvas bounds — the crop + # overhang case (finding 1). + "courses": [{"class": 2, "points": [[-9000, 0], [0, 0], [10, 0]], "terminus": ""}], + } + layer.set_frame(canvas, Vector2.ZERO, "District", Vector2i(4, 4)) + assert_object(layer).is_not_null() + + +## An interior-source course (source point inside the canvas bounds) +## exercises the taper=true ribbon-head + polyline-tail hybrid path. +func test_set_frame_with_an_interior_source_course_does_not_crash() -> void: + var layer: StepCanvasAnnotationLayer = auto_free(StepCanvasAnnotationLayer.new()) + add_child(layer) + var canvas := { + "width": 4, + "height": 4, + "courses": [{"class": 2, "points": [[0, 0], [500, 0], [1000, 0], [1500, 0]], "terminus": "Mouth"}], + } + layer.set_frame(canvas, Vector2.ZERO, "District", Vector2i(4, 4)) + assert_object(layer).is_not_null() diff --git a/client/ui/implant/apps/atlas/step_canvas/step_canvas_annotation_layer.gd b/client/ui/implant/apps/atlas/step_canvas/step_canvas_annotation_layer.gd index 3612a9da1..1658d0e79 100644 --- a/client/ui/implant/apps/atlas/step_canvas/step_canvas_annotation_layer.gd +++ b/client/ui/implant/apps/atlas/step_canvas/step_canvas_annotation_layer.gd @@ -90,6 +90,66 @@ const TAPER_ARC_FRACTION: float = 0.15 ## layer draws at (District..Chunk, 1px/gridunit) while staying a valid strip. const TAPER_MIN_WIDTH_PX: float = 0.15 +## PR #207 round, finding 1 (Araminta, blocking) — CROP-EDGE FALSE HEADWATERS. +## `points[0]` is only the course's TRUE upstream source when the course's +## full extent fits inside this canvas. When a course enters the canvas +## MID-RIVER, server-side cropping (`layer_proxy::crop_course_to_window` / +## `step_canvas::crop_course_for_canvas`, both: "Crop range: one station +## beyond each edge... `let lo = first_in.saturating_sub(1)`") keeps exactly +## ONE point beyond the window edge on the upstream side — SILENTLY: there is +## no wire-carried upstream analog of `CourseTerminus` (that enum only +## resolves the DOWNSTREAM end, `terminus` field). Tapering `points[0]` +## unconditionally therefore draws a fake spring at the crop line for every +## passthrough course. The one-point overhang IS the detection signal, +## client-side, with no wire change: a cropped course's `points[0]` lies +## OUTSIDE this canvas's own declared bounds (that overhang point exists +## PRECISELY so a consumer can see one station of "what's beyond the edge" — +## a consumer checking whether it's IN the window is reading that signal as +## intended); a true-source course's `points[0]` is the source itself, +## which is only kept because it was already inside (`first_in == 0` implies +## `lo == 0 == first_in`). CONTRACT: if a future crop change ever ships ZERO +## points beyond the window (or more than one), this gate silently breaks — +## re-derive it against `crop_course_to_window`'s own `lo`/`first_in` logic +## before touching either side. +## +## Epsilon absorbs float roundtrip slop between the server's `window_rect`/ +## `canvas_rect` (f64, half-extent split via integer `width/2`+`height-half_w`, +## step_canvas.rs's `fixed_canvas_world_rect`) and this file's own SYMMETRIC +## `extent_cells * 0.5` half-extent (`world_m_to_canvas_local`'s existing +## formula, reused as-is here rather than introduced as a second, subtly +## different bounds formula) — at worst a fraction of one gridunit for an +## odd extent, never enough to misclassify a real interior source as +## boundary-adjacent. A source sitting exactly at the boundary (within +## epsilon) is treated as OUTSIDE (no taper) — the conservative default per +## the ruling: "behaves conservatively (no taper)". +const CROP_EDGE_EPSILON_M: float = 1.0 + +## PR #207 findings 2 (Hoshe, verified numerically — ribbon self-intersects +## at turn radii below half-width) and 3 (Hoshe, verified — the OLD averaged- +## unit-normal joint under-widened the ribbon by cos(theta/2), a 29% pinch +## at a right-angle bend) — ONE FIX, per the lead ruling. A proper mitre: +## the offset length along the averaged (bisector) direction must be +## `half_w / cos(theta/2)` to restore the TRUE perpendicular width on both +## adjacent segments (the averaged-unit-normal approach implicitly used +## `half_w` unscaled, which is only correct for theta=0 — a straight run). +## Two clamps, both required (the ruling: "ONE FIX FOR BOTH"): +## 1. MITRE_LIMIT_FACTOR caps the offset at ~2x half_w for sharp angles +## (the classic mitre-limit / bevel-fallback threshold — an +## unclamped 1/cos(theta/2) blows up toward a near-180-degree fold-back). +## 2. An ADDITIONAL clamp to `HAIRPIN_SEGMENT_FACTOR` of the SHORTER +## adjacent segment length — this is the piece that actually prevents +## the self-intersecting bowtie at a tight hairpin (turn radius < +## half-width): capping by half_w alone still lets the offset exceed +## the segment's own length when the segment is shorter than half_w, +## which is exactly the self-crossing condition Hoshe's numeric check +## caught. +## Guarantee restated (the old doc's "always simple" claim was FALSE, per +## Hoshe): this ribbon is a simple (non-self-intersecting) polygon up to the +## mitre limit; beyond it, the offset is clamped — a bounded, visible +## flattening at an extreme hairpin, never a bowtie fold. +const MITRE_LIMIT_FACTOR: float = 2.0 +const HAIRPIN_SEGMENT_FACTOR: float = 0.45 + const MOUTH_RING_RADIUS_PX: float = 5.0 const MOUTH_HALO_RADIUS_PX: float = 8.0 const MOUTH_HALO_ALPHA: float = 0.30 @@ -145,10 +205,15 @@ func _draw_one_course(course: Dictionary) -> void: return var screen_pts := PackedVector2Array() + # First WELL-FORMED point's world position — the crop-edge gate (finding + # 1) reads THIS, not screen_pts[0], which is already screen-projected. + var first_world_m: Variant = null for pt: Variant in points_raw: if not (pt is Array and pt.size() >= 2): continue var world_m := Vector2(float(pt[0]), float(pt[1])) + if first_world_m == null: + first_world_m = world_m screen_pts.append(_world_to_local(world_m)) if screen_pts.size() < 2: return @@ -156,35 +221,158 @@ func _draw_one_course(course: Dictionary) -> void: var width: float = float(COURSE_CLASS_WIDTH_PX.get(cls, COURSE_CLASS_WIDTH_PX[RIVER_CLASS_STREAM])) var opacity: float = float(COURSE_CLASS_OPACITY.get(cls, COURSE_CLASS_OPACITY[RIVER_CLASS_STREAM])) var color := Color(COLOR_RIVER.r, COLOR_RIVER.g, COLOR_RIVER.b, COLOR_RIVER.a * opacity) - _draw_tapered_course(screen_pts, width, color) + + # Finding 1 (PR #207, Araminta, blocking): only taper when points[0] is + # the course's TRUE source (inside this canvas's own declared bounds) — + # see _is_true_source_in_canvas()'s own doc for the crop-overhang gate. + var is_true_source: bool = _is_true_source_in_canvas(first_world_m) + _draw_tapered_course(screen_pts, width, color, is_true_source) var terminus: String = str(course.get("terminus", "")) if terminus == COURSE_TERMINUS_MOUTH: _draw_mouth_ring(screen_pts[screen_pts.size() - 1]) -## Draw one course as a source-tapered ribbon (T-1175 seeded item 2): -## `screen_pts[0]` (the upstream source) narrows to TAPER_MIN_WIDTH_PX, -## ramping linearly by ARC LENGTH (not by vertex index — a course's own -## points are not evenly spaced, so an index-based ramp would taper faster -## or slower depending on point density) to `full_width` at -## TAPER_ARC_FRACTION of the total run, then holds `full_width` to the -## mouth end. Built as a single `draw_polygon()` triangle-strip-shaped -## ribbon: one offset vertex pair (left/right of the course direction) per -## centerline point, side-A vertices first then side-B vertices REVERSED — -## `draw_polygon()` triangulates whatever simple polygon its point winding -## describes, and a strip laid out this way (there-and-back around the -## ribbon's own outline) is always simple (non-self-intersecting) for a -## non-self-crossing centerline, which every real river course is. -func _draw_tapered_course(screen_pts: PackedVector2Array, full_width: float, color: Color) -> void: - var widths := _course_widths_by_arc_length(screen_pts, full_width) +## PR #207 finding 1 — the crop-overhang detection gate. `canvas_rect` is +## reconstructed client-side from the held frame (`_world_center`/`_rung`/ +## `_extent_cells`) using the SAME symmetric half-extent formula +## `world_m_to_canvas_local()` already uses to project every point in this +## file (see CROP_EDGE_EPSILON_M's own doc for why this — not the server's +## asymmetric integer split — is the right formula to mirror here). A point +## strictly outside those bounds (beyond the epsilon) is the one-station +## crop overhang (`crop_course_to_window`'s `lo = first_in.saturating_sub(1)`) +## — a passthrough course, not a real headwater. `null` (a course with no +## well-formed points at all, already unreachable by the caller's own +## `screen_pts.size() < 2` guard, but defensive here too) is conservatively +## NOT a true source. +func _is_true_source_in_canvas(world_m: Variant) -> bool: + if not world_m is Vector2: + return false + var p: Vector2 = world_m + var half_w_m: float = float(_extent_cells.x) * 0.5 * StepCanvasTransport.spacing_for_rung(_rung) + var half_h_m: float = float(_extent_cells.y) * 0.5 * StepCanvasTransport.spacing_for_rung(_rung) + var lo_x: float = _world_center.x - half_w_m + CROP_EDGE_EPSILON_M + var hi_x: float = _world_center.x + half_w_m - CROP_EDGE_EPSILON_M + var lo_y: float = _world_center.y - half_h_m + CROP_EDGE_EPSILON_M + var hi_y: float = _world_center.y + half_h_m - CROP_EDGE_EPSILON_M + return p.x >= lo_x and p.x <= hi_x and p.y >= lo_y and p.y <= hi_y + + +## PR #207 finding 4 (both reviewers, blocking) — ANTIALIASING. A flat +## `draw_polygon()` ribbon for the WHOLE course silently dropped the AA +## `draw_polyline(..., antialiased=true)` shipped with (a hard-rasterized +## 0.6px stream at 0.8 alpha stairsteps). LEAD RULING'S HYBRID: only the +## TAPER HEAD (source -> the point where width first reaches full class +## width, i.e. TAPER_ARC_FRACTION of the run) draws as a ribbon — width +## varies there, and `draw_polyline()` has no per-vertex-width primitive, so +## the ribbon is unavoidable for that span. The remaining ~85% of the run +## (the visually dominant part, constant full width) draws via the ORIGINAL +## `draw_polyline()` call, AA intact, unchanged from pre-T-1175 behaviour. +## The two pieces meet at a BUTT joint: the ribbon's own last cross-section +## is exactly `full_width` wide at exactly the polyline's first point — same +## width, same position, no gap or overlap by construction (see +## _split_course_at_arc_length()'s own doc for how that shared point is +## derived). Only applies when `taper` is true; a crop-passthrough course +## (finding 1) skips the ribbon path entirely and draws as a single +## constant-width AA polyline, matching the pre-taper flat-cut look +## Araminta already prefers as the default for that case. +func _draw_tapered_course( + screen_pts: PackedVector2Array, full_width: float, color: Color, taper: bool +) -> void: + if not taper: + draw_polyline(screen_pts, color, full_width, true) + return + + var split := _split_course_at_arc_length(screen_pts, TAPER_ARC_FRACTION) + var head_pts: PackedVector2Array = split[0] + var tail_pts: PackedVector2Array = split[1] + + _draw_ribbon_head(head_pts, full_width, color) + if tail_pts.size() >= 2: + draw_polyline(tail_pts, color, full_width, true) + + +## Splits `screen_pts` into a HEAD (source through the arc-length fraction +## `t_fraction` of the total run, inclusive of an INTERPOLATED point exactly +## at that fraction) and a TAIL (that same interpolated point through the +## mouth) — the shared interpolated point is the butt-joint seam +## `_draw_tapered_course()` relies on for a gapless/overlapless hybrid. +## Snapping to the nearest EXISTING vertex instead (no interpolation) was +## rejected: station spacing is a rung's own gridunit spacing (2048 m at +## District down to 64 m at Chunk) — coarse enough, relative to a canvas's +## screen footprint, that a single segment can span the ENTIRE taper +## fraction, which would make an index-snapped seam land far from the +## intended taper length rather than close to it. +## +## Returns `[head, tail]`; `tail` is empty (size 0) when the course is +## shorter than `t_fraction` of itself, i.e. the taper fraction as measured +## would run past the mouth — the caller's `tail_pts.size() >= 2` guard +## then draws nothing beyond the ribbon head, which itself already reaches +## `full_width` by its own last point in that case (`_head_widths_by_arc_ +## length()`'s own ramp always ends at `full_width` at the head's last +## vertex) — a course too short for a meaningful post-taper span still ends +## up at full width, just without a separate polyline tail. +func _split_course_at_arc_length(screen_pts: PackedVector2Array, t_fraction: float) -> Array: + var n := screen_pts.size() + if n < 2: + return [screen_pts, PackedVector2Array()] + + var cumulative := PackedFloat32Array() + cumulative.resize(n) + cumulative[0] = 0.0 + for i in range(1, n): + cumulative[i] = cumulative[i - 1] + screen_pts[i].distance_to(screen_pts[i - 1]) + var total_len: float = cumulative[n - 1] + + if total_len <= 0.0: + # Degenerate (coincident-point) course — nothing meaningful to + # split; the whole thing is the "head" (drawn at full width per + # _course_widths_by_arc_length()'s own degenerate-course fallback). + return [screen_pts, PackedVector2Array()] + + var taper_len: float = total_len * t_fraction + if taper_len >= total_len: + # The taper fraction covers the whole course (a very short course) — + # no constant-width tail exists; the ribbon head IS the whole course. + return [screen_pts, PackedVector2Array()] + + # Find the segment straddling taper_len and interpolate the exact split + # point along it. + var split_idx := n - 1 + for i in range(1, n): + if cumulative[i] >= taper_len: + split_idx = i + break + var seg_start_len: float = cumulative[split_idx - 1] + var seg_len: float = cumulative[split_idx] - seg_start_len + var seg_t: float = 0.0 if seg_len <= 0.0 else (taper_len - seg_start_len) / seg_len + var split_point: Vector2 = screen_pts[split_idx - 1].lerp(screen_pts[split_idx], seg_t) + + var head := PackedVector2Array() + for i in range(split_idx): + head.append(screen_pts[i]) + head.append(split_point) + + var tail := PackedVector2Array() + tail.append(split_point) + for i in range(split_idx, n): + tail.append(screen_pts[i]) + return [head, tail] + + +## Draws the tapered ribbon for the HEAD span only (source through the +## constant-full-width handoff point, `head_pts`'s own last point) — +## `full_width` is the width AT the handoff (the head's own last vertex), +## matching the tail polyline's constant width exactly (the butt joint). +func _draw_ribbon_head(head_pts: PackedVector2Array, full_width: float, color: Color) -> void: + var widths := _head_widths_by_arc_length(head_pts, full_width) var left := PackedVector2Array() var right := PackedVector2Array() - for i in range(screen_pts.size()): - var normal: Vector2 = _segment_normal(screen_pts, i) + for i in range(head_pts.size()): var half_w: float = widths[i] * 0.5 - left.append(screen_pts[i] + normal * half_w) - right.append(screen_pts[i] - normal * half_w) + var offset: Vector2 = _mitred_offset(head_pts, i, half_w) + left.append(head_pts[i] + offset) + right.append(head_pts[i] - offset) var ribbon := PackedVector2Array() ribbon.append_array(left) @@ -193,18 +381,21 @@ func _draw_tapered_course(screen_pts: PackedVector2Array, full_width: float, col if ribbon.size() < 3: return - var colors := PackedColorArray() - colors.resize(ribbon.size()) - colors.fill(color) - draw_polygon(ribbon, colors) + # Finding 5 (Hoshe, trivial) — draw_polygon() accepts a 1-element color + # array for a flat fill; no need to replicate `color` across every + # ribbon vertex. + draw_polygon(ribbon, PackedColorArray([color])) -## Per-vertex width for a tapered course — arc-length parameterized so the -## taper reads consistently regardless of how densely a course's own points -## are spaced. `full_width` for every vertex beyond TAPER_ARC_FRACTION of -## the cumulative length from the source (index 0). -func _course_widths_by_arc_length(screen_pts: PackedVector2Array, full_width: float) -> PackedFloat32Array: - var n := screen_pts.size() +## Per-vertex width for the RIBBON HEAD ONLY — linear ramp from +## TAPER_MIN_WIDTH_PX at the source (index 0) to `full_width` at the head's +## own LAST point (the butt-joint handoff, always exactly `full_width` by +## construction: `_split_course_at_arc_length()` places that point at +## exactly `t_fraction` of the FULL course's arc length, and this function +## is handed only the head span, so the head's own last point is always the +## ramp's 100% mark). +func _head_widths_by_arc_length(head_pts: PackedVector2Array, full_width: float) -> PackedFloat32Array: + var n := head_pts.size() var widths := PackedFloat32Array() widths.resize(n) if n == 0: @@ -217,41 +408,68 @@ func _course_widths_by_arc_length(screen_pts: PackedVector2Array, full_width: fl cumulative.resize(n) cumulative[0] = 0.0 for i in range(1, n): - cumulative[i] = cumulative[i - 1] + screen_pts[i].distance_to(screen_pts[i - 1]) - var total_len: float = cumulative[n - 1] + cumulative[i] = cumulative[i - 1] + head_pts[i].distance_to(head_pts[i - 1]) + var head_len: float = cumulative[n - 1] - # A degenerate (zero-length, coincident-point) course has no meaningful - # arc-length ramp — hold every vertex at full width rather than divide - # by zero. - if total_len <= 0.0: + if head_len <= 0.0: widths.fill(full_width) return widths - var taper_len: float = total_len * TAPER_ARC_FRACTION for i in range(n): - if taper_len <= 0.0 or cumulative[i] >= taper_len: - widths[i] = full_width - else: - var t: float = cumulative[i] / taper_len - widths[i] = lerpf(TAPER_MIN_WIDTH_PX, full_width, t) + var t: float = cumulative[i] / head_len + widths[i] = lerpf(TAPER_MIN_WIDTH_PX, full_width, t) return widths -## The ribbon-offset direction at vertex `i` — perpendicular to the local -## course tangent, averaged between the incoming and outgoing segment when -## both exist (a mitred join at interior vertices, avoiding a visible kink -## in the ribbon edge at each point) and falling back to the single -## adjacent segment's normal at either end. -func _segment_normal(screen_pts: PackedVector2Array, i: int) -> Vector2: - var n := screen_pts.size() - var dir := Vector2.ZERO +## PR #207 findings 2/3 — see the MITRE_LIMIT_FACTOR/HAIRPIN_SEGMENT_FACTOR +## const doc (top of file) for the full rationale; this is the function that +## consumes them. +func _mitred_offset(points: PackedVector2Array, i: int, half_w: float) -> Vector2: + var n := points.size() + var incoming: Vector2 = Vector2.ZERO + var outgoing: Vector2 = Vector2.ZERO + var incoming_len: float = 0.0 + var outgoing_len: float = 0.0 if i > 0: - dir += (screen_pts[i] - screen_pts[i - 1]).normalized() + var seg: Vector2 = points[i] - points[i - 1] + incoming_len = seg.length() + if incoming_len > 0.0: + incoming = seg / incoming_len if i < n - 1: - dir += (screen_pts[i + 1] - screen_pts[i]).normalized() + var seg: Vector2 = points[i + 1] - points[i] + outgoing_len = seg.length() + if outgoing_len > 0.0: + outgoing = seg / outgoing_len + + var dir: Vector2 = incoming + outgoing if dir == Vector2.ZERO: return Vector2.ZERO - return dir.normalized().orthogonal() + dir = dir.normalized() + var normal: Vector2 = dir.orthogonal() + + # cos(theta/2) where theta is the turn angle between incoming and + # outgoing tangents — the bisector-normal `normal` sits exactly + # theta/2 off each segment's own perpendicular, so the dot product + # against EITHER segment's unit normal recovers cos(theta/2) directly + # (no explicit angle/trig call needed). + var half_angle_cos: float = normal.dot(incoming.orthogonal()) if incoming_len > 0.0 else 1.0 + if outgoing_len > 0.0: + var alt_cos: float = normal.dot(outgoing.orthogonal()) + half_angle_cos = maxf(absf(half_angle_cos), absf(alt_cos)) + half_angle_cos = maxf(absf(half_angle_cos), 0.05) # guard near-180 fold-back (cos -> 0) + + var mitre_len: float = half_w / half_angle_cos + mitre_len = minf(mitre_len, half_w * MITRE_LIMIT_FACTOR) + + var shortest_adjacent: float = INF + if incoming_len > 0.0: + shortest_adjacent = minf(shortest_adjacent, incoming_len) + if outgoing_len > 0.0: + shortest_adjacent = minf(shortest_adjacent, outgoing_len) + if is_finite(shortest_adjacent): + mitre_len = minf(mitre_len, shortest_adjacent * HAIRPIN_SEGMENT_FACTOR) + + return normal * mitre_len func _draw_mouth_ring(local_pt: Vector2) -> void: