Shape inspection - #228
Conversation
|
@torusJKL Thanks for another PR, I'll try to get to reviewing this soon, it'll take a bit of time as it's a larger PR. |
bschwind
left a comment
There was a problem hiding this comment.
Thanks for being patient! In general the changes are good but there are some improvements we can make to reduce the C++ boilerplate, and remove things like runtime string matching.
| new opencascade::handle<Poly_Triangulation>(BRep_Tool::Triangulation(face, location))); | ||
| } | ||
|
|
||
| inline bool BRep_Tool_IsClosed(const TopoDS_Wire &wire) { return BRep_Tool::IsClosed(wire); } |
There was a problem hiding this comment.
It's slightly clumsier in this case (because you have to cast a Wire to a Shape), but you can bind to static class functions like this:
#[Self = "BRep_Tool"]
pub fn IsClosed(shape: &TopoDS_Shape) -> bool;Just noting that as an option though, this code is fine as-is.
| let edge = Edge::segment(DVec3::new(0.0, 0.0, 0.0), DVec3::new(10.0, 0.0, 0.0)); | ||
| let tangent = edge.tangent_at(0.5, PositionMode::Parameter); | ||
| let expected = DVec3::new(1.0, 0.0, 0.0); | ||
| assert!(tangent.distance_squared(expected) < 1e-6, "expected {expected:?}, got {tangent:?}"); |
There was a problem hiding this comment.
A dot product being approximately equal to 1.0 would probably be a more appropriate check here.
| fn approx_equal(a: f64, b: f64) { | ||
| let diff = (a - b).abs(); | ||
| let rel = diff / b.abs().max(1e-12); | ||
| assert!(rel < 1e-4 || diff < 1e-6, "expected {b}, got {a} (diff={diff}, rel={rel})"); | ||
| } |
There was a problem hiding this comment.
Let's move this into a shared test module that edge_inspection can also use.
| inline void BRepAdaptor_Curve_D1(const BRepAdaptor_Curve &curve, const Standard_Real U, gp_Pnt &P, gp_Vec &V1) { | ||
| curve.D1(U, P, V1); | ||
| } |
There was a problem hiding this comment.
This one can fortunately be auto-bound
| inline void BRepAdaptor_Curve_D1(const BRepAdaptor_Curve &curve, const Standard_Real U, gp_Pnt &P, gp_Vec &V1) { | |
| curve.D1(U, P, V1); | |
| } |
In b_rep_adaptor.rs, you can write:
#[cxx_name = "D1"]
pub fn d1(self: &BRepAdaptor_Curve, u: f64, point: Pin<&mut gp_Pnt>, vec: Pin<&mut gp_Vec>);And then in edge.rs, it can be called like so:
curve.d1(param, point.pin_mut(), vec.pin_mut());In general I try to avoid as much C++ code as possible in the wrappers. For things like wrapping code in unique_ptr, or handles, it's hard to avoid, but for simpler types you can usually get cxx-rs to automatically bind things.
| pub fn BRepAdaptor_Curve_D1( | ||
| curve: &BRepAdaptor_Curve, | ||
| u: f64, | ||
| point: Pin<&mut gp_Pnt>, | ||
| vec: Pin<&mut gp_Vec>, | ||
| ); |
There was a problem hiding this comment.
As noted in another comment, this can become:
#[cxx_name = "D1"]
pub fn d1(self: &BRepAdaptor_Curve, u: f64, point: Pin<&mut gp_Pnt>, vec: Pin<&mut gp_Vec>);| let param = match mode { | ||
| PositionMode::Parameter => position, | ||
| PositionMode::Length => { | ||
| let first = curve.FirstParameter(); | ||
| let last = curve.LastParameter(); | ||
| first + position * (last - first) | ||
| }, | ||
| }; |
There was a problem hiding this comment.
Do we really want PositionMode at all here? The API is confusing, and I don't see any tests demonstrating what it's useful for. It interpolates between the first and last parameter of the curve, but when would you use that?
| inline double HandleGeomSphericalSurface_Radius(const Handle_Geom_SphericalSurface &sphere) { return sphere->Radius(); } | ||
|
|
||
| inline const gp_Ax3 &HandleGeomSphericalSurface_Position(const Handle_Geom_SphericalSurface &sphere) { | ||
| return sphere->Position(); | ||
| } |
| inline double HandleGeomConicalSurface_RefRadius(const Handle_Geom_ConicalSurface &cone) { return cone->RefRadius(); } | ||
|
|
||
| inline double HandleGeomConicalSurface_SemiAngle(const Handle_Geom_ConicalSurface &cone) { return cone->SemiAngle(); } | ||
|
|
||
| inline const gp_Ax3 &HandleGeomConicalSurface_Position(const Handle_Geom_ConicalSurface &cone) { | ||
| return cone->Position(); | ||
| } |
There was a problem hiding this comment.
Same here, let's try to auto-bind these.
| inline double HandleGeomToroidalSurface_MajorRadius(const Handle_Geom_ToroidalSurface &torus) { | ||
| return torus->MajorRadius(); | ||
| } | ||
|
|
||
| inline double HandleGeomToroidalSurface_MinorRadius(const Handle_Geom_ToroidalSurface &torus) { | ||
| return torus->MinorRadius(); | ||
| } | ||
|
|
||
| inline const gp_Ax3 &HandleGeomToroidalSurface_Position(const Handle_Geom_ToroidalSurface &torus) { | ||
| return torus->Position(); | ||
| } |
| pub fn new_HandleGeomCylindricalSurface_from_HandleGeomSurface( | ||
| geom_surface_handle: &Handle_Geom_Surface, | ||
| ) -> UniquePtr<Handle_Geom_CylindricalSurface>; | ||
| pub fn HandleGeomCylindricalSurface_Radius(cyl: &Handle_Geom_CylindricalSurface) -> f64; | ||
| pub fn HandleGeomCylindricalSurface_Position( | ||
| cyl: &Handle_Geom_CylindricalSurface, | ||
| ) -> &gp_Ax3; |
There was a problem hiding this comment.
Sorry the code for Handles is pretty atrocious right now, I need to find a better way to abstract this, maybe a macro or something.
This PR adds inspection/query capabilities across shape types.
I included all the different types in one PR since they are all about inspection of properties but I can split them based on shape type if that is preferable.
The following can be inspected:
Solid
Face / Surface
Edge
Wire
Shape