Merge main into codex/fillet-rollover-concrete-solver-ops

Co-Authored-By: Codex <noreply@openai.com>
2026-11-08 13:01:52 +00:00 · 2026-10-26 16:31:48 +01:00
parent c16bca0c3e d9266b4b62
commit bc697973c1
5 changed files with 296 additions and 11 deletions
@@ -424,6 +424,22 @@ pub(super) enum OpIn {
    CloseLoop {
        v: Hin,
    },
+    /// A marker, dropped by the resolver: later `FaceFace` ops (with no input
+    /// vertices) may follow a `VertexFace` or `EdgeFace` op, taking only face handles
+    /// (faces keep their handles through those edits; edges and vertices do not).
+    /// Existing cases never carry it, so their resolution is unchanged.
+    ChainFaces,
+    /// `pBrep->m_bEditingEnabled = enable` on the intersector's B-rep `which` (0 or 1),
+    /// which `IntersectTopology` sets around its stages and the edits need (shim op 34).
+    SetEditing {
+        which: u8,
+        enable: bool,
+    },
+    /// `pPrimaryBrep->MergeBrep(*pOtherBrep, NULL, NULL, FALSE)`, as `SmMerge` does before
+    /// it intersects, so both operands' faces share an owner. Only as the first op (shim
+    /// op 35); the other B-rep stays as it was, and the primary's handles then number the
+    /// primary's entities followed by the copies.
+    MergeBreps,
 }

 #[derive(Debug, Clone, Serialize, Deserialize)]
@@ -1056,6 +1072,7 @@ impl ClassGeometry<H> for Two {
            K_FACE => EntityRef::Face(fid(o)),
            K_LOOP => EntityRef::Loop(LoopId::from_index(o.i)),
            K_EDGEUSE => EntityRef::Edgeuse(EdgeuseId::from_index(o.i)),
+            K_VERTEXUSE => EntityRef::Vertexuse(VertexuseId::from_index(o.i)),
            _ => return None,
        };
        Some((b, e))
@@ -2558,6 +2575,8 @@ fn resolve(
    let mut curves = 0usize;
    let mut has_mgr = false;
    let mut mutated = false;
+    let mut chain = false;
+    let mut soft = false;
    // the intersector's B-reps (the FFI's `have_breps`)
    let mut have_breps = [true, c.breps[1].is_some()];
    let mut closed = false;
@@ -2594,13 +2613,20 @@ fn resolve(
                | OpIn::ShellVerts { .. }
                | OpIn::WireLamina { .. }
                | OpIn::MgrNew { .. }
+                | OpIn::SetEditing { .. }
+                | OpIn::MergeBreps
                | OpIn::CommonEdges
                | OpIn::CommonVertices
                | OpIn::IntersectTopo
                | OpIn::SetRels
                | OpIn::Iir
        );
-        if mutated && takes_handles {
+        if matches!(op, OpIn::ChainFaces) {
+            chain = true;
+            continue;
+        }
+        let face_only = matches!(op, OpIn::FaceFace { verts, .. } if verts.is_empty());
+        if (mutated || (soft && !face_only)) && takes_handles {
            return Err(fail("after_mutation"));
        }
        // part 2: what the FFI refuses
@@ -2869,6 +2895,12 @@ fn resolve(
                }
                op.clone()
            }
+            OpIn::SetEditing { which, .. } => {
+                if *which > 1 || (*which == 1 && c.breps[1].is_none()) {
+                    return Err(fail("brep"));
+                }
+                op.clone()
+            }
            OpIn::MgrNew { .. } => {
                if has_mgr || c.breps[1].is_none() {
                    return Err(fail("manager"));
@@ -2940,6 +2972,14 @@ fn resolve(
            OpIn::CloseLoop { v } => OpIn::CloseLoop {
                v: hk(*v, &[K_VERTEX], true)?,
            },
+            // handled above (never reaches here)
+            OpIn::ChainFaces => continue,
+            OpIn::MergeBreps => {
+                if idx != 0 || c.breps[1].is_none() {
+                    return Err(fail("merge"));
+                }
+                op.clone()
+            }
        };
        if matches!(
            r,
@@ -2958,7 +2998,11 @@ fn resolve(
                | OpIn::EdgeFaceCoin { .. }
                | OpIn::CloseLoop { .. }
        ) {
-            mutated = true;
+            if chain && !mutated && matches!(r, OpIn::VertexFace { .. } | OpIn::EdgeFace { .. }) {
+                soft = true;
+            } else {
+                mutated = true;
+            }
        }
        out.push(ROp { op: r });
    }
@@ -3247,6 +3291,9 @@ fn encode(i: &Inputs, ops: &[ROp]) -> Result<Vec<f64>, HarnessError> {
                capture,
                override_tol,
            } => v.extend([33.0, *capture, *override_tol]),
+            OpIn::SetEditing { which, enable } => {
+                v.extend([34.0, f64::from(*which), flag(*enable)]);
+            }
            OpIn::RemoveRel { a, b } => {
                v.push(41.0);
                push_h(&mut v, *a);
@@ -3307,6 +3354,9 @@ fn encode(i: &Inputs, ops: &[ROp]) -> Result<Vec<f64>, HarnessError> {
                v.push(52.0);
                push_h(&mut v, *bv);
            }
+            // dropped by the resolver
+            OpIn::ChainFaces => {}
+            OpIn::MergeBreps => v.push(35.0),
        }
    }
    Ok(v)
@@ -3342,6 +3392,20 @@ fn put_cut(o: &mut Vec<f64>, cut: Option<&(usize, String)>) {
    }
 }

+/// `target->MergeBrep(other, NULL, NULL, FALSE)`: `topology::brep::p3::merge_brep`.
+fn merge_primary(target: &mut Brep, other: &mut Brep) -> Result<(), HarnessError> {
+    use solidcore::topology::brep::p3 as bp3;
+    let mut k = Kernel::default();
+    let serial = bp3::notify_serial(&mut k.context, other)
+        .map_err(|e| harness(format!("merge: {e}")))?;
+    let source_key = move |e| bp3::topology_ref(serial, e);
+    let other = &*other;
+    bp3::with_kernel_callbacks(&mut k, target, |k, b, env| {
+        bp3::merge_brep(k, env, b, other, &source_key, None, None, false)
+    })
+    .map_err(|e| harness(format!("merge: {e}")))
+}
+
 /// Exports and imports the B-reps and resolves the script; `Err(Ok(output))` is the
 /// output both sides report when the B-reps cannot be built or an op cannot be
 /// resolved.
@@ -3355,6 +3419,13 @@ fn prepare(k: &Kernel, i: &Inputs) -> Result<Prepared, Box<Result<Output, Harnes
        Some(b) => Some(import(b).map_err(|e| Box::new(Err(e)))?.0),
        None => None,
    };
+    let mut brep_a = brep_a;
+    let mut brep_b = brep_b;
+    if matches!(i.ops.first(), Some(OpIn::MergeBreps)) {
+        if let Some(other) = brep_b.as_mut() {
+            merge_primary(&mut brep_a, other).map_err(|e| Box::new(Err(e)))?;
+        }
+    }
    let count3 = |b: &Brep| [b.vertices().len(), b.edges().len(), b.faces().len()];
    let counts = Counts {
        breps: [Some(count3(&brep_a)), brep_b.as_ref().map(count3)],
@@ -3926,6 +3997,15 @@ fn run_rust_script(i: &Inputs, p: Prepared) -> Result<(Vec<f64>, Vec<usize>), Fa
                o.push(r.max_gap);
                o.push(r.min_unstitched);
            }
+            OpIn::MergeBreps => {
+                // (merged in `prepare`, which the resolution needs the counts of)
+                o.push(35.0);
+                o.push(0.0);
+            }
+            OpIn::SetEditing { which, enable } => {
+                o.push(34.0);
+                two.set_editing_enabled(brep_h(i8::try_from(*which).unwrap_or(0)), *enable)?;
+            }
            OpIn::MgrNew {
                capture,
                override_tol,
@@ -5536,6 +5616,130 @@ fn edge_cases() -> Vec<(String, Value)> {
            },
        );
    }
+    // Crossing faces: the intersection edge is inserted inside each face
+    // (`SmBrep::MakeEdgeInFace`), appended after the cases above.
+    let plane = |o: [f64; 3], n: [f64; 3], dom: [f64; 4]| BrepIn::Face {
+        surface: SrfIn::Plane { o, n },
+        dom,
+    };
+    let cross = |name: &str,
+                 a: BrepIn,
+                 b: BrepIn,
+                 approx: f64,
+                 tail: Vec<OpIn>,
+                 add: &mut dyn FnMut(&str, Inputs)| {
+        let mut ops = vec![OpIn::FaceFace {
+            verts: vec![],
+            faces1: vec![f(0, 0)],
+            faces2: vec![f(1, 0)],
+            ignore: false,
+            nesting: false,
+        }];
+        ops.extend(tail);
+        add(
+            name,
+            Inputs {
+                brep_a: a,
+                brep_b: Some(b),
+                ctor: 0,
+                approx,
+                ang: 0.1,
+                ops,
+            },
+        );
+    };
+    let xy = [-1.0, -1.0, 1.0, 1.0];
+    for (name, na, nb, da, db) in [
+        (
+            "cross_planes_perpendicular",
+            [0.0, 0.0, 1.0],
+            [1.0, 0.0, 0.0],
+            xy,
+            xy,
+        ),
+        (
+            "cross_planes_tilted",
+            [0.0, 0.0, 1.0],
+            [1.0, 0.0, 1.0],
+            xy,
+            xy,
+        ),
+        (
+            "cross_planes_partial_overlap",
+            [0.0, 0.0, 1.0],
+            [0.0, 1.0, 0.0],
+            [-1.0, -1.0, 1.0, 1.0],
+            [-3.0, -1.0, 0.5, 1.0],
+        ),
+        (
+            "cross_planes_one_wide",
+            [0.0, 0.0, 1.0],
+            [1.0, 1.0, 0.0],
+            [-3.0, -3.0, 3.0, 3.0],
+            [-0.5, -1.0, 0.5, 1.0],
+        ),
+    ] {
+        for (suffix, tail) in [
+            ("", vec![]),
+            ("_resolve", vec![OpIn::Resolve, OpIn::Imprint]),
+        ] {
+            cross(
+                &format!("{name}{suffix}"),
+                plane([0.0, 0.0, 0.0], na, da),
+                plane([0.0, 0.0, 0.0], nb, db),
+                1e-5,
+                tail,
+                &mut add,
+            );
+        }
+    }
+    // Faces of one B-rep, as `SmMerge` intersects them: the other operand is merged into
+    // the primary B-rep first, so `GlueEdgesGeneral` sees edges of one owner. The edge
+    // intersections come first (they give the interval ends their vertices).
+    for (fa, na) in [("plane_square.smb", 4i64), ("disk.smb", 2)] {
+        for (fb, nb) in [("sphere.smb", 1i64), ("cone.smb", 2), ("torus.smb", 2)] {
+            add(
+                &format!("merged_{fa}_{fb}"),
+                Inputs {
+                    brep_a: file(fa),
+                    brep_b: Some(file(fb)),
+                    ctor: 0,
+                    approx: 1e-5,
+                    ang: 0.1,
+                    ops: vec![
+                        OpIn::MergeBreps,
+                        OpIn::SetEditing {
+                            which: 0,
+                            enable: true,
+                        },
+                        OpIn::ChainFaces,
+                        OpIn::EdgeFace {
+                            edges: (0..na + nb).map(|k| e(0, k)).collect(),
+                            faces: vec![f(0, 0), f(0, 1)],
+                            manifold: false,
+                            ignore: false,
+                        },
+                        OpIn::FaceFace {
+                            verts: vec![],
+                            faces1: vec![f(0, 0)],
+                            faces2: vec![f(0, 1)],
+                            ignore: false,
+                            nesting: false,
+                        },
+                    ],
+                },
+            );
+        }
+    }
+    // A plane through a B-spline surface and a file face.
+    cross(
+        "cross_plane_file_sphere",
+        plane([0.0, 0.0, 0.0], [0.0, 0.0, 1.0], [-3.0, -3.0, 3.0, 3.0]),
+        file("sphere.smb"),
+        1e-5,
+        vec![],
+        &mut add,
+    );
    out
 }

@@ -73,7 +73,8 @@
 //! `.smb` files, well-formed clamped B-splines, handles inside the B-reps' exports (it
 //! builds the B-reps once more to count), the registered curves that exist, edges with a
 //! curve and vertices and faces with a surface where upstream dereferences them, no
-//! handle-taking op after an op that may delete topology (23 to 28, 30), ops 7 and 8
+//! handle-taking op after an op that may delete topology (23 to 28, 30; only a `FaceFace`
+//! without input vertices may follow a `VertexFace` or an `EdgeFace`), ops 7 and 8
 //! only while the intersector's primary B-rep is set and 27 to 30 only while both are
 //! (either constructor sets both from the blocks, then `SetBreps` may change them; they
 //! dereference them unchecked); ops 29 to 33 need the
@@ -157,6 +158,8 @@ pub enum ScriptOp {
    ShellVerts = 31,
    WireLamina = 32,
    MgrNew = 33,
+    SetEditing = 34,
+    MergeBreps = 35,
 }

 impl ScriptOp {
@@ -173,10 +176,10 @@ impl ScriptOp {
            BrepMate, CanImprint, CheckInterior, CheckSsi, ClearSsi, CommonEdges, CommonVertices,
            EdgeFace, FaceFace, FaceFaceCoin, FixupEnd, FixupMid, Getters, Imprint, ImprintFace,
            IsMated, Lowest, MappedBrep, MgrNew, OtherMate, RegisterSsi, Relate, Resolve, SetBreps,
-            SetEdgeClass, SetSubset, SetTols, ShellVerts, SplitTwo, ThisMate, VertexFace,
+            MergeBreps, SetEdgeClass, SetEditing, SetSubset, SetTols, ShellVerts, SplitTwo, ThisMate, VertexFace,
            WireLamina,
        };
-        const ALL: [ScriptOp; 32] = [
+        const ALL: [ScriptOp; 34] = [
            Relate,
            OtherMate,
            ThisMate,
@@ -209,6 +212,8 @@ impl ScriptOp {
            ShellVerts,
            WireLamina,
            MgrNew,
+            SetEditing,
+            MergeBreps,
        ];
        ALL.iter().copied().find(|o| o.code() == code)
    }
@@ -716,6 +721,9 @@ fn script(
    rd.finite()?; // ang
    let n_ops = rd.index(MAX_OPS)?;
    let mut mutated = false;
+    // after a `VertexFace` / `EdgeFace` only: face handles stay valid (neither deletes
+    // a face), so a `FaceFace` without input vertices may still follow
+    let mut soft = false;
    let mut has_mgr = false;
    // the intersector's B-reps: both from the blocks with either constructor (1 sets
    // them with SetPrimaryBrep / SetOtherBrep), then whatever SetBreps sets
@@ -729,13 +737,13 @@ fn script(
        HandleKind::Curve,
        Brep,
    ];
-    for _ in 0..n_ops {
-        let code = rd.int(1, extra.map_or(33, |x| x.max_code().max(33)))?;
+    for op_index in 0..n_ops {
+        let code = rd.int(1, extra.map_or(35, |x| x.max_code().max(35)))?;
        let Some(op) = ScriptOp::from_code(code) else {
            let extra = extra.ok_or_else(|| bad("bad script op"))?;
            let mut st = ScriptState {
                ctor,
-                mutated,
+                mutated: mutated || soft,
                has_mgr,
                have_breps,
                closed: false,
@@ -746,6 +754,7 @@ fn script(
                return Err(bad("bad script op"));
            }
            mutated = st.mutated;
+            soft = false;
            has_mgr = st.has_mgr;
            have_breps = st.have_breps;
            if st.closed {
@@ -765,10 +774,12 @@ fn script(
                | ScriptOp::ShellVerts
                | ScriptOp::WireLamina
                | ScriptOp::MgrNew
+                | ScriptOp::SetEditing
+                | ScriptOp::MergeBreps
                | ScriptOp::CommonEdges
                | ScriptOp::CommonVertices
        );
-        if mutated && takes_handles {
+        if (mutated || (soft && !matches!(op, ScriptOp::FaceFace))) && takes_handles {
            return Err(bad("handle-taking op after a mutating op"));
        }
        // the ops that dereference the intersector's B-reps without checking
@@ -876,7 +887,13 @@ fn script(
                rd.flag()?;
            }
            ScriptOp::FaceFace => {
-                c.list(rd, &[Vertex], false)?;
+                if soft {
+                    if rd.index(MAX_LIST)? != 0 {
+                        return Err(bad("vertex handles after a mutating op"));
+                    }
+                } else {
+                    c.list(rd, &[Vertex], false)?;
+                }
                c.list(rd, &[Face], false)?;
                c.list(rd, &[Face], false)?;
                rd.flag()?;
@@ -908,6 +925,23 @@ fn script(
                rd.flag()?;
                rd.flag()?;
            }
+            ScriptOp::MergeBreps => {
+                // the first op only; the primary B-rep then holds both operands
+                let (Some(a), Some(b)) = (c.breps[0], c.breps[1]) else {
+                    return Err(bad("no second B-rep"));
+                };
+                if op_index != 0 {
+                    return Err(bad("merge is not the first op"));
+                }
+                c.breps[0] = Some([a[0] + b[0], a[1] + b[1], a[2] + b[2]]);
+            }
+            ScriptOp::SetEditing => {
+                let which = rd.index(2)?;
+                if which == 1 && c.breps[1].is_none() {
+                    return Err(bad("no second B-rep"));
+                }
+                rd.flag()?;
+            }
            ScriptOp::MgrNew => {
                if has_mgr || c.breps[1].is_none() {
                    return Err(bad("manager already made or no second B-rep"));
@@ -918,7 +952,11 @@ fn script(
            }
        }
        if op.mutates() {
-            mutated = true;
+            if !mutated && matches!(op, ScriptOp::VertexFace | ScriptOp::EdgeFace) {
+                soft = true;
+            } else {
+                mutated = true;
+            }
        }
    }
    Ok(())
@@ -917,6 +917,8 @@ enum TiScriptOp : int64_t
    S_SHELL_VERTS = 31,
    S_WIRE_LAMINA = 32,
    S_MGR_NEW = 33,
+    S_SET_EDITING = 34,
+    S_MERGE_BREPS = 35,
 };

 // Handle kinds (`kind` of a `brep kind index` triple).
@@ -1661,6 +1663,35 @@ void run_script(Rd& rd, Two& t, std::vector<double>& o)
            o.push_back(mn);
            break;
        }
+        case S_MERGE_BREPS:
+        {
+            // the first op only: copies the other B-rep into the primary one, as SmMerge
+            // does before it intersects, so both operands' faces share an owner
+            if (t.brep[1] == nullptr)
+            {
+                o.push_back(ST_NULL_DEREF);
+                break;
+            }
+            SmStatus s = t.brep[0]->MergeBrep(*t.brep[1], NULL, NULL, FALSE);
+            o.push_back(st(s));
+            t.g[0] = Graph();
+            walk(t.brep[0], t.g[0]);
+            t.index_geometry();
+            break;
+        }
+        case S_SET_EDITING:
+        {
+            // pBrep->m_bEditingEnabled = flag (IntersectTopology sets it around the stages)
+            int64_t which = rd.idx();
+            SmBoolean enable = rd.bflag();
+            if (which < 0 || which > 1 || t.brep[which] == nullptr)
+            {
+                o.push_back(ST_NULL_DEREF);
+                break;
+            }
+            t.brep[which]->m_bEditingEnabled = enable;
+            break;
+        }
        case S_MGR_NEW:
        {
            double capture = rd.num();
@@ -0,0 +1,3 @@
+Updates: plan/findings/boolean.topology_intersector_p1--cross-brep-glue-edges.md
+
+Whether C++ is defined for the cross-B-rep `GlueEdgesGeneral` was not established (no ASan run), so no FFI guard was added for it. The committed fixtures take the route `SmMerge` takes: a `MergeBreps` runner op (shim op 35, `SmBrep::MergeBrep`) puts both operands into the primary B-rep, then `SetEditing`, `EdgeFace`, `FaceFace` on faces of that one B-rep. `merged_*` cases (plane_square or disk x sphere, cone, torus) reach `MakeEdgeInFace` in both faces and agree with C++ at 1e-9.
@@ -0,0 +1,9 @@
+# FaceFaceIntersect with faces of two B-reps: GlueEdgesGeneral across B-reps
+
+Unit: boolean.topology_intersector_p1 (PR #1165 runner work).
+
+Inputs: plane_square.smb x sphere.smb (and torus.smb), `SetEditing` on both B-reps, one `EdgeFace` over all edges and both faces, then `FaceFace` with faces f(0,i), f(1,j) (8 cases, temporary names `cand_*`, not committed).
+
+Both sides reach `MakeEdgeInFace` for the interval (Vertex/Vertex ends) in both faces. Upstream then calls `m_pBrep->GlueEdgesGeneral(pNewEdge1, ..., pNewEdge2)` with edges of two different B-reps. C++ continues (state length 36); the Rust runner returns "not an edge of this B-rep" because `brep_glue_edges_general` takes one arena (state length 25).
+
+Suspected cause: upstream only calls this with both faces in one (merged) B-rep; the two-B-rep call is outside the contract and `topology::brep::p7::glue_edges_general` cannot model it. A fixture needs both faces in one B-rep, or the FFI should reject the cross-B-rep case as undefined.