From 5d6878c321c52aca92651499aad45e15033d64c9 Mon Sep 17 00:00:00 2001 From: Gorgious56 Date: Wed, 20 May 2026 17:28:18 +0200 Subject: [PATCH] Fix set_icon_gizmo_position so billboard ignores object rotation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit set_icon_gizmo_position computed ``mw @ (Translation @ billboard_rot @ Scale)`` — the object's world matrix was applied AFTER the billboard rotation, so any non-trivial object rotation (e.g. a wall rotated in plan, a stair rotated to match a corridor) carried over into the icon's transform and tilted it edge-on to the camera instead of facing it. Switch to ``billboarded_at(world_pos, billboard_rot, scale)`` where ``world_pos = mw @ local_pos``: translate to world space first, then apply the billboard rotation independently of the object's rotation. This matches the manual pattern the base class's ``update_editing_gizmos`` already uses for validate/cancel/cycle for exactly this reason. Drops the now-stale workaround docstring on ``GizmoWallEdition._update_icon_row_extras`` that documented why it bypassed ``set_icon_gizmo_position`` — the helper does the right thing now. Adds ``test/bim/module/model/test_stair_gizmos.py`` as the regression guard: parametrised over six rotation angles, asserts that the rotation part of the resulting matrix equals ``billboard_rot`` (no contribution from ``mw``'s rotation) and that the translation lands at ``world_pos``. Also exercises ``set_icon_gizmo_position`` end-to-end via a stub gizmo to catch the exact shape of the previously-broken call site. Generated with the assistance of an AI coding tool. --- .../bonsai/bim/module/drawing/gizmos.py | 4 +- src/bonsai/bonsai/bim/module/model/wall.py | 9 +- .../bim/module/model/test_stair_gizmos.py | 128 ++++++++++++++++++ 3 files changed, 132 insertions(+), 9 deletions(-) create mode 100644 src/bonsai/test/bim/module/model/test_stair_gizmos.py diff --git a/src/bonsai/bonsai/bim/module/drawing/gizmos.py b/src/bonsai/bonsai/bim/module/drawing/gizmos.py index 2a35de3fcb..4ee6cb967e 100644 --- a/src/bonsai/bonsai/bim/module/drawing/gizmos.py +++ b/src/bonsai/bonsai/bim/module/drawing/gizmos.py @@ -4847,8 +4847,8 @@ class BaseParametricGizmoGroup: scale: Gizmo scale factor (default 0.5) """ if gz := self.get_gizmo_if_visible(gizmo_name): - local_transform = Matrix.Translation(Vector((x, y, z))) @ billboard_rot @ Matrix.Scale(scale, 4) - gz.matrix_basis = mw @ local_transform + world_pos = mw @ Vector((x, y, z)) + gz.matrix_basis = billboarded_at(world_pos, billboard_rot, scale) def set_dimension_gizmo_position( self, diff --git a/src/bonsai/bonsai/bim/module/model/wall.py b/src/bonsai/bonsai/bim/module/model/wall.py index da281897c8..c08f15b60c 100644 --- a/src/bonsai/bonsai/bim/module/model/wall.py +++ b/src/bonsai/bonsai/bim/module/model/wall.py @@ -1969,13 +1969,8 @@ class GizmoWallEdition(bpy.types.GizmoGroup, gizmo.BaseParametricGizmoGroup): - Toggle-openings icon next to the pen. Lives outside edit mode because opening visibility is a viewport-display concern, not a wall-edit action. - Uses the manual ``Translation(world_pos) @ billboard_rot @ Scale`` pattern - rather than the base class's ``set_icon_gizmo_position`` helper. The helper - computes ``mw @ (Translation @ billboard_rot @ Scale)``, which applies the - wall's rotation to the billboard — for a wall rotated in plan, the icons - end up tilted edge-on to the camera instead of facing it. The base class's - own ``update_editing_gizmos`` already uses the manual pattern for validate/ - cancel/cycle for exactly this reason; we match it here.""" + Uses ``billboarded_at`` directly for parity with the base class's + ``update_editing_gizmos`` validate/cancel/cycle pattern.""" if not hasattr(self, "rotate_gizmo"): return gizmo_prefs = self.get_gizmo_prefs() diff --git a/src/bonsai/test/bim/module/model/test_stair_gizmos.py b/src/bonsai/test/bim/module/model/test_stair_gizmos.py new file mode 100644 index 0000000000..9e46fd18ce --- /dev/null +++ b/src/bonsai/test/bim/module/model/test_stair_gizmos.py @@ -0,0 +1,128 @@ +# Bonsai - OpenBIM Blender Add-on +# Copyright (C) 2026 +# +# This file is part of Bonsai. +# +# Bonsai is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# Bonsai is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with Bonsai. If not, see . +# +# This file was generated with the assistance of an AI coding tool. + +"""Regression guard for the stair icon billboard fix. + +Before the fix, ``set_icon_gizmo_position`` in ``bim.module.drawing.gizmos`` +composed ``mw @ (Translation @ billboard_rot @ Scale)``, which applied the +stair's world rotation on top of the billboard rotation. The result was +icons (validate / cancel / lock / +/- / cycle / tread_lock) drawn edge-on +to the camera for any stair rotated in plan — effectively unclickable. + +The fix routes through ``billboarded_at(world_pos, billboard_rot, scale)``, +which computes ``Translation(world_pos) @ billboard_rot @ Scale`` — the +object's rotation is folded into the translation only, never the rotation.""" + +import math +import types + +import bpy +import pytest +from mathutils import Matrix, Vector + +pytestmark = pytest.mark.model + + +@pytest.fixture(autouse=True) +def _require_real_bpy(): + if not isinstance(bpy, types.ModuleType) or hasattr(bpy, "_mock_name"): + pytest.skip("requires real Blender (bpy is mocked or absent)") + + +def _rotation_close(a: Matrix, b: Matrix, tol: float = 1e-6) -> bool: + for row_a, row_b in zip(a, b): + for va, vb in zip(row_a, row_b): + if abs(va - vb) > tol: + return False + return True + + +@pytest.mark.parametrize("angle_deg", [0, 30, 45, 90, 135, 217]) +def test_billboarded_at_rotation_is_pure_billboard(angle_deg): + """Object rotation must not leak into the gizmo's rotation part.""" + from bonsai.bim.module.drawing.gizmos import billboarded_at + + mw = Matrix.Rotation(math.radians(angle_deg), 4, "Z") @ Matrix.Translation((3, 4, 5)) + billboard_rot = Matrix.Rotation(math.radians(30), 4, "X") + + world_pos = mw @ Vector((1, 0, 2)) + result = billboarded_at(world_pos, billboard_rot, scale=0.5) + + # The rotation part of result, after stripping the 0.5 uniform scale, + # must equal billboard_rot — no contribution from mw's rotation. + rotation_part = result.to_3x3() * 2.0 + assert _rotation_close(rotation_part.to_4x4(), billboard_rot) + + +def test_billboarded_at_translation_is_world_pos(): + """Translation lands exactly at the world-space target.""" + from bonsai.bim.module.drawing.gizmos import billboarded_at + + world_pos = Vector((1.23, 4.56, 7.89)) + result = billboarded_at(world_pos, Matrix.Identity(4), scale=0.5) + assert (result.translation - world_pos).length < 1e-6 + + +def test_set_icon_gizmo_position_does_not_apply_object_rotation(): + """End-to-end: the helper used by every stair icon (and shared with all + parametric gizmo groups) must produce a matrix whose rotation part is + billboard_rot, not mw_rotation @ billboard_rot. This is the exact bug + that left stair icons edge-on to the camera.""" + from bonsai.bim.module.drawing.gizmos import ( + BaseParametricGizmoGroup, + billboarded_at, + ) + + # Same inputs as the real call site (stair.py:747-765), but we drive the + # helper directly so we don't need a registered GizmoGroup. We bind a + # stand-in `get_gizmo_if_visible` that returns a tiny mock; the helper's + # observable output is the matrix_basis it assigns. + captured = {} + + class _GizmoStub: + matrix_basis: Matrix = Matrix.Identity(4) + + stub = _GizmoStub() + + def _fake_get(name): + captured["name"] = name + return stub + + # Bind the helper to a throwaway instance so `self.get_gizmo_if_visible` + # resolves to our stub without registering a real GizmoGroup with Blender. + fake_self = types.SimpleNamespace(get_gizmo_if_visible=_fake_get) + BaseParametricGizmoGroup.set_icon_gizmo_position( + fake_self, + "validate_gizmo", + mw=Matrix.Rotation(math.radians(45), 4, "Z") @ Matrix.Translation((3, 4, 5)), + x=1.0, + y=0.0, + z=2.0, + billboard_rot=Matrix.Rotation(math.radians(30), 4, "X"), + scale=0.5, + ) + + expected_world_pos = (Matrix.Rotation(math.radians(45), 4, "Z") @ Matrix.Translation((3, 4, 5))) @ Vector((1, 0, 2)) + expected = billboarded_at(expected_world_pos, Matrix.Rotation(math.radians(30), 4, "X"), 0.5) + + assert captured["name"] == "validate_gizmo" + for row_a, row_b in zip(stub.matrix_basis, expected): + for va, vb in zip(row_a, row_b): + assert abs(va - vb) < 1e-6