diff --git a/crates/difftest/src/units/primitive_primitive_creation_p2.rs b/crates/difftest/src/units/primitive_primitive_creation_p2.rs index e8b7e6ac3..7dafa6ae2 100644 --- a/crates/difftest/src/units/primitive_primitive_creation_p2.rs +++ b/crates/difftest/src/units/primitive_primitive_creation_p2.rs @@ -9,24 +9,18 @@ use crate::{Case, HarnessError, Output, Rng, UnitDiff}; use serde::{Deserialize, Serialize}; use serde_json::Value; -use solidcore::curve::composite_curve::{OffsetCornerType, OffsetMapAttribute}; +use solidcore::curve::composite_curve::OffsetCornerType; use solidcore::curve::curve::Curve; use solidcore::error::{Result, SolidError}; -use solidcore::geom::attribute::TagData; use solidcore::geom::axis2_placement::Axis2Placement; -use solidcore::geom::context::MarkType; use solidcore::kernel::Kernel; -use solidcore::math::{Point3, Vec3}; +use solidcore::math::Vec3; use solidcore::nurbs::b_spline_curve::p1::{BSplineCurve, NurbCircleParam}; use solidcore::primitive::primitive_creation::{ - p1::PrimitiveCreation, p2::PrimitiveCreationAccess, p3::PrimitiveAccess, SweepOptions, + model::PrimitiveModel, p1::PrimitiveCreation, SweepOptions, }; -use solidcore::sweep::topology_sweep::{ - access::{EdgeRef, FaceRef, SweepAccess, SweepMergeId}, - SweepEntity, -}; -use solidcore::topology::brep::p10::TrimType; -use solidcore::topology::{Brep, BrepId, FaceId, Region, RegionId}; +use solidcore::sweep::topology_sweep::access::SweepAccess; +use solidcore::topology::{Brep, BrepId, Region, RegionId}; use solidcore_ffi::units::primitive_primitive_creation_p2 as ffi; /// Differential runner. @@ -91,20 +85,19 @@ fn options(f: &[f64]) -> SweepOptions { } } -// The native fixture is a fresh B-rep with only its infinite region. This adapter -// resolves exactly that state. New B-reps need the unported constructor; every other -// unsupported service fails explicitly if a case unexpectedly reaches it, so no -// topology is faked. -struct EmptyModel { - k: Kernel, - notification_records: std::collections::BTreeMap< - solidcore::geom::attribute::ObjectKey, - solidcore::topology::topology::Topology, - >, - notification_attributes: solidcore::geom::attribute::AttributeStore, - // `SmAObject` tag attributes of the swept topology (none on the fresh B-rep). - tags: std::collections::BTreeMap, - b: Brep, +// The native fixture is a fresh B-rep with only its infinite region. The entry points +// run on it through the production adapter (`PrimitiveModel`), which also makes the +// scratch B-reps (`new SmBrep()`) and copies faces between them; the calls it cannot +// resolve yet fail as `Unported` with their unit. +fn fixture_brep(editing: bool) -> Result { + let mut b = Brep::new(); + let region = b.add_region_record(Region { + in_brep: true, + ..Default::default() + })?; + b.set_infinite_region(Some(region)); + b.set_editing_enabled(editing); + Ok(b) } fn unavailable() -> Result { Err(SolidError::invalid_input( @@ -112,206 +105,6 @@ fn unavailable() -> Result { "fixture has no topology/service for this call", )) } -impl EmptyModel { - fn new(editing: bool) -> Result { - let mut b = Brep::new(); - let region = b.add_region_record(Region { - in_brep: true, - ..Default::default() - })?; - b.set_infinite_region(Some(region)); - b.set_editing_enabled(editing); - Ok(Self { - k: Kernel::default(), - notification_records: std::collections::BTreeMap::default(), - notification_attributes: solidcore::geom::attribute::AttributeStore::default(), - tags: std::collections::BTreeMap::default(), - b, - }) - } -} -impl SweepAccess for EmptyModel { - fn brep(&self, _: BrepId) -> Result<&Brep> { - Ok(&self.b) - } - fn brep_mut(&mut self, _: BrepId) -> Result<&mut Brep> { - Ok(&mut self.b) - } - fn kernel(&self, _: BrepId) -> Result<&Kernel> { - Ok(&self.k) - } - fn kernel_mut(&mut self, _: BrepId) -> Result<&mut Kernel> { - Ok(&mut self.k) - } - fn with_brep_mut( - &mut self, - id: BrepId, - body: impl FnOnce( - &mut Kernel, - &mut Brep, - &dyn Fn(FaceId) -> solidcore::geom::object::ObjectRef, - &mut solidcore::topology::edge::NotifyEnv<'_>, - ) -> Result, - ) -> Result { - if id != BrepId::from_index(0) { - return unavailable(); - } - // As primitive.primitive_creation_p3's fixture: this fixture owns the face-key - // namespace, including future allocations. - let face_objects = |face: FaceId| solidcore::geom::object::ObjectRef { - type_code: solidcore::geom::assert_array::labels::FACE_TYPE, - key: solidcore::geom::attribute::ObjectKey(100_000 + u64::from(face.index()) * 97), - }; - let key_of = |entity: solidcore::EntityRef| { - if let solidcore::EntityRef::Face(face) = entity { - face_objects(face) - } else { - super::boolean_topology_intersector_p1::p9_entity_ref(entity) - } - }; - let mut notifications = solidcore::topology::edge::NotifyEnv { - records: &mut self.notification_records, - attributes: &mut self.notification_attributes, - context: None, - key_of: &key_of, - }; - body(&mut self.k, &mut self.b, &face_objects, &mut notifications) - } - fn with_breps_mut( - &mut self, - _source: BrepId, - _destination: BrepId, - _body: impl FnOnce( - &mut Kernel, - &mut Brep, - &mut Brep, - &dyn Fn(solidcore::EntityRef) -> solidcore::geom::object::ObjectRef, - &dyn Fn(FaceId) -> solidcore::geom::object::ObjectRef, - Option<&mut solidcore::geom::context::Context>, - &mut solidcore::topology::edge::NotifyEnv<'_>, - ) -> Result, - ) -> Result { - // This fixture has one B-rep; no distinct pair can be resolved. - unavailable() - } - fn with_brep_notifications( - &mut self, - id: BrepId, - body: impl FnOnce(&mut Brep, &mut solidcore::topology::edge::NotifyEnv<'_>) -> Result, - ) -> Result { - if id != BrepId::from_index(0) { - return unavailable(); - } - // The same face-key namespace as with_brep_mut (primitive.primitive_creation_p3's fixture). - let key_of = |entity: solidcore::EntityRef| { - if let solidcore::EntityRef::Face(face) = entity { - solidcore::geom::object::ObjectRef { - type_code: solidcore::geom::assert_array::labels::FACE_TYPE, - key: solidcore::geom::attribute::ObjectKey( - 100_000 + u64::from(face.index()) * 97, - ), - } - } else { - super::boolean_topology_intersector_p1::p9_entity_ref(entity) - } - }; - let mut env = solidcore::topology::edge::NotifyEnv { - records: &mut self.notification_records, - attributes: &mut self.notification_attributes, - context: Some(&mut self.k.context), - key_of: &key_of, - }; - body(&mut self.b, &mut env) - } - fn retain_hcr(&mut self, _: BrepId, _: solidcore::blend::hcr::Hcr) -> Result { - unavailable() - } - fn hcr_session(&self, _: BrepId) -> Result<&solidcore::blend::hcr::Hcr> { - unavailable() - } - fn new_brep(&mut self, _: BrepId) -> Result { - // UNPORTED(topology.brep_p3): SmBrep::SmBrep - Err(SolidError::unported("SmBrep::SmBrep", "topology.brep_p3")) - } - fn delete_brep(&mut self, _: BrepId) {} - fn new_merge(&mut self, _: BrepId, _: BrepId, _: BrepId, _: f64, _: f64) -> Result { unavailable() } - fn merge_operands(&self, _: SweepMergeId) -> Result<(BrepId, BrepId)> { unavailable() } - fn merge_relations(&mut self, _: SweepMergeId) -> Result<&mut solidcore::boolean::topology_intersector::p1::Relations> { unavailable() } - fn copy_faces(&mut self, _: BrepId, _: &[FaceId], _: BrepId, _: SweepMergeId, _: &mut solidcore::sweep::topology_sweep::access::BTreeEdgeMap) -> Result<()> { unavailable() } - fn piecewise_merge(&mut self, _: SweepMergeId, _: bool, _: bool) -> Result { unavailable() } - fn union(&mut self, _: SweepMergeId) -> Result { unavailable() } - fn delete_merge(&mut self, _: SweepMergeId) {} - fn tag(&self, e: SweepEntity) -> Result> { - Ok(self.tags.get(&e).copied()) - } - fn add_tag(&mut self, e: SweepEntity, tag: TagData) -> Result<()> { - self.tags.insert(e, tag); - Ok(()) - } - fn curve_tag(&self, edge: EdgeRef) -> Result> { - // The fixture's curves carry no tag attributes. - self.brep(edge.0)? - .edge(edge.1)? - .curve - .as_ref() - .ok_or_else(|| SolidError::null_pointer("primitive_p2_fixture"))?; - Ok(None) - } - fn set_editing(&mut self, _: BrepId, value: bool) -> Result { - let previous = self.b.editing_enabled(); - self.b.set_editing_enabled(value); - Ok(previous) - } - fn lock_mark(&mut self, _: BrepId, _: i32) -> Result { - unavailable() - } - fn unlock_mark(&mut self, _: BrepId, _: MarkType) -> Result<()> { - unavailable() - } - fn is_marked(&self, _: SweepEntity, _: MarkType) -> Result { - unavailable() - } - fn mark(&mut self, _: SweepEntity, _: MarkType) -> Result<()> { - unavailable() - } - fn collect_faces(&mut self, _: FaceRef, _: MarkType, _: &mut Vec) -> Result<()> { - unavailable() - } -} -impl PrimitiveAccess for EmptyModel { - fn delete_primitive_faces(&mut self, _: BrepId, _: &[FaceId]) -> Result<()> { - unavailable() - } - fn with_primitive_brep( - &mut self, - _: BrepId, - body: impl FnOnce(&mut Kernel, &mut Brep) -> Result, - ) -> Result { - body(&mut self.k, &mut self.b) - } -} -impl PrimitiveCreationAccess for EmptyModel { - fn copy_faces_into(&mut self, _: BrepId, _: &[FaceId], _: BrepId) -> Result<()> { - unavailable() - } - fn attach_offset_map(&mut self, _: FaceRef, _: OffsetMapAttribute) -> Result<()> { - unavailable() - } - fn offset_map(&self, _: FaceRef) -> Result> { - unavailable() - } - fn project_and_trim( - &mut self, - _: BrepId, - _: &BSplineCurve, - _: Vec3, - _: Point3, - _: TrimType, - _: f64, - ) -> Result<()> { - unavailable() - } -} /// Run one entry point on the fixture and build its record. fn entry(i: &Inputs) -> Result> { @@ -322,7 +115,8 @@ fn entry(i: &Inputs) -> Result> { let mut ends = Vec::new(); let mut extra = Vec::new(); if i.op == 9 { - let mut model = EmptyModel::new(false)?; + let mut k = Kernel::default(); + let mut model = PrimitiveModel::new(&mut k, fixture_brep(false)?); let profile_count = d[4] as usize; let profiles = curves(&d[6..6 + 7 * profile_count])?; let paths = curves(&d[6 + 7 * profile_count..])?; @@ -344,7 +138,7 @@ fn entry(i: &Inputs) -> Result> { return Ok(vec![ result.as_ref().err().map_or(0., error), 0., - f64::from(model.k.context.doing_boolean()), + f64::from(model.kernel(b)?.context.doing_boolean()), 0., 0., 0., @@ -355,7 +149,8 @@ fn entry(i: &Inputs) -> Result> { f64::from(result.is_ok()), ]); } - let mut model = EmptyModel::new(d[0] != 0.)?; + let mut k = Kernel::default(); + let mut model = PrimitiveModel::new(&mut k, fixture_brep(d[0] != 0.)?); let mut primitive = PrimitiveCreation::new(Some(RegionId::from_index(0)), 0, false); let result = match i.op { 1 => primitive.create_partial_cone( @@ -478,8 +273,8 @@ fn entry(i: &Inputs) -> Result> { } let mut record = vec![ result.as_ref().err().map_or(0., error), - f64::from(model.b.editing_enabled()), - f64::from(model.k.context.doing_boolean()), + f64::from(model.brep(b)?.editing_enabled()), + f64::from(model.kernel(b)?.context.doing_boolean()), primitive.maps.to_same.len() as f64, primitive.maps.to_higher.len() as f64, primitive.maps.from_same.len() as f64, diff --git a/fixtures/primitive.primitive_creation_p2/README.md b/fixtures/primitive.primitive_creation_p2/README.md index 1b649ba90..d38c27dc2 100644 --- a/fixtures/primitive.primitive_creation_p2/README.md +++ b/fixtures/primitive.primitive_creation_p2/README.md @@ -30,15 +30,24 @@ Every record compares, on both sides: ## What is deferred (not certified) -Every case that builds topology reaches a dependency that is not ported yet, and the -Rust side defers (no value is compared): +The Rust side runs every entry point through the production adapter +(`primitive::primitive_creation::model::PrimitiveModel`), which makes the scratch and +piece B-reps (`new SmBrep()`), copies faces between them and keys faces as the B-rep +caches resolve them. Cases that build topology go on until a call that is not ported +yet and defer there (no value is compared): | Entry point | First unported call | Blocking unit | | --- | --- | --- | -| skin (two or more profiles), taper extrude, planar path | the scratch / piece B-rep (`SmBrep::SmBrep`) | `topology.brep_p3` | -| partial cone, sphere and torus (valid input) | p3's rotational sweep: planar faces of the profile | `topology.brep_p8` | -| pipe sweep | p3's curve sweep: the profile's placement (`SmBrep::Notify` inside `SmBrep::Transform`) or the faces of the sweep surface | `topology.brep_p2`, `topology.brep_p8` | -| full-turn primitives (valid input) | p1's analytic face | `topology.brep_p8` | +| skin, pipe sweep, planar path, some partial solids and tapers | the shell ray fire of `StitchAndOrient`'s region split reaches a face's trimmed surface cache | `surface.trim_srf_cache` | +| taper extrude | the merge of its pieces (`SmMerge::SmMerge`; the model keeps this deferral, `SweepStore` owns the merge services) | `boolean.merge_p2` | +| partial cone, sphere and torus | `SmFace::FindVertexSector`, a face handle of a surface's face, `SmFace::EdgeCurveOnClassify` | `topology.face_p4`, `topology.face_p2`, `query.face_p1` | +| a few of each | an opaque curve handle's `SmCurve::GetNaturalInterval` | `curve.curve_p2` | + +Before the production adapter, a one-B-rep fixture keyed faces in its own namespace, +which the B-rep caches cannot resolve to a face: the ray fire then treated the faces' +surfaces as untrimmed and 12 pipe, skin and planar-path cases matched C++'s status on +that path. With the production keys they reach the trimmed cache like C++ and defer +(see `plan/findings.md`). The upstream skin (`TestSmCreateSkinPrimitive`), draft sweep (`TestSmCreateDraftSweep`), taper (`TestSmTaperExtrude`) and pipe (`TestSmPipeSweep`) cases run the real C++ and diff --git a/plan/findings.md b/plan/findings.md index eafcf566b..c0fc52dc3 100644 --- a/plan/findings.md +++ b/plan/findings.md @@ -17574,3 +17574,22 @@ a status-only or vacuous pass. `approx_tol_3d` to the general intersector without the `SmApproxTol3d` conversion. Only a negative (or `SM_USE_DEFAULT`) tolerance is affected; a generated negative-tolerance case in those runners would show whether they diverge. +## primitive.primitive_creation_p2: fixture face keys hid the trimmed surface cache (2026-10-23) + +- **What.** The p2 runner's one-B-rep fixture gave new faces object keys from its own + namespace (`FACE_TYPE`, `100000 + 97 * index`). The B-rep caches of the shell ray fire + (`StitchAndOrient` -> region split -> `FindInfiniteRegion` -> `SmShell::RayFire`) cannot + resolve those keys to a face, so the faces' surfaces were solved as untrimmed, which is + the shortcut `SurfaceCache::get_tree` refuses for a face-owned surface. 12 cases + (`pipe_sweep_curved_path` and 11 generated pipe/skin/planar-path cases) matched C++ + only because the record compares status and list sizes, not geometry. +- **How found.** With the same fixture, only the face keys changed to the production + `notify_ref` formula, those 12 cases defer at `TrimSrfCache::build_tree_with_face for a + face-owned surface` (passes 196 -> 184, `surface.trim_srf_cache` deferrals 6 -> 18). +- **Now.** The runner uses the production `PrimitiveModel`: 194 passes (the 10 cases that + waited for `SmBrep::SmBrep` pass, the 12 above defer honestly), 0 failed, 114 deferred. + The p3 fixture (#617) keys faces the same way; #615 moves p3 to the production adapter + and reports the same 295 passes, so no p3 comparison depended on it. +- **Follow-up.** `PrimitiveModel::new_merge` still returns `Unported(SmMerge::SmMerge, + boolean.merge_p2)` although the unit is ported (34 taper cases); `SweepStore` has the + merge services. diff --git a/plan/units/primitive.primitive_creation_p2.toml b/plan/units/primitive.primitive_creation_p2.toml index 860b63086..c1be55e6e 100644 --- a/plan/units/primitive.primitive_creation_p2.toml +++ b/plan/units/primitive.primitive_creation_p2.toml @@ -9,4 +9,4 @@ tests = ["SM_API_test:TestSmCreateSkinPrimitive", "SM_API_test:TestSmPipeSweep", status = "partial" claimed_by = "" claimed_at = "" -notes = "CO-PORTED in sweep.api_brep (PR #110): SmSweepOptions -> solidcore::primitive::primitive_creation::SweepOptions (crates/solidcore/src/primitive/primitive_creation.rs, parent module of p1..p3); 12 fns; largest: SmPrimitiveCreation::CreateTaperExtrude, SmPrimitiveCreation::CreateSweepAlongPlanarPath, sm_CreateSkinPrimitiveInternal, SmPrimitiveCreation::CreateCone; 45 upstream test entry points reach this unit; PR #250: all twelve owned functions ported through PrimitiveCreationAccess (extends p3's PrimitiveAccess; Unported defaults for B-rep services of unported units), with full tails after unavailable calls. Difftest 328 cases: 129 bit-identical passes (entry-point early exits: cone height/turn/side/axis with the editing flag left on, degenerate sphere/torus radii, full turns through p1, skins of fewer than two profiles with their cleared lists and ownership report, curve sweeps of nothing), 179 deferrals (topology.brep_p3 81, topology.brep_p8 78, topology.brep_p1 20), 20 guarded rejections (UB-215, HANG-35, UB-50, UB-247), 0 failures; no absolute floors; perturb 129/129; fuzz 1000 clean. PR #250 review (2026-10-20): the taper's FFI guard replays FixChainCurvesList (both input passes; the reversed pass only where upstream may take it; with caps 2/3, where upstream may reverse, a closed chain whose short curve meets one over 100 times longer at a corner sharper than 3e-5 rad is refused for the offset end cap's double free, UB-50; with caps 1/3 curves that close a loop while an end meets no other end or more than one, UB-247); review_duplicate_curves.json keeps 31 tapers that run natively; ComputeCrvsNormal failures zero the normal as upstream; api_primitives' sphere/cone/torus stubs wired to their next unported call (topology.brep_p3) and the blend retagged to topology.brep_p8. Construction tails (partial sweeps, taper, pipe, planar path, skin faces, blend) are not certified. PR #250 second review (2026-10-21): SmBrep::SetTolerance, IsManifoldSolid and SmMerge::BooleanTrees wired (topology.brep_p2, boolean.merge_p3); merged #233 f810c60a and main ca3bbd2b. New upstream issues UB-215, HANG-35, WR-191, UB-247. Caller must supply a concrete PrimitiveCreationAccess. See fixtures/primitive.primitive_creation_p2/README.md and plan/findings.md. PrimitiveAccess forwards caller notifications into the shared SweepStore constructor/merge services. Wire (2026-10-23, with primitive.primitive_creation_p3): PrimitiveAccess's B-rep face services and axis classification are real defaults; the runner adapter keeps a tag store and untagged curves. Difftest 328 cases: 196 bit-identical passes (was 146), 0 failed, 112 deferred (was 162; topology.brep_p8 71 -> 0, now topology.brep_p3 81, topology.face_p4 14, surface.trim_srf_cache 6, topology.face_p2 5, curve.curve_p2 3, query.face_p1 3), 20 rejected." +notes = "CO-PORTED in sweep.api_brep (PR #110): SmSweepOptions -> solidcore::primitive::primitive_creation::SweepOptions (crates/solidcore/src/primitive/primitive_creation.rs, parent module of p1..p3); 12 fns; largest: SmPrimitiveCreation::CreateTaperExtrude, SmPrimitiveCreation::CreateSweepAlongPlanarPath, sm_CreateSkinPrimitiveInternal, SmPrimitiveCreation::CreateCone; 45 upstream test entry points reach this unit; PR #250: all twelve owned functions ported through PrimitiveCreationAccess (extends p3's PrimitiveAccess; Unported defaults for B-rep services of unported units), with full tails after unavailable calls. Difftest 328 cases: 129 bit-identical passes (entry-point early exits: cone height/turn/side/axis with the editing flag left on, degenerate sphere/torus radii, full turns through p1, skins of fewer than two profiles with their cleared lists and ownership report, curve sweeps of nothing), 179 deferrals (topology.brep_p3 81, topology.brep_p8 78, topology.brep_p1 20), 20 guarded rejections (UB-215, HANG-35, UB-50, UB-247), 0 failures; no absolute floors; perturb 129/129; fuzz 1000 clean. PR #250 review (2026-10-20): the taper's FFI guard replays FixChainCurvesList (both input passes; the reversed pass only where upstream may take it; with caps 2/3, where upstream may reverse, a closed chain whose short curve meets one over 100 times longer at a corner sharper than 3e-5 rad is refused for the offset end cap's double free, UB-50; with caps 1/3 curves that close a loop while an end meets no other end or more than one, UB-247); review_duplicate_curves.json keeps 31 tapers that run natively; ComputeCrvsNormal failures zero the normal as upstream; api_primitives' sphere/cone/torus stubs wired to their next unported call (topology.brep_p3) and the blend retagged to topology.brep_p8. Construction tails (partial sweeps, taper, pipe, planar path, skin faces, blend) are not certified. PR #250 second review (2026-10-21): SmBrep::SetTolerance, IsManifoldSolid and SmMerge::BooleanTrees wired (topology.brep_p2, boolean.merge_p3); merged #233 f810c60a and main ca3bbd2b. New upstream issues UB-215, HANG-35, WR-191, UB-247. Caller must supply a concrete PrimitiveCreationAccess. See fixtures/primitive.primitive_creation_p2/README.md and plan/findings.md. PrimitiveAccess forwards caller notifications into the shared SweepStore constructor/merge services. Wire (2026-10-23, with primitive.primitive_creation_p3): PrimitiveAccess's B-rep face services and axis classification are real defaults; the runner adapter keeps a tag store and untagged curves. Difftest 328 cases: 196 bit-identical passes (was 146), 0 failed, 112 deferred (was 162; topology.brep_p8 71 -> 0, now topology.brep_p3 81, topology.face_p4 14, surface.trim_srf_cache 6, topology.face_p2 5, curve.curve_p2 3, query.face_p1 3), 20 rejected. Runner on the production adapter (2026-10-23): entry points run through primitive::primitive_creation::model::PrimitiveModel (scratch B-reps, face copies, production face keys) instead of a one-B-rep fixture. Difftest 328 cases: 194 bit-identical passes, 0 failed, 114 deferred (surface.trim_srf_cache 51, boolean.merge_p2 34 via PrimitiveModel::new_merge, topology.face_p4 14, curve.curve_p2 7, topology.face_p2 5, query.face_p1 3), 20 rejected; 10 cases that waited for SmBrep::SmBrep now pass, 12 earlier passes relied on fixture face keys the B-rep caches cannot resolve and now defer (plan/findings.md)."