Ports the 7-commit v0.8.0 SVG edge-classification feature (f0970b90b +
6 follow-ups) onto wgpu's heavily-diverged serializer. Reconstructed
block-by-block rather than merged, because both sides rewrote
SvgSerializer (v0.8.0 +419, wgpu +778) and git's conflict alignment was
misleading.
Key wgpu adaptations reasoned per block:
- IfcUtil::IfcBaseEntity* (pointer identity) -> express::Base (value),
incl. as a std::map key in draw_hlr (express::Base has operator<);
nullptr fallback -> express::Base{}
- boost::optional -> std::optional (css_class, dash_array)
- hlr_calc::result_type pair -> 3-tuple (adds per-edge class label)
- draw_hlr restructured with a group_by_product map: ONE path_object per
product so multiple class buckets share a group and per-path classes
survive Bonsai's merge (NOT naive per-item threading, which fragments
groups -- caught during visual verification)
- settings wired into wgpu's apply_settings() (ctor-called), NOT the
feature's added ready() call which wgpu already solved differently
- logger_.Warning -> logger().warning in write(geometry_data)
Verified end-to-end via IfcConvert on a curved-geometry stress scene
(Suzanne/Torus/Sphere/Cube/Cone/...): edges classify into outline/sharp/
crease/boundary/flush with correct per-product grouping and CSS styling.
Requires the model's storey to carry an Elevation and --svg-project to
trigger the HLR projection.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The Reassign Class operator refused to reassign an element to a different
IFC product family unless it was an IfcElement <-> IfcElementType swap, so a
piece of geometry mistakenly hosted on IfcSite could not be turned into
IfcFurniture even though root.reassign_class handles it fine.
Loosen the guard: only block the case that actually matters - a spatial
element (IfcSpatialElement / IfcSpatialStructureElement for IFC2X3) with no
geometry, which would be a real containment-hierarchy container rather than
a stray modelled object. Everything else reassigns freely.
Closes#8664
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
(cherry picked from commit b5a0f1fc74)
When converting a wall representation to a parametric extrusion via the
Representation Utilities buttons, an element that has openings would report
"has openings - representation cannot be updated" and stop, without telling
the user there is an ALT+click path that bakes the openings into the new
representation. Point the message at that path so the error is actionable.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
(cherry picked from commit 25441bd816)
* Bonsai: place auto-generated opening boundaries at their real position #8237
auto_generate_boundaries (single-space mode) built each opening/filling boundary
from the opening's LOCAL geometry (get_vertices) but first did
mat.translation = (0, 0, 0) on its placement matrix. Because the vertices are
local, that placement translation is exactly what carries the opening to its
real location, so zeroing it collapsed every window/door boundary onto the
origin. This is why the auto path misplaced window boundaries while the
single-element path (create_element_boundary) placed them correctly, as
@MDHering observed with the two modes. Keep the full placement matrix.
Verified on the reporter's file: the opening's real placement is (0.1, 1.5, 1.0);
a vertex went from (0.6, 0, 0) under the old code to (0.7, 1.5, 1.0) with the fix,
i.e. moved by exactly the (0.1, 1.5, 1.0) that was being discarded.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Remove superfluous comment from #8237 fix
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: CyrilWaechter <cyril@biminsight.ch>
(cherry picked from commit 65811ac7c9)
Surveyor test was failing because `get_z_rotation` and `set_z_rotation` were not implemented.
The code was added in 230cbe1fd8, but it was never used.
(cherry picked from commit 9e25c12b16)
My resolution of #25 (e38993909) over-took v0.8.0's explicit
aggregate_of<...>::ptr type, which the rewrite renamed. Every other
get_related call in this file uses auto (it's a deduced-return template);
wgpu's HEAD already used auto here. Only the SCHEMA_HAS_ macro typo fix
was actually needed. Revert the type to auto.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
versionURLs in brand.html used http:// while the docs sites are
served over https://, so currentURL.includes(url) never matched and
the <select> never reflected/switched to Unstable. Fixes#8023.
Generated with the assistance of an AI coding tool.
(cherry picked from commit 780739719f)
Two TestImplementsTool failures on v0.8.0:
- test_cost.py: Cost could not be instantiated because
core.tool.Cost declared abstract get_direct_cost_item_products, which
tool.cost.Cost never implements. The method is dead (zero call sites;
get_cost_item_products(is_deep=False) already covers the 'direct'
case), so remove the abstract declaration.
- test_ifcgit.py: tool.ifcgit.IfcGit was not declared as a subclass of
its core.tool.IfcGit interface (unlike every sibling tool class), so
the isinstance check failed. Add the base class (and the
bonsai.core.tool import it needs). All 50 interface methods are
already implemented on the concrete class.
No behaviour change. Verified in headless Blender: isinstance(Cost(), core.tool.Cost) and isinstance(IfcGit(), core.tool.IfcGit) both True (were TypeError / False); repo abstract-vs-impl diff confirms all IfcGit abstracts are implemented.
This change was made with the assistance of an AI tool.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
(cherry picked from commit 4a717ca7ff)
black (the version CI's psf/black@stable resolves to) flags three spots
in util/selector.py: the chained .replace() in FormatTransformer.number,
the suppress_zero_inches kwarg in format_length, and the long
`elif key in (...) and hasattr(...)` placement-key tuple in
set_element_value. Reformat all three to black's multi-line style.
Formatting only, no behavioural change (all keys preserved).
This change was made with the assistance of an AI tool.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
(cherry picked from commit 5a831e3d21)
generate_annotation built the annotation list from a set union and sorted it by
ZIndex and TEXT-ness only. Annotations that tied on that key kept set iteration
order, which follows entity hash (step id plus the process memory address), so
the order of tied annotations (for example a label and its background fill)
shuffled between Blender restarts and flipped their draw order.
Add the stable IFC step id as a final tiebreaker so the order is total and
session independent. Behavior preserving, no z-layer semantics changed.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
(cherry picked from commit d30286225c)
coerce_value assumed value_str was always a CLI string, but ifcmcp
passes JSON-decoded native types (int, None) straight through. Guard
the Union/Optional "none" check so it only calls .lower() on strings,
and handle native None explicitly.
(cherry picked from commit 65695fb878)
IfcSpace is not a subtype of IfcElement, so quantify.run_quantify()'s
default selector silently skipped all spaces, reporting
elements_quantified: 0 with no error or warning.
Generated with the assistance of an AI coding tool.
(cherry picked from commit ab15750747)
Importing a Primavera P6 XML crashed with
`AttributeError: 'NoneType' object has no attribute 'text'` in
P62Ifc.parse_activity_xml, which read
activity.find("pr:CalendarObjectId").text unconditionally. CalendarObjectId
is optional on a P6 Activity; when omitted, the activity inherits the
project's ActivityDefaultCalendarObjectId.
Capture the project default in parse_xml and fall back to it when an
activity has no CalendarObjectId (`calendar_id or self.default_calendar_id`).
Verified on the reporter's attached file (20241021 Cronograma.xml): 3 of 14
activities lack a CalendarObjectId and reproduced the exact crash on
v0.8.0; after the fix parse_xml completes and those activities resolve to
the project default calendar "2" (a valid calendar in the file). An
activity with an explicit CalendarObjectId keeps its own value.
Fixes the P6 re-import crash reported in #5617 (that issue tracks several
Gantt items; this addresses the import AttributeError).
Generated with the assistance of an AI coding tool.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
(cherry picked from commit 694a44e638)
get_pset and get_psets assumed RelatingPropertyDefinition is a single property
definition and read definition.Name directly. When it is an
IfcPropertySetDefinitionSet (a defined type wrapping a list of property set
definitions) that attribute access raised AttributeError, so an element whose
psets are grouped in a set returned none of them.
Unpack IfcPropertySetDefinitionSet into its members in both loops and process
each one. Single property definitions and the psets_only and qtos_only filters
are unchanged.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
(cherry picked from commit a3950ac191)
Several BDD scenarios hardcode absolute representation-item object names
whose trailing number is the IFC STEP line id
(f"Item/{item.is_a()}/{item.id()}"). Those ids drift when file-creation
order changes; a recent shift moved all of them by a uniform -4, so the
scenarios failed with "Item/.../NN does not exist".
The failing step (the_object_name_exists in test_feature.py) dumps the
full bpy.data.objects listing on failure, so the correct current ids are
recoverable directly from the CI log (run 29208793599, tested commit
36e21e882f, an ancestor of HEAD with only a .gitignore commit between).
Renumber to match:
IfcExtrudedAreaSolid/77->73, IfcPolygonalFaceSet/76->72,
IfcVertexPoint/69->65, IfcEdge/72->68, IfcFace/74->70.
Verified against the CI failure dump (a local build produces different
ids, so this is validated by CI's own object listing rather than a local
run). boolean.feature also hardcodes IfcHalfSpaceSolid/90 and panel text
[91] downstream of the failing assertion, which CI never reached and so
never dumped; left as-is to avoid guessing - they will print a fresh dump
next run for a follow-up if still stale.
This change was made with the assistance of an AI tool.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
(cherry picked from commit 6f90badda8)
Reported: Blender segfaults when clicking Cancel on the "newer
autosave found" recovery popup shown by LoadProject at startup.
Root cause: LoadProject.execute()/invoke() triggered the recovery
popup via bpy.ops.bim.load_autosaved_recovery_popup("INVOKE_DEFAULT",
...) and returned that call's result ({'RUNNING_MODAL'}) as their own
return value, without LoadProject itself ever calling
modal_handler_add(). Blender's window manager takes a RUNNING_MODAL
return as a promise the operator registered its own modal handler;
since it hadn't, the WM's operator bookkeeping was left corrupted -
silently, since this is heap/state corruption rather than an
immediate crash. It only surfaced later, when the real modal operator
(the popup) closed and the WM reconciled its modal stack, which lines
up with the crash occurring specifically on dialog close regardless
of which button was pressed. check_autosave_recovery() now returns a
plain bool and fires the popup fire-and-forget; LoadProject reports
its own honest {"FINISHED"}.
Also hardened, as defense in depth: LoadAutosavedRecoveryPopup's
execute()/cancel() call back into bim.load_project(...), which (with
should_start_fresh_session) calls wm.read_homefile() and tears down
the window manager/screens. Doing that synchronously from inside this
popup's own execute()/cancel() - itself invoked from deep inside
Blender's modal handling for the popup's button click - risks the
same class of use-after-free as the timer bug fixed in the previous
commit. The reload is now deferred by one timer tick so it runs after
the popup's modal handling has fully unwound, and the deferred
callback closes over plain values rather than `self`, since the
operator instance may not survive past cancel()/execute() returning.
This defer-only change was tried and tested first, on the (incorrect)
assumption it was the root cause: it produced a byte-for-byte
identical crash backtrace on retest, which is what pointed at the
RUNNING_MODAL bug above as the actual cause - the defer change alone
was insufficient because the corruption happens when the popup is
first shown, not when it's closed.
Generated with the assistance of an AI coding tool.
(cherry picked from commit d0eca6fa90)
The periodic autosave timer called reset_timer() at the end of its
own callback, which unregistered the timer that was still executing
(itself). Blender frees the timer's internal registry entry on that
manual unregister, then frees it again when the callback returns
None - a double free that corrupts the heap and can crash Blender
later, once the corrupted memory is reused.
Reschedule by returning the next interval from the callback instead,
which is the safe, documented way to repeat a bpy.app.timers
callback. External reset_timer() calls (from SaveProject,
LoadProject, AutosavePrompt) are unaffected since they run from a
separate call stack (UI events), not from inside the timer.
Found while investigating a segfault reported when cancelling the
autosave recovery popup; not itself the cause of that crash (see the
following commit), but the same reentrant-unregister pattern and a
real, independent latent bug in the periodic reminder path.
Generated with the assistance of an AI coding tool.
(cherry picked from commit 6306ce0f80)
The project-unit to Blender-unit mapping in format_distance only knew
FOOT/INCH/METRE/DECIMETRE/CENTIMETRE/MILLIMETRE, so creating a project
with Kilometers or Miles in the New Project Wizard crashed with
KeyError: 'KILOMETRE' (or 'MILE') as soon as the spatial tree formatted
an elevation. Add the missing Blender-supported units (kilometre, mile,
micrometre) and fall through gracefully for anything else (for example
HECTOMETRE) so unknown units use the adaptive formatting branch instead
of raising.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
(cherry picked from commit 980988f208)
Expose the Euler rotation of an element's placement in degrees through
get_element_value, alongside the existing x/y/z and easting/northing/
elevation keys. This makes element rotation exportable through ifccsv,
e.g. for placing oriented symbols in GIS.
Adopts the approach agreed in the review of the stale PR #6272 by
@TZwielehner: reuse util.shape_builder.np_matrix_to_euler and do the
degree conversion inside get_element_value.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
(cherry picked from commit d4805387ef)
Project loading set scene length_unit to f"{Prefix}METERS", but Blender's
enum only defines KILOMETERS, CENTIMETERS, MILLIMETERS and MICROMETERS.
A model with a DECIMETRE (or HECTO/DECA/etc.) length unit therefore raised
on the enum assignment and the file failed to open. Guard with the set of
supported values and fall back to ADAPTIVE display for the rest.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
(cherry picked from commit 69a4be68e8)
The quickstart ended with three empty sections whose bodies were only
"TODO" (placing occurrences, changing locations, modeling a building),
which read as a dead end on docs.bonsaibim.org. The page now ends on the
completed save-and-view flow.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
(cherry picked from commit 06da416b8f)
The guard that avoids re-assigning the same object to the same resource
tested is_a("IfclRelAssignsToResource") (stray "l"), so it never matched.
A repeat assignment therefore fell through and appended the related object
to RelatedObjects a second time. Corrected to "IfcRelAssignsToResource".
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
(cherry picked from commit 21ae78fbc2)
IfcDiff defaulted to relationships=["geometry"], so a plain diff only ever
compared geometry. Attribute-only edits on an element that kept its GlobalId
(a modified or removed PredefinedType, a renamed element, etc.) were silently
missed. The CLI made this worse: --relationships did not list "attributes" or
"geometry" as valid values, so there was no documented way to enable it.
The default is now ["attributes", "geometry"], so a plain `ifcdiff old new`
reports attribute changes alongside geometry changes. The CLI help and the
IfcDiff docstring now document all valid relationship values.
Added a regression test covering a PredefinedType change detected with the
default configuration.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
(cherry picked from commit 0a8ae14789)
FormatTransformer.round() called Decimal() directly on the input value,
which raises decimal.InvalidOperation when the value is a non-numeric
string (a text property, or a value carrying a unit suffix like "12.5 m").
In a spreadsheet export this crashed the entire operation as soon as one
element carried such a value.
Now round() catches InvalidOperation and returns the value unchanged, the
same graceful-fallback convention used by add(). Numeric rounding is
unaffected.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
(cherry picked from commit 2eea7728d2)
In the cobie24 Coordinate sheet, Floor rows use get_local_placement, whose values
are in the project length unit, but Space rows come from ifcopenshell.geom
create_shape, whose vertices are in SI metres, and the space branch never scaled
them back. So on a non metre model (for example millimetres) the Coordinate sheet
mixed units a thousandfold apart and disagreed with the Facility sheet's declared
LinearUnits.
Scale the space bounding box by the project unit scale so the whole Coordinate
sheet is consistent. A metre model is unchanged since the scale is 1.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
(cherry picked from commit 6b3cc54afc)
The filter_elements selector grammar had no way to comment out part of a
query, so users had to delete and retype text to temporarily toggle a
facet. Add a /* ... */ block comment terminal that is ignored by the
lexer, and tolerate a trailing "+" so that commenting out the final
operand (e.g. "IfcWall + /* IfcSlab */") parses cleanly. Comments may
span multiple lines; a /* sequence inside a quoted string is not treated
as a comment. Only the filter grammar is affected, not get_element or
format which use "/" for regex and division.
Adds a regression test and documents the syntax.
Generated with the assistance of an AI coding tool.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
(cherry picked from commit 5c11946470)
The location and parent filters both match at any depth in the spatial
hierarchy, which surprises users who want only the elements immediately
under a given container. Document that the parent query key resolves the
direct parent only (e.g. query:"parent.Name"="My Site"), add a matching
filter example, and note the immediacy on the parent value key.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
(cherry picked from commit 0b7e25a3ef)
SHIFT+CTRL+CLICK on Activate Drawing now imports the
annotations of all selected drawings without switching
the active view or camera, then selects their cameras with
the first as active. SHIFT+CTRL+ALT+CLICK also selects the
loaded annotation objects. The drawing camera is imported
when missing so annotations land in the correct collection.
Loading is idempotent.
Generated with the assistance of an AI coding tool.
(cherry picked from commit d16c283aef)
Converting to a path whose directory does not exist (or is not writable)
failed silently: the serializer's ready() check correctly returned false,
but IfcConvert deleted the temp file and returned EXIT_FAILURE without any
message, so the user saw no reason for the failure.
Log a SYS error naming the output file before returning, matching the
existing "Unable to open output file" reporting used elsewhere.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
(cherry picked from commit a0f493b471)
Property sets contained in an IfcPropertySetDefinitionSet were exported as
an empty element in XML. The XmlSerializer already had a block to expand
such a set into its member property sets, but it was gated behind
emits SCHEMA_HAS_IfcPropertySetDefinitionSet (singular). The plural spelling
is defined nowhere, so the block was dead code and a RelatingPropertyDefinition
holding a set produced nothing.
Correct the macro name so the set is expanded and its property sets are
serialized. The parse layer already reads these nested sets (they are
reachable from util.element), so this only completes the XML path.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
(cherry picked from commit e389939092)
escape_xml escaped the five XML metacharacters but passed control
characters (0x00 to 0x1F other than tab, newline and carriage return)
through unchanged. Those bytes are illegal in XML 1.0 and cannot be
represented even as numeric character references, so any IFC string
containing them produced non-well-formed XML and SVG output.
Strip those illegal control characters before escaping. Bytes belonging to
a valid UTF-8 multibyte sequence are always >= 0x80, so filtering on the low
control range leaves real text intact. This is the shared helper used by the
SVG serializer text and attribute sites (audited: all route through it) and
by the XML/Collada paths, so both reports are resolved at one place.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
(cherry picked from commit 380675e214)
Fixes clean-but-broken breakage from replayed v0.8.0 commits that
compiled on v0.8.0's API but not wgpu's renamed one (caught by the
checkpoint build, not by any merge conflict):
- face.cpp: logger().Warning -> warning (from #527)
- IfcAsymmetricIShapeProfileDef.cpp (from #1367): map_impl takes a
reference not a pointer (matches wgpu's BIND convention); inst-> -> inst.;
boost get_value_or -> std::optional value_or; logger_.Message/Logger:: ->
message/::logger::
- IfcTriangulatedFaceSet.cpp: inst->PnIndex() -> inst. (my own port slip;
wgpu's triangulated map_impl is also a reference)
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
IfcTriangulatedFaceSet and IfcPolygonalFaceSet used CoordIndex values to
index Coordinates.CoordList directly, ignoring the optional PnIndex
attribute. When PnIndex is present it remaps point references, so a
CoordIndex value i must resolve as CoordList[PnIndex[i-1]-1] (both 1-based).
Without the indirection any model carrying a PnIndex was built from the wrong
points.
Add a resolve() helper in both mappings that applies the PnIndex indirection
when present and is a plain bounds-checked lookup otherwise, with bounds
checks at both index levels. When PnIndex is absent the behavior is unchanged.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
(cherry picked from commit 3e55c5126c)
Address maintainer request on #8368: instead of a deflection floor on top
of a fixed CircleSegments count, use one mode or the other. When
CircleSegments == 0 (the new default) the CGAL kernel derives the conic
segment count from MesherLinearDeflection, matching the deflection based
meshing OpenCascade already does and fixing #8051. When CircleSegments is
non zero it is used directly as a fixed, radius independent count.
CircleSegments is only read by the CGAL kernel; OpenCascade meshes by
deflection and never reads it, so the new default has no effect there.
Update the setting description and the ifcconvert / geometry-settings docs.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
(cherry picked from commit 0d70812641)
The CGAL kernels (cgal and cgal-simple) allocate arc segments as a
fraction of the full circle via CircleSegments, ignoring the radius.
A large-radius arc that spans a small angle therefore collapsed to a
single chord, turning curved curtain-wall mullions straight while the
OpenCascade kernel (which meshes by deflection) kept them curved.
evaluate_conic now also enforces a deflection-based floor on the number
of segments, keeping the chord deviation within mesher-linear-deflection,
matching OpenCascade. Small circles are unchanged (CircleSegments floor
still dominates); only large-radius curves get denser.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
(cherry picked from commit dd9fa65629)
Dion Moult