Skip to content

At frame - #8

Open
chenkasirer wants to merge 4 commits into
mainfrom
at_frame
Open

At frame#8
chenkasirer wants to merge 4 commits into
mainfrom
at_frame

Conversation

@chenkasirer

Copy link
Copy Markdown
Contributor
  • added outer and inner loop indicators to BrepFace and BrepLoop
  • added frame_at and normal_at to BrepFace for surface evaluation

chenkasirer and others added 4 commits August 19, 2026 13:59
`compas.geometry.Brep` consumers written against compas_rhino use
`loop.is_outer` to tell a face's boundary from its holes; compas_occ has no
equivalent, so such code carries a try/except fallback to `face.loops[0]`.
Neither accessor existed here.

Adds `BrepFace.boundary` and `BrepFace.holes` (the face-level view), and
`BrepLoop.is_outer` / `is_inner` / `loop_type` with the `LoopType` constants
(the loop-level view compas_rhino exposes).

Outer/inner is a face-loop relationship rather than an intrinsic property of a
loop, so the owning face tags its loops in `_mark_loops()`. The backend faces
don't chain to `BrepFace.__init__`, so each constructor calls it; a loop that
is never tagged reports itself as a boundary.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`face.surface` returns the underlying surface as the kernel stores it, which
ignores the face's orientation flag. On a box that means opposite faces report
identical normals - the three pairs are indistinguishable - so any consumer
sorting faces by normal gets a degenerate result. Neither compas_occ nor
compas_rhino ever exposed an orientation-corrected normal, so there was no
accessor to reach for.

Adds `BrepFace.frame_at(u, v)` and `BrepFace.normal_at(u, v)`. Both flip the
yaxis when `is_reversed` is set, so the zaxis is the face normal rather than
the surface normal, and both return fresh objects - `surface` is cached and
shared, so handing out a reference invites callers to corrupt the face.

`surface` itself is deliberately left unflipped: that matches both old
backends, and `__data__` stores `surface` and `is_reversed` as a pair, so
flipping the surface would double-count on a round-trip.

A planar face has no parametrization to honour - `Plane` carries no
x-direction - so there `u`/`v` are distances along the plane's own frame and
omitting them puts the frame on the face centroid.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Both old backends expose `face.nurbssurface`, and consumers use it to reach
`frame_at` on a face whose surface is analytic. compas_brep had no equivalent,
so that call site raised AttributeError.

Adds a `face_to_nurbssurface` pluggable with implementations on both backends:
OCC trims the surface to the face's UV bounds and converts (which already
existed inside `_extract_surface`, now factored out as
`occ_face_to_nurbssurface` and shared), Rhino converts the underlying surface
directly.

Like the old implementations this returns the unflipped surface; `frame_at` is
the accessor to use when a face normal is what is wanted.

A face carrying no native handle can't be converted, so `BrepFace.native_face`
now returns None on the base class and `nurbssurface` raises a BrepError that
says so, rather than failing inside a backend with an AttributeError.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Consumers written against compas_rhino reach for `edge.start_vertex` /
`end_vertex`, where compas_brep followed compas_occ's `first_vertex` /
`last_vertex`. Adds the aliases, so the same code runs on either.

Also fills in the remaining accessors the base `compas.geometry` classes
declare but compas_brep had left out: `BrepFace.is_bspline` and
`BrepFace.type` (a `SurfaceType`), `BrepEdge.type` (a `CurveType`), and
`BrepEdge.centroid`, which is length-weighted and sampled for a NURBS edge.

`BrepEdge.to_line` had two identical branches; dropped the dead one and
documented that a non-linear edge yields its chord, as in compas_occ.

Not added: `edge.orientation`. compas_occ returns a raw TopAbs orientation and
Rhino has no equivalent on an edge - here orientation lives on the trim, since
edges are shared between faces, so a portable `edge.orientation` would have
nothing to mean.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant