Merge remote-tracking branch 'origin/main' into fix2708

# Conflicts:
#	crates/io/src/tests.rs

crates/engine/src/autosave.rs

@@ -405,11 +405,10 @@ mod native {
     pub fn prune_locks(dir: &Path) {
         let Ok(entries) = std::fs::read_dir(dir) else { return };
         let names: Vec<String> = entries.flatten().take(10_000).map(|e| e.file_name().to_string_lossy().to_string()).collect();
-        // The instances that own autosave files, parsed once rather than once per lock file.
-        let with_files: std::collections::HashSet<&str> = names.iter().filter_map(|f| parse_file_name(f)).map(|(_, i, _)| i).collect();
         for n in &names {
             let Some(instance) = parse_lock_name(n) else { continue };
-            if !with_files.contains(instance) && !owner_alive(dir, instance) {
+            let has_files = names.iter().any(|f| parse_file_name(f).is_some_and(|(_, i, _)| i == instance));
+            if !has_files && !owner_alive(dir, instance) {
                 let _ = std::fs::remove_file(dir.join(n));
             }
         }
@@ -436,22 +435,7 @@ mod native {
         }
         // Create a fresh temporary file exclusively and rename it into place, so a planted
         // `<path>.tmp` symlink in a shared autosave directory cannot redirect the write (CWE-59).
-        // The file holds unsaved work: owner-only from its creation, whatever the umask.
-        crate::atomic_write::write_atomic_with(Path::new(path), private_mode(), |f| f.write_all(&bytes)).map_err(|e| e.to_string())
-    }
-
-    /// Owner read/write only (`None` where file modes don't exist).
-    fn private_mode() -> Option<std::fs::Permissions> {
-        #[cfg(unix)]
-        return Some(std::os::unix::fs::PermissionsExt::from_mode(0o600));
-        #[cfg(not(unix))]
-        None
-    }
-
-    /// Write this app's lock file at `path` without following a link planted there: the file is
-    /// replaced (exclusive temporary file, then rename), never written through (CWE-59).
-    fn write_lock(path: &Path, id: u32) -> std::io::Result<()> {
-        crate::atomic_write::write_atomic_with(path, private_mode(), |f| f.write_all(id.to_string().as_bytes()))
+        crate::atomic_write::write_atomic(Path::new(path), &bytes).map_err(|e| e.to_string())
     }
 
     impl Autosaver {
@@ -602,7 +586,7 @@ mod native {
             for (held, file) in &self.locks {
                 let refreshed = match file {
                     Some(file) => file.set_modified(SystemTime::now()),
-                    None => write_lock(held, id),
+                    None => std::fs::write(held, id.to_string()),
                 };
                 if held == path {
                     current = Some(refreshed);
@@ -611,22 +595,10 @@ mod native {
             if let Some(refreshed) = current {
                 return refreshed;
             }
-            let mut options = OpenOptions::new();
-            options.write(true).create(true).truncate(false);
-            #[cfg(unix)]
-            {
-                use std::os::unix::fs::OpenOptionsExt;
-                // A symlink planted at the lock path must not be opened (and truncated) through,
-                // nor a FIFO wait for a reader.
-                options.custom_flags(libc::O_NOFOLLOW | libc::O_NONBLOCK).mode(0o600);
-            }
-            let mut file = options.open(path)?;
-            if !file.metadata()?.is_file() {
-                return Err(std::io::Error::new(std::io::ErrorKind::InvalidInput, "the lock file is not a plain file"));
-            }
+            let mut file = OpenOptions::new().write(true).create(true).truncate(false).open(path)?;
             if file.try_lock().is_err() {
                 self.locks.push((path.to_path_buf(), None));
-                return write_lock(path, id);
+                return std::fs::write(path, id.to_string());
             }
             file.set_len(0)?;
             file.write_all(format!("{id}{LOCK_HELD}").as_bytes())?;
@@ -655,30 +627,6 @@ mod native {
             }
         }
     }
-
-    #[cfg(all(test, unix))]
-    mod tests {
-        use super::*;
-
-        /// A symlink planted at the lock path is neither opened through nor written through.
-        #[test]
-        fn lock_writes_do_not_follow_a_planted_link() {
-            let dir = std::env::temp_dir().join(format!("cadcraft-lock-link-{}", std::process::id()));
-            std::fs::create_dir_all(&dir).unwrap();
-            let victim = dir.join("victim.txt");
-            std::fs::write(&victim, b"precious").unwrap();
-            let link = dir.join("1-a.lock");
-            std::os::unix::fs::symlink(&victim, &link).unwrap();
-            let mut a = Autosaver::new("1-a");
-            assert!(a.refresh_lock(&link).is_err());
-            assert_eq!(std::fs::read(&victim).unwrap(), b"precious");
-            write_lock(&link, 42).unwrap();
-            assert_eq!(std::fs::read(&victim).unwrap(), b"precious");
-            assert!(!std::fs::symlink_metadata(&link).unwrap().file_type().is_symlink());
-            assert_eq!(std::fs::read(&link).unwrap(), b"42");
-            std::fs::remove_dir_all(dir).unwrap();
-        }
-    }
 }
 
 #[cfg(test)]

crates/io/src/dxf_read/document.rs

@@ -333,17 +333,7 @@ pub(super) fn apply_sortents(
     paper_for: &HashMap<String, String>,
     first_layout: &str,
 ) {
-    // Each sort rebuilds its whole space, so a space named by many tables is sorted once, by the
-    // last of them (which decided its order anyway); repeated tables made import quadratic.
-    let target = |owner: &String| {
-        let up = brs.get(owner)?.to_ascii_uppercase();
-        Some(if up.starts_with("*PAPER_SPACE") { paper_for.get(&up).cloned().unwrap_or_else(|| first_layout.to_string()) + "\0paper" } else { up })
-    };
-    let last: HashMap<String, usize> = tables.iter().enumerate().filter_map(|(i, (owner, _))| Some((target(owner)?, i))).collect();
-    for (i, (owner, map)) in tables.iter().enumerate() {
-        if target(owner).and_then(|t| last.get(&t)) != Some(&i) {
-            continue;
-        }
+    for (owner, map) in tables {
         let Some(name) = brs.get(owner) else { continue };
         let up = name.to_ascii_uppercase();
         // A sort handle of 0 draws last (ezdxf `reorder`, which notes AutoCAD defines it, undocumented).

crates/io/src/tests.rs

@@ -337,28 +337,6 @@ fn entity_transparency_roundtrips_as_group_440() {
     assert_eq!(d.model.iter().map(|e| e.common.transparency).collect::<Vec<_>>(), [Transparency::ByLayer, Transparency::Percent(90)]);
 }
 
-/// Several SORTENTSTABLEs for one space sort it once, in the last table's order.
-#[test]
-fn repeated_sortents_tables_use_the_last() {
-    let table =
-        |a: u32, b: u32| format!("0\nSORTENTSTABLE\n5\n{:X}\n100\nAcDbSortentsTable\n330\n1F\n331\nA\n5\n{a:X}\n331\nB\n5\n{b:X}\n", 0x100 + a);
-    let mut t = String::from(
-        "0\nSECTION\n2\nTABLES\n0\nTABLE\n2\nBLOCK_RECORD\n0\nBLOCK_RECORD\n5\n1F\n2\n*Model_Space\n0\nENDTAB\n0\nENDSEC\n\
-         0\nSECTION\n2\nENTITIES\n0\nLINE\n5\nA\n330\n1F\n10\n0\n20\n0\n11\n1\n21\n1\n0\nLINE\n5\nB\n330\n1F\n10\n0\n20\n0\n11\n2\n21\n2\n0\nENDSEC\n\
-         0\nSECTION\n2\nOBJECTS\n0\nDICTIONARY\n5\nC\n",
-    );
-    for _ in 0..1000 {
-        t.push_str(&table(2, 1));
-    }
-    t.push_str(&table(1, 2));
-    t.push_str("0\nENDSEC\n0\nEOF\n");
-    let d = read(t.as_bytes(), "s.dxf").unwrap();
-    assert_eq!(d.model.iter().map(|e| e.handle.0).collect::<Vec<_>>(), [0xA, 0xB]);
-    let reversed = t.replacen(&table(1, 2), &table(2, 1), 1);
-    let d = read(reversed.as_bytes(), "s.dxf").unwrap();
-    assert_eq!(d.model.iter().map(|e| e.handle.0).collect::<Vec<_>>(), [0xB, 0xA]);
-}
-
 /// A table declaring 10 000 rows and no columns (or the reverse) is read as an empty table.
 #[test]
 fn table_with_an_empty_dimension_allocates_nothing() {

crates/render/src/blocks.rs

@@ -56,15 +56,13 @@ pub(crate) fn insert(b: &mut Builder, ctx: &Ctx, e: &Entity, ins: &cadcraft_doc:
                 let m = Mat3::translate(off).then_before(ins.transform(blk.base.xy()));
                 let sub = Ctx { block_pstyle: b.named, block_transparency: b.transparency, ..sub_ctx(ctx, e, m) };
                 for be in blk.entities.iter() {
-                    // Every visit is charged, drawn or not, so a block of definitions can't make
-                    // the copies loop free.
-                    if !b.expand() {
-                        break 'copies;
-                    }
                     // Constant/visible attribute definitions inside blocks are not drawn; attribs are.
                     if matches!(be.kind, EntityKind::AttDef(_)) {
                         continue;
                     }
+                    if !b.expand() {
+                        break 'copies;
+                    }
                     entity(b, &sub, be);
                 }
             }
@@ -75,17 +73,6 @@ pub(crate) fn insert(b: &mut Builder, ctx: &Ctx, e: &Entity, ins: &cadcraft_doc:
         let attmode = ctx.d.header.i64("ATTMODE", 1);
         // The reference's transparency; each attribute draws with its own.
         let reference = b.transparency;
-        // The block's first definition per tag (without case), built once for all the attributes.
-        let mut defs: std::collections::HashMap<String, &Entity> = std::collections::HashMap::new();
-        if !ins.attribs.is_empty()
-            && let Some(blk) = ctx.d.block(&ins.block)
-        {
-            for be in blk.entities.iter() {
-                if let EntityKind::AttDef(d) = &be.kind {
-                    defs.entry(d.tag.to_ascii_lowercase()).or_insert(be.as_ref());
-                }
-            }
-        }
         for a in &ins.attribs {
             if attmode == 0 || (a.invisible && attmode != 2) {
                 continue;
@@ -96,7 +83,7 @@ pub(crate) fn insert(b: &mut Builder, ctx: &Ctx, e: &Entity, ins: &cadcraft_doc:
             // Properties of the attribute's definition (as AutoCAD copies them to the attribute
             // on insertion), except those set on the attribute itself: ByBlock = the reference's,
             // layer 0 = the reference's layer.
-            let def = defs.get(&a.tag.to_ascii_lowercase()).copied();
+            let def = ctx.d.block(&ins.block).and_then(|blk| blk.entities.iter().find(|be| attdef_tag(be, &a.tag)));
             let (ink, lw, shown, t) = match (def, a.props.is_empty()) {
                 (None, true) => (rgb, resolve(ctx, e, b.plotting).1, true, reference),
                 (def, _) => {
@@ -130,3 +117,8 @@ fn by_block() -> cadcraft_doc::Common {
         ..cadcraft_doc::Common::default()
     }
 }
+
+/// The attribute definition with tag `tag` (any case).
+fn attdef_tag(e: &Entity, tag: &str) -> bool {
+    matches!(&e.kind, EntityKind::AttDef(d) if d.tag.eq_ignore_ascii_case(tag))
+}

crates/render/src/outline.rs

@@ -106,12 +106,10 @@ fn chain(mut segs: Vec<(Vec2, Vec2)>) -> Vec<Vec<Vec2>> {
         }
     }
     segs.sort_unstable_by(|(a, b), (c, d)| cmp(*a, *c).then_with(|| cmp(*b, *d)));
-    // Segments at each vertex with a cursor past the used ones, so many segments meeting at one
-    // vertex are each examined once rather than from the start on every visit.
-    let mut at: HashMap<Key, (Vec<usize>, usize)> = HashMap::new();
+    let mut at: HashMap<Key, Vec<usize>> = HashMap::new();
     for (i, (a, b)) in segs.iter().enumerate() {
-        at.entry(key(*a)).or_default().0.push(i);
-        at.entry(key(*b)).or_default().0.push(i);
+        at.entry(key(*a)).or_default().push(i);
+        at.entry(key(*b)).or_default().push(i);
     }
     let mut used = vec![false; segs.len()];
     let mut out = Vec::new();
@@ -127,15 +125,7 @@ fn chain(mut segs: Vec<(Vec2, Vec2)>) -> Vec<Vec<Vec2>> {
         let mut end = b;
         // Each step uses one segment, so the walk ends within `segs.len()` steps.
         loop {
-            let next = at.get_mut(&key(end)).and_then(|(ids, cursor)| {
-                while let Some(&i) = ids.get(*cursor) {
-                    if !used.get(i).copied().unwrap_or(true) {
-                        return Some(i);
-                    }
-                    *cursor += 1;
-                }
-                None
-            });
+            let next = at.get(&key(end)).and_then(|ids| ids.iter().copied().find(|&i| !used.get(i).copied().unwrap_or(true)));
             let Some(i) = next else { break };
             let Some(&(p, q)) = segs.get(i) else { break };
             if let Some(u) = used.get_mut(i) {
@@ -157,20 +147,6 @@ mod tests {
         lines.iter().flat_map(|l| l.windows(2).map(|w| w[0].dist(w[1]))).sum()
     }
 
-    /// Thousands of lobes meeting at one vertex chain in one pass over that vertex's segments.
-    #[test]
-    fn many_lobes_at_one_vertex_chain_quickly() {
-        let mut tris = Vec::new();
-        for i in 0..20_000 {
-            let x = f64::from(i + 1);
-            tris.extend_from_slice(&[Vec2::ZERO, Vec2::new(x, 1.0), Vec2::new(x + 0.5, 2.0)]);
-        }
-        // No wall-clock bound (flaky on loaded runners): a quadratic scan would make this test crawl
-        // rather than fail, so check the output is complete instead.
-        let lines = fill_outlines(&tris);
-        assert_eq!(lines.iter().map(|l| l.len() - 1).sum::<usize>(), 60_000, "every triangle edge is emitted once");
-    }
-
     #[test]
     fn a_square_of_two_triangles_is_one_loop() {
         let (a, b, c, d) = (Vec2::new(0.0, 0.0), Vec2::new(2.0, 0.0), Vec2::new(2.0, 1.0), Vec2::new(0.0, 1.0));

crates/render/src/wide.rs

@@ -95,9 +95,7 @@ pub fn dashed(p: &LwPolyline, tol: f64, lt: &Linetype, scale: f64, min_pattern:
                 continue;
             }
             let (Some(first), Some(last)) = (at(run, a), at(run, b)) else { continue };
-            // The samples strictly between a and b: `run` is ordered by `s`, so a slice, not a scan.
-            let (lo, hi) = (run.partition_point(|x| x.s <= a), run.partition_point(|x| x.s < b));
-            let inner = run.get(lo..hi.max(lo)).unwrap_or_default().iter().copied();
+            let inner = run.iter().filter(|x| x.s > a && x.s < b).copied();
             let pts: Vec<Sample> = std::iter::once(first).chain(inner).chain(std::iter::once(last)).collect();
             for w in pts.windows(2) {
                 if let [x, y] = w {