mirror of
https://github.com/IfcOpenShell/IfcOpenShell.git
synced 2026-08-06 07:51:47 +00:00
Fix wall edit lifecycle + drain wall_offset_gizmos cache on load
Bundled bug fixes + the forward-compat AST guard that prevents the
underlying class of bug from coming back.
* bim/module/model/wall.py: FinishEditingWall._execute early-returns
CANCELLED when props.is_editing is False. Without this guard, a
failed enable (e.g. on a wall without IfcMaterialLayerSetUsage)
leaves is_editing False but a press on finish still walked the
sub-ops below, which dereferenced layer-set-dependent state and
crashed.
* tool/model.py: Model.offset_wall now guards against
ifcopenshell.util.element.get_material returning None before
calling .is_a("IfcMaterialLayerSetUsage"). Fixes the pre-existing
test/bim/module/model/test_wall_header_refresh.py crash that has
been the only failing test in the wall lane since this branch
started.
* bim/handler.py: _apply_save_file_invariants drains
wall_offset_gizmos.clear_caches() on load_post. The module-scope
GenerationKeyedCache instance survives the .blend reload; without
the drain the cache may serve entries whose bpy_struct references
point into the freed bpy.data of the previous file.
* test/bim/test_handler_forward_compat.py: AST-walk test that
enumerates every bim/module/model/*.py source declaring both a
module-scope GenerationKeyedCache assignment AND a top-level
clear_caches function, and asserts each module appears as a
<module>.clear_caches() call in _apply_save_file_invariants. Pins
the contract: any future module-scope geom cache that exposes
clear_caches must wire into the load_post drain.
* test/bim/feature/model.feature + test/bim/test_feature.py: wall
edit-lifecycle scenarios switch from "add cube + assign as
IfcWallType" to "load the demo construction library + add an
occurrence of the WAL100 wall type", so the parametric edit runs
against a real LAYER2 wall with IfcMaterialLayerSetUsage rather
than a vanilla-mesh promotion that lacks one. The demo-library
step also picks the schema-matching library file (IFC2X3 /
IFC4 / IFC4X3) so the appended types remain valid across schemas.
Door saved-height assertion updates from 2.5 → 2500 to reflect
that BBIM_Door pset stores project units (METRIC_MM in the
empty-project fixture).
Generated with the assistance of an AI coding tool.
This commit is contained in:
@@ -41,6 +41,7 @@ from bonsai.bim.decorator_cache import (
|
||||
from bonsai.bim.ifc import IfcStore, get_cache_or_detect_lock
|
||||
from bonsai.bim.module.aggregate.decorator import AggregateDecorator
|
||||
from bonsai.bim.module.georeference.decorator import GeoreferenceDecorator
|
||||
from bonsai.bim.module.model import wall_offset_gizmos
|
||||
from bonsai.bim.module.model.data import AuthoringData
|
||||
from bonsai.bim.module.model.decorator import (
|
||||
ArrayPreviewDecorator,
|
||||
@@ -447,6 +448,7 @@ def _apply_save_file_invariants(scene: bpy.types.Scene) -> None:
|
||||
|
||||
tool.Parametric.heal_stale_edit_flags()
|
||||
discard_pending_previews(scene)
|
||||
wall_offset_gizmos.clear_caches()
|
||||
|
||||
if tool.Ifc.get() and bpy.data.is_saved:
|
||||
props = tool.Blender.get_bim_props()
|
||||
|
||||
@@ -1821,6 +1821,12 @@ class FinishEditingWall(bpy.types.Operator, tool.Ifc.Operator):
|
||||
if not element:
|
||||
return {"CANCELLED"}
|
||||
props = tool.Model.get_wall_props(obj)
|
||||
# No edit session in progress — finish is a true no-op. Without this guard,
|
||||
# an enable that failed validation (e.g. wall without IfcMaterialLayerSetUsage)
|
||||
# leaves is_editing=False but a press on finish still walks the sub-ops below,
|
||||
# which dereference layer-set-dependent state and crash.
|
||||
if not props.is_editing:
|
||||
return {"CANCELLED"}
|
||||
|
||||
length_changed = not tool.Cad.is_x(props.length, props.snap_length, tolerance=1e-5)
|
||||
height_changed = not tool.Cad.is_x(props.height, props.snap_height, tolerance=1e-5)
|
||||
|
||||
@@ -2943,7 +2943,7 @@ class Model(bonsai.core.tool.Model):
|
||||
def offset_wall(cls, wall: bpy.types.Object, baseline: Literal["EXTERIOR", "INTERIOR", "CENTER"]) -> None:
|
||||
element = tool.Ifc.get_entity(wall)
|
||||
usage = ifcopenshell.util.element.get_material(element)
|
||||
if not usage.is_a("IfcMaterialLayerSetUsage"):
|
||||
if usage is None or not usage.is_a("IfcMaterialLayerSetUsage"):
|
||||
return
|
||||
layer_set = usage.ForLayerSet
|
||||
if baseline == "CENTER":
|
||||
|
||||
@@ -687,8 +687,10 @@ Scenario: Saving with a door mid-edit auto-commits the draft value to the IFC ps
|
||||
Then "active_object.BIMDoorProperties.is_editing" is "True"
|
||||
When I press "bim.save_project(filepath='{temp_project_path}', should_save_as=True)"
|
||||
Then "active_object.BIMDoorProperties.is_editing" is "False"
|
||||
# BBIM_<Type> psets store project units, not raw Blender SI. The empty project
|
||||
# used in an_empty_blender_session is METRIC_MM, so 2.5 m → 2500 mm in the pset.
|
||||
And the variable "saved_height" is "__import__('json').loads(ifcopenshell.util.element.get_pset({ifc}.by_type('IfcDoor')[0], 'BBIM_Door', 'Data'))['overall_height']"
|
||||
And the variable "saved_height" equals "2.5"
|
||||
And the variable "saved_height" equals "2500.0"
|
||||
|
||||
Scenario: Saving with no parametric edits in progress leaves the door pset unchanged
|
||||
Given an empty IFC project
|
||||
@@ -705,14 +707,10 @@ Scenario: Saving with no parametric edits in progress leaves the door pset uncha
|
||||
|
||||
Scenario: Saving with a wall mid-edit auto-commits the draft to IFC
|
||||
Given an empty IFC project
|
||||
And I add a cube
|
||||
And the object "Cube" is selected
|
||||
And I set "scene.BIMRootProperties.ifc_product" to "IfcElementType"
|
||||
And I set "scene.BIMRootProperties.ifc_class" to "IfcWallType"
|
||||
And I press "bim.assign_class"
|
||||
And I load the demo construction library
|
||||
And I set "scene.BIMModelProperties.ifc_class" to "IfcWallType"
|
||||
And the variable "cube" is "{ifc}.by_type('IfcWallType')[0].id()"
|
||||
And I set "scene.BIMModelProperties.relating_type_id" to "{cube}"
|
||||
And the variable "element_type" is "[e for e in {ifc}.by_type('IfcWallType') if e.Name == 'WAL100'][0].id()"
|
||||
And I set "scene.BIMModelProperties.relating_type_id" to "{element_type}"
|
||||
And I press "bim.add_occurrence"
|
||||
And the object "IfcWall/Wall" is selected
|
||||
And I press "bim.enable_editing_wall()"
|
||||
@@ -722,21 +720,18 @@ Scenario: Saving with a wall mid-edit auto-commits the draft to IFC
|
||||
|
||||
Scenario: Enabling and finishing a wall edit with no drag is a no-op
|
||||
Given an empty IFC project
|
||||
And I add a cube
|
||||
And the object "Cube" is selected
|
||||
And I set "scene.BIMRootProperties.ifc_product" to "IfcElementType"
|
||||
And I set "scene.BIMRootProperties.ifc_class" to "IfcWallType"
|
||||
And I press "bim.assign_class"
|
||||
And I load the demo construction library
|
||||
And I set "scene.BIMModelProperties.ifc_class" to "IfcWallType"
|
||||
And the variable "cube" is "{ifc}.by_type('IfcWallType')[0].id()"
|
||||
And I set "scene.BIMModelProperties.relating_type_id" to "{cube}"
|
||||
And the variable "element_type" is "[e for e in {ifc}.by_type('IfcWallType') if e.Name == 'WAL100'][0].id()"
|
||||
And I set "scene.BIMModelProperties.relating_type_id" to "{element_type}"
|
||||
And I press "bim.add_occurrence"
|
||||
And the object "IfcWall/Wall" is selected
|
||||
And the variable "entity_count_before" is "len(list({ifc}))"
|
||||
When I press "bim.enable_editing_wall()"
|
||||
And I press "bim.finish_editing_wall()"
|
||||
Then "active_object.BIMWallProperties.is_editing" is "False"
|
||||
And "len(list({ifc}))" is "{entity_count_before}"
|
||||
And the variable "entity_count_after" is "len(list({ifc}))"
|
||||
And the variable "entity_count_after" equals "{entity_count_before}"
|
||||
|
||||
Scenario: Cancelling a wall edit clears is_editing
|
||||
Given an empty IFC project
|
||||
@@ -756,14 +751,10 @@ Scenario: Cancelling a wall edit clears is_editing
|
||||
|
||||
Scenario: Wall parametric edit works on IFC2X3 projects
|
||||
Given an empty IFC2X3 project
|
||||
And I add a cube
|
||||
And the object "Cube" is selected
|
||||
And I set "scene.BIMRootProperties.ifc_product" to "IfcElementType"
|
||||
And I set "scene.BIMRootProperties.ifc_class" to "IfcWallType"
|
||||
And I press "bim.assign_class"
|
||||
And I load the demo construction library
|
||||
And I set "scene.BIMModelProperties.ifc_class" to "IfcWallType"
|
||||
And the variable "cube" is "{ifc}.by_type('IfcWallType')[0].id()"
|
||||
And I set "scene.BIMModelProperties.relating_type_id" to "{cube}"
|
||||
And the variable "element_type" is "[e for e in {ifc}.by_type('IfcWallType') if e.Name == 'WAL100'][0].id()"
|
||||
And I set "scene.BIMModelProperties.relating_type_id" to "{element_type}"
|
||||
And I press "bim.add_occurrence"
|
||||
And the object "IfcWall/Wall" is selected
|
||||
When I press "bim.enable_editing_wall()"
|
||||
|
||||
@@ -1764,7 +1764,17 @@ def i_load_the_ifc_test_file(filepath):
|
||||
@given("I load the demo construction library")
|
||||
@when("I load the demo construction library")
|
||||
def i_add_a_construction_library():
|
||||
lib_path = "./bonsai/bim/data/libraries/IFC4 Demo Library.ifc"
|
||||
# Pick the library file whose schema matches the current project so the
|
||||
# appended types are valid (IFC2X3-vs-IFC4 entity attributes differ).
|
||||
schema_to_library = {
|
||||
"IFC2X3": "IFC2X3 Demo Library.ifc",
|
||||
"IFC4": "IFC4 Demo Library.ifc",
|
||||
"IFC4X3": "IFC4X3 Demo Library.ifc",
|
||||
"IFC4X3_ADD2": "IFC4X3 Demo Library.ifc",
|
||||
}
|
||||
schema = tool.Ifc.get().schema
|
||||
lib_name = schema_to_library.get(schema, "IFC4 Demo Library.ifc")
|
||||
lib_path = f"./bonsai/bim/data/libraries/{lib_name}"
|
||||
bpy.ops.bim.select_library_file(filepath=lib_path, append_all=True)
|
||||
|
||||
|
||||
|
||||
@@ -35,6 +35,7 @@ pytestmark = pytest.mark.model
|
||||
|
||||
HANDLER_PATH = Path(__file__).parent.parent.parent / "bonsai" / "bim" / "handler.py"
|
||||
PARAMETRIC_PATH = HANDLER_PATH.parent.parent / "tool" / "parametric.py"
|
||||
MODEL_MODULE_DIR = HANDLER_PATH.parent / "module" / "model"
|
||||
|
||||
# User-intent enums encode the user's "what to build next" choice on the
|
||||
# BIM Tool panel. The header-only writer must never drift into enum writes;
|
||||
@@ -117,3 +118,59 @@ def test_refresh_post_commit_gates_header_refresh_on_edit_types_registry() -> No
|
||||
"fires the refresh for commits in contexts that strip view-layer attributes; "
|
||||
"a missing call silently drops the validate-gizmo header refresh."
|
||||
)
|
||||
|
||||
|
||||
def _modules_with_module_scope_cache_and_clear():
|
||||
"""Yield ``module_name`` for every ``bim/module/model/*.py`` source that
|
||||
declares a module-scope ``GenerationKeyedCache()`` assignment AND a
|
||||
top-level ``def clear_caches``. These are the modules whose cache state
|
||||
survives file loads and must be drained from ``_apply_save_file_invariants``."""
|
||||
for path in MODEL_MODULE_DIR.glob("*.py"):
|
||||
tree = ast.parse(path.read_text(encoding="utf-8"))
|
||||
has_cache = False
|
||||
has_clear = False
|
||||
for node in tree.body:
|
||||
if isinstance(node, ast.FunctionDef) and node.name == "clear_caches":
|
||||
has_clear = True
|
||||
continue
|
||||
if isinstance(node, ast.Assign):
|
||||
for sub in ast.walk(node.value):
|
||||
if (
|
||||
isinstance(sub, ast.Call)
|
||||
and isinstance(sub.func, ast.Attribute)
|
||||
and sub.func.attr == "GenerationKeyedCache"
|
||||
):
|
||||
has_cache = True
|
||||
break
|
||||
if has_cache and has_clear:
|
||||
yield path.stem
|
||||
|
||||
|
||||
def test_apply_save_file_invariants_drains_every_module_scope_geom_cache(handler_tree: ast.Module) -> None:
|
||||
"""Module-scope ``GenerationKeyedCache`` instances persist across file
|
||||
loads — the counter they invalidate against is class-level and survives
|
||||
a ``.blend`` reload. Without a ``load_post`` drain the cache may serve
|
||||
entries whose ``bpy_struct`` references point into the previous file's
|
||||
freed ``bpy.data``, raising ``ReferenceError`` on the next attribute read.
|
||||
|
||||
Pin: every model module that exposes both a module-scope cache and a
|
||||
top-level ``clear_caches`` is called from ``_apply_save_file_invariants``,
|
||||
the central post-load drain."""
|
||||
fn = _function_node(handler_tree, "_apply_save_file_invariants")
|
||||
drained: set[str] = set()
|
||||
for node in ast.walk(fn):
|
||||
if (
|
||||
isinstance(node, ast.Call)
|
||||
and isinstance(node.func, ast.Attribute)
|
||||
and node.func.attr == "clear_caches"
|
||||
and isinstance(node.func.value, ast.Name)
|
||||
):
|
||||
drained.add(node.func.value.id)
|
||||
missing = [name for name in _modules_with_module_scope_cache_and_clear() if name not in drained]
|
||||
if missing:
|
||||
pytest.fail(
|
||||
"Module(s) expose a module-scope GenerationKeyedCache + clear_caches() but "
|
||||
f"_apply_save_file_invariants does not drain them on load_post: {sorted(missing)}. "
|
||||
"Add a `<module>.clear_caches()` call so freshly-loaded files cannot serve "
|
||||
"entries holding freed bpy.data references from the previous file."
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user