diff --git a/.beads/issues.jsonl b/.beads/issues.jsonl index 3b85cc9..0c5faa4 100644 --- a/.beads/issues.jsonl +++ b/.beads/issues.jsonl @@ -2,7 +2,7 @@ {"id":"ifcurl-p60","title":"Viewer: no element click-to-identify — can't get GlobalId or properties","description":"There is no way to click on a mesh in the viewer and see which IFC element it represents. Reviewers can see geometry but cannot reference specific elements — they can't discover a GlobalId to put in a selector URL, can't see the element name/type/Psets, and can't say 'this specific wall' in a BCF or Forgejo comment. This breaks the collaboration workflow at its foundation: every useful feedback comment needs to identify specific elements. Need: click or hover on a surface to highlight it and show a properties panel (name, type, GlobalId, key Psets). The GlobalId should be copyable into the selector field or URL. The ThatOpen components library provides the tools for this (OBC.IfcRelationsIndexer, element property queries).","status":"closed","priority":1,"issue_type":"feature","assignee":"Bruno Postle","owner":"bruno@postle.net","created_at":"2026-04-24T11:02:33Z","created_by":"Bruno Postle","updated_at":"2026-04-24T11:47:23Z","started_at":"2026-04-24T11:14:57Z","closed_at":"2026-04-24T11:47:23Z","close_reason":"Closed","dependency_count":0,"dependent_count":0,"comment_count":0} {"id":"ifcurl-4l7","title":"viewer.html: scale= URLs don't load in orthographic mode","description":"applyCameraParam() reads the camera= and fov= parameters from the URL and applies them, but never reads scale= or switches the camera to orthographic (parallel) projection. URLs like ?camera=...\u0026scale=50 will load in perspective mode instead of orthographic. The OBC OrthoPerspectiveCamera can switch modes, but the code path to set parallel projection and apply the scale value is missing from applyCameraParam.","status":"closed","priority":1,"issue_type":"bug","assignee":"Bruno Postle","owner":"bruno@postle.net","created_at":"2026-04-24T09:04:51Z","created_by":"Bruno Postle","updated_at":"2026-04-24T10:56:29Z","started_at":"2026-04-24T10:50:19Z","closed_at":"2026-04-24T10:56:29Z","close_reason":"Closed","dependency_count":0,"dependent_count":0,"comment_count":0} {"id":"ifcurl-362","title":"Add missing server-config files: gitconfig-ifcmerge and gitattributes","description":"forgejo/README.md documents deploying two files that don't exist in the repo: forgejo/server-config/gitconfig-ifcmerge and forgejo/server-config/gitattributes. Both are required for the ifcmerge git merge driver to work in bare Forgejo repos. The README even includes 'sudo cp forgejo/server-config/gitattributes /etc/gitattributes' and 'sudo git config --system include.path .../gitconfig-ifcmerge', but the files themselves are absent. Anyone following the deployment instructions will hit a 'file not found' error. Need to create both files with the content described in the README.","status":"closed","priority":1,"issue_type":"bug","assignee":"Bruno Postle","owner":"bruno@postle.net","created_at":"2026-04-24T09:04:31Z","created_by":"Bruno Postle","updated_at":"2026-04-24T10:51:47Z","started_at":"2026-04-24T10:50:15Z","closed_at":"2026-04-24T10:51:47Z","close_reason":"Files already exist: forgejo/server-config/gitconfig-ifcmerge and gitattributes are both present and correct. Missed in initial review.","dependency_count":0,"dependent_count":0,"comment_count":0} -{"id":"ifcurl-ndf","title":"gitconfig-ifcmerge: update to use ifcmerge --prioritise-local instead of swapping LOCAL/REMOTE","description":"The current gitconfig-ifcmerge uses the old technique of swapping the order of %A and %B arguments to preserve the base branch's STEP ID space:\n\n driver = ifcmerge %O %A %B %L (standard)\n driver = ifcmerge %O %B %A %L (ours variant)\n\nBonsai's current ifcgit code (tool/ifcgit.py, config_ifcmerge) uses the newer --prioritise-local flag instead:\n\n ifcmerge --prioritise-local $BASE $LOCAL $REMOTE $MERGED\n\nThe --prioritise-local flag tells ifcmerge to preserve LOCAL's STEP ID space without needing to swap argument order. This is cleaner, more readable, and follows the canonical ifcmerge interface.\n\nThe gitconfig-ifcmerge needs updating to:\n driver = ifcmerge --prioritise-local %O %A %B %A\n\n(where %A is both LOCAL and the output file — git merge drivers write their result to %A)\n\nThe ifcmerge_ours variant (for rebase direction where base branch arrives as %B) would become:\n driver = ifcmerge --prioritise-local %O %B %A %B\n\nVerify that the --prioritise-local flag exists in the version of ifcmerge being used before merging.","status":"open","priority":2,"issue_type":"task","owner":"bruno@postle.net","created_at":"2026-04-25T06:06:59Z","created_by":"Bruno Postle","updated_at":"2026-04-25T06:06:59Z","dependency_count":0,"dependent_count":0,"comment_count":0} +{"id":"ifcurl-ndf","title":"gitconfig-ifcmerge: update to use ifcmerge --prioritise-local instead of swapping LOCAL/REMOTE","description":"The current gitconfig-ifcmerge uses the old technique of swapping the order of %A and %B arguments to preserve the base branch's STEP ID space:\n\n driver = ifcmerge %O %A %B %L (standard)\n driver = ifcmerge %O %B %A %L (ours variant)\n\nBonsai's current ifcgit code (tool/ifcgit.py, config_ifcmerge) uses the newer --prioritise-local flag instead:\n\n ifcmerge --prioritise-local $BASE $LOCAL $REMOTE $MERGED\n\nThe --prioritise-local flag tells ifcmerge to preserve LOCAL's STEP ID space without needing to swap argument order. This is cleaner, more readable, and follows the canonical ifcmerge interface.\n\nThe gitconfig-ifcmerge needs updating to:\n driver = ifcmerge --prioritise-local %O %A %B %A\n\n(where %A is both LOCAL and the output file — git merge drivers write their result to %A)\n\nThe ifcmerge_ours variant (for rebase direction where base branch arrives as %B) would become:\n driver = ifcmerge --prioritise-local %O %B %A %B\n\nVerify that the --prioritise-local flag exists in the version of ifcmerge being used before merging.","status":"closed","priority":2,"issue_type":"task","owner":"bruno@postle.net","created_at":"2026-04-25T06:06:59Z","created_by":"Bruno Postle","updated_at":"2026-04-25T06:41:59Z","closed_at":"2026-04-25T06:41:59Z","close_reason":"Closed","dependency_count":0,"dependent_count":0,"comment_count":0} {"id":"ifcurl-qjd","title":"render_diff: auto-fit camera to changed elements, not whole model","description":"When rendering a diff of a large building model, the auto-fit camera fits to the entire head model. A wall added to one corner of a multi-storey building would appear as a tiny green speck. The camera should instead zoom to the bounding box of just the added + modified + removed elements so the changed area fills the viewport.\n\nImplementation sketch:\n- In render_diff(), after adding all shapes to plotter1, call plotter1.reset_camera() as now, but also compute the bounds of only the diff-coloured meshes (added + modified) and re-fit to those bounds.\n- For pass 2 (removed elements), the elements are already isolated in the iterator, so reset_camera() naturally fits to them. The pass 1 camera capture then needs to consider that pass 2 might be out of that frustum.\n- A combined approach: compute the bounding box union of added + modified + removed element meshes, then set the camera to fit that box with a small margin. This is the camera used for both passes.\n- pyvista Plotter has plotter.reset_camera(bounds=(xmin,xmax,ymin,ymax,zmin,zmax)) which can fit to an explicit box.\n- The bounds can be accumulated during the _add_shape loop for diff-coloured elements.\n\nAcceptance: a single modified wall in a large building renders with the wall clearly visible and filling most of the image.","status":"open","priority":2,"issue_type":"bug","owner":"bruno@postle.net","created_at":"2026-04-25T06:06:39Z","created_by":"Bruno Postle","updated_at":"2026-04-25T06:06:39Z","dependency_count":0,"dependent_count":0,"comment_count":0} {"id":"ifcurl-zxs","title":"sandbox.py: add seccomp filter to block execve and socket in render subprocess","description":"The subprocess isolation in sandbox.py (ifcurl-vm5) prevents DoS but not RCE: if a crafted IFC file achieves code execution inside the render child, that child still has full access to execve and the network. A seccomp BPF filter applied at the start of _worker() would block the two most useful post-exploitation steps even under active code execution.\n\nThe filter should:\n- Block execve/execveat (cannot exec a shell or any binary)\n- Block socket() for AF_INET/AF_INET6/AF_UNIX (cannot open a reverse shell or connect to the Forgejo Unix socket)\n- Allow clone/fork (ifcopenshell's geom.iterator spawns its own worker processes)\n- Allow everything else needed by Python + ifcopenshell + pyvista\n\nThe python-seccomp library (pip install seccomp, wraps libseccomp) is the cleanest way to do this. The filter would be installed via resource.setrlimit companion code already present in sandbox._worker().\n\nThe main risk is over-filtering: if the seccomp policy blocks a syscall that ifcopenshell or pyvista needs, renders fail with EPERM. The policy needs to be validated against the full render path (including the geometry iterator subprocesses, which inherit the filter across fork).\n\nAcceptance criteria:\n- A render subprocess cannot exec /bin/sh even with arbitrary code execution (verified by replacing fn with a ctypes execve call — should get EPERM/killed, not a shell)\n- Normal renders continue to produce correct PNG output\n- The seccomp library is optional (graceful fallback if libseccomp is absent, e.g. on macOS or minimal containers)","status":"open","priority":2,"issue_type":"task","owner":"bruno@postle.net","created_at":"2026-04-24T20:47:00Z","created_by":"Bruno Postle","updated_at":"2026-04-24T20:47:00Z","dependency_count":0,"dependent_count":0,"comment_count":0} {"id":"ifcurl-9cr","title":"ifc:// URLs in Forgejo issues and PR comments not documented as a collaboration tool","description":"Forgejo's markdown extension renders ifc:// URLs as clickable 3D preview images in file README views, but it works equally in issue bodies, PR descriptions, and comment threads — which is where the actual collaboration happens. This is potentially the most useful feature: a reviewer can paste an ifc:// URL in a PR comment and everyone in the thread sees the 3D view inline. This is not mentioned anywhere in README.md, forgejo/README.md, or SPECIFICATION.md. Add a 'Collaboration workflow' section to the README showing: (1) navigate to view in viewer, (2) copy URL from address bar, (3) paste as [label](ifc://...) in a Forgejo comment/issue, (4) collaborators click the preview image to open the view.","status":"closed","priority":2,"issue_type":"task","assignee":"Bruno Postle","owner":"bruno@postle.net","created_at":"2026-04-24T11:03:44Z","created_by":"Bruno Postle","updated_at":"2026-04-24T11:47:24Z","started_at":"2026-04-24T11:14:59Z","closed_at":"2026-04-24T11:47:24Z","close_reason":"Closed","dependency_count":0,"dependent_count":0,"comment_count":0} diff --git a/ifcurl/render.py b/ifcurl/render.py index 99a00e1..4a3325d 100644 --- a/ifcurl/render.py +++ b/ifcurl/render.py @@ -496,6 +496,43 @@ def render( return _render_iterator(iterator, selection_ids, visibility, clips, camera, fov, scale) +def _entity_bounds(model: ifcopenshell.file, entities: list) -> list[float] | None: + """Return [xmin,xmax,ymin,ymax,zmin,zmax] for *entities* in world coords, or None.""" + if not entities: + return None + settings = _build_geom_settings(model) + it = ifcopenshell.geom.iterator(settings, model, _WORKER_COUNT, include=entities) + if not it.initialize(): + return None + all_verts: list[np.ndarray] = [] + while True: + try: + v = np.array(it.get().geometry.verts, dtype=float).reshape(-1, 3) + if v.size > 0: + all_verts.append(v) + except Exception: + pass + if not it.next(): + break + if not all_verts: + return None + v = np.vstack(all_verts) + return [float(v[:, 0].min()), float(v[:, 0].max()), + float(v[:, 1].min()), float(v[:, 1].max()), + float(v[:, 2].min()), float(v[:, 2].max())] + + +def _merge_bounds(a: list[float] | None, b: list[float] | None) -> list[float] | None: + """Merge two [xmin,xmax,ymin,ymax,zmin,zmax] extents, either may be None.""" + if a is None: + return b + if b is None: + return a + return [min(a[0], b[0]), max(a[1], b[1]), + min(a[2], b[2]), max(a[3], b[3]), + min(a[4], b[4]), max(a[5], b[5])] + + def render_diff( model_head: ifcopenshell.file, model_base: ifcopenshell.file, @@ -540,6 +577,38 @@ def render_diff( frozen_diff = {k: frozenset(v) for k, v in diff_ids.items()} + # ------------------------------------------------------------------ + # Pre-compute removed entities (needed for bounds and pass 2) + # ------------------------------------------------------------------ + removed_ids = diff_ids.get("removed", set()) + removed_entities = [] + for eid in removed_ids: + try: + removed_entities.append(model_base.by_id(eid)) + except Exception: + pass + + # ------------------------------------------------------------------ + # Pre-compute bounding box of all changed elements for camera fitting. + # added/modified bounds come from a subset iterator on the head model; + # removed bounds come from a subset iterator on the base model. + # Both are fast because they operate on small subsets. + # ------------------------------------------------------------------ + if camera is None: + changed_head_ids = frozen_diff.get("added", frozenset()) | frozen_diff.get("modified", frozenset()) + changed_head_entities = [] + for eid in changed_head_ids: + try: + changed_head_entities.append(model_head.by_id(eid)) + except Exception: + pass + diff_bounds = _merge_bounds( + _entity_bounds(model_head, changed_head_entities), + _entity_bounds(model_base, removed_entities), + ) + else: + diff_bounds = None + # ------------------------------------------------------------------ # Pass 1: head model — full scene with diff colouring # ------------------------------------------------------------------ @@ -565,6 +634,12 @@ def render_diff( if camera is not None: _apply_camera(plotter1, camera, fov, scale) + elif diff_bounds is not None: + plotter1.reset_camera(bounds=diff_bounds) + plotter1.camera.up = (0, 0, 1) + if scale is not None: + plotter1.camera.parallel_projection = True + plotter1.camera.parallel_scale = scale else: plotter1.reset_camera() plotter1.camera.up = (0, 0, 1) @@ -586,16 +661,6 @@ def render_diff( # Pass 2: base model — removed elements only, in red. # Apply the same camera that pass 1 used. # ------------------------------------------------------------------ - removed_ids = diff_ids.get("removed", set()) - if not removed_ids: - return pass1_png - - removed_entities = [] - for eid in removed_ids: - try: - removed_entities.append(model_base.by_id(eid)) - except Exception: - pass if not removed_entities: return pass1_png