Review: recalculate_id_counter() scanned every i|<id>|_ record because
ids were written as decimal text, which doesn't sort numerically. Make
every numeric key segment fixed-width 16-digit lowercase hex, produced
and parsed by key_to_string()/key_from_string() in rocksdb_map_adapter.h,
with named builders (rocksdb_key::attribute, header_attribute,
type_record, inverse, inverse_prefix, type_list, upper_bound) that the
storage, entity_instance_data.cpp, read_schema() and the serializer use
instead of assembling "i|" + std::to_string(id) + ... by hand. Keys now
sort by id, an instance's records are contiguous in id order, and the
largest id is the last key under i|, which recalculate_id_counter()
seeks to. The two dormant to_string_fixed_width() helpers are gone.
This changes the on-disk layout; databases converted before this commit
have to be re-converted.
Also review: instance_cache_ eviction went through an on-erase hook on
set_to_map_transformer, a std::function call under a mutex per deleted
instance. Drop the hook; file::remove_entity() and unbatch() call
file::erase_instances_(ids), which on RocksDB is
rocks_db_file_storage::erase_instances(): one WriteBatch of DeleteRanges
and one lock for the whole batch.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HNrXDmR88wKPCYwGE21SyH
Review: argument_type enumerates the members of type_variant_parameter_pack
in order, so express that once as argument_storage_type_t<A> (pinned by
static_asserts) and let attribute_value::size() on RocksDB go through a
single aggregate_size_<A>() helper instead of spelling each vector type
out in the switch. size() now returns size_t; its only caller already
took size_t.
Also build the RocksDB DeleteRange upper bounds as prefix + ('|' + 1)
rather than a literal '}', which read as the {id} placeholder notation.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HNrXDmR88wKPCYwGE21SyH
file.remove() on a RocksDB-backed file segfaulted. Reducing it turned up
five gaps that each made editing such a file crash or silently do
nothing:
- process_deletion_inverse() decoded the v| inverse-record values as
size_t while the serializer, register_inverse(), unregister_inverse()
and instances_by_reference() use uint32_t, so std::find failed and
vals.erase(end()) was undefined behaviour. It also took the DeleteRange
end from an iterator that is invalid when the instance has no inverse
records. Decode as uint32_t, remove every occurrence guarded on
"found", and derive the range end from the prefix itself.
- attribute_value::size() ignored storage_model_, so every aggregate
assignment on a RocksDB instance threw "Invalid variant index" from
set_attribute_value(). Branch on the storage model like the sibling
accessors and count the deserialized aggregate.
- rocks_db_file_storage::create() was a stub returning an empty handle,
which anything creating an instance then dereferenced. Implement it
after in_memory_file_storage::create().
- max_id_ is only initialised by the in-memory parse, so a RocksDB file
would have handed out ids that overwrite existing instances.
Implement the recalculate_id_counter() stub per backend and run it
once before the first fresh_id() on RocksDB.
- byid_.erase() was a no-op: set_to_map_transformer::erase() and
rocksdb_set_view::erase() were stubs. The deleted instance's attribute
keys and cached handle survived, entity_names() still listed it and
reopening the database threw. Erase deletes every key under the
instance's prefix; the transformer forwards to it and takes an
on-erase hook the storage uses to drop the cached handle.
root.remove_product on the first 200 products of a 61 MB model now
leaves the same surviving ids and inverse counts whether the file was
opened from SPF or converted to RocksDB.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HNrXDmR88wKPCYwGE21SyH
ae2f27c3a replaced the global include_directories(${Boost_INCLUDE_DIRS})
with a Boost::headers target appended to Boost_LIBRARIES. The wasm branch of
IfcParse's link line never included Boost_LIBRARIES, so it lost the include
path and the Pyodide build has failed since with
src/ifcparse/exception.h:25:10: fatal error: 'boost/lexical_cast.hpp' file not found
Nobody noticed because no Pyodide build had run since 2026-08-28.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Fixes the following compilation error when building on MSVC with `USE_MMAP`.
```
parse.cpp(2202,23): error C2338: static_assert failed: 'Default instance_streamer requires a pushed sequential reader'
```
Apparently, when class has `IFC_PARSE_API`, compiler is very eager to instantiate all possible templates leading to this error. We already have explicit instantiation with export for the ones we actually use (e.g. `template class IFC_PARSE_API ifcopenshell::instance_streamer<file_reader<full_buffer_impl>>;`), so it should be no-op.
Route all polyline tools through a single PolylineOperator.cleanup that tears down the decorators, polyline, and raycast cache, and uninstall ProductDecorator unconditionally since it is idempotent.
Generated with the assistance of an AI coding tool.
- use draw handler to move the expensive synchronous GPU readback
out of the mouse-event loop.
- restore GPU state (depth, blend, face culling) around the offscreen
Initial implementation of GPU object detection. It uses an offscreen
buffer with object ID enconded as colors, then reads the pixel(s) under
the cursor to find which object is hit.
It has two modes, one to detect faces and other the detect boundary edges and isolated vertices.
An IfcSurfaceStyle with a NULL Styles set raised a TypeError when
loading a project. Guard the iterations, matching the existing
defensive pattern in import_presentation_styles.
Generated with the assistance of an AI coding tool.
See the comment, if there was couple whitespaces at the end of the file without newline, it would take running `check-whitespace` twice to finally fix it.
The stat()-based path helpers were added in 573e53ebf as a speculative
workaround for #7131 ("Ugly workarounds to not depend on
std::filesystem"). That issue turned out to be a hardcoded schema list
missing HEADER_SECTION_SCHEMA and was fixed separately.
Since the plug-in architecture landed, libIfcParse already depends on
std::filesystem: schema.h exports schema_plugin_directory() returning a
std::filesystem::path, and plugin.cpp uses it throughout. The build also
mandates C++17. The workaround therefore no longer avoids anything and
its comment is misleading.
Restore the std::filesystem version, using the error_code overloads so
inaccessible paths are still reported as "not there" rather than
throwing, and route the path through ifcopenshell::path::from_utf8 so
non-ASCII paths work on Windows, as file_reader.cpp already does.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WcUNL8YygRrNC5KpxELMHs
Also drop-in replacement mostly, except extra cmake args need to be provided now after `--` - `python run-cmake.py vs2022-x64 -- -DGLTF_SUPPORT=ON`.
Internally, script relies on env variables much much less.
Mainly drop-in replacement for `build-ifcopenshell.bat` with `--help`, kw args and args validation, but there's a small caveat.
Previously it was possible to pass args to the underlying build tool as simple positionals - e.g. `build-ifcopenshell vs2022-x64 Release /p:Foo=bar`.
This behaviour is disabled now, because it doesn't allow validating provided args - it's impossible to tell whether `--config Release` is meant to be passed to msbuild or was meant as `--build-cfg Release` for `build-ifcopenshell`.
But it's still possible to pass args to msbuild by using `--` - `python build-ifcopenshell.py vs2022-x64 Release -- /p:Foo=bar`
`install-ifcopenshell.py` is now just a small wrapper passing `--target INSTALL` arg.
* Rewrite to_string() to use switch{} and handle Token_NONE and identifier without as_string()
* Add regression tests for to_string() on tokens without a string form
Cover both halves of the recursion that made a whitespace-only file
segfault: token::to_string() on the EOF marker and on an instance name,
and a parse of input that lexes to zero tokens, which is how the header
parser reaches token::as_string() on the EOF marker.
Generated with the assistance of an AI coding tool.
---------
Co-authored-by: Bruno Postle <bruno@postle.net>
process_deletion_inverse() called inverse_index::remove_source(), which
walked every record in the file's inverse index to find the ones whose
source is the deleted instance: O(R) per deletion, the dominant cost of
file.remove() on large files now that the lookup side no longer re-sorts.
The records a deleted instance contributed are exactly the entity
references in its own attributes, so walk those with the same visitor
build_inverses_() uses for registration and remove each record with a
targeted binary search instead. remove_source() has no callers left and
is deleted.
Also use the ordered view of batch_deletion_ids_ (a boost multi_index
that already had one) for the is-this-referencer-also-being-deleted
check in process_deletion_(), which was a linear std::find over the
sequenced view: O(b) per referencing instance made batch deletion of b
instances quadratic.
file.remove on 300 IfcPropertySet of a 155 MB IFC4 model (201k IfcRoot)
drops from 3.15 ms to 0.17 ms per call, batched removal of 2000 from
3.34 ms to 0.17 ms per call, root.remove_product on 100 walls from
332 ms to 131 ms per call.
Claude-Session: https://claude.ai/code/session_01HNrXDmR88wKPCYwGE21SyH
(cherry picked from commit 938442303f)
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Requiring quotes around a decimal was a surprise with no way to predict it:
Foobar.Baz>1 works, Foobar.Baz>1.5 is a syntax error, and > >= < <= are
almost always used on numbers. Writing Width>"0.2" also reads like a text
comparison even though it is not.
The "." is excluded from unquoted_string because it separates a pset from a
property, but that separator has already been consumed by the time a value is
read, so a number is unambiguous in value position. Add a decimal_string
alternative to the value rule only. Signed and leading dot forms are accepted,
1. is not, and pset, prop and query keys are unchanged - they still need
quoting for a ".".
The transformer returns the same string a quoted value would, so =1.5 and
="1.5" are the same query and compare() does the existing type coercion.
Nothing that parses today changes meaning; only inputs that used to be a
syntax error now work.
Update the quoting docs accordingly, and assert the Bay property that the
filter test set up but never checked.
Generated with the assistance of an AI coding tool.
An unquoted pset, prop, key or value may not contain any of , . = > < * !
or whitespace. This was never stated, and the only motivation the docs gave
for quoting was the "Level 3" example having a space, so there was nothing
to suggest that Pset_WallCommon.ThermalTransmittance=1.5 is a syntax error
while FireRating=2HR is fine. Decimal numbers are the common way to hit it.
State the rule where the reader meets values, in a new subsection under each
of the two value tables. The two grammars exclude different characters, so
the element value section states its own set and only the delta. Note that
all of these characters are a syntax error except the comma, which is read
as the filter separator instead, so Name=Foo,IfcWall silently means "named
Foo and an IfcWall" rather than matching the literal value.
Also fix the Query filter example, which had the same problem in the file
itself: query:types.count=0 does not error, it is silently parsed as a
property filter for a count property in a property set named query:types,
and matches nothing. Quote the keys, as the other query examples already do.
Generated with the assistance of an AI coding tool.
Moving to Python to make Windows build scripts more maintainable.
It's intended to be a drop-in replacement, so it should be possible to just switch `.\build-deps.cmd` to `python build-deps.py`, keeping exactly the same arguments and behaviour will be the same.
`build-deps.cmd` is deprecated, but not yet removed, but will be shortly after more testing.
Other batch files will be migrated to Python shortly after too.
Apparently `ty` is being too strict here and warning about `Any` possibly being `PathLike` which is not supported on older Pythons.
```
error[deprecated]: The overload of `which` is deprecated
--> src\bonsai\bonsai\bim\module\drawing\operator.py:2267:34
|
2267 | command[0] = shutil.which(command[0]) or command[0]
| ^^^^^^^^^^^^ On Windows before Python 3.12, using a PathLike as `cmd` would always fail or return `None`.
error[deprecated]: The overload of `which` is deprecated
--> src\bonsai\bonsai\tool\drawing.py:1324:30
|
1324 | command[0] = shutil.which(command[0]) or command[0]
| ^^^^^^^^^^^^ On Windows before Python 3.12, using a PathLike as `cmd` would always fail or return `None`.
```
Dion Moult