Skip to content

Commit d8d2d03

Browse files
committed
refactor: harden color-aa CLI parsing and de-magic charset count
Make --color-aa a typed Option<AaStrength> via FromStr so clap rejects invalid values with a helpful error instead of silently falling back to Off; remove the hand-rolled parse-and-ignore paths in resolved_color_aa and the runner. Replace the duplicated [AaStrength; 7] literals with a NUM_CHARSETS const tied to ALL_CHARSETS.len(), so per-charset arrays stay locked to the charset count. Drop two provably-dead per-cell bounds checks in the blur hot loop in favor of a single debug_assert on the width*height invariant.
1 parent 0b91f6e commit d8d2d03

8 files changed

Lines changed: 43 additions & 19 deletions

File tree

src/app/runner.rs

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -324,10 +324,8 @@ pub fn run_simulation(
324324
renderer.set_dither_mode(dither_mode);
325325

326326
// Apply CLI --color-aa override to the launch charset (else the per-charset default stands).
327-
if let Some(ref s) = args.color_aa {
328-
if let Some(aa) = crate::render::antialiasing::AaStrength::parse_cli(s) {
329-
runtime_state.apply_cli_color_aa(aa);
330-
}
327+
if let Some(aa) = args.color_aa {
328+
runtime_state.apply_cli_color_aa(aa);
331329
}
332330
// Push the resolved launch AA (default or CLI override) to the renderer so the
333331
// FIRST frame already reflects it (renderer's color_aa otherwise inits to Off).

src/cli.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1715,7 +1715,7 @@ pub struct Args {
17151715
help = "Color anti-aliasing for subcell charsets: off | subtle | strong (default: auto — strong for braille, off otherwise)"
17161716
)]
17171717
/// Color anti-aliasing mode for subcell charsets.
1718-
pub color_aa: Option<String>,
1718+
pub color_aa: Option<crate::render::antialiasing::AaStrength>,
17191719

17201720
#[arg(
17211721
long = "bg-color",
@@ -1870,8 +1870,8 @@ impl Args {
18701870
charset: &crate::render::charset::Charset,
18711871
) -> crate::render::antialiasing::AaStrength {
18721872
use crate::render::antialiasing::AaStrength;
1873-
if let Some(ref s) = self.color_aa {
1874-
return AaStrength::parse_cli(s).unwrap_or(AaStrength::Off);
1873+
if let Some(aa) = self.color_aa {
1874+
return aa;
18751875
}
18761876
if matches!(charset, crate::render::charset::Charset::Braille) {
18771877
AaStrength::Strong

src/config_defaults.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -488,7 +488,8 @@ pub mod rendering {
488488
/// Default per-charset color-AA strength, indexed by `ALL_CHARSETS` order:
489489
/// [HalfBlock, HalfBlockDual, Ascii, Braille, Quadrant, Shade, Points].
490490
/// Braille defaults to Strong; everything else Off.
491-
pub const DEFAULT_COLOR_AA: [crate::render::antialiasing::AaStrength; 7] = {
491+
pub const DEFAULT_COLOR_AA: [crate::render::antialiasing::AaStrength;
492+
crate::render::charset::NUM_CHARSETS] = {
492493
use crate::render::antialiasing::AaStrength::{Off, Strong};
493494
[Off, Off, Off, Strong, Off, Off, Off]
494495
};

src/config_manager.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -252,7 +252,7 @@ impl SavedConfig {
252252
palette_cycle: crate::render::palette::PaletteCycle,
253253
glyph: crate::render::charset::GlyphConfig,
254254
temporal_accent: Option<crate::render::palette::RgbColor>,
255-
color_aa: [crate::render::antialiasing::AaStrength; 7],
255+
color_aa: [crate::render::antialiasing::AaStrength; crate::render::charset::NUM_CHARSETS],
256256
) -> Self {
257257
let diffusion_kernel_str = match sim_config.diffusion_kernel {
258258
DiffusionKernel::Mean3x3 => "mean3x3",

src/render/antialiasing.rs

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,19 @@ impl AaStrength {
6060
}
6161
}
6262

63+
impl std::str::FromStr for AaStrength {
64+
type Err = String;
65+
66+
/// Parse a CLI token, rejecting unrecognized values with a helpful message
67+
/// (used by clap so bad `--color-aa` input errors instead of silently
68+
/// falling back).
69+
fn from_str(s: &str) -> Result<Self, Self::Err> {
70+
Self::parse_cli(s).ok_or_else(|| {
71+
format!("invalid color-aa mode: {s}. Valid options: off, subtle, strong")
72+
})
73+
}
74+
}
75+
6376
/// True for charsets whose shape resolution exceeds their color resolution and
6477
/// therefore benefit from color anti-aliasing.
6578
pub fn charset_aa_eligible(charset: &Charset) -> bool {
@@ -78,13 +91,18 @@ pub fn blur_field(src: &[f32], width: usize, height: usize, strength: AaStrength
7891
if strength == AaStrength::Off || width == 0 || height == 0 {
7992
return src.to_vec();
8093
}
94+
// Invariant: src is exactly the row-major field. With it, idx = y*width+x and
95+
// every in-range nidx are provably < src.len(), so the inner indexing needs no
96+
// per-cell bounds check (debug builds catch a caller that violates this).
97+
debug_assert_eq!(
98+
src.len(),
99+
width * height,
100+
"blur_field: src must be width*height"
101+
);
81102
let mut out = vec![0.0_f32; src.len()];
82103
for y in 0..height {
83104
for x in 0..width {
84105
let idx = y * width + x;
85-
if idx >= src.len() {
86-
continue;
87-
}
88106
let center = src[idx];
89107
let mut neighbor_sum = 0.0_f32;
90108
let mut neighbor_count = 0u32;
@@ -97,10 +115,8 @@ pub fn blur_field(src: &[f32], width: usize, height: usize, strength: AaStrength
97115
let ny = y as i32 + dy;
98116
if nx >= 0 && nx < width as i32 && ny >= 0 && ny < height as i32 {
99117
let nidx = ny as usize * width + nx as usize;
100-
if nidx < src.len() {
101-
neighbor_sum += src[nidx];
102-
neighbor_count += 1;
103-
}
118+
neighbor_sum += src[nidx];
119+
neighbor_count += 1;
104120
}
105121
}
106122
}

src/render/charset.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,10 @@ pub const ALL_CHARSETS: [Charset; 7] = [
138138
Charset::Points,
139139
];
140140

141+
/// Number of distinct charsets. Single source of truth for any per-charset
142+
/// array (e.g. color-AA strength) so they stay locked to [`ALL_CHARSETS`].
143+
pub const NUM_CHARSETS: usize = ALL_CHARSETS.len();
144+
141145
const HALF_BLOCK_CHARS: [char; 9] = [
142146
' ', '\u{2581}', '\u{2582}', '\u{2583}', '\u{2584}', '\u{2585}', '\u{2586}', '\u{2587}',
143147
'\u{2588}',

src/terminal/frame_buffer.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -566,6 +566,10 @@ impl FrameBuffer {
566566
// Color anti-aliasing: when active for this (frame-uniform) charset,
567567
// pre-blur two per-cell color fields. The glyph/shape path below reads
568568
// raw quadrant data and is unaffected.
569+
// PERF: when active this allocates ~3 Vec<f32> per frame (base, diff, and
570+
// each blur_field output). Only runs for AA-eligible charsets (braille by
571+
// default). Revisit with reusable scratch buffers if a frame-time profile
572+
// shows braille-AA as hot; not worth the restructure unprofiled.
569573
use crate::render::antialiasing::{blur_field, charset_aa_eligible};
570574
let aa_active = aa_strength != crate::render::antialiasing::AaStrength::Off
571575
&& charset_aa_eligible(&charset);

src/terminal/state.rs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -387,7 +387,7 @@ pub struct ParameterState {
387387
/// Window frame display mode.
388388
pub window_frame: WindowFrame,
389389
/// Per-charset color-AA strength, indexed by `charset_index`.
390-
pub color_aa: [crate::render::antialiasing::AaStrength; 7],
390+
pub color_aa: [crate::render::antialiasing::AaStrength; crate::render::charset::NUM_CHARSETS],
391391
}
392392

393393
#[derive(Debug, Clone, Copy, PartialEq)]
@@ -487,7 +487,7 @@ pub struct RuntimeState {
487487
/// Index of current charset.
488488
pub charset_index: usize,
489489
/// Per-charset color-AA strength, indexed by `charset_index`.
490-
pub color_aa: [crate::render::antialiasing::AaStrength; 7],
490+
pub color_aa: [crate::render::antialiasing::AaStrength; crate::render::charset::NUM_CHARSETS],
491491
/// Random seed used for initialization.
492492
pub original_seed: u64,
493493
/// Initialization mode used.
@@ -586,7 +586,8 @@ pub struct RuntimeState {
586586
/// (`cli_overrides: SimConfig` covers sim-layer params; these cover the render layer.)
587587
initial_palette_index: usize,
588588
initial_charset_index: usize,
589-
initial_color_aa: [crate::render::antialiasing::AaStrength; 7],
589+
initial_color_aa:
590+
[crate::render::antialiasing::AaStrength; crate::render::charset::NUM_CHARSETS],
590591
initial_intensity_mapping: IntensityMapping,
591592
initial_window_frame: WindowFrame,
592593
/// Undo history stack.

0 commit comments

Comments
 (0)