diff --git a/src/app/update/file.rs b/src/app/update/file.rs index b2044180c..d4404fad9 100644 --- a/src/app/update/file.rs +++ b/src/app/update/file.rs @@ -5612,7 +5612,12 @@ mod plot_paper_tests { env!("CARGO_MANIFEST_DIR"), "/tests/fixtures/plot/page-setups-metric.dxf" ); - let reply = app.automation_op(&format!(r#"{{"op":"open","path":"{path}"}}"#)); + // json! escapes the path: a Windows CARGO_MANIFEST_DIR carries + // backslashes, and a raw format! would put invalid \X escapes in the + // JSON string. + let reply = app.automation_op( + &serde_json::json!({"op": "open", "path": path}).to_string(), + ); assert_eq!(reply["ok"], true, "{reply}"); app } @@ -5911,6 +5916,10 @@ mod plot_paper_tests { assert!(app.plot_dialog.custom_editor.is_none()); } + // The driver-options editor is the CUPS flow; on Windows + // PrinterProperties routes to the system preferences dialog instead, so + // the editor under test never opens there. + #[cfg(not(target_os = "windows"))] #[test] fn driver_options_are_edited_remembered_and_sent_with_the_job() { use crate::io::print_to_printer::parse_lpoptions; diff --git a/src/io/pdf_export.rs b/src/io/pdf_export.rs index 7d6702bf1..774ec9afa 100644 --- a/src/io/pdf_export.rs +++ b/src/io/pdf_export.rs @@ -605,6 +605,7 @@ fn append_pdf_page( emit_wire_fills( &mut ops, std::slice::from_ref(&wire.wire), + wire.draw_depth, ox, oy, plot_style, @@ -1036,6 +1037,7 @@ fn plotted_color( fn emit_wire_fills( ops: &mut Vec, wires: &[WireModel], + wire_depth: f32, ox: f64, oy: f64, plot_style: Option<&PlotStyleTable>, @@ -1090,7 +1092,11 @@ fn emit_wire_fills( line_weight_px: wire.line_weight_px, angle_offset: 0.0, scale: 1.0 / scale.max(1.0e-6), - draw_depth: wire.depth_override.unwrap_or(0.0), + // The host wire's composed draw depth (PlotWire carries + // wire_draw_depth). depth_override alone is a per-block + // child label and would sort the fill outside its block's + // band; keep the pattern fill co-sorted with its wire. + draw_depth: wire_depth, }; emit_hatch( ops, diff --git a/src/scene/cache/block_cache.rs b/src/scene/cache/block_cache.rs index f1fc4de2c..d327d8a92 100644 --- a/src/scene/cache/block_cache.rs +++ b/src/scene/cache/block_cache.rs @@ -1703,6 +1703,14 @@ impl Batches { ); } } + // Glyph quads stay at neutral depth on purpose: the per-wire + // depth_override (b.local_depth) is composed with the insert's + // scene-graph level at upload time (wire_draw_depth: + // depths[insert] + override * half), the same place the + // wipeout/hatch fills get theirs. Baking the raw child rank + // into the vertices here would double-count it in per-block + // label units — for a negative rank that sinks the text below + // its own block's wipes instead of ahead of them. if !b.pattern_stations.is_empty() { b.pattern_stations = encode_pattern_stations( std::mem::take(&mut b.pattern_stations), @@ -2571,11 +2579,17 @@ fn emit_wire( .map(|marker| transformed_point_marker(marker, accum_xform)); let point_marker = marker_transform.map(|(marker, _)| marker); - // Only band wires take a per-child composed depth: their solid area is - // what covers siblings, and their width already splits them into their own - // batches — thin wires keep the shared whole-insert depth so same-style - // batches stay merged. - let local_depth = (lw.world_width > 0.0) + // Band wires take a per-child composed depth: their solid area is what + // covers siblings, and their width already splits them into their own + // batches. Text-bearing wires need it too — but as `depth_override`, not + // baked into vertices: the upload resolves it against the insert's + // scene-graph entry (depths[insert] + override * half), which is what + // keeps text interleaved with its sibling wipeout/hatch fills. Without a + // composed rank the text sits at the bare insert level, an exact tie with + // every sibling fill, and the later wipeout pass erases it (unselected + // block text vanished under its wipeout; selecting won only because the + // xray pass ignores depth). + let local_depth = (lw.world_width > 0.0 || !lw.text_verts.is_empty()) .then(|| d_range.0 + lw.local_rank * d_range.1); let plot_visible = ctx.plot_visible && lw.plot_visible diff --git a/src/scene/mod.rs b/src/scene/mod.rs index 8aca49e99..31d691c80 100644 --- a/src/scene/mod.rs +++ b/src/scene/mod.rs @@ -2204,7 +2204,8 @@ pub struct Scene { RefCell>>>, styled_wire_cache: RefCell>)>>, styled_hatch_cache: RefCell>>>, - styled_wire_fill_cache: RefCell>>>, + styled_wire_fill_cache: + RefCell>>>, /// Per-viewport projected wire cache for paper-space content viewports. /// Stores projected + clipped wires in paper-space coordinates. /// Maps vp_handle → (geometry_epoch, Vec). diff --git a/src/scene/pipeline/mod.rs b/src/scene/pipeline/mod.rs index 4fad1d098..8989c8538 100644 --- a/src/scene/pipeline/mod.rs +++ b/src/scene/pipeline/mod.rs @@ -53,6 +53,13 @@ use crate::scene::model::image_model::ImageModel; use crate::scene::model::mesh_model::MeshLodSet; use crate::scene::model::wire_model::WireModel; +/// Worst-case raster depth bias (in 24-bit quanta, toward the camera) block +/// text is required to clear. The wipeout pipeline currently carries no bias +/// (a raster bias can jump a block's narrow depth band and erase its +/// foreground), so this is a safety budget: the block-text regression asserts +/// a larger margin over whatever bias the pipeline could reintroduce. +pub const WIPEOUT_DEPTH_BIAS_QUANTA: i32 = 2; + struct SilhouetteChunk { vertex_buffer: wgpu::Buffer, instance_buffer: wgpu::Buffer, @@ -1056,7 +1063,13 @@ impl Pipeline { depth_compare: Some(wgpu::CompareFunction::LessEqual), stencil: content_stencil.clone(), // The shader already applies draw order. A raster bias can - // jump across a block's narrow range and erase its foreground. + // jump across a block's narrow range and erase its foreground + // — block text clears its siblings' wipes by only 9-16 depth + // quanta, so the pipeline must stay bias-free. Coincident ties + // still resolve toward the mask, which draws later under + // LessEqual. WIPEOUT_DEPTH_BIAS_QUANTA is the budget the + // block-text regression asserts against if a bias ever + // returns. bias: wgpu::DepthBiasState::default(), }), multisample: wgpu::MultisampleState { diff --git a/src/scene/view/render.rs b/src/scene/view/render.rs index 5cbeb605e..a1f79be28 100644 --- a/src/scene/view/render.rs +++ b/src/scene/view/render.rs @@ -3102,14 +3102,19 @@ impl Scene { let Some(style) = self.display_plot_style() else { return None; }; + // Depth generation rides in the key: the fills below compose their + // depth from the live depth map, which can move independently of the + // wire set (a DRAWORDER edit re-ranks without retessellating). let key = ( source_gen, style.name.to_ascii_lowercase(), pattern_scale.to_bits(), + self.draw_depth_generation(), ); if let Some(hatches) = self.styled_wire_fill_cache.borrow().get(&key) { return Some(Arc::clone(hatches)); } + let depths = self.draw_depth_map(); let mut hatches = Vec::new(); for wire in wires.iter().filter(|wire| wire.fill_is_2d_solid && wire.aci > 0) { let Some(pattern) = style @@ -3161,7 +3166,14 @@ impl Scene { line_weight_px, angle_offset: 0.0, scale: pattern_scale, - draw_depth: wire.depth_override.unwrap_or(0.0), + // Compose against the scene graph exactly like the wire + // pipeline does. The raw depth_override is a per-block + // child label (or None for top-level wires) — either way + // it would place the fill outside its host's depth band + // and let sibling wipes/masks bury it. + draw_depth: crate::scene::pipeline::wire_gpu::wire_draw_depth( + wire, &depths, + ), }); } } diff --git a/tests/block_text_draw_depth.rs b/tests/block_text_draw_depth.rs new file mode 100644 index 000000000..691aac58c --- /dev/null +++ b/tests/block_text_draw_depth.rs @@ -0,0 +1,149 @@ +// Regression: text INSIDE a block must reach the GPU at neutral vertex depth +// with its composed child rank carried in `WireModel::depth_override`, so the +// upload resolves it against the insert's scene-graph entry +// (depths[insert] + override * half) — the same composition the wipeout/hatch +// fills get. Baking the raw per-block rank into the vertices instead sinks +// text with a negative rank below its own block's wipes (the "G-3" mark +// labels of BS33-11) and catapults text with a positive rank above later +// draw-order geometry. +// +// The scene mirrors the real-world shape of the bug: a block whose children +// are [wipeout, line, text], inserted into a drawing big enough that the +// insert's child band is as fine-grained as a real one. +use acadrust::entities::{Insert, Text, Wipeout}; +use acadrust::tables::BlockRecord; +use acadrust::types::Vector3; +use acadrust::EntityType; +use OpenCADStudio::scene::cache::block_cache::{expand_insert, BlockCache}; +use OpenCADStudio::scene::view::render::InheritStyle; +use OpenCADStudio::scene::Scene; +use OpenCADStudio::scene::WireModel; + +const DRAW_ORDER_BIAS: f32 = 0.001; +const DEPTH_QUANTA: f32 = 16_777_216.0; // 2^24 (Depth24Plus) + +#[test] +fn block_text_depth_composes_through_the_instance_path() { + let mut scene = Scene::new(); + + let br_h = acadrust::Handle::new(scene.document.next_handle()); + let mut br = BlockRecord::new("MARK"); + br.handle = br_h; + scene.document.block_records.add(br).unwrap(); + + // Child 0: the wipeout the text is drawn after (the text must win). + let mut wipe = Wipeout::new(); + wipe.insertion_point = Vector3::new(-10.0, -10.0, 0.0); + wipe.u_vector = Vector3::new(20.0, 0.0, 0.0); + wipe.v_vector = Vector3::new(0.0, 20.0, 0.0); + let mut wipe_e = EntityType::Wipeout(wipe); + wipe_e.common_mut().owner_handle = br_h; + let wipe_handle = scene.document.add_entity(wipe_e).unwrap(); + + // Child 1: a spacer so the text sits two sibling steps above the wipe, + // the same gap as the real BS33-11 mark blocks. + let mut spacer = acadrust::entities::Line::default(); + spacer.start = Vector3::new(-10.0, -10.0, 0.0); + spacer.end = Vector3::new(10.0, 10.0, 0.0); + let mut spacer_e = EntityType::Line(spacer); + spacer_e.common_mut().owner_handle = br_h; + let _ = scene.document.add_entity(spacer_e).unwrap(); + + // Child 2: the text. + let mut txt = Text::new(); + txt.value = "G-3".to_string(); + txt.height = 2.5; + txt.insertion_point = Vector3::new(0.0, 0.0, 0.0); + let mut txt_e = EntityType::Text(txt); + txt_e.common_mut().owner_handle = br_h; + let text_handle = scene.document.add_entity(txt_e).unwrap(); + + // A large model space, so the insert's child band compresses to the + // fine granularity real drawings have (~600 top-level entities puts the + // per-sibling spacing at a handful of 24-bit quanta, where an oversized + // wipeout depth bias would swallow it). + for i in 0..600 { + let mut line = acadrust::entities::Line::default(); + line.start = Vector3::new(f64::from(i) * 10.0 - 3000.0, -50.0, 0.0); + line.end = Vector3::new(f64::from(i) * 10.0 - 3000.0, 50.0, 0.0); + let _ = scene.document.add_entity(EntityType::Line(line)); + } + + let ins = Insert::new("MARK", Vector3::new(0.0, 0.0, 0.0)); + let ins_handle = scene.add_entity(EntityType::Insert(ins)); + + let depths = scene.draw_depth_map(); + let [ins_d, ins_half] = depths + .get(&ins_handle.value()) + .copied() + .expect("insert must be ranked"); + let text_label = depths + .get(&text_handle.value()) + .expect("block children must be ranked")[0]; + let wipe_label = depths.get(&wipe_handle.value()).expect("wipe ranked")[0]; + assert!( + text_label > wipe_label, + "precondition: the drawing orders the text after the wipeout" + ); + + let cache = BlockCache::build( + &scene.document, 1.0, None, true, [0.0, 0.0, 0.0, 1.0], None, &depths, + ); + let wires = expand_insert( + &scene.document, &cache, &Insert::new("MARK", Vector3::new(0.0, 0.0, 0.0)), + ins_handle, + [1.0, 1.0, 1.0, 1.0], 0, 0.0, [0.0; 8], 1.0, + InheritStyle { color: [1.0, 1.0, 1.0, 1.0], pat_len: 0.0, pat: [0.0; 8], lw_px: 1.0 }, + 0, false, false, 1.0, None, None, false, [0.0, 0.0, 0.0, 1.0], 1.0, + OpenCADStudio::scene::BlockScalePolicy::FromInsert, false, + ) + .expect("block defn cached"); + + let text_wires: Vec<&WireModel> = + wires.iter().filter(|w| !w.text_verts.is_empty()).collect(); + assert!( + !text_wires.is_empty(), + "block TEXT must tessellate into glyph quads" + ); + + // The wire is named after its insert (decimal handle), and the upload + // resolves that name against the depth map: this is the composition the + // GPU finally sees. + for w in &text_wires { + assert!( + w.text_verts.iter().all(|v| v.draw_depth == 0.0), + "glyph vertices must stay at neutral depth; the instance path owns it" + ); + assert_eq!( + w.depth_override, + Some(text_label), + "the wire must carry the text's child rank as its depth override" + ); + let resolved = w + .name + .parse::() + .ok() + .and_then(|h| depths.get(&h).copied()); + let instance = match (resolved, w.depth_override) { + (Some([d, half]), Some(local)) => d + local * half, + (Some([d, _]), None) => d, + _ => 0.0, + }; + assert_eq!( + instance, + ins_d + text_label * ins_half, + "instance depth must compose to depths[insert] + label * half" + ); + + // The wipeout mask, composed the same way the scene graph does it. + let mask = ins_d + wipe_label * ins_half; + let margin_quanta = + (instance - mask) * DRAW_ORDER_BIAS * DEPTH_QUANTA; + let bias = OpenCADStudio::scene::pipeline::WIPEOUT_DEPTH_BIAS_QUANTA as f32; + assert!( + margin_quanta > bias + 2.0, + "text drawn after its block's wipeout must clear it by more than \ + the wipeout depth bias plus headroom, got {margin_quanta} quanta" + ); + } +}