diff --git a/src/entities/dimension.rs b/src/entities/dimension.rs index 106197000..b6dd2621c 100644 --- a/src/entities/dimension.rs +++ b/src/entities/dimension.rs @@ -1261,9 +1261,11 @@ fn tessellate_dimension_inner( plinegen: true, vp_scissor: None, fill_tris: geom.arrow_fill, - // FIXME: fill_tris_low left empty — needs double-single split to match - // fill_tris. Any geometry from this path inside a block definition will - // trip the debug_assert_eq! in emit_wire (block_cache.rs:1397). + // fill_tris_low intentionally empty: this fill renders on the top-level + // path, where consumers (face3d_gpu, xclip) treat a short low half as + // all-zero, so it draws at f32 precision (sub-metre error at UTM scale) + // — not a crash. Follow-up: double-single-split via points_to_ds to + // match emit_wire's paired fill path. fill_tris_low: Vec::new(), }); @@ -1296,11 +1298,13 @@ fn tessellate_dimension_inner( aabb: WireModel::UNBOUNDED_AABB, plinegen: true, vp_scissor: None, - fill_tris: rect, - // FIXME: fill_tris_low left empty — needs double-single split to match - // fill_tris. Any geometry from this path inside a block definition will - // trip the debug_assert_eq! in emit_wire (block_cache.rs:1397). - fill_tris_low: Vec::new(), + fill_tris: rect, + // fill_tris_low intentionally empty: this fill renders on the + // top-level path, where consumers (face3d_gpu, xclip) treat a + // short low half as all-zero, so it draws at f32 precision + // (sub-metre error at UTM scale) — not a crash. Follow-up: + // double-single-split via points_to_ds to match emit_wire. + fill_tris_low: Vec::new(), }); } } diff --git a/src/entities/multileader.rs b/src/entities/multileader.rs index 73ee80002..7ab571c02 100644 --- a/src/entities/multileader.rs +++ b/src/entities/multileader.rs @@ -1304,9 +1304,11 @@ impl MultiLeaderTess for MultiLeader { plinegen: true, vp_scissor: None, fill_tris: arrow_fill, - // FIXME: fill_tris_low left empty — needs double-single split to match - // fill_tris. Any geometry from this path inside a block definition will - // trip the debug_assert_eq! in emit_wire (block_cache.rs:1397). + // fill_tris_low intentionally empty: this fill renders on the + // top-level path, where consumers (face3d_gpu, xclip) treat a short + // low half as all-zero, so it draws at f32 precision (sub-metre + // error at UTM scale) — not a crash. Follow-up: double-single-split + // via points_to_ds to match emit_wire's paired fill path. fill_tris_low: Vec::new(), }); @@ -1596,9 +1598,11 @@ impl MultiLeaderTess for MultiLeader { plinegen: true, vp_scissor: None, fill_tris: greek_tris, - // FIXME: fill_tris_low left empty — needs double-single split to match - // fill_tris. Any geometry from this path inside a block definition will - // trip the debug_assert_eq! in emit_wire (block_cache.rs:1397). + // fill_tris_low intentionally empty: this fill renders on + // the top-level path, where consumers treat a short low + // half as all-zero, so it draws at f32 precision (sub- + // metre error at UTM scale) — not a crash. Follow-up: + // double-single-split via points_to_ds to match emit_wire. fill_tris_low: Vec::new(), }); } @@ -1676,9 +1680,11 @@ impl MultiLeaderTess for MultiLeader { plinegen: true, vp_scissor: None, fill_tris: text_fill_tris, - // FIXME: fill_tris_low left empty — needs double-single split to match - // fill_tris. Any geometry from this path inside a block definition will - // trip the debug_assert_eq! in emit_wire (block_cache.rs:1397). + // fill_tris_low intentionally empty: this fill renders on + // the top-level path, where consumers treat a short low + // half as all-zero, so it draws at f32 precision (sub- + // metre error at UTM scale) — not a crash. Follow-up: + // double-single-split via points_to_ds to match emit_wire. fill_tris_low: Vec::new(), }); } diff --git a/src/entities/table.rs b/src/entities/table.rs index b615c1cb8..9fca6e4b8 100644 --- a/src/entities/table.rs +++ b/src/entities/table.rs @@ -755,9 +755,11 @@ pub fn tessellate_table( plinegen: true, vp_scissor: None, fill_tris, - // FIXME: fill_tris_low left empty — needs double-single split to match - // fill_tris. Any geometry from this path inside a block definition will - // trip the debug_assert_eq! in emit_wire (block_cache.rs:1397). + // fill_tris_low intentionally empty: this fill renders on the + // top-level path, where consumers (face3d_gpu, xclip) treat a + // short low half as all-zero, so it draws at f32 precision + // (sub-metre error at UTM scale) — not a crash. Follow-up: + // double-single-split via points_to_ds to match emit_wire. fill_tris_low: Vec::new(), } }; diff --git a/src/scene/cache/block_cache.rs b/src/scene/cache/block_cache.rs index db07a4e1c..9284946ef 100644 --- a/src/scene/cache/block_cache.rs +++ b/src/scene/cache/block_cache.rs @@ -1138,6 +1138,17 @@ fn expand_defn( } } else { if h_px < 1.0 { + // Colour-split MTEXT: only the first outline + // wire from a source entity emits the baseline + // substitute. Siblings share the same + // text_obb_local and would stack identical + // overlapping lines (last colour wins) — the + // same dedup the greek branch does, one tier + // down. + if lw.text_obb_local == last_lod_obb { + continue; + } + last_lod_obb = lw.text_obb_local; emit_text_baseline(lw, accum_xform, ctx, out, wpp); continue; } @@ -1431,7 +1442,6 @@ fn emit_wire( entry.fill_tris.push([hx, hy, hz]); entry.fill_tris_low.push([lx, ly, lz]); } - } fn transform_tangent( diff --git a/src/scene/text/sysfont.rs b/src/scene/text/sysfont.rs index b326752d7..7e24c4d2a 100644 --- a/src/scene/text/sysfont.rs +++ b/src/scene/text/sysfont.rs @@ -6,7 +6,8 @@ // TTF glyph engine). LFF stroke fonts stay separate — this is purely the // TrueType side of the renderer. -use std::sync::OnceLock; +use std::collections::HashMap; +use std::sync::{Mutex, OnceLock}; struct SysFonts { db: fontdb::Database, @@ -38,7 +39,27 @@ pub fn families() -> &'static [String] { } /// Resolve a requested family name to the canonical installed system family name (with exact case). +/// +/// Memoised process-wide: `resolve_font` calls this once per word on the MTEXT +/// measure hot path for inline-`\f` TTF runs, and `Face::resolve` re-runs it +/// immediately after — the underlying fontdb query plus linear family scans are +/// not free. The cache keys on the raw request string; results are stable for +/// the process lifetime (the font DB is loaded once via `OnceLock`). pub fn canonical_family_name(family: &str) -> Option { + static CACHE: OnceLock>>> = OnceLock::new(); + let cache = CACHE.get_or_init(|| Mutex::new(HashMap::new())); + if let Some(hit) = cache.lock().unwrap().get(family) { + return hit.clone(); + } + let resolved = canonical_family_name_uncached(family); + cache + .lock() + .unwrap() + .insert(family.to_string(), resolved.clone()); + resolved +} + +fn canonical_family_name_uncached(family: &str) -> Option { let db = &fonts().db; // 1. Try exact match first diff --git a/tests/text_font_rendering.rs b/tests/text_font_rendering.rs index 968c1e6f6..b0e1aa477 100644 --- a/tests/text_font_rendering.rs +++ b/tests/text_font_rendering.rs @@ -3,20 +3,28 @@ use acadrust::tables::{BlockRecord, TextStyle}; use acadrust::types::Vector3; use acadrust::{CadDocument, EntityType, Handle}; use OpenCADStudio::scene::cache::block_cache::{expand_insert, BlockCache}; +use OpenCADStudio::scene::WireModel; -fn drawable_point_count(wires: &[OpenCADStudio::scene::WireModel]) -> usize { +fn drawable_point_count(wires: &[WireModel]) -> usize { wires .iter() .map(|w| w.points.iter().filter(|p| p[0].is_finite()).count() + w.fill_tris.len()) .sum() } -#[test] -fn block_nested_mtext_uses_its_style_font() { +/// Build a one-block document holding a single MTEXT and expand the INSERT +/// through the block cache, returning the finalized wires. `mtext_at` is the +/// MTEXT's position inside the block; `insert_at` is where the block is placed. +fn expand_block_mtext( + value: &str, + font_file: &str, + mtext_at: Vector3, + insert_at: Vector3, +) -> Vec { let mut doc = CadDocument::new(); let mut style = TextStyle::new("SHOP"); - style.font_file = "arial.ttf".to_string(); + style.font_file = font_file.to_string(); doc.text_styles.add(style).unwrap(); let br_h = Handle::new(doc.next_handle()); @@ -24,7 +32,7 @@ fn block_nested_mtext_uses_its_style_font() { br.handle = br_h; doc.block_records.add(br).unwrap(); - let mut mtext = MText::with_value("FERRAGAMO", Vector3::new(0.0, 0.0, 0.0)); + let mut mtext = MText::with_value(value, mtext_at); mtext.style = "SHOP".to_string(); mtext.height = 20.0; mtext.rectangle_width = 0.0; @@ -32,10 +40,10 @@ fn block_nested_mtext_uses_its_style_font() { sub.common_mut().owner_handle = br_h; doc.add_entity(sub).unwrap(); - let ins = Insert::new("LABEL_BLOCK", Vector3::new(100.0, 50.0, 0.0)); + let ins = Insert::new("LABEL_BLOCK", insert_at); doc.add_entity(EntityType::Insert(ins.clone())).unwrap(); let cache = BlockCache::build(&doc, 1.0, [0.0, 0.0, 0.0, 1.0]); - let wires = expand_insert( + expand_insert( &cache, &ins, Handle::new(999), @@ -50,7 +58,22 @@ fn block_nested_mtext_uses_its_style_font() { false, [0.0, 0.0, 0.0, 1.0], ) - .expect("block defn is cached"); + .expect("block defn is cached") +} + +#[test] +fn block_nested_mtext_uses_its_style_font() { + // NB: on a host without Arial, `arial.ttf` resolves to the LFF stroke + // fallback, so this exercises the "something renders" path rather than the + // TTF canonicalisation specifically. The font-canonicalisation logic itself + // is covered by the unit tests in `text_support`; the colour-split and + // fill-pairing tests below cover the block-cache rendering changes. + let wires = expand_block_mtext( + "FERRAGAMO", + "arial.ttf", + Vector3::new(0.0, 0.0, 0.0), + Vector3::new(100.0, 50.0, 0.0), + ); assert!( drawable_point_count(&wires) > 0, @@ -58,7 +81,84 @@ fn block_nested_mtext_uses_its_style_font() { ); assert!( - wires.iter().all(|w| w.points.is_empty() || w.fill_tris.is_empty()), + wires + .iter() + .all(|w| w.points.is_empty() || w.fill_tris.is_empty()), "outline and fill wires should be separate for correct GPU classification" ); } + +#[test] +fn block_nested_colour_split_mtext_keeps_per_wire_colour() { + // `\C1;` = ACI red, `\C2;` = ACI yellow → two colour bins. The block cache + // must keep them as separate per-wire colours; the fold-to-one-colour bug + // this PR fixes silently collapsed every segment to the first colour. Uses + // the builtin stroke font so the split is observable without a system TTF. + let wires = expand_block_mtext( + "\\C1;AAA\\C2;BBB", + "txt", + Vector3::new(0.0, 0.0, 0.0), + Vector3::new(100.0, 50.0, 0.0), + ); + + let mut colours: Vec<[u8; 3]> = wires + .iter() + .filter(|w| !w.points.is_empty() || !w.fill_tris.is_empty()) + .map(|w| { + [ + (w.color[0] * 255.0).round() as u8, + (w.color[1] * 255.0).round() as u8, + (w.color[2] * 255.0).round() as u8, + ] + }) + .collect(); + colours.sort(); + colours.dedup(); + + assert!( + colours.len() >= 2, + "colour-split MTEXT in a block must keep ≥2 distinct wire colours, got {colours:?}" + ); +} + +#[test] +fn block_nested_mtext_fill_tris_keep_paired_low_half_at_utm() { + // TTF glyph fills are host-dependent (builtin LFF stroke fonts produce + // none). When the host resolves a TTF and fills ARE produced, every fill + // wire must carry an index-paired `fill_tris_low`: emit_wire reconstructs + // `fill_tris[i] + fill_tris_low[i]`, so an unpaired wire panics (release + // too) and a dropped low half quantizes fills to ~0.5 m at UTM scale. + // The MTEXT sits at a UTM-scale coordinate so the low half is significant. + // "DejaVu Sans" is present on most Linux hosts; where no TTF resolves the + // fill set is empty and the assertions are skipped (gate below). + let wires = expand_block_mtext( + "FERRAGAMO", + "DejaVu Sans", + Vector3::new(500_000.0, 4_000_000.0, 0.0), + Vector3::new(0.0, 0.0, 0.0), + ); + + let fill_wires: Vec<&WireModel> = wires.iter().filter(|w| !w.fill_tris.is_empty()).collect(); + if fill_wires.is_empty() { + eprintln!("no TTF fills resolvable on this host; skipping fill-pairing assertions"); + return; + } + + for w in &fill_wires { + assert_eq!( + w.fill_tris.len(), + w.fill_tris_low.len(), + "fill_tris and fill_tris_low must be index-paired for emit_wire" + ); + } + + let any_nonzero_low = fill_wires + .iter() + .flat_map(|w| w.fill_tris_low.iter()) + .any(|p| p.iter().any(|&c| c != 0.0)); + assert!( + any_nonzero_low, + "UTM-scale fills must keep a non-zero low half — a dropped fill_tris_low \ + silently quantizes fill triangles to ~0.5 m" + ); +}