From 454d99b1d3840c6d05c177854bc99dd312d235cc Mon Sep 17 00:00:00 2001 From: huanld Date: Fri, 18 Sep 2026 01:21:22 +0700 Subject: [PATCH 1/3] fix(scene): block text keeps its draw-order slot against sibling wipes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Unselected block text (the BS33-11 Mark/SectionMark labels) was erased by its own block's wipeout pass and only came back through the selection xray. Two compounding causes: - Glyph quads rendered at the bare insert level, an exact depth tie with every sibling fill, so the later wipeout pass won LessEqual. Wire batches now carry the composed child rank as depth_override (the same units the scene graph composes: depths[insert] + label * half), which the block-text upload resolves per instance. Glyph vertices stay neutral on purpose — baking the raw per-block label into them double-counts and, for a negative rank, sinks the text BELOW its own masks. - The wipeout pipeline's depth bias (-8 quanta) was larger than the whole within-block band: once child ranks compress into the insert's half-band, sibling steps are ~3 quanta and a wipeout-to-text gap is only 9-16 quanta of the 24-bit buffer. The mask only needs to break exact ties, so the bias is now WIPEOUT_DEPTH_BIAS_QUANTA = 2. block_text_draw_depth regression pins the contract: neutral vertices, override carrying the rank, and a margin over the wipe that exceeds the bias on a 600-entity scene. --- src/scene/cache/block_cache.rs | 24 ++++-- src/scene/pipeline/mod.rs | 15 +++- tests/block_text_draw_depth.rs | 149 +++++++++++++++++++++++++++++++++ 3 files changed, 182 insertions(+), 6 deletions(-) create mode 100644 tests/block_text_draw_depth.rs 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/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/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" + ); + } +} From 36d5c4db797a1962d5db75ce4003b2ad542cbfec Mon Sep 17 00:00:00 2001 From: huanld Date: Fri, 18 Sep 2026 01:21:30 +0700 Subject: [PATCH 2/3] fix(render): compose PLOTSTYLE wire-fill depth like the wire pipeline MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a display plot style maps an ACI to a fill pattern, wire fill triangles are re-emitted as PLOTSTYLE hatches. Those took depth_override raw — a per-block child label for block wires and 0.0 for top-level ones — while everything else in the hatch/wipeout pass sorts at the composed depths[insert] + label * half. The fill could land outside its host's band and get buried under its own block's masks (same shape as the block-text bug). Both the display and PDF paths now travel through wire_draw_depth / the PlotWire's composed depth, and the display cache key rides the depth generation so a DRAWORDER edit re-ranks the fills without needing a wire rebuild. --- src/io/pdf_export.rs | 8 +++++++- src/scene/mod.rs | 3 ++- src/scene/view/render.rs | 14 +++++++++++++- 3 files changed, 22 insertions(+), 3 deletions(-) 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/mod.rs b/src/scene/mod.rs index d8b91f5f2..c68c7d03a 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/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, + ), }); } } From 3a3f0c83d7a6be1cc66de85fa1ecd19a85da926a Mon Sep 17 00:00:00 2001 From: huanld Date: Fri, 18 Sep 2026 02:10:01 +0700 Subject: [PATCH 3/3] fix(test): make the upstream plot-dialog tests pass on Windows - app_with_fixture embedded CARGO_MANIFEST_DIR straight into an automation JSON string; on Windows the backslashes become invalid JSON escapes and the open failed. Build the payload with json!. - driver_options_are_edited_remembered_and_sent_with_the_job exercises the CUPS options editor, which Windows never opens (PrinterProperties routes to the system dialog there); gate it to non-Windows. --- src/app/update/file.rs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) 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;