Skip to content

Add BTLx UserReferencePlane support, remove CutPlaneSpec/MiterPlaneSpec - #803

Open
papachap wants to merge 32 commits into
mainfrom
UserRefPlane-against-spec-classes
Open

Add BTLx UserReferencePlane support, remove CutPlaneSpec/MiterPlaneSpec#803
papachap wants to merge 32 commits into
mainfrom
UserRefPlane-against-spec-classes

Conversation

@papachap

Copy link
Copy Markdown
Contributor

This PR adds support for BTLx's UserReferencePlane concept and uses it to replace the ad-hoc plane-override system joints were using before.

  • TimberElements can now register arbitrary named planes via add_user_ref_plane() / get_user_ref_plane(), stored on TimberElement.attributes (that I believe is much underused) relative to the element's ref_frame. These round-trip through BTLx XML, survive transform(), and survive model (de)serialization.
  • ButtJoint/LButtJoint/TButtJoint and LMiterJoint now reference an override plane by a plain int ID (butt_plane_id, back_plane_id, miter_plane_id) instead of building a CutPlaneSpec/MiterPlaneSpec object.
  • CutPlaneSpec and MiterPlaneSpec are removed entirely — they each encoded basically the same thing (a plane relative to a beam) in a slightly different way per joint family. One mechanism now covers all of them.
  • Reworked a bit ButtJoint's pocket logic, to add a Pocket instead of a Lap in case the butt_plane is overridden since the override plane may not be perpendicular to the cross beam's face, which a Lap can't represent.

What type of change is this?

  • Bug fix in a backwards-compatible manner.
  • New feature in a backwards-compatible manner.
  • Breaking change: bug fix or new feature that involve incompatible API changes.
  • Other (e.g. doc update, configuration, etc)

Checklist

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.

  • I added a line to the CHANGELOG.md file in the Unreleased section under the most fitting heading (e.g. Added, Changed, Removed).
  • I ran all tests on my computer and it's all green (i.e. invoke test).
  • I ran lint on my computer and there are no errors (i.e. invoke lint).
  • I added new functions/classes and made them available on a second-level import, e.g. compas_timber.datastructures.Beam.
  • I have added tests that prove my fix is effective or that my feature works.
  • I have added necessary documentation, including updating class_diagrams.rst (if appropriate).

papachap added 22 commits March 20, 2026 15:17
…d avoid circular imports

refactor so that the plane is stored in local coords
…te ButtJoint and LButtJoint to use user_ref_plane IDs
Copilot AI review requested due to automatic review settings July 17, 2026 17:25
@papachap papachap linked an issue Jul 17, 2026 that may be closed by this pull request
@codecov

codecov Bot commented Jul 17, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 92.64706% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.30%. Comparing base (148d51b) to head (559b9ca).

Files with missing lines Patch % Lines
src/compas_timber/base.py 92.98% 4 Missing ⚠️
src/compas_timber/btlx/reader.py 90.47% 2 Missing ⚠️
src/compas_timber/fabrication/btlx.py 91.30% 2 Missing ⚠️
src/compas_timber/connections/butt_joint.py 95.23% 1 Missing ⚠️
src/compas_timber/connections/l_miter.py 87.50% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #803      +/-   ##
==========================================
- Coverage   82.52%   82.30%   -0.22%     
==========================================
  Files          81       81              
  Lines       11861    11910      +49     
==========================================
+ Hits         9788     9803      +15     
- Misses       2073     2107      +34     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@papachap
papachap requested review from chenkasirer and obucklin July 17, 2026 17:28

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR introduces first-class support for BTLx UserReferencePlane objects on TimberElement, and refactors multiple joints/processings to reference override planes by BTLx plane ID (>= 100) instead of the removed ad-hoc CutPlaneSpec / MiterPlaneSpec objects. This aligns the joint plane-override workflow with the BTLx model, improves round-tripping through BTLx XML, and updates tests and changelog accordingly.

Changes:

  • Added TimberElement.user_ref_planes API (add/get/remove) plus UserReferencePlane data class, including BTLx XML write/read support.
  • Refactored ButtJoint/LButtJoint/TButtJoint and LMiterJoint to use *_plane_id overrides instead of *PlaneSpec objects; removed exports of the spec types.
  • Updated tests and changelog to cover new round-trips and the updated pocket/lap behavior when a butt plane is overridden.

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
tests/compas_timber/test_t_butt_joint.py Updates T-butt serialization expectations and adds tests for overridden butt planes with/without mill depth.
tests/compas_timber/test_L_miter_joint.py Migrates LMiter tests from miter_plane_args to miter_plane_id using user_ref_planes.
tests/compas_timber/test_L_butt_joint.py Migrates L-butt override-plane tests to butt_plane_id/back_plane_id and adds new pocket-depth direction assertions.
tests/compas_timber/test_btlx.py Adds BTLx write/read roundtrip coverage for user reference planes.
tests/compas_timber/test_beam.py Adds unit tests for user reference plane lifecycle + transform/model-transform behavior.
src/compas_timber/fabrication/btlx.py Emits UserReferencePlanes XML and updates ReferencePlaneID handling to pass through user plane IDs (>=100).
src/compas_timber/connections/t_butt.py Updates TButtJoint signature/docs to accept butt_plane_id.
src/compas_timber/connections/l_miter.py Removes MiterPlaneSpec and switches LMiter override to miter_plane_id resolution via user_ref_planes.
src/compas_timber/connections/l_butt.py Switches LButt override planes to ID-based resolution (butt_plane_id, back_plane_id).
src/compas_timber/connections/butt_joint.py Removes CutPlaneSpec, adds _resolve_user_ref_plane, and updates pocket/lap selection logic for overridden planes.
src/compas_timber/connections/init.py Removes CutPlaneSpec / MiterPlaneSpec exports.
src/compas_timber/btlx/reader.py Parses UserReferencePlanes from XML and registers them on elements.
src/compas_timber/base.py Introduces UserReferencePlane and TimberElement user-plane registration/lookup/removal APIs.
CHANGELOG.md Documents the new feature, API changes, and removals.

Comment thread src/compas_timber/base.py Outdated
Comment thread src/compas_timber/base.py Outdated
Comment thread src/compas_timber/connections/t_butt.py
Comment thread src/compas_timber/connections/butt_joint.py Outdated
Comment thread src/compas_timber/fabrication/btlx.py
Comment thread src/compas_timber/fabrication/btlx.py Outdated
@papachap
papachap requested a review from Copilot July 17, 2026 17:35

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated 9 comments.

Comment thread src/compas_timber/connections/t_butt.py Outdated
Comment thread src/compas_timber/base.py Outdated
Comment thread src/compas_timber/base.py Outdated
Comment thread src/compas_timber/base.py Outdated
Comment thread src/compas_timber/connections/butt_joint.py
Comment thread src/compas_timber/fabrication/btlx.py
Comment thread src/compas_timber/fabrication/btlx.py Outdated
Comment thread src/compas_timber/base.py Outdated
Comment thread src/compas_timber/base.py Outdated

@chenkasirer chenkasirer left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally fine by me but wrote some more high-level take on the recent movements in this system here Hopefully it'll help us evaluate the alternatives in a more engineer-y fashion.

In the meantime, I would jut push back about using the attributes dict - I think it's too unstructured for this purpose, and should be reserved for user or project specific data, not part of core functionality.

@chenkasirer chenkasirer left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!
left some comments + documented the different alternatives we went through in #804

Comment thread src/compas_timber/base.py Outdated
Comment thread src/compas_timber/base.py Outdated
Comment thread src/compas_timber/base.py Outdated
self.attributes.setdefault("user_ref_planes", []).append(UserReferencePlane(frame=local_frame, ID=ID))
return ID

def add_user_ref_plane(self, frame: Frame, ID: int = None) -> int:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should probably guard here against planes which are completely unrelated to the beam. like a plane whose origin is on the other side of the scene

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm yeah it makes sense.. But I am wondering if add_user_ref_plane is the best spot for this. It's generic storage with no idea what the plane will be used for, so there's no principled distance threshold to check against.

The BTLx processing itself will throw a ValueError if the geometry can't be used, or a joint using that plane would have its own lazy checks against it. Maybe that's enough already?

Happy to hear your ideas on it.

papachap and others added 4 commits July 20, 2026 14:37
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@papachap

Copy link
Copy Markdown
Contributor Author

@obucklin I have implemented some changes based on @chenkasirer comments.. please have another look :)

@papachap
papachap requested a review from anjakunic July 21, 2026 09:03

@obucklin obucklin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @papachap, I'm glad that you finally implemented this in BTLx. However there is a critical issue when applying this to the Joints with custom plane inputs:

In order to use this as a design input, you will have to find a way to deal with joint extensions. in hindsight, the old *_plane_spec didn't take into account the extensions, and fails when extensions are added on the start end of the beam. (see the image below)

Image

This is of course because we use the ref_sides for parameterizing the plane and not the beam.frame or some other "hard-coded" property. I would fix this by using the beam.frame as the initial reference frame, not the ref_side. (Actually I would still just serialize the Plane in the beam local space.) This may be a case where we need to differentiate between the design-side and fabrication-side of model.process_joinery and say that the input is different than the output, in which case the BTLx.UserReferencePlane is only a result of joinery used for fabrication.

…ranfroming it to ref_frame happens only when writing the btlx
@papachap

Copy link
Copy Markdown
Contributor Author

@obucklin Thanks for pointing the bug out! please have another look

@obucklin obucklin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One comment in the code. I added thoughts to #804 @chenkasirer s conversation about this topic. I am glad you fixed the extension bug but I see two issues:

  1. Joint doesn't own a joint attribute, meaning e.g. the LMiterJoint.miter_plane can be changed in the TimberElement.add_user_ref_plane() method. I think the Joints should own those attributes.
  2. you now essentially have 2 user_plane types. one is the UserReferencePlaneCollection on TimberElement, and one is the BTLx user_plane, which is generated via 2 transformations and is a different object. IMO this deviates from the "feature is a BTLxprocessing" philosophy of CT.

Since this is a recurring issue, I suggest we sit together and discuss to come to a consensus on how to solve this.

user_plane = self._resolve_user_ref_plane(self.cross_beam, self.butt_plane_id, "butt_plane")
if self.mill_depth:
main_centerline = point_centerline_towards_joint(self.main_beam, self.cross_beam)
user_plane.translate(-main_centerline.direction * self.mill_depth) # offset the plane along the beam's centerline direction

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm glad you are implementing this! I would think the mill depth would offset normal to the user_plane.

@papachap

Copy link
Copy Markdown
Contributor Author

@obucklin Thanks for your feedback! I would like to reflect to the two points that you raised:

1. Joint owning the plane:

  • You have a point there. If for whatever reason a referenced user_reference_plane gets removed, then a joint that was making use of it would definitely fail.
  • Probably a locking mechanism for user_reference_planes that have been accessed and are in use, would be one way to guard these planes.

2. BTLx alignment issue:

  • This might be a misconception. There aren't two user_reference_planes, the API only ever exposes one, and it's always in world coordinates via .add/.get_user_reference_plane(). The UserReferencePlaneCollection mechanism is what takes care of storing it, by referencing it to the beam's immutable frame rather than its (mutable, blank derived) ref_frame; nothing in the public API ever returns the collection class itself.
  • What you're reading as "a dual frame" is just the BTLx serialization step, which re-expresses that plane relative to the element's ref_frame so it matches the BTLx schema. But that's no different from the duality between a TimberElement and a BTLxPart, which also isn't meant to be touched by a user, it's just what the writer uses internally to serialize.
  • The only genuinely 1-to-1 system we have is BTLxProcessings, and even there it's not perfectly clean, since Python and BTLx aren't always fully compatible (see FreeContour).
    One small note: BTLx UserReferencePlanes aren't on the same level as BTLxProcessings to begin with, they're on the same level as BTLx Transformation/ReferenceSide: Part level reference geometry, not a machining operation. So comparing it to the processing 1:1 pattern is comparing it to the wrong thing in the schema.

@obucklin

Copy link
Copy Markdown
Contributor

@papachap fair points. I still think the joint should own their own properties. For simplicity sake I would do this as a transformation from the element_a.frame to the plane. I think we should sit with @chenkasirer and discuss to decide.

daniel-locatelli added a commit that referenced this pull request Aug 4, 2026
…BTLx standard, remove diagram pipeline from repo

- CutPlaneSpec/MiterPlaneSpec removed from the Connections diagram and prose
  (mechanism contested in #803); LMiterJoint's miter_plane attribute keeps its
  extracted type.
- Fabrication intro links the BTLx standard specification.
- The generation pipeline moves out of the repo to
  https://github.com/daniel-locatelli/compas_timber-class-diagrams; the page
  header no longer points at in-repo scripts.

Co-Authored-By: Claude Fable 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.

add UserReferencePlane functionality in TimberElement

5 participants