fix(ui): PR #207 review round — crop-gated taper, true mitres, hybrid AA (T-1175)

Five findings from the araminta+hoshe round. Tapering now fires only on
a TRUE upstream source: the course's first raw world point is tested
against the canvas's own world bounds (conservative 1m epsilon) —
exact detection because the server crop keeps one point beyond the
window (layer_proxy crop_course_to_window lo = first_in-1, contract
documented), so crop passthroughs draw the old flat full-width cut and
never a false headwater. The averaged-normal joint is replaced by a
real mitre (half_w/cos(theta/2) recovered trig-free via the bisector
normal), clamped by a 2x mitre limit AND 0.45x the shorter adjacent
segment — restoring true perpendicular width at bends (the 29% pinch at
confluences is gone) and preventing the hairpin bowtie; the winding doc
now states the actual bounded guarantee. Antialiasing restored via the
hybrid: only the varying-width taper head draws as a ribbon; the
constant-width ~85% of every course keeps the original antialiased
draw_polyline (byte-identical for untapered courses), split at an
interpolated arc-length point sharing position and width — junction
capture evidence in .cache/screenshots/t1175-fix-round/. Flat-fill
single-element color array. Suite 24 -> 48 tests.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
2026-07-25 16:05:17 +02:00
co-authored by Claude Fable 5
parent 28d2e6c80b
commit 686021bee8
2 changed files with 527 additions and 121 deletions
+256 -68
View File
@@ -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()