Skip to content

[CTX-1007] feat(render): underline shader fidelity curly/dotted/dashed + SGR 58 (#1761) - #1770

Merged
Xuepoo merged 1 commit into
mainfrom
ctx-1007/feat-underline-shader-fidelity
Oct 7, 2026
Merged

Xuepoo merged 1 commit into
mainfrom
ctx-1007/feat-underline-shader-fidelity

Conversation

@Xuepoo

@Xuepoo Xuepoo commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

Implements #1761: WGSL underline-pattern library plus DPI-aware CPU quantization in bitty-render::grid.

Scope:

  • pipeline::UNDERLINE_WGSL routines for curly sine/wavy, dotted, dashed, double gap, pattern thickness; re-exported from bitty-render root.
  • grid.rs CPU mirrors (pattern_thickness, curly step/wavelength/amplitude, sine plus quantized segment offsets, dotted/dashed params, double gap) with phase-aligned absolute-origin patterns; 8x16 representative geometry unchanged.
  • Double-spacing refinement via double_gap_px; patterned bars use DPI-aware thickness while Single/Double keep the pinned underline_thickness contract (runtime mirror still green).
  • SGR 58/59 honored via existing underline_color resolution, now covered by render tests proving wavy underline color independent of foreground.

Verification:

  • cargo fmt --check: clean.
  • cargo clippy --workspace --all-targets --locked -- -D warnings: clean.
  • cargo check --workspace --all-targets --locked: clean.
  • cargo check --target x86_64-pc-windows-gnu --workspace --all-targets --locked: clean.
  • cargo test -p bitty-render --lib: 243 passed (8 new ctx1007 tests).
  • cargo nextest run -p bitty-render -p bitty-vt -p bitty-term-state: 691 passed.
  • bitty-runtime underline_thickness_mirror: passed (pinned Single formula untouched).
  • just ci-local: BLOCKED by known infra CTX-0983 (sccache GHA backend ConfigInvalid under act, affects every branch; remote GHA Quality gates expected green). Host equivalents above are green.
  • ./scripts/check-scratch-paths.sh: OK. git diff --check: clean. No TODO/FIXME.

Closes #1761.

Priority: P2 | Area: area:render | Labels: feat,area:render,P2 | Milestone: v0.1.0 | RFC: OQ-004 | Task: CTX-1007

Summary by CodeRabbit

  • New Features
    • Dotted, dashed, double, and curly underlines now scale with cell dimensions, with patterns aligned across adjacent cells.
    • Underline colors are applied consistently across supported styles, including indexed colors and the default foreground color.
    • Added public shader resources for rendering underline patterns.

@Xuepoo Xuepoo added this to the v0.1.0 milestone Oct 7, 2026
@Xuepoo Xuepoo added feat Feature area:render Area: rendering P2 Priority: medium labels Oct 7, 2026
@coderabbitai

coderabbitai Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

⚙️ Run configuration
  • Configuration used: Repository UI (base), Organization UI (inherited)
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: a044f0c9-599d-463b-a1f2-4e89d13bc8a3
📥 Commits

Reviewing files that changed from the base of the PR and between 143b49d and 5d2390b.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository UI (base), Organization UI (inherited)
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 74235bf4-19d7-4258-b479-56e1bffa18b3
📥 Commits

Reviewing files that changed from the base of the PR and between d5ca8e5 and 8911faa.

📒 Files selected for processing (4)
  • crates/bitty-render/src/grid.rs
  • crates/bitty-render/src/grid/tests.rs
  • crates/bitty-render/src/lib.rs
  • crates/bitty-render/src/pipeline.rs

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

Patterned underline geometry now scales with cell dimensions. Grid rendering applies the geometry to double, dotted, dashed, and curly styles. The crate exposes geometry helpers and a WGSL reference library. Tests cover geometry, phase continuity, distinct styles, and underline color handling.

Changes

Underline rendering

Layer / File(s) Summary
Geometry helpers and WGSL reference
crates/bitty-render/src/grid.rs, crates/bitty-render/src/pipeline.rs, crates/bitty-render/src/lib.rs
Public helpers define patterned underline thickness, spacing, dimensions, and curly-wave offsets. The public WGSL library defines corresponding pattern routines, and the crate root re-exports these APIs.
Scaled grid rendering and verification
crates/bitty-render/src/grid.rs, crates/bitty-render/src/grid/tests.rs
Grid rendering uses scaled geometry for double, dotted, dashed, and curly underlines. Tests check reference dimensions, scaling, cell-boundary phase continuity, distinct styles, and custom underline colors with foreground fallback.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Feature · Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 8911f

The reported thickness difference is expected, so no actionable merge-blocking issue remains after normal checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 8911f

The change adds public underline calculations without an identified new privileged capability. Rendering remains bounded, but downstream use has not been fully established, so the assessment remains cautious.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The demonstrated affected outcome is underline rectangle geometry for visited snapshot cells through the existing rendering entrypoint. The scoped evidence does not establish the complete upstream terminal ingress or enumerate every external consumer of the new exports.

Trust Boundaries and Controls

  • observed — Cell style selects the existing underline path, while renderer-owned metrics supply its dimensions. Pattern emission clips horizontal runs, constrains vertical placement, coarsens excessive detail and stops if saturated arithmetic cannot advance.

Resilience and Maintainability Implications

  • observed — Existing atlas-exhaustion recovery discards the first render pass, restores accounting, resets the atlas and rebuilds the draw list. New decoration output remains confined to those per-pass lists rather than surviving as stale partial output.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the main changes: shader support for curly, dotted, and dashed underlines, plus SGR 58 underline colors.
Linked Issues check ✅ Passed Issue #1761 objectives are implemented. pipeline.rs defines and exports UNDERLINE_WGSL routines for curly, dotted, dashed, double-gap, and DPI-aware thickness. grid.rs adds corresponding CPU geo…
Out of Scope Changes check ✅ Passed The changes are limited to underline shader routines, grid geometry and color behavior, public re-exports, and tests for issue #1761. No unrelated changes are evident in the reviewed diff.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 24 functions across 4 files.
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@Xuepoo

Xuepoo commented Oct 7, 2026

Copy link
Copy Markdown
Contributor Author

Independent review: APPROVE (CTX-1007 reviewer identity)

Reviewed the full diff of ctx-1007/feat-underline-shader-fidelity against the issue #1761 claims. What verifies:

  • WGSL curly/dotted/dashed + double refinement: UNDERLINE_WGSL defines all six routines (pattern thickness, curly offset/phase, dotted/dashed masks, double gap), honestly documented as the analytic fidelity reference rather than a fourth pipeline — the CPU quantizes to FillRect runs.
  • CPU mirrors hand-checked: pattern_thickness, double_gap_px, dotted/dashed_params_px, curly step/wavelength/amplitude, sine plus the quantized [0, amp/2, amp, amp/2] stair all reproduce the claimed 8x16 goldens ((4,2) dots, (6,3) dashes, 4px double gap) and the scaled spot values; Single/Double keep the pinned underline_thickness contract.
  • Phase alignment genuinely proven: ctx1007_dashed_continues_across_cells_phase_aligned expects the middle dash split at the cell boundary ([6,8) + [8,9)), which only a continuous origin-anchored pattern produces — a per-cell restart would fail that assertion.
  • SGR 58/59 independent color: RGB independence on curly, indexed color on dotted/dashed, and SGR 59 Default-falls-back-to-foreground are all covered by render tests.
  • DPI scaling: the 8x16 vs 13x26 vs 16x32 scale test checks thickening, crest rise, gap widening, in-cell containment, and pairwise style distinctness at every scale.
  • Robustness: saturating arithmetic and loop-advance guards keep hostile geometry total; spot-checked for TODO/FIXME — none.
  • CI: all required checks green. CodeRabbit: review completed with 0 inline findings.

Non-blocking note for the commander (no action for this PR): #1771 and #1772 both introduce a bitty_platform CursorIcon / WindowHandle::set_cursor_icon and will need reconciliation with each other at merge time.

Verdict: APPROVE.

@Xuepoo
Xuepoo force-pushed the ctx-1007/feat-underline-shader-fidelity branch from 8911faa to 143b49d Compare October 7, 2026 14:47
@Xuepoo
Xuepoo force-pushed the ctx-1007/feat-underline-shader-fidelity branch from 143b49d to 5d2390b Compare October 7, 2026 15:17
@Xuepoo
Xuepoo merged commit d0007c7 into main Oct 7, 2026
17 checks passed
@Xuepoo
Xuepoo deleted the ctx-1007/feat-underline-shader-fidelity branch October 7, 2026 15:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:render Area: rendering feat Feature P2 Priority: medium

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[P2] Underline style shader fidelity for curly, dotted, and dashed lines with SGR 58 support

1 participant