From 5e784e4175c83e5659335a709eea00ac4f35b876 Mon Sep 17 00:00:00 2001 From: Bruno Postle Date: Wed, 1 Apr 2026 23:37:32 +0100 Subject: [PATCH 01/25] Refactor ifcgit, fix UI bugs and performance Move all business logic into bonsai core and tool. Performance fixes to minimise file IO, various minor bug fixes and tests. Generated with the assistance of an AI coding tool. --- src/bonsai/bonsai/bim/module/ifcgit/data.py | 143 +++--- .../bonsai/bim/module/ifcgit/operator.py | 68 ++- src/bonsai/bonsai/bim/module/ifcgit/prop.py | 26 +- src/bonsai/bonsai/bim/module/ifcgit/ui.py | 18 +- src/bonsai/bonsai/core/ifcgit.py | 63 ++- src/bonsai/bonsai/core/tool.py | 50 ++ src/bonsai/bonsai/tool/ifcgit.py | 180 +++---- src/bonsai/test/core/bootstrap.py | 7 + src/bonsai/test/core/test_ifcgit.py | 274 +++++++++++ src/bonsai/test/tool/test_ifcgit.py | 456 ++++++++++++++++++ 10 files changed, 1053 insertions(+), 232 deletions(-) create mode 100644 src/bonsai/test/core/test_ifcgit.py create mode 100644 src/bonsai/test/tool/test_ifcgit.py diff --git a/src/bonsai/bonsai/bim/module/ifcgit/data.py b/src/bonsai/bonsai/bim/module/ifcgit/data.py index 92da517a1c..7dbd26b5e0 100644 --- a/src/bonsai/bonsai/bim/module/ifcgit/data.py +++ b/src/bonsai/bonsai/bim/module/ifcgit/data.py @@ -21,65 +21,71 @@ class IfcGitData: @classmethod def load(cls): + repo = None + if bool(tool.Ifc.get()): + path_ifc = tool.Ifc.get_path() + if os.path.isfile(path_ifc): + repo = tool.IfcGit.repo_from_path(path_ifc) + cls.data = { - "repo": cls.repo(), - "remotes": cls.remotes(), - "branch_names": cls.branch_names(), - "remote_names": cls.remote_names(), - "remote_urls": cls.remote_urls(), + "repo": repo, + "remotes": repo.remotes if repo else None, + "branch_names": cls.branch_names(repo), + "tag_names": cls.tag_names(repo), + "remote_names": cls.remote_names(repo), + "remote_urls": {r.name: r.url for r in repo.remotes} if repo else {}, "path_ifc": cls.path_ifc(), "branches_by_hexsha": cls.branches_by_hexsha(), "tags_by_hexsha": cls.tags_by_hexsha(), - "name_ifc": cls.name_ifc(), + "name_ifc": cls.name_ifc(repo), "dir_name": cls.dir_name(), "base_name": cls.base_name(), - "working_dir": cls.working_dir(), - "untracked_files": cls.untracked_files(), - "is_detached": cls.is_detached(), - "active_branch_name": cls.active_branch_name(), - "is_dirty": cls.is_dirty(), - "commit": cls.commit(), - "current_revision": cls.current_revision(), + "working_dir": repo.working_dir if repo else None, + "ifc_is_untracked": cls.ifc_is_untracked(repo), + "is_detached": repo.head.is_detached if repo else None, + "active_branch_name": repo.active_branch.name if repo and not repo.head.is_detached else None, + "is_dirty": cls.is_dirty(repo), + "current_revision": cls.current_revision(repo), "git_exe": cls.git_exe(), "ifcmerge_exe": cls.ifcmerge_exe(), } cls.is_loaded = True @classmethod - def repo(cls): - if bool(tool.Ifc.get()): - path_ifc = tool.Ifc.get_path() - if os.path.isfile(path_ifc): - return tool.IfcGit.repo_from_path(path_ifc) - return None + def branch_names(cls, repo): + if not repo or not repo.heads: + return [] + names = sorted([b.name for b in repo.branches]) + if "main" in names: + names.remove("main") + names = ["main"] + names + if repo.remotes: + for remote in repo.remotes: + for ref in remote.refs: + names.append(ref.name) + return names @classmethod - def remotes(cls): - if cls.repo(): - return cls.repo().remotes - return None + def tag_names(cls, repo): + if not repo: + return [] + return [t.name for t in repo.tags] @classmethod - def branch_names(cls): - return [] - - @classmethod - def remote_names(cls): - return [] - - @classmethod - def remote_urls(cls): - result = {} - if cls.repo(): - for remote in cls.repo().remotes: - result[remote.name] = remote.url - return result + def remote_names(cls, repo): + if not repo: + return [] + names = sorted([r.name for r in repo.remotes]) + if "origin" in names: + names.remove("origin") + names = ["origin"] + names + return names @classmethod def path_ifc(cls): path_ifc = tool.Ifc.get_path() if os.path.isfile(path_ifc): - return tool.Ifc.get_path() + return path_ifc return None @classmethod @@ -88,7 +94,8 @@ class IfcGitData: if tool.IfcGitRepo.repo.branches: return tool.IfcGit.branches_by_hexsha(tool.IfcGitRepo.repo) except AttributeError: - return {} + pass + return {} @classmethod def tags_by_hexsha(cls): @@ -97,12 +104,11 @@ class IfcGitData: return {} @classmethod - def name_ifc(cls): - if bool(tool.Ifc.get()): + def name_ifc(cls, repo): + if bool(tool.Ifc.get()) and repo: path_ifc = tool.Ifc.get_path() - if tool.IfcGitRepo.repo and os.path.isfile(path_ifc): - working_dir = tool.IfcGitRepo.repo.working_dir - return os.path.relpath(path_ifc, working_dir) + if os.path.isfile(path_ifc): + return os.path.relpath(path_ifc, repo.working_dir) return None @classmethod @@ -122,49 +128,28 @@ class IfcGitData: return None @classmethod - def working_dir(cls): - if cls.repo(): - return cls.repo().working_dir + def ifc_is_untracked(cls, repo): + """Return True if the IFC file exists in the repo but has not been added to git.""" + if not repo: + return False + path_ifc = tool.Ifc.get_path() + if not os.path.isfile(path_ifc): + return False + return not bool(repo.git.ls_files(path_ifc)) @classmethod - def untracked_files(cls): - if cls.repo(): - return cls.repo().untracked_files - return [] - - @classmethod - def is_detached(cls): - if cls.repo(): - return cls.repo().head.is_detached - - @classmethod - def active_branch_name(cls): - if cls.repo() and not cls.is_detached(): - return cls.repo().active_branch.name - - @classmethod - def is_dirty(cls): - if cls.repo() and cls.git_exe(): + def is_dirty(cls, repo): + if repo and cls.git_exe(): path_ifc = tool.Ifc.get_path() if os.path.isfile(path_ifc): - return cls.repo().is_dirty(path=path_ifc) + return repo.is_dirty(path=path_ifc) return False @classmethod - def commit(cls): + def current_revision(cls, repo): props = tool.IfcGit.get_ifcgit_props() - if cls.repo() and len(props.ifcgit_commits) > 0: - item = props.ifcgit_commits[props.commit_index] - try: - return cls.repo().commit(rev=item.hexsha) - except ValueError: - return - - @classmethod - def current_revision(cls): - props = tool.IfcGit.get_ifcgit_props() - if cls.repo() and cls.repo().head.is_valid() and len(props.ifcgit_commits) > 0: - return tool.IfcGitRepo.repo.commit() + if repo and repo.head.is_valid() and len(props.ifcgit_commits) > 0: + return repo.commit() @classmethod def git_exe(cls): diff --git a/src/bonsai/bonsai/bim/module/ifcgit/operator.py b/src/bonsai/bonsai/bim/module/ifcgit/operator.py index 829a0a62a2..5f01ef78ea 100644 --- a/src/bonsai/bonsai/bim/module/ifcgit/operator.py +++ b/src/bonsai/bonsai/bim/module/ifcgit/operator.py @@ -120,11 +120,11 @@ class CommitChanges(bpy.types.Operator): if props.commit_message == "": return False if repo: - if props.new_branch_name in [branch.name for branch in repo.branches]: + if props.new_branch_name in IfcGitData.data["branch_names"]: cls.poll_message_set("Branch already exists!") return False elif not tool.IfcGit.is_valid_ref_format(props.new_branch_name): - if repo.head.is_detached: + if IfcGitData.data["is_detached"]: cls.poll_message_set("Branch name is invalid or empty!") return False elif props.new_branch_name != "": @@ -134,10 +134,17 @@ class CommitChanges(bpy.types.Operator): def execute(self, context): - repo = IfcGitData.data["repo"] - core.commit_changes(tool.IfcGit, tool.Ifc, repo) - core.refresh_revision_list(tool.IfcGit, repo, tool.Ifc) + props = tool.IfcGit.get_ifcgit_props() + commit_message = props.commit_message + new_branch_name = props.new_branch_name + core.commit_changes(tool.IfcGit, tool.Ifc, commit_message, new_branch_name) + props.new_branch_name = "" + props.commit_message = "" + core.refresh_revision_list(tool.IfcGit, tool.Ifc) refresh() + IfcGitData.load() + if new_branch_name: + props.display_branch = new_branch_name return {"FINISHED"} @@ -157,7 +164,7 @@ class AddTag(bpy.types.Operator): repo = IfcGitData.data["repo"] if repo and ( not tool.IfcGit.is_valid_ref_format(props.new_tag_name) - or props.new_tag_name in [tag.name for tag in repo.tags] + or props.new_tag_name in IfcGitData.data["tag_names"] ): return False return True @@ -165,8 +172,12 @@ class AddTag(bpy.types.Operator): def execute(self, context): repo = IfcGitData.data["repo"] - core.add_tag(tool.IfcGit, repo) - core.refresh_revision_list(tool.IfcGit, repo, tool.Ifc) + props = tool.IfcGit.get_ifcgit_props() + item = props.ifcgit_commits[props.commit_index] + core.add_tag(tool.IfcGit, repo, item.hexsha, props.new_tag_name, props.new_tag_message) + props.new_tag_name = "" + props.new_tag_message = "" + core.refresh_revision_list(tool.IfcGit, tool.Ifc) refresh() return {"FINISHED"} @@ -183,7 +194,7 @@ class DeleteTag(bpy.types.Operator): repo = IfcGitData.data["repo"] core.delete_tag(tool.IfcGit, repo, self.tag_name) - core.refresh_revision_list(tool.IfcGit, repo, tool.Ifc) + core.refresh_revision_list(tool.IfcGit, tool.Ifc) refresh() return {"FINISHED"} @@ -205,8 +216,7 @@ class RefreshGit(bpy.types.Operator): def execute(self, context): - repo = IfcGitData.data["repo"] - core.refresh_revision_list(tool.IfcGit, repo, tool.Ifc) + core.refresh_revision_list(tool.IfcGit, tool.Ifc) refresh() tool.IfcGit.decolourise() return {"FINISHED"} @@ -284,7 +294,7 @@ class Merge(bpy.types.Operator): def execute(self, context): - if core.merge_branch(tool.IfcGit, tool.Ifc, self): + if core.merge_branch(tool.IfcGit, tool.Ifc, self) is not False: refresh() return {"FINISHED"} else: @@ -314,9 +324,9 @@ class Fetch(bpy.types.Operator): def execute(self, context): props = tool.IfcGit.get_ifcgit_props() - repo = IfcGitData.data["repo"] - remote = repo.remotes[props.select_remote] - remote.fetch() + core.fetch(tool.IfcGit, props.select_remote) + core.refresh_revision_list(tool.IfcGit, tool.Ifc) + refresh() return {"FINISHED"} @@ -336,7 +346,7 @@ class AddRemote(bpy.types.Operator): not repo or not tool.IfcGit.is_valid_ref_format(props.remote_name) or not props.remote_url - or props.remote_name in [remote.name for remote in repo.remotes] + or props.remote_name in IfcGitData.data["remote_names"] ): return False return True @@ -344,8 +354,11 @@ class AddRemote(bpy.types.Operator): def execute(self, context): repo = IfcGitData.data["repo"] - core.add_remote(tool.IfcGit, repo) - core.refresh_revision_list(tool.IfcGit, repo, tool.Ifc) + props = tool.IfcGit.get_ifcgit_props() + core.add_remote(tool.IfcGit, repo, props.remote_name, props.remote_url) + props.remote_name = "" + props.remote_url = "" + core.refresh_revision_list(tool.IfcGit, tool.Ifc) refresh() return {"FINISHED"} @@ -360,8 +373,19 @@ class DeleteRemote(bpy.types.Operator): def execute(self, context): repo = IfcGitData.data["repo"] - core.delete_remote(tool.IfcGit, repo) - core.refresh_revision_list(tool.IfcGit, repo, tool.Ifc) + props = tool.IfcGit.get_ifcgit_props() + remote_name = props.select_remote + if props.display_branch.startswith(remote_name + "/"): + active = IfcGitData.data["active_branch_name"] + if active: + props.display_branch = active + else: + local_branches = [b for b in IfcGitData.data["branch_names"] if "/" not in b] + if local_branches: + props.display_branch = local_branches[0] + core.delete_remote(tool.IfcGit, repo, remote_name) + tool.IfcGit.select_first_remote() + core.refresh_revision_list(tool.IfcGit, tool.Ifc) refresh() return {"FINISHED"} @@ -375,8 +399,8 @@ class ObjectLog(bpy.types.Operator): @classmethod def poll(cls, context): - if not (obj := context.active_object): - cls.poll_message_set("No Active Object") + if not (obj := context.active_object) or not obj.select_get(): + cls.poll_message_set("No selected object") elif not tool.Blender.get_ifc_definition_id(obj): cls.poll_message_set("Active Object doesn't have an IFC definition") else: diff --git a/src/bonsai/bonsai/bim/module/ifcgit/prop.py b/src/bonsai/bonsai/bim/module/ifcgit/prop.py index cba3ca632c..865cb4eacf 100644 --- a/src/bonsai/bonsai/bim/module/ifcgit/prop.py +++ b/src/bonsai/bonsai/bim/module/ifcgit/prop.py @@ -17,28 +17,14 @@ from bonsai.bim.module.ifcgit.data import IfcGitData def git_branches(self: "IfcGitProperties", context: bpy.types.Context) -> tool.Blender.BLENDER_ENUM_ITEMS: # NOTE "Python must keep a reference to the strings returned by # the callback or Blender will misbehave or even crash" - IfcGitData.data["branch_names"] = sorted([branch.name for branch in IfcGitData.data["repo"].heads]) - - if "main" in IfcGitData.data["branch_names"]: - IfcGitData.data["branch_names"].remove("main") - IfcGitData.data["branch_names"] = ["main"] + IfcGitData.data["branch_names"] - - if IfcGitData.data["remotes"]: - for remote in IfcGitData.data["remotes"]: - for remote_branch in remote.refs: - IfcGitData.data["branch_names"].append(remote_branch.name) - - return [(myname, myname, myname) for myname in IfcGitData.data["branch_names"]] + # Branch list (local + remote, main first) is computed once in IfcGitData.load() + IfcGitData.make_sure_is_loaded() + return [(name, name, name) for name in IfcGitData.data["branch_names"]] def git_remotes(self: "IfcGitProperties", context: bpy.types.Context) -> tool.Blender.BLENDER_ENUM_ITEMS: - IfcGitData.data["remote_names"] = sorted([remote.name for remote in IfcGitData.data["remotes"]]) - - if "origin" in IfcGitData.data["remote_names"]: - IfcGitData.data["remote_names"].remove("origin") - IfcGitData.data["remote_names"] = ["origin"] + IfcGitData.data["remote_names"] - - return [(myname, myname, myname) for myname in IfcGitData.data["remote_names"]] + IfcGitData.make_sure_is_loaded() + return [(name, name, name) for name in IfcGitData.data["remote_names"]] def update_revlist(self: "IfcGitProperties", context: bpy.types.Context) -> None: @@ -90,6 +76,7 @@ class IfcGitListItem(PropertyGroup): name="Commit Message", default="", ) + committed_date: IntProperty(name="Committed Date", default=0) tags: CollectionProperty(type=IfcGitTag, name="List of revision tags") if TYPE_CHECKING: @@ -98,6 +85,7 @@ class IfcGitListItem(PropertyGroup): author_name: str author_email: str message: str + committed_date: int tags: bpy.types.bpy_prop_collection_idprop[IfcGitTag] diff --git a/src/bonsai/bonsai/bim/module/ifcgit/ui.py b/src/bonsai/bonsai/bim/module/ifcgit/ui.py index d8b7357e7b..6ac4aceb74 100644 --- a/src/bonsai/bonsai/bim/module/ifcgit/ui.py +++ b/src/bonsai/bonsai/bim/module/ifcgit/ui.py @@ -52,7 +52,7 @@ class IFCGIT_PT_panel(bpy.types.Panel): if IfcGitData.data["repo"] and os.path.exists(IfcGitData.data["repo"].git_dir): name_ifc = IfcGitData.data["name_ifc"] row.label(text=IfcGitData.data["working_dir"], icon="SYSTEM") - if name_ifc in IfcGitData.data["untracked_files"]: + if IfcGitData.data["ifc_is_untracked"]: row.operator( "ifcgit.addfile", text="Add '" + name_ifc + "' to repository", @@ -216,13 +216,7 @@ class COMMIT_UL_List(bpy.types.UIList): ): current_revision = IfcGitData.data["current_revision"] - - # TODO Figure how this "item" can be acesse in "data.py" - # so it's possible to move the ".commit" - try: - commit = IfcGitData.data["repo"].commit(rev=item.hexsha) - except ValueError: - return + current_hexsha = current_revision.hexsha if current_revision else None lookup = IfcGitData.data["branches_by_hexsha"] refs = "" @@ -236,11 +230,11 @@ class COMMIT_UL_List(bpy.types.UIList): for tag in lookup[item.hexsha]: refs += "{" + tag.name + "} " - if commit == current_revision: - layout.label(text="[HEAD] " + refs + commit.message.split("\n")[0], icon="DECORATE_KEYFRAME") + if item.hexsha == current_hexsha: + layout.label(text="[HEAD] " + refs + item.message.split("\n")[0], icon="DECORATE_KEYFRAME") else: - layout.label(text=refs + commit.message.split("\n")[0], icon="DECORATE_ANIMATE") - layout.label(text=time.strftime("%c", time.localtime(commit.committed_date))) + layout.label(text=refs + item.message.split("\n")[0], icon="DECORATE_ANIMATE") + layout.label(text=time.strftime("%c", time.localtime(item.committed_date))) def draw_filter(self, context, layout): diff --git a/src/bonsai/bonsai/core/ifcgit.py b/src/bonsai/bonsai/core/ifcgit.py index fb23652ebc..f14dda6cbb 100644 --- a/src/bonsai/bonsai/core/ifcgit.py +++ b/src/bonsai/bonsai/core/ifcgit.py @@ -56,42 +56,45 @@ def discard_uncommitted(ifcgit: type[tool.IfcGit], ifc: type[tool.Ifc]) -> None: ifcgit.load_project(path_ifc) -def commit_changes(ifcgit: type[tool.IfcGit], ifc: type[tool.Ifc], repo: git.Repo) -> None: +def commit_changes( + ifcgit: type[tool.IfcGit], ifc: type[tool.Ifc], commit_message: str, new_branch_name: str = "" +) -> None: """Commit and create new branches as required""" path_ifc = ifc.get_path() - if repo.head.is_detached: - ifcgit.git_commit(path_ifc) - ifcgit.create_new_branch() + if ifcgit.is_head_detached(): + ifcgit.git_commit(path_ifc, commit_message) + ifcgit.create_new_branch(new_branch_name) else: - ifcgit.checkout_new_branch(path_ifc) - ifcgit.git_commit(path_ifc) + if new_branch_name: + ifcgit.checkout_new_branch(path_ifc, new_branch_name) + ifcgit.git_commit(path_ifc, commit_message) -def add_tag(ifcgit: type[tool.IfcGit], repo: git.Repo) -> None: - ifcgit.add_tag(repo) +def add_tag(ifcgit: type[tool.IfcGit], repo: git.Repo, hexsha: str, tag_name: str, tag_message: str = "") -> None: + ifcgit.add_tag(repo, hexsha, tag_name, tag_message) def delete_tag(ifcgit: type[tool.IfcGit], repo: git.Repo, tag_name: git.TagReference) -> None: ifcgit.delete_tag(repo, tag_name) -def add_remote(ifcgit: type[tool.IfcGit], repo: git.Repo) -> None: - ifcgit.add_remote(repo) +def add_remote(ifcgit: type[tool.IfcGit], repo: git.Repo, remote_name: str, remote_url: str) -> None: + ifcgit.add_remote(repo, remote_name, remote_url) -def delete_remote(ifcgit: type[tool.IfcGit], repo: git.Repo) -> None: - ifcgit.delete_remote(repo) +def delete_remote(ifcgit: type[tool.IfcGit], repo: git.Repo, remote_name: str) -> None: + ifcgit.delete_remote(repo, remote_name) def push(ifcgit: type[tool.IfcGit], repo: git.Repo, remote_name: str, operator: bpy.types.Operator) -> None: - error_message = ifcgit.push(repo, remote_name, repo.active_branch.name) + error_message = ifcgit.push(repo, remote_name, ifcgit.get_active_branch_name()) if error_message: operator.report({"ERROR"}, error_message) -def refresh_revision_list(ifcgit: type[tool.IfcGit], repo: git.Repo, ifc: type[tool.Ifc]) -> None: - if repo.heads: +def refresh_revision_list(ifcgit: type[tool.IfcGit], ifc: type[tool.Ifc]) -> None: + if ifcgit.repo_has_commits(): ifcgit.refresh_revision_list(ifc.get_path()) @@ -125,10 +128,32 @@ def switch_revision(ifcgit: type[tool.IfcGit], ifc: type[tool.Ifc]) -> None: ifcgit.decolourise() -def merge_branch(ifcgit: type[tool.IfcGit], ifc: type[tool.Ifc], operator: bpy.types.Operator) -> None: +def merge_branch(ifcgit: type[tool.IfcGit], ifc: type[tool.Ifc], operator: bpy.types.Operator) -> bool | None: path_ifc = ifc.get_path() ifcgit.config_ifcmerge() - ifcgit.execute_merge(path_ifc, operator) + + branch_name = ifcgit.get_selected_branch() + if branch_name is None: + return + + mergetool = ifcgit.get_merge_tool(branch_name) + merge_result = ifcgit.git_merge(branch_name) + + if merge_result == "error": + operator.report({"ERROR"}, "Unknown IFC Merge failure") + return False + elif merge_result == "conflict": + error = ifcgit.git_mergetool(mergetool) + if error: + ifcgit.git_merge_abort() + operator.report({"ERROR"}, "IFC Merge failed:" + error) + return False + ifcgit.commit_merge(path_ifc) + + ifcgit.set_display_branch() + ifcgit.load_project(path_ifc) + ifcgit.refresh_revision_list(path_ifc) + ifcgit.decolourise() def entity_log(ifcgit: type[tool.IfcGit], ifc: type[tool.Ifc], step_id: int, operator: bpy.types.Operator) -> None: @@ -145,5 +170,9 @@ def install_git(ifcgit: type[tool.IfcGit], operator: bpy.types.Operator) -> None print("install_git() not implemented") +def fetch(ifcgit: type[tool.IfcGit], remote_name: str) -> None: + ifcgit.fetch(remote_name) + + def run_git_diff(ifcgit: type[tool.IfcGit], operator: bpy.types.Operator, save_to_temp: bool) -> None: ifcgit.run_git_diff(operator, save_to_temp) diff --git a/src/bonsai/bonsai/core/tool.py b/src/bonsai/bonsai/core/tool.py index f815bf0963..910ed79627 100644 --- a/src/bonsai/bonsai/core/tool.py +++ b/src/bonsai/bonsai/core/tool.py @@ -535,6 +535,56 @@ class Ifc: def get_all_element_occurrences(cls, element): pass +@interface +class IfcGit: + def add_file_to_repo(cls, repo, path_file): pass + def add_remote(cls, repo, remote_name, remote_url): pass + def add_tag(cls, repo, hexsha, tag_name, tag_message): pass + def branches_by_hexsha(cls, repo): pass + def checkout_new_branch(cls, path_file, branch_name): pass + def clear_commits_list(cls): pass + def clone_repo(cls, remote_url, local_folder): pass + def colourise(cls, step_ids): pass + def config_ifcmerge(cls): pass + def create_new_branch(cls, branch_name): pass + def decolourise(cls): pass + def delete_remote(cls, repo, remote_name): pass + def delete_tag(cls, repo, tag_name): pass + def dos2unix(cls, path_file): pass + def commit_merge(cls, path_ifc): pass + def entity_log(cls, path_ifc, step_id): pass + def fetch(cls, remote_name): pass + def get_commits_list(cls, path_ifc, lookup): pass + def get_merge_tool(cls, branch_name): pass + def get_selected_branch(cls): pass + def git_merge(cls, branch_name): pass + def git_merge_abort(cls): pass + def git_mergetool(cls, mergetool): pass + def set_display_branch(cls): pass + def get_active_branch_name(cls): pass + def get_ifcgit_props(cls): pass + def get_modified_step_ids(cls, step_ids): pass + def get_path_dir(cls, path_ifc): pass + def get_revisions_step_ids(cls): pass + def is_head_detached(cls): pass + def repo_has_commits(cls): pass + def git_checkout(cls, path_file): pass + def git_commit(cls, path_file, commit_message): pass + def ifc_diff_ids(cls, repo, hash_a, hash_b, path_ifc): pass + def init_repo(cls, path_dir): pass + def install_git_windows(cls, operator): pass + def is_valid_ref_format(cls, string): pass + def load_anyifc(cls, repo): pass + def load_project(cls, path_ifc): pass + def push(cls, repo, remote_name, branch_name): pass + def refresh_revision_list(cls, path_ifc): pass + def repo_from_path(cls, path): pass + def run_git_diff(cls, operator, save_to_temp): pass + def switch_to_revision_item(cls): pass + def tags_by_hexsha(cls, repo): pass + def update_step_ids(cls, step_ids, modified_step_ids): pass + + @interface class Layer: pass diff --git a/src/bonsai/bonsai/tool/ifcgit.py b/src/bonsai/bonsai/tool/ifcgit.py index 64b3957169..afff9fa3dc 100644 --- a/src/bonsai/bonsai/tool/ifcgit.py +++ b/src/bonsai/bonsai/tool/ifcgit.py @@ -24,7 +24,7 @@ import re import subprocess import tempfile from pathlib import Path -from typing import TYPE_CHECKING, Any, Literal, Union +from typing import TYPE_CHECKING, Any, Union import bpy @@ -128,39 +128,27 @@ class IfcGit: cls.dos2unix(path_file) repo.index.add(os.path.normpath(path_file)) repo.index.commit(message="Added " + os.path.relpath(path_file, repo.working_dir)) - bpy.ops.ifcgit.refresh() @classmethod def git_checkout(cls, path_file: str) -> None: IfcGitRepo.repo.git.checkout(path_file) @classmethod - def checkout_new_branch(cls, path_file: str) -> None: + def checkout_new_branch(cls, path_file: str, branch_name: str) -> None: """Create a branch and move uncommitted changes to this branch""" - props = cls.get_ifcgit_props() - if props.new_branch_name: - IfcGitRepo.repo.git.checkout(b=props.new_branch_name) - props.display_branch = props.new_branch_name - props.new_branch_name = "" - bpy.ops.ifcgit.refresh() + IfcGitRepo.repo.git.checkout(b=branch_name) @classmethod - def git_commit(cls, path_file: str) -> None: - props = cls.get_ifcgit_props() + def git_commit(cls, path_file: str, commit_message: str) -> None: repo = IfcGitRepo.repo if os.name == "nt": cls.dos2unix(path_file) repo.index.add(os.path.normpath(path_file)) - repo.index.commit(message=props.commit_message) - props.commit_message = "" + repo.index.commit(message=commit_message) @classmethod - def add_tag(cls, repo: git.Repo) -> None: - props = cls.get_ifcgit_props() - item = props.ifcgit_commits[props.commit_index] - repo.create_tag(props.new_tag_name, ref=item.hexsha, message=props.new_tag_message) - props.new_tag_name = "" - props.new_tag_message = "" + def add_tag(cls, repo: git.Repo, hexsha: str, tag_name: str, tag_message: str = "") -> None: + repo.create_tag(tag_name, ref=hexsha, message=tag_message) @classmethod def delete_tag(cls, repo: git.Repo, tag_name: git.TagReference) -> None: @@ -168,20 +156,13 @@ class IfcGit: repo.delete_tag(tag_name) @classmethod - def add_remote(cls, repo: git.Repo) -> None: - props = cls.get_ifcgit_props() - repo.create_remote(name=props.remote_name, url=props.remote_url) - props.remote_name = "" - props.remote_url = "" + def add_remote(cls, repo: git.Repo, remote_name: str, remote_url: str) -> None: + repo.create_remote(name=remote_name, url=remote_url) @classmethod - def delete_remote(cls, repo: git.Repo) -> None: - props = cls.get_ifcgit_props() - remote_name = props.select_remote + def delete_remote(cls, repo: git.Repo, remote_name: str) -> None: if remote_name in repo.remotes: repo.delete_remote(remote_name) - if repo.remotes: - props.select_remote = repo.remotes[0].name @classmethod def push(cls, repo: git.Repo, remote_name: str, branch_name: str) -> Union[str, None]: @@ -193,16 +174,25 @@ class IfcGit: return exc.stderr @classmethod - def create_new_branch(cls) -> None: - """Convert a detached HEAD into a branch""" - props = cls.get_ifcgit_props() - repo = IfcGitRepo.repo - new_branch = repo.create_head(props.new_branch_name) - new_branch.checkout() - props.display_branch = props.new_branch_name - props.new_branch_name = "" + def is_head_detached(cls) -> bool: + return bool(IfcGitRepo.repo.head.is_detached) - bpy.ops.ifcgit.refresh() + @classmethod + def repo_has_commits(cls) -> bool: + if IfcGitRepo.repo: + return bool(IfcGitRepo.repo.heads) + return False + + @classmethod + def get_active_branch_name(cls) -> str: + return IfcGitRepo.repo.active_branch.name + + @classmethod + def create_new_branch(cls, branch_name: str) -> None: + """Convert a detached HEAD into a branch""" + repo = IfcGitRepo.repo + new_branch = repo.create_head(branch_name) + new_branch.checkout() @classmethod def clear_commits_list(cls) -> None: @@ -243,6 +233,7 @@ class IfcGit: list_item.message = commit.message list_item.author_name = commit.author.name list_item.author_email = commit.author.email + list_item.committed_date = int(commit.committed_date) if commit in commits_relevant: list_item.relevant = True if commit.hexsha in lookup: @@ -286,6 +277,10 @@ class IfcGit: bpy.data.orphans_purge(do_recursive=True) # ty:ignore[unknown-argument] + from bonsai.bim.module.root.data import IfcClassData + + IfcClassData.is_loaded = False + settings = import_ifc.IfcImportSettings.factory(bpy.context, path_ifc, logging.getLogger("ImportIFC")) settings.should_setup_viewport_camera = False ifc_importer = import_ifc.IfcImporter(settings) @@ -469,15 +464,6 @@ class IfcGit: bpy.data.objects.remove(obj, do_unlink=True) bpy.data.collections.remove(blender_collection) - @classmethod - def is_valid_branch_name(cls, new_branch_name: str): - """Check if a branch name is valid and doesn't conflict with existing branches""" - if not cls.is_valid_ref_format(new_branch_name): - return False - if new_branch_name in [branch.name for branch in IfcGitRepo.repo.branches]: - return False - return True - @classmethod def config_ifcmerge(cls) -> None: config_reader = IfcGitRepo.repo.config_reader() @@ -519,49 +505,65 @@ class IfcGit: output.write(line + b"\n") @classmethod - def execute_merge(cls, path_ifc: str, operator: bpy.types.Operator) -> Union[None, Literal[False]]: + def get_selected_branch(cls) -> Union[str, None]: + """Return the name of the branch at the selected commit matching display_branch, or None.""" props = cls.get_ifcgit_props() repo = IfcGitRepo.repo item = props.ifcgit_commits[props.commit_index] lookup = cls.branches_by_hexsha(repo) - if item.hexsha in lookup: - for branch in lookup[item.hexsha]: - if branch.name == props.display_branch: - # this is a branch! - if re.match("^(origin/)?(HEAD|main|master)$", branch.name): - # preserve remote IDs in origin/main or main - mergetool = "ifcmerge" - else: - # rewrite remote IDs - mergetool = "ifcmerge-forward" - try: - # NOTE this is calling the git binary in a subprocess - repo.git.merge(branch) - except git.exc.GitCommandError: - # merge is expected to fail, run ifcmerge - try: - repo.git.mergetool(tool=mergetool) - except git.exc.GitCommandError as exc: - message = re.sub("( stderr: '|')", "", exc.stderr) - # ifcmerge failed, rollback - repo.git.merge(abort=True) + if item.hexsha not in lookup: + return None + for branch in lookup[item.hexsha]: + if branch.name == props.display_branch: + return branch.name + return None - operator.report({"ERROR"}, "IFC Merge failed:" + message) - return False - else: - if os.name == "nt": - cls.dos2unix(path_ifc) - repo.index.add(os.path.normpath(path_ifc)) - repo.git.commit("--no-edit") - except git.exc.GitError: - operator.report({"ERROR"}, "Unknown IFC Merge failure") - return False + @classmethod + def get_merge_tool(cls, branch_name: str) -> str: + if re.match("^(origin/)?(HEAD|main|master)$", branch_name): + return "ifcmerge" + return "ifcmerge-forward" - props.display_branch = repo.active_branch.name + @classmethod + def git_merge(cls, branch_name: str) -> Union[str, None]: + """Attempt a git merge. Returns None on clean merge, 'conflict' on expected + GitCommandError, or 'error' on an unknown GitError.""" + repo = IfcGitRepo.repo + branch = repo.branches[branch_name] + try: + repo.git.merge(branch) + return None + except git.exc.GitCommandError: + return "conflict" + except git.exc.GitError: + return "error" - cls.load_project(path_ifc) - cls.refresh_revision_list(path_ifc) - cls.decolourise() + @classmethod + def git_mergetool(cls, mergetool: str) -> Union[str, None]: + """Run ifcmerge tool. Returns None on success, error message string on failure.""" + repo = IfcGitRepo.repo + try: + repo.git.mergetool(tool=mergetool) + return None + except git.exc.GitCommandError as exc: + return re.sub("( stderr: '|')", "", exc.stderr) + + @classmethod + def git_merge_abort(cls) -> None: + IfcGitRepo.repo.git.merge(abort=True) + + @classmethod + def commit_merge(cls, path_ifc: str) -> None: + repo = IfcGitRepo.repo + if os.name == "nt": + cls.dos2unix(path_ifc) + repo.index.add(os.path.normpath(path_ifc)) + repo.git.commit("--no-edit") + + @classmethod + def set_display_branch(cls) -> None: + props = cls.get_ifcgit_props() + props.display_branch = IfcGitRepo.repo.active_branch.name @classmethod def entity_log(cls, path_ifc: str, step_id: int) -> str: @@ -589,6 +591,18 @@ class IfcGit: except FileNotFoundError: operator.report({"ERROR"}, "Winget is not available. Make sure Windows Package Manager is installed.") + @classmethod + def select_first_remote(cls) -> None: + props = cls.get_ifcgit_props() + repo = IfcGitRepo.repo + if repo and repo.remotes: + props.select_remote = repo.remotes[0].name + + @classmethod + def fetch(cls, remote_name: str) -> None: + repo = IfcGitRepo.repo + repo.remotes[remote_name].fetch() + @classmethod def run_git_diff(cls, operator: bpy.types.Operator, save_to_temp: bool) -> None: path = tool.Ifc.get_path() diff --git a/src/bonsai/test/core/bootstrap.py b/src/bonsai/test/core/bootstrap.py index 057ee9ffab..cd8371e1c3 100644 --- a/src/bonsai/test/core/bootstrap.py +++ b/src/bonsai/test/core/bootstrap.py @@ -102,6 +102,13 @@ def geometry(): prophet.verify() +@pytest.fixture +def ifcgit(): + prophet = Prophecy(bonsai.core.tool.IfcGit) + yield prophet + prophet.verify() + + @pytest.fixture def georeference(): prophet = Prophecy(bonsai.core.tool.Georeference) diff --git a/src/bonsai/test/core/test_ifcgit.py b/src/bonsai/test/core/test_ifcgit.py new file mode 100644 index 0000000000..b4d80985b2 --- /dev/null +++ b/src/bonsai/test/core/test_ifcgit.py @@ -0,0 +1,274 @@ +# Bonsai - OpenBIM Blender Add-on +# Copyright (C) 2025 Dion Moult +# +# 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. + +import pytest + +import bonsai.core.ifcgit as subject +from test.core.bootstrap import ifcgit, ifc + + +class MockOperator: + def __init__(self): + self.reports = [] + + def report(self, level, message): + self.reports.append((level, message)) + + +class TestCreateRepo: + def test_run(self, ifcgit, ifc): + ifc.get_path().should_be_called().will_return("path/to/model.ifc") + ifcgit.get_path_dir("path/to/model.ifc").should_be_called().will_return("path/to") + ifcgit.init_repo("path/to").should_be_called() + subject.create_repo(ifcgit, ifc) + + +class TestAddFile: + def test_run(self, ifcgit, ifc): + ifc.get_path().should_be_called().will_return("path/to/model.ifc") + ifcgit.repo_from_path("path/to/model.ifc").should_be_called().will_return("repo") + ifcgit.add_file_to_repo("repo", "path/to/model.ifc").should_be_called() + subject.add_file(ifcgit, ifc) + + +class TestCloneRepo: + def test_successful_clone(self, ifcgit): + ifcgit.clone_repo("http://example.com/repo.git", "/local/folder").should_be_called().will_return("repo") + ifcgit.load_anyifc("repo").should_be_called() + op = MockOperator() + subject.clone_repo(ifcgit, "http://example.com/repo.git", "/local/folder", operator=op) + assert op.reports == [({"INFO"}, "Repository cloned")] + + def test_failed_clone_reports_error(self, ifcgit): + ifcgit.clone_repo("http://example.com/repo.git", "/local/folder").should_be_called().will_return(None) + op = MockOperator() + subject.clone_repo(ifcgit, "http://example.com/repo.git", "/local/folder", operator=op) + assert op.reports == [({"ERROR"}, "Clone failed")] + + +class TestDiscardUncommitted: + def test_run(self, ifcgit, ifc): + ifc.get_path().should_be_called().will_return("path/to/model.ifc") + ifcgit.git_checkout("path/to/model.ifc").should_be_called() + ifcgit.load_project("path/to/model.ifc").should_be_called() + subject.discard_uncommitted(ifcgit, ifc) + + +class TestCommitChanges: + def test_commit_on_branch_without_new_branch(self, ifcgit, ifc): + ifc.get_path().should_be_called().will_return("path/to/model.ifc") + ifcgit.is_head_detached().should_be_called().will_return(False) + ifcgit.git_commit("path/to/model.ifc", "my message").should_be_called() + subject.commit_changes(ifcgit, ifc, "my message", "") + + def test_commit_on_branch_with_new_branch(self, ifcgit, ifc): + ifc.get_path().should_be_called().will_return("path/to/model.ifc") + ifcgit.is_head_detached().should_be_called().will_return(False) + ifcgit.checkout_new_branch("path/to/model.ifc", "feature").should_be_called() + ifcgit.git_commit("path/to/model.ifc", "my message").should_be_called() + subject.commit_changes(ifcgit, ifc, "my message", "feature") + + def test_commit_on_detached_head(self, ifcgit, ifc): + ifc.get_path().should_be_called().will_return("path/to/model.ifc") + ifcgit.is_head_detached().should_be_called().will_return(True) + ifcgit.git_commit("path/to/model.ifc", "my message").should_be_called() + ifcgit.create_new_branch("feature").should_be_called() + subject.commit_changes(ifcgit, ifc, "my message", "feature") + + +class TestAddTag: + def test_run(self, ifcgit): + ifcgit.add_tag("repo", "abc123", "v1.0", "Release notes").should_be_called() + subject.add_tag(ifcgit, "repo", "abc123", "v1.0", "Release notes") + + +class TestDeleteTag: + def test_run(self, ifcgit): + ifcgit.delete_tag("repo", "v1.0").should_be_called() + subject.delete_tag(ifcgit, "repo", "v1.0") + + +class TestAddRemote: + def test_run(self, ifcgit): + ifcgit.add_remote("repo", "origin", "http://example.com").should_be_called() + subject.add_remote(ifcgit, "repo", "origin", "http://example.com") + + +class TestDeleteRemote: + def test_run(self, ifcgit): + ifcgit.delete_remote("repo", "origin").should_be_called() + subject.delete_remote(ifcgit, "repo", "origin") + + +class TestPush: + def test_push_succeeds_silently(self, ifcgit): + ifcgit.get_active_branch_name().should_be_called().will_return("main") + ifcgit.push("repo", "origin", "main").should_be_called().will_return(None) + subject.push(ifcgit, "repo", "origin", operator=None) + + def test_push_failure_reports_error(self, ifcgit): + ifcgit.get_active_branch_name().should_be_called().will_return("main") + ifcgit.push("repo", "origin", "main").should_be_called().will_return("stderr: rejected") + op = MockOperator() + subject.push(ifcgit, "repo", "origin", operator=op) + assert op.reports == [({"ERROR"}, "stderr: rejected")] + + +class TestRefreshRevisionList: + def test_refreshes_when_repo_has_heads(self, ifcgit, ifc): + ifcgit.repo_has_commits().should_be_called().will_return(True) + ifc.get_path().should_be_called().will_return("path/to/model.ifc") + ifcgit.refresh_revision_list("path/to/model.ifc").should_be_called() + subject.refresh_revision_list(ifcgit, ifc) + + def test_skips_when_repo_has_no_heads(self, ifcgit, ifc): + ifcgit.repo_has_commits().should_be_called().will_return(False) + subject.refresh_revision_list(ifcgit, ifc) + # nothing else should be called — Prophecy will verify + + +class TestColouriseRevision: + def test_skips_when_no_step_ids(self, ifcgit): + ifcgit.get_revisions_step_ids().should_be_called().will_return(None) + subject.colourise_revision(ifcgit) + + def test_colourises_with_step_ids(self, ifcgit): + ifcgit.get_revisions_step_ids().should_be_called().will_return("step_ids") + ifcgit.get_modified_step_ids("step_ids").should_be_called().will_return("modified_step_ids") + ifcgit.update_step_ids("step_ids", "modified_step_ids").should_be_called().will_return("final_step_ids") + ifcgit.colourise("final_step_ids").should_be_called() + subject.colourise_revision(ifcgit) + + +class TestColouriseUncommitted: + def test_skips_when_no_step_ids(self, ifcgit, ifc): + ifc.get_path().should_be_called().will_return("path/to/model.ifc") + ifcgit.ifc_diff_ids("repo", None, "HEAD", "path/to/model.ifc").should_be_called().will_return(None) + subject.colourise_uncommitted(ifcgit, ifc, "repo") + + def test_colourises_with_step_ids(self, ifcgit, ifc): + ifc.get_path().should_be_called().will_return("path/to/model.ifc") + ifcgit.ifc_diff_ids("repo", None, "HEAD", "path/to/model.ifc").should_be_called().will_return("step_ids") + ifcgit.get_modified_step_ids("step_ids").should_be_called().will_return("modified_step_ids") + ifcgit.update_step_ids("step_ids", "modified_step_ids").should_be_called().will_return("final_step_ids") + ifcgit.colourise("final_step_ids").should_be_called() + subject.colourise_uncommitted(ifcgit, ifc, "repo") + + +class TestSwitchRevision: + def test_run(self, ifcgit, ifc): + ifc.get_path().should_be_called().will_return("path/to/model.ifc") + ifcgit.switch_to_revision_item().should_be_called() + ifcgit.load_project("path/to/model.ifc").should_be_called() + ifcgit.refresh_revision_list("path/to/model.ifc").should_be_called() + ifcgit.decolourise().should_be_called() + subject.switch_revision(ifcgit, ifc) + + +class TestMergeBranch: + def test_no_branch_at_selected_commit(self, ifcgit, ifc): + ifc.get_path().should_be_called().will_return("path/to/model.ifc") + ifcgit.config_ifcmerge().should_be_called() + ifcgit.get_selected_branch().should_be_called().will_return(None) + subject.merge_branch(ifcgit, ifc, operator=None) + + def test_clean_merge(self, ifcgit, ifc): + ifc.get_path().should_be_called().will_return("path/to/model.ifc") + ifcgit.config_ifcmerge().should_be_called() + ifcgit.get_selected_branch().should_be_called().will_return("feature") + ifcgit.get_merge_tool("feature").should_be_called().will_return("ifcmerge-forward") + ifcgit.git_merge("feature").should_be_called().will_return(None) + ifcgit.set_display_branch().should_be_called() + ifcgit.load_project("path/to/model.ifc").should_be_called() + ifcgit.refresh_revision_list("path/to/model.ifc").should_be_called() + ifcgit.decolourise().should_be_called() + subject.merge_branch(ifcgit, ifc, operator=None) + + def test_conflict_mergetool_success(self, ifcgit, ifc): + ifc.get_path().should_be_called().will_return("path/to/model.ifc") + ifcgit.config_ifcmerge().should_be_called() + ifcgit.get_selected_branch().should_be_called().will_return("feature") + ifcgit.get_merge_tool("feature").should_be_called().will_return("ifcmerge-forward") + ifcgit.git_merge("feature").should_be_called().will_return("conflict") + ifcgit.git_mergetool("ifcmerge-forward").should_be_called().will_return(None) + ifcgit.commit_merge("path/to/model.ifc").should_be_called() + ifcgit.set_display_branch().should_be_called() + ifcgit.load_project("path/to/model.ifc").should_be_called() + ifcgit.refresh_revision_list("path/to/model.ifc").should_be_called() + ifcgit.decolourise().should_be_called() + subject.merge_branch(ifcgit, ifc, operator=None) + + def test_conflict_mergetool_failure(self, ifcgit, ifc): + ifc.get_path().should_be_called().will_return("path/to/model.ifc") + ifcgit.config_ifcmerge().should_be_called() + ifcgit.get_selected_branch().should_be_called().will_return("feature") + ifcgit.get_merge_tool("feature").should_be_called().will_return("ifcmerge-forward") + ifcgit.git_merge("feature").should_be_called().will_return("conflict") + ifcgit.git_mergetool("ifcmerge-forward").should_be_called().will_return("merge error") + ifcgit.git_merge_abort().should_be_called() + op = MockOperator() + subject.merge_branch(ifcgit, ifc, op) + assert op.reports == [({"ERROR"}, "IFC Merge failed:merge error")] + + def test_unknown_merge_error(self, ifcgit, ifc): + ifc.get_path().should_be_called().will_return("path/to/model.ifc") + ifcgit.config_ifcmerge().should_be_called() + ifcgit.get_selected_branch().should_be_called().will_return("feature") + ifcgit.get_merge_tool("feature").should_be_called().will_return("ifcmerge-forward") + ifcgit.git_merge("feature").should_be_called().will_return("error") + op = MockOperator() + subject.merge_branch(ifcgit, ifc, op) + assert op.reports == [({"ERROR"}, "Unknown IFC Merge failure")] + + +class TestEntityLog: + def test_run(self, ifcgit, ifc): + ifc.get_path().should_be_called().will_return("path/to/model.ifc") + ifcgit.entity_log("path/to/model.ifc", 42).should_be_called().will_return("log text") + op = MockOperator() + subject.entity_log(ifcgit, ifc, 42, op) + assert op.reports == [({"ERROR"}, "log text")] + + +class TestInstallGit: + def test_windows(self, ifcgit): + import unittest.mock as mock + + with mock.patch("platform.system", return_value="Windows"): + ifcgit.install_git_windows(operator="op").should_be_called() + subject.install_git(ifcgit, "op") + + def test_non_windows_does_nothing(self, ifcgit): + import unittest.mock as mock + + with mock.patch("platform.system", return_value="Linux"): + subject.install_git(ifcgit, "op") + # no tool method should be called — Prophecy will verify + + +class TestFetch: + def test_run(self, ifcgit): + ifcgit.fetch("origin").should_be_called() + subject.fetch(ifcgit, "origin") + + +class TestRunGitDiff: + def test_run(self, ifcgit): + ifcgit.run_git_diff("operator", False).should_be_called() + subject.run_git_diff(ifcgit, "operator", False) diff --git a/src/bonsai/test/tool/test_ifcgit.py b/src/bonsai/test/tool/test_ifcgit.py new file mode 100644 index 0000000000..5cd051aa5f --- /dev/null +++ b/src/bonsai/test/tool/test_ifcgit.py @@ -0,0 +1,456 @@ +# Bonsai - OpenBIM Blender Add-on +# Copyright (C) 2025 Dion Moult +# +# 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. + +import os +import tempfile + +import bonsai.core.tool +from bonsai.tool.ifcgit import IfcGit, IfcGitRepo +from test.bim.bootstrap import NewFile + +try: + import git + import git.exc + + HAS_GIT = True +except ImportError: + HAS_GIT = False + +import pytest + +requires_git = pytest.mark.skipif(not HAS_GIT, reason="GitPython not available") + + +def _make_repo(tmpdir: str) -> "git.Repo": + """Initialise a git repo with user config (needed for commits).""" + repo = git.Repo.init(tmpdir) + with repo.config_writer() as cfg: + cfg.set_value("user", "name", "Test User") + cfg.set_value("user", "email", "test@example.com") + return repo + + +def _commit_ifc(repo: "git.Repo", tmpdir: str, content: str, message: str) -> str: + """Write content to model.ifc, stage and commit it. Returns commit hexsha.""" + ifc_path = os.path.join(tmpdir, "model.ifc") + with open(ifc_path, "w") as f: + f.write(content) + repo.index.add([os.path.normpath(ifc_path)]) + commit = repo.index.commit(message) + return commit.hexsha + + +# --------------------------------------------------------------------------- +# Interface conformance +# --------------------------------------------------------------------------- + + +class TestImplementsTool(NewFile): + def test_run(self): + assert isinstance(IfcGit(), bonsai.core.tool.IfcGit) + + +# --------------------------------------------------------------------------- +# Pure-logic (no git required) +# --------------------------------------------------------------------------- + + +class TestIsValidRefFormat(NewFile): + def test_simple_name(self): + assert IfcGit.is_valid_ref_format("main") + + def test_name_with_slash(self): + assert IfcGit.is_valid_ref_format("feature/my-feature") + + def test_name_with_numbers(self): + assert IfcGit.is_valid_ref_format("release-2024") + + def test_rejects_leading_dot(self): + assert not IfcGit.is_valid_ref_format(".hidden") + + def test_rejects_leading_dash(self): + assert not IfcGit.is_valid_ref_format("-branch") + + def test_rejects_space(self): + assert not IfcGit.is_valid_ref_format("my branch") + + def test_rejects_double_dot(self): + assert not IfcGit.is_valid_ref_format("my..branch") + + def test_rejects_tilde(self): + assert not IfcGit.is_valid_ref_format("my~branch") + + def test_rejects_caret(self): + assert not IfcGit.is_valid_ref_format("my^branch") + + def test_rejects_trailing_dot(self): + assert not IfcGit.is_valid_ref_format("branch.") + + def test_rejects_trailing_slash(self): + assert not IfcGit.is_valid_ref_format("branch/") + + def test_rejects_dot_lock_suffix(self): + assert not IfcGit.is_valid_ref_format("branch.lock") + + def test_rejects_empty_string(self): + assert not IfcGit.is_valid_ref_format("") + + def test_rejects_at_brace(self): + assert not IfcGit.is_valid_ref_format("branch@{upstream}") + + +class TestGetPathDir(NewFile): + def test_returns_parent_directory(self): + assert IfcGit.get_path_dir("/some/path/model.ifc") == "/some/path" + + def test_handles_nested_path(self): + assert IfcGit.get_path_dir("/a/b/c/d.ifc") == "/a/b/c" + + def test_returns_absolute_path(self): + result = IfcGit.get_path_dir("/a/b/model.ifc") + assert os.path.isabs(result) + + +# --------------------------------------------------------------------------- +# File I/O +# --------------------------------------------------------------------------- + + +class TestDos2Unix(NewFile): + def test_converts_crlf_to_lf(self): + with tempfile.NamedTemporaryFile(suffix=".ifc", delete=False, mode="wb") as f: + f.write(b"line1\r\nline2\r\nline3\r\n") + path = f.name + try: + IfcGit.dos2unix(path) + with open(path, "rb") as f: + assert f.read() == b"line1\nline2\nline3\n" + finally: + os.unlink(path) + + def test_lf_only_file_is_unchanged(self): + with tempfile.NamedTemporaryFile(suffix=".ifc", delete=False, mode="wb") as f: + f.write(b"line1\nline2\nline3\n") + path = f.name + try: + IfcGit.dos2unix(path) + with open(path, "rb") as f: + assert f.read() == b"line1\nline2\nline3\n" + finally: + os.unlink(path) + + def test_empty_file_unchanged(self): + with tempfile.NamedTemporaryFile(suffix=".ifc", delete=False, mode="wb") as f: + f.write(b"") + path = f.name + try: + IfcGit.dos2unix(path) + with open(path, "rb") as f: + assert f.read() == b"" + finally: + os.unlink(path) + + +# --------------------------------------------------------------------------- +# Git repo initialisation +# --------------------------------------------------------------------------- + + +class TestInitRepo(NewFile): + @requires_git + def test_creates_git_repo(self): + with tempfile.TemporaryDirectory() as tmpdir: + IfcGitRepo.repo = None + IfcGit.init_repo(tmpdir) + assert IfcGitRepo.repo is not None + assert os.path.isdir(IfcGitRepo.repo.git_dir) + IfcGitRepo.repo = None + + @requires_git + def test_creates_info_attributes_file(self): + with tempfile.TemporaryDirectory() as tmpdir: + IfcGitRepo.repo = None + IfcGit.init_repo(tmpdir) + attrs_path = os.path.join(IfcGitRepo.repo.git_dir, "info", "attributes") + assert os.path.isfile(attrs_path) + with open(attrs_path) as f: + assert "*.ifc text" in f.read() + IfcGitRepo.repo = None + + +class TestRepoFromPath(NewFile): + @requires_git + def test_finds_repo_from_file_in_root(self): + with tempfile.TemporaryDirectory() as tmpdir: + IfcGitRepo.repo = None + repo = _make_repo(tmpdir) + ifc_path = os.path.join(tmpdir, "model.ifc") + open(ifc_path, "w").close() + result = IfcGit.repo_from_path(ifc_path) + assert result is not None + assert os.path.abspath(result.working_dir) == os.path.abspath(tmpdir) + IfcGitRepo.repo = None + + @requires_git + def test_finds_repo_from_subdirectory(self): + with tempfile.TemporaryDirectory() as tmpdir: + IfcGitRepo.repo = None + _make_repo(tmpdir) + subdir = os.path.join(tmpdir, "sub", "dir") + os.makedirs(subdir) + result = IfcGit.repo_from_path(subdir) + assert result is not None + IfcGitRepo.repo = None + + @requires_git + def test_returns_none_for_path_outside_any_repo(self): + with tempfile.TemporaryDirectory() as tmpdir: + IfcGitRepo.repo = None + # a plain directory with no .git anywhere above it (using /tmp directly + # is safe since /tmp is not a git repo on this system) + result = IfcGit.repo_from_path("/nonexistent/path/that/does/not/exist") + assert result is None + IfcGitRepo.repo = None + + +# --------------------------------------------------------------------------- +# Git repo configuration +# --------------------------------------------------------------------------- + + +class TestConfigInfoAttributes(NewFile): + @requires_git + def test_creates_attributes_file(self): + with tempfile.TemporaryDirectory() as tmpdir: + repo = _make_repo(tmpdir) + IfcGit.config_info_attributes(repo) + attrs_path = os.path.join(repo.git_dir, "info", "attributes") + assert os.path.isfile(attrs_path) + with open(attrs_path) as f: + assert "*.ifc text" in f.read() + + @requires_git + def test_does_not_overwrite_existing_attributes(self): + with tempfile.TemporaryDirectory() as tmpdir: + repo = _make_repo(tmpdir) + attrs_path = os.path.join(repo.git_dir, "info", "attributes") + os.makedirs(os.path.dirname(attrs_path), exist_ok=True) + with open(attrs_path, "w") as f: + f.write("*.png binary\n") + IfcGit.config_info_attributes(repo) + with open(attrs_path) as f: + content = f.read() + assert "*.png binary" in content # original content preserved + + +class TestConfigPush(NewFile): + @requires_git + def test_sets_push_defaults(self): + with tempfile.TemporaryDirectory() as tmpdir: + repo = _make_repo(tmpdir) + IfcGit.config_push(repo) + reader = repo.config_reader() + assert reader.get_value("push", "default") == "current" + assert reader.get_value("push", "autoSetupRemote") is True + + @requires_git + def test_does_not_overwrite_existing_push_section(self): + with tempfile.TemporaryDirectory() as tmpdir: + repo = _make_repo(tmpdir) + with repo.config_writer() as w: + w.set_value("push", "default", "simple") + IfcGit.config_push(repo) + reader = repo.config_reader() + assert reader.get_value("push", "default") == "simple" + + +# --------------------------------------------------------------------------- +# Branch / tag lookups +# --------------------------------------------------------------------------- + + +class TestBranchesByHexsha(NewFile): + @requires_git + def test_maps_head_commit_to_active_branch(self): + with tempfile.TemporaryDirectory() as tmpdir: + repo = _make_repo(tmpdir) + _commit_ifc(repo, tmpdir, "#1=IFCPROJECT('abc',$,$,$,$,$,$,$,$);\n", "init") + result = IfcGit.branches_by_hexsha(repo) + head_sha = repo.head.commit.hexsha + assert head_sha in result + names = [b.name for b in result[head_sha]] + assert repo.active_branch.name in names + + @requires_git + def test_returns_empty_dict_when_no_commits(self): + with tempfile.TemporaryDirectory() as tmpdir: + repo = _make_repo(tmpdir) + result = IfcGit.branches_by_hexsha(repo) + assert result == {} + + @requires_git + def test_includes_both_branches_when_pointing_to_same_commit(self): + with tempfile.TemporaryDirectory() as tmpdir: + repo = _make_repo(tmpdir) + _commit_ifc(repo, tmpdir, "#1=IFCPROJECT('abc',$,$,$,$,$,$,$,$);\n", "init") + repo.create_head("feature") + result = IfcGit.branches_by_hexsha(repo) + head_sha = repo.head.commit.hexsha + names = [b.name for b in result[head_sha]] + assert len(names) == 2 + + +class TestTagsByHexsha(NewFile): + @requires_git + def test_returns_empty_dict_when_no_tags(self): + with tempfile.TemporaryDirectory() as tmpdir: + repo = _make_repo(tmpdir) + _commit_ifc(repo, tmpdir, "#1=IFCPROJECT('abc',$,$,$,$,$,$,$,$);\n", "init") + assert IfcGit.tags_by_hexsha(repo) == {} + + @requires_git + def test_maps_commit_to_annotated_tag(self): + with tempfile.TemporaryDirectory() as tmpdir: + repo = _make_repo(tmpdir) + _commit_ifc(repo, tmpdir, "#1=IFCPROJECT('abc',$,$,$,$,$,$,$,$);\n", "init") + repo.create_tag("v1.0", message="Release 1.0") + result = IfcGit.tags_by_hexsha(repo) + head_sha = repo.head.commit.hexsha + assert head_sha in result + assert result[head_sha][0].name == "v1.0" + + @requires_git + def test_maps_commit_to_lightweight_tag(self): + with tempfile.TemporaryDirectory() as tmpdir: + repo = _make_repo(tmpdir) + _commit_ifc(repo, tmpdir, "#1=IFCPROJECT('abc',$,$,$,$,$,$,$,$);\n", "init") + repo.create_tag("v0.1") + result = IfcGit.tags_by_hexsha(repo) + head_sha = repo.head.commit.hexsha + assert head_sha in result + + +class TestDeleteTag(NewFile): + @requires_git + def test_removes_existing_tag(self): + with tempfile.TemporaryDirectory() as tmpdir: + repo = _make_repo(tmpdir) + _commit_ifc(repo, tmpdir, "#1=IFCPROJECT('abc',$,$,$,$,$,$,$,$);\n", "init") + repo.create_tag("v1.0") + IfcGit.delete_tag(repo, "v1.0") + assert "v1.0" not in [t.name for t in repo.tags] + + @requires_git + def test_does_not_raise_for_nonexistent_tag(self): + with tempfile.TemporaryDirectory() as tmpdir: + repo = _make_repo(tmpdir) + _commit_ifc(repo, tmpdir, "#1=IFCPROJECT('abc',$,$,$,$,$,$,$,$);\n", "init") + IfcGit.delete_tag(repo, "does-not-exist") # must not raise + + +# --------------------------------------------------------------------------- +# IFC diff parsing +# --------------------------------------------------------------------------- + + +class TestIfcDiffIds(NewFile): + @requires_git + def test_detects_modified_entity(self): + with tempfile.TemporaryDirectory() as tmpdir: + repo = _make_repo(tmpdir) + sha_a = _commit_ifc(repo, tmpdir, "#1=IFCPROJECT('abc',$,$,$,$,$,$,$,$);\n", "init") + ifc_path = os.path.join(tmpdir, "model.ifc") + sha_b = _commit_ifc(repo, tmpdir, "#1=IFCPROJECT('xyz',$,$,$,$,$,$,$,$);\n", "update") + result = IfcGit.ifc_diff_ids(repo, sha_a, sha_b, ifc_path) + assert 1 in result["modified"] + assert result["added"] == set() + assert result["removed"] == set() + + @requires_git + def test_detects_added_entity(self): + with tempfile.TemporaryDirectory() as tmpdir: + repo = _make_repo(tmpdir) + sha_a = _commit_ifc(repo, tmpdir, "#1=IFCPROJECT('abc',$,$,$,$,$,$,$,$);\n", "init") + ifc_path = os.path.join(tmpdir, "model.ifc") + sha_b = _commit_ifc( + repo, + tmpdir, + "#1=IFCPROJECT('abc',$,$,$,$,$,$,$,$);\n#2=IFCSITE('new',$,$,$,$,$,$,$,$,$,$,$,$);\n", + "add site", + ) + result = IfcGit.ifc_diff_ids(repo, sha_a, sha_b, ifc_path) + assert 2 in result["added"] + assert 1 not in result["modified"] + assert result["removed"] == set() + + @requires_git + def test_detects_removed_entity(self): + with tempfile.TemporaryDirectory() as tmpdir: + repo = _make_repo(tmpdir) + sha_a = _commit_ifc(repo, tmpdir, "#1=IFCPROJECT('abc',$,$,$,$,$,$,$,$);\n", "init") + ifc_path = os.path.join(tmpdir, "model.ifc") + sha_b = _commit_ifc(repo, tmpdir, "", "clear") + result = IfcGit.ifc_diff_ids(repo, sha_a, sha_b, ifc_path) + assert 1 in result["removed"] + assert result["added"] == set() + assert result["modified"] == set() + + @requires_git + def test_no_changes_between_identical_commits(self): + with tempfile.TemporaryDirectory() as tmpdir: + repo = _make_repo(tmpdir) + sha = _commit_ifc(repo, tmpdir, "#1=IFCPROJECT('abc',$,$,$,$,$,$,$,$);\n", "init") + ifc_path = os.path.join(tmpdir, "model.ifc") + result = IfcGit.ifc_diff_ids(repo, sha, sha, ifc_path) + assert result["modified"] == set() + assert result["added"] == set() + assert result["removed"] == set() + + @requires_git + def test_diff_against_working_tree_with_none_hash_a(self): + with tempfile.TemporaryDirectory() as tmpdir: + repo = _make_repo(tmpdir) + _commit_ifc(repo, tmpdir, "#1=IFCPROJECT('abc',$,$,$,$,$,$,$,$);\n", "init") + ifc_path = os.path.join(tmpdir, "model.ifc") + # Make an uncommitted change in working tree + with open(ifc_path, "w") as f: + f.write("#1=IFCPROJECT('xyz',$,$,$,$,$,$,$,$);\n") + result = IfcGit.ifc_diff_ids(repo, None, "HEAD", ifc_path) + assert 1 in result["modified"] + + @requires_git + def test_handles_multiple_changed_entities(self): + with tempfile.TemporaryDirectory() as tmpdir: + repo = _make_repo(tmpdir) + sha_a = _commit_ifc( + repo, + tmpdir, + "#1=IFCPROJECT('abc',$,$,$,$,$,$,$,$);\n#2=IFCSITE('s',$,$,$,$,$,$,$,$,$,$,$,$);\n", + "init", + ) + ifc_path = os.path.join(tmpdir, "model.ifc") + sha_b = _commit_ifc( + repo, + tmpdir, + "#1=IFCPROJECT('xyz',$,$,$,$,$,$,$,$);\n#2=IFCSITE('t',$,$,$,$,$,$,$,$,$,$,$,$);\n", + "update both", + ) + result = IfcGit.ifc_diff_ids(repo, sha_a, sha_b, ifc_path) + assert 1 in result["modified"] + assert 2 in result["modified"] From fdb29473452e6f90a384dbc9daf751ca097fa7f9 Mon Sep 17 00:00:00 2001 From: Ryan Schultz Date: Wed, 1 Apr 2026 19:45:22 -0500 Subject: [PATCH 02/25] Add git branch to system info debug output Include bonsai_git_branch in get_debug_info(). For dev environments using the GitPython-based update_commit_data() path, the branch is read from repo.active_branch.name. For built extensions, a 7777777 placeholder is replaced at build time via the Makefile, matching the existing pattern for bonsai_commit_hash and bonsai_commit_date. Generated with the assistance of an AI coding tool. --- src/bonsai/Makefile | 2 ++ src/bonsai/bonsai/__init__.py | 13 +++++++++++++ 2 files changed, 15 insertions(+) diff --git a/src/bonsai/Makefile b/src/bonsai/Makefile index f21f1b42ac..60ed304532 100644 --- a/src/bonsai/Makefile +++ b/src/bonsai/Makefile @@ -48,6 +48,7 @@ VERSION_PATCH:=$(shell cat '../../VERSION' | cut -d '.' -f 3) VERSION_DATE:=$(shell date '+%y%m%d') LAST_COMMIT_HASH:=$(shell git rev-parse HEAD) LAST_COMMIT_DATE:=$(shell git show -s --format=%cI) +LAST_GIT_BRANCH:=$(shell git rev-parse --abbrev-ref HEAD) PYPI_IMP:=cp ifdef PYVERSION @@ -261,6 +262,7 @@ else $(SED) "s/0.0.0/$(VERSION)-alpha$(VERSION_DATE)/" build/bonsai/blender_manifest.toml $(SED) "s/8888888/$(LAST_COMMIT_HASH)/" build/bonsai/__init__.py $(SED) "s/9999999/$(LAST_COMMIT_DATE)/" build/bonsai/__init__.py + $(SED) "s/7777777/$(LAST_GIT_BRANCH)/" build/bonsai/__init__.py $(SED) 's/version = "0.0.0"/version = "$(VERSION)-alpha$(VERSION_DATE)"/' build/pyproject.toml endif diff --git a/src/bonsai/bonsai/__init__.py b/src/bonsai/bonsai/__init__.py index 915071855f..c6ec6405c0 100644 --- a/src/bonsai/bonsai/__init__.py +++ b/src/bonsai/bonsai/__init__.py @@ -43,6 +43,7 @@ from typing import TYPE_CHECKING, Any, Union last_commit_hash = "8888888" last_commit_date = "9999999" +last_git_branch = "7777777" def get_last_commit_hash() -> Union[str, None]: @@ -60,6 +61,15 @@ def get_last_commit_date() -> Union[str, None]: return last_commit_date +def get_git_branch() -> Union[str, None]: + # Using this weird way to write 7777777, + # so makefile won't accidentally replace it here + # we'll be able to distinguish branch from placeholder value. + if last_git_branch == str(7_777777): + return None + return last_git_branch + + # Accessed from bonsai extension: bbim_semver: dict[str, Any] = {} @@ -125,6 +135,7 @@ def get_debug_info(*, bonsai_failed_to_load: bool = False) -> dict[str, Any]: "bonsai_version": bbim_version, "bonsai_commit_hash": get_last_commit_hash(), "bonsai_commit_date": get_last_commit_date(), + "bonsai_git_branch": get_git_branch(), "last_actions": last_actions, "last_error": last_error, } @@ -251,10 +262,12 @@ if IN_BLENDER: global last_commit_hash global last_commit_date + global last_git_branch path = Path(__file__).resolve().parent repo = git.Repo(str(path), search_parent_directories=True) last_commit_hash = repo.head.object.hexsha last_commit_date = repo.head.object.committed_datetime.isoformat() + last_git_branch = repo.active_branch.name except: pass From 9d42f4c1eee7fc5ed928f3ec3a8416ae76d529c7 Mon Sep 17 00:00:00 2001 From: Bruno Postle Date: Thu, 2 Apr 2026 07:54:20 +0100 Subject: [PATCH 03/25] Update Bonsai to latest ifcmerge (#7581 #3096) This version has some functional differences: - Structured JSON error message instead of free text (on STDOUT not STDERR) - New --prioritise-local flag to control which side wins in merge conflicts (not used by Bonsai yet) - IfcLocalPlacement conflicts now auto-resolve instead of failing the merge (partial solution to #6885) - Float values are normalised when comparing entities (workaround for #7696) --- src/bonsai/Makefile | 6 +++--- src/bonsai/bonsai/tool/ifcgit.py | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/bonsai/Makefile b/src/bonsai/Makefile index 60ed304532..b517bc572b 100644 --- a/src/bonsai/Makefile +++ b/src/bonsai/Makefile @@ -64,6 +64,7 @@ PYNUMBER:=3$(PYMINOR) PYPI_VERSION:=3.$(PYMINOR) endif # def PYVERSION +IFCMERGE_VERSION:=2026-04-02 ifdef PLATFORM SUPPORTED_PLATFORMS := linux macos macosm1 win @@ -240,10 +241,9 @@ endif # required for three-way git merging ifeq ($(PLATFORM), win) - cd build/bonsai/libs/bin && wget https://github.com/brunopostle/ifcmerge/releases/download/2025-01-26/ifcmerge.zip - cd build/bonsai/libs/bin && unzip ifcmerge.zip && rm ifcmerge.zip + cd build/bonsai/libs/bin && wget https://github.com/brunopostle/ifcmerge/releases/download/$(IFCMERGE_VERSION)/ifcmerge.exe else - cd build/bonsai/libs/bin && wget https://raw.githubusercontent.com/brunopostle/ifcmerge/main/ifcmerge && chmod +x ifcmerge + cd build/bonsai/libs/bin && wget https://raw.githubusercontent.com/brunopostle/ifcmerge/$(IFCMERGE_VERSION)/ifcmerge && chmod +x ifcmerge endif # Generate translations module for Bonsai build diff --git a/src/bonsai/bonsai/tool/ifcgit.py b/src/bonsai/bonsai/tool/ifcgit.py index afff9fa3dc..1843716f58 100644 --- a/src/bonsai/bonsai/tool/ifcgit.py +++ b/src/bonsai/bonsai/tool/ifcgit.py @@ -546,7 +546,7 @@ class IfcGit: repo.git.mergetool(tool=mergetool) return None except git.exc.GitCommandError as exc: - return re.sub("( stderr: '|')", "", exc.stderr) + return re.sub("( stdout: '|')", "", exc.stdout) @classmethod def git_merge_abort(cls) -> None: From 95851ff94c82f647313f9a98843dcf0d3cdd69f7 Mon Sep 17 00:00:00 2001 From: DesertSpringsCivil Date: Thu, 2 Apr 2026 18:50:27 -0600 Subject: [PATCH 04/25] feat: Add Anthropic Claude API support to ifcchat Add a provider selector (OpenAI / Anthropic) to the ifcchat web UI, allowing users to use their Anthropic API key with Claude models instead of only OpenAI. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/ifcchat/app.js | 198 +++++++++++++++++++++++++++++++++++++---- src/ifcchat/index.html | 26 ++++-- 2 files changed, 203 insertions(+), 21 deletions(-) diff --git a/src/ifcchat/app.js b/src/ifcchat/app.js index 516ec8f846..541f3aef43 100644 --- a/src/ifcchat/app.js +++ b/src/ifcchat/app.js @@ -7,10 +7,57 @@ const sendBtn = $("send"); const inputEl = $("input"); const apiKeyEl = $("apiKey"); const modelEl = $("model"); +const providerEl = $("provider"); const ifcFileEl = $("ifcFile"); const newBtn = $("newModel"); const downloadBtn = $("downloadIfc"); +// ---- Provider switching ---- + +const PROVIDER_MODELS = { + openai: ["gpt-5", "gpt-4.1"], + anthropic: ["claude-sonnet-4-6", "claude-opus-4-6", "claude-haiku-4-5-20251001"], +}; + +const PROVIDER_LABELS = { + openai: "OpenAI API key", + anthropic: "Anthropic API key", +}; + +const PROVIDER_PLACEHOLDERS = { + openai: "sk-...", + anthropic: "sk-ant-...", +}; + +function getProvider() { + return providerEl.value; +} + +function updateProviderUI() { + const provider = getProvider(); + $("apiKeyLabel").innerHTML = `${PROVIDER_LABELS[provider]}stored in browser memory; only sent to provider servers`; + apiKeyEl.placeholder = PROVIDER_PLACEHOLDERS[provider]; + + // Enable/disable model optgroups and select first available model + for (const [key, models] of Object.entries(PROVIDER_MODELS)) { + const group = $(`modelGroup${key.charAt(0).toUpperCase() + key.slice(1)}`); + if (!group) continue; + const isActive = key === provider; + group.disabled = !isActive; + for (const opt of group.querySelectorAll("option")) { + opt.disabled = !isActive; + } + } + + modelEl.value = PROVIDER_MODELS[provider][0]; + + // Reset conversation on provider switch + openaiInputItems = []; + anthropicMessages = []; +} + +providerEl.addEventListener("change", updateProviderUI); + function setBusy(isBusy, reason = "") { const controls = [ $("send"), @@ -75,8 +122,7 @@ function callWorker(type, payload = {}) { }); } -// ---- OpenAI Responses API tool schemas (should match ifcmcp.core openai_tools()) ---- -// Docs show Responses API function_call items + function_call_output loop. :contentReference[oaicite:4]{index=4} +// ---- Tool schemas (OpenAI Responses API format) ---- const tools = [ { type: "function", name: "ifc_new", description: "Create a new empty IFC model in memory.", @@ -146,6 +192,15 @@ const tools = [ }, ]; +// Convert OpenAI tool format to Anthropic tool format +function toolsForAnthropic() { + return tools.map((t) => ({ + name: t.name, + description: t.description, + input_schema: t.parameters, + })); +} + const SYSTEM_INSTRUCTIONS = ` You are an IFC copilot running in a browser. You can call tools to inspect or modify the currently loaded IFC model. Rules: @@ -156,7 +211,15 @@ Rules: Be concise. Avoid dumping huge trees unless asked. `; -let inputItems = []; // running conversation state (Responses API style) +// ---- OpenAI state ---- +let openaiInputItems = []; + +// ---- Anthropic state ---- +let anthropicMessages = []; + +// ============================================================ +// OpenAI Responses API +// ============================================================ async function openAIResponsesCreate({ apiKey, model, input, tools }) { const res = await fetch("https://api.openai.com/v1/responses", { @@ -180,7 +243,7 @@ async function openAIResponsesCreate({ apiKey, model, input, tools }) { return await res.json(); } -function extractAssistantText(response) { +function extractAssistantTextOpenAI(response) { const out = []; for (const item of response.output ?? []) { if (item.type === "message" && item.role === "assistant") { @@ -192,27 +255,23 @@ function extractAssistantText(response) { return out.join("\n").trim(); } -async function runAgentTurn(userText) { +async function runOpenAITurn(userText) { const apiKey = apiKeyEl.value.trim(); if (!apiKey) throw new Error("Missing API key"); - // Add user message - inputItems.push({ role: "user", content: userText }); + openaiInputItems.push({ role: "user", content: userText }); - // Tool-calling loop (Responses API): append response.output, execute function_call items, append function_call_output. for (let i = 0; i < 64; i++) { const response = await openAIResponsesCreate({ apiKey, model: modelEl.value, - input: inputItems, + input: openaiInputItems, tools, }); - // Keep ALL output items (incl reasoning/tool calls) in the running state. - inputItems.push(...(response.output ?? [])); + openaiInputItems.push(...(response.output ?? [])); - // Show any assistant text immediately - const text = extractAssistantText(response); + const text = extractAssistantTextOpenAI(response); if (text) addMessage("assistant", text); const calls = (response.output ?? []).filter((x) => x.type === "function_call"); @@ -227,8 +286,7 @@ async function runAgentTurn(userText) { const toolRes = await callWorker("toolCall", { name: call.name, args }); - // Feed tool result back to the model - inputItems.push({ + openaiInputItems.push({ type: "function_call_output", call_id: call.call_id, output: JSON.stringify(toolRes.result), @@ -241,6 +299,113 @@ async function runAgentTurn(userText) { addMessage("assistant", "I hit the tool-call loop limit. Try narrowing your request."); } +// ============================================================ +// Anthropic Messages API +// ============================================================ + +async function anthropicMessagesCreate({ apiKey, model, system, messages, tools }) { + const res = await fetch("https://api.anthropic.com/v1/messages", { + method: "POST", + headers: { + "Content-Type": "application/json", + "x-api-key": apiKey, + "anthropic-version": "2023-06-01", + "anthropic-dangerous-direct-browser-access": "true", + }, + body: JSON.stringify({ + model, + max_tokens: 4096, + system, + tools, + messages, + }), + }); + + if (!res.ok) { + const text = await res.text(); + throw new Error(`Anthropic error ${res.status}: ${text}`); + } + return await res.json(); +} + +function extractAssistantTextAnthropic(response) { + const out = []; + for (const block of response.content ?? []) { + if (block.type === "text") out.push(block.text); + } + return out.join("\n").trim(); +} + +async function runAnthropicTurn(userText) { + const apiKey = apiKeyEl.value.trim(); + if (!apiKey) throw new Error("Missing API key"); + + anthropicMessages.push({ role: "user", content: userText }); + + const claudeTools = toolsForAnthropic(); + + for (let i = 0; i < 64; i++) { + const response = await anthropicMessagesCreate({ + apiKey, + model: modelEl.value, + system: SYSTEM_INSTRUCTIONS, + messages: anthropicMessages, + tools: claudeTools, + }); + + // Show any assistant text + const text = extractAssistantTextAnthropic(response); + if (text) addMessage("assistant", text); + + // Append the full assistant response to conversation history + anthropicMessages.push({ role: "assistant", content: response.content }); + + // Check for tool use + const toolUseBlocks = (response.content ?? []).filter((b) => b.type === "tool_use"); + if (response.stop_reason !== "tool_use" || toolUseBlocks.length === 0) return; + + // Execute each tool call and collect results + const toolResultBlocks = []; + for (const toolUse of toolUseBlocks) { + const args = toolUse.input ?? {}; + + addMessage("tool", `→ ${toolUse.name}(${JSON.stringify(args)})`); + + let resultContent; + try { + const toolRes = await callWorker("toolCall", { name: toolUse.name, args }); + resultContent = JSON.stringify(toolRes.result); + } catch (e) { + resultContent = JSON.stringify({ error: e.message }); + } + + toolResultBlocks.push({ + type: "tool_result", + tool_use_id: toolUse.id, + content: resultContent, + }); + + addMessage("tool", `← ${toolUse.name}: ${resultContent}`); + } + + // Feed all tool results back as a single user message + anthropicMessages.push({ role: "user", content: toolResultBlocks }); + } + + addMessage("assistant", "I hit the tool-call loop limit. Try narrowing your request."); +} + +// ============================================================ +// Unified agent turn +// ============================================================ + +async function runAgentTurn(userText) { + if (getProvider() === "anthropic") { + return runAnthropicTurn(userText); + } + return runOpenAITurn(userText); +} + sendBtn.onclick = async () => { const text = inputEl.value.trim(); if (!text) return; @@ -312,9 +477,10 @@ downloadBtn.onclick = async () => { try { setBusy(true, "Initializing Pyodide and IfcOpenShell for in-memory IFC access…"); await callWorker("init", {}); + updateProviderUI(); setBusy(false, "Ready"); } catch (e) { setBusy(true, "Error"); addMessage("assistant", `Worker init failed: ${e.message}`); } -})(); \ No newline at end of file +})(); diff --git a/src/ifcchat/index.html b/src/ifcchat/index.html index ac878ae615..0697bfa790 100644 --- a/src/ifcchat/index.html +++ b/src/ifcchat/index.html @@ -143,7 +143,8 @@ padding: 8px 10px; } - #model { + #model, + #provider { background: white; color: gray; border: solid 1px #eee; @@ -239,14 +240,22 @@
- + + +
+ +
+
- +
From eaf7950677005f6916ab4ebc9ae1e847f15b0071 Mon Sep 17 00:00:00 2001 From: Ryan Schultz Date: Thu, 2 Apr 2026 10:56:56 -0500 Subject: [PATCH 05/25] Fix TypeError in ray_cast_by_proximity_2d degenerate edge A degenerate edge (zero-length segment) caused an early `return` of a tuple instead of continuing the loop, resulting in a TypeError when snap.py iterated the result and tried to assign `point["group"]` on a float. Generated with the assistance of an AI coding tool. --- src/bonsai/bonsai/tool/raycast.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/bonsai/bonsai/tool/raycast.py b/src/bonsai/bonsai/tool/raycast.py index c16bc55564..6b613aecaf 100644 --- a/src/bonsai/bonsai/tool/raycast.py +++ b/src/bonsai/bonsai/tool/raycast.py @@ -434,11 +434,8 @@ class Raycast(bonsai.core.tool.Raycast): seg_len_sq = sx * sx + sy * sy if seg_len_sq == 0.0: - # degenerate segment: return distance to p0 - dx = px - p0x - dy = py - p0y - dist = math.hypot(dx, dy) - return dist, (p0x, p0y), 0.0 + # degenerate segment: skip it + continue # project (p - p0) onto seg: t = dot(p-p0, seg) / |seg|^2 apx = px - p0x From 1d7a8ca2491226549603f828787113ac3ac37131 Mon Sep 17 00:00:00 2001 From: geronimi73 Date: Thu, 2 Apr 2026 13:19:16 +0200 Subject: [PATCH 06/25] separate API calls --- src/ifcchat/api_openai.js | 16 ++++++++++++++++ src/ifcchat/api_openrouter.js | 16 ++++++++++++++++ 2 files changed, 32 insertions(+) create mode 100644 src/ifcchat/api_openai.js create mode 100644 src/ifcchat/api_openrouter.js diff --git a/src/ifcchat/api_openai.js b/src/ifcchat/api_openai.js new file mode 100644 index 0000000000..0fa6d7a88f --- /dev/null +++ b/src/ifcchat/api_openai.js @@ -0,0 +1,16 @@ + +export async function chat({ apiKey, model, messages, tools }) { + const res = await fetch("https://api.openai.com/v1/chat/completions", { + method: "POST", + headers: { + "Content-Type": "application/json", + "Authorization": `Bearer ${apiKey}`, + }, + body: JSON.stringify({ model, messages, tools }), + }); + if (!res.ok) { + const text = await res.text(); + throw new Error(`OpenAI error ${res.status}: ${text}`); + } + return await res.json(); +} diff --git a/src/ifcchat/api_openrouter.js b/src/ifcchat/api_openrouter.js new file mode 100644 index 0000000000..9a02ee3025 --- /dev/null +++ b/src/ifcchat/api_openrouter.js @@ -0,0 +1,16 @@ + +export async function chat({ apiKey, model, messages, tools }) { + const res = await fetch("https://openrouter.ai/api/v1/chat/completions", { + method: "POST", + headers: { + "Content-Type": "application/json", + "Authorization": `Bearer ${apiKey}`, + }, + body: JSON.stringify({ model, messages, tools }), + }); + if (!res.ok) { + const text = await res.text(); + throw new Error(`OpenRouter error ${res.status}: ${text}`); + } + return await res.json(); +} From b7fc5daf828794297ffc30767591ad52119b284f Mon Sep 17 00:00:00 2001 From: geronimi73 Date: Thu, 2 Apr 2026 13:19:28 +0200 Subject: [PATCH 07/25] ui: choose openai/openrouter --- src/ifcchat/index.html | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/ifcchat/index.html b/src/ifcchat/index.html index ac878ae615..2bf0b6c4f1 100644 --- a/src/ifcchat/index.html +++ b/src/ifcchat/index.html @@ -239,7 +239,15 @@
- + + +
+ +
+
@@ -295,10 +303,7 @@ IfcOpenShell AI Assistant - +
From 0f7f960b29ab47204f9b6f533696a2acf6689a07 Mon Sep 17 00:00:00 2001 From: geronimi73 Date: Thu, 2 Apr 2026 13:25:16 +0200 Subject: [PATCH 08/25] let claude code openrouter compatibility --- src/ifcchat/app.js | 169 ++++++++++++++++++++++----------------------- 1 file changed, 82 insertions(+), 87 deletions(-) diff --git a/src/ifcchat/app.js b/src/ifcchat/app.js index 516ec8f846..642ef275df 100644 --- a/src/ifcchat/app.js +++ b/src/ifcchat/app.js @@ -1,4 +1,26 @@ // app.js +import * as openaiApi from "./api_openai.js"; +import * as openrouterApi from "./api_openrouter.js"; + +const PROVIDERS = { + openai: { + api: openaiApi, + models: [ + { value: "gpt-5", label: "gpt-5" }, + { value: "gpt-4.1", label: "gpt-4.1" }, + ], + }, + openrouter: { + api: openrouterApi, + models: [ + { value: "openai/gpt-oss-20b", label: "gpt-oss-20b" }, + { value: "anthropic/claude-sonnet-4-5", label: "claude-sonnet-4-5" }, + { value: "openai/gpt-4.1", label: "gpt-4.1" }, + { value: "google/gemini-2.5-pro-preview", label: "gemini-2.5-pro" }, + ], + }, +}; + const $ = (id) => document.getElementById(id); const statusEl = $("status"); @@ -7,10 +29,19 @@ const sendBtn = $("send"); const inputEl = $("input"); const apiKeyEl = $("apiKey"); const modelEl = $("model"); +const providerEl = $("provider"); const ifcFileEl = $("ifcFile"); const newBtn = $("newModel"); const downloadBtn = $("downloadIfc"); +function onProviderChange() { + const p = PROVIDERS[providerEl.value]; + modelEl.innerHTML = p.models.map(m => ``).join(""); +} + +providerEl.addEventListener("change", onProviderChange); +onProviderChange(); + function setBusy(isBusy, reason = "") { const controls = [ $("send"), @@ -75,74 +106,73 @@ function callWorker(type, payload = {}) { }); } -// ---- OpenAI Responses API tool schemas (should match ifcmcp.core openai_tools()) ---- -// Docs show Responses API function_call items + function_call_output loop. :contentReference[oaicite:4]{index=4} +// ---- Tool schemas (should match ifcmcp.core openai_tools()) ---- const tools = [ { - type: "function", name: "ifc_new", description: "Create a new empty IFC model in memory.", - parameters: { type: "object", properties: { schema: { type: "string" } }, required: [], additionalProperties: false } + type: "function", function: { name: "ifc_new", description: "Create a new empty IFC model in memory.", + parameters: { type: "object", properties: { schema: { type: "string" } }, required: [], additionalProperties: false } } }, { - type: "function", name: "ifc_summary", description: "Get a concise overview of the loaded IFC model.", - parameters: { type: "object", properties: {}, required: [], additionalProperties: false } + type: "function", function: { name: "ifc_summary", description: "Get a concise overview of the loaded IFC model.", + parameters: { type: "object", properties: {}, required: [], additionalProperties: false } } }, { - type: "function", name: "ifc_tree", description: "Get the full spatial hierarchy tree.", - parameters: { type: "object", properties: {}, required: [], additionalProperties: false } + type: "function", function: { name: "ifc_tree", description: "Get the full spatial hierarchy tree.", + parameters: { type: "object", properties: {}, required: [], additionalProperties: false } } }, { - type: "function", name: "ifc_select", description: "Select elements using ifcopenshell selector syntax (e.g. 'IfcWall').", - parameters: { type: "object", properties: { query: { type: "string" } }, required: ["query"], additionalProperties: false } + type: "function", function: { name: "ifc_select", description: "Select elements using ifcopenshell selector syntax (e.g. 'IfcWall').", + parameters: { type: "object", properties: { query: { type: "string" } }, required: ["query"], additionalProperties: false } } }, { - type: "function", name: "ifc_info", description: "Inspect an entity by STEP id.", - parameters: { type: "object", properties: { element_id: { type: "integer" } }, required: ["element_id"], additionalProperties: false } + type: "function", function: { name: "ifc_info", description: "Inspect an entity by STEP id.", + parameters: { type: "object", properties: { element_id: { type: "integer" } }, required: ["element_id"], additionalProperties: false } } }, { - type: "function", name: "ifc_relations", description: "Get relationships for an element. traverse='up' walks to IfcProject.", + type: "function", function: { name: "ifc_relations", description: "Get relationships for an element. traverse='up' walks to IfcProject.", parameters: { type: "object", properties: { element_id: { type: "integer" }, traverse: { type: "string" } }, required: ["element_id"], additionalProperties: false - } + } } }, { - type: "function", name: "ifc_clash", description: "Run clash/clearance checks for an element.", + type: "function", function: { name: "ifc_clash", description: "Run clash/clearance checks for an element.", parameters: { type: "object", properties: { element_id: { type: "integer" }, clearance: { type: "number" }, tolerance: { type: "number" }, scope: { type: "string" } }, required: ["element_id"], additionalProperties: false - } + } } }, { - type: "function", name: "ifc_list", description: "List ifcopenshell.api modules or functions within a module.", - parameters: { type: "object", properties: { module: { type: "string" } }, required: [], additionalProperties: false } + type: "function", function: { name: "ifc_list", description: "List ifcopenshell.api modules or functions within a module.", + parameters: { type: "object", properties: { module: { type: "string" } }, required: [], additionalProperties: false } } }, { - type: "function", name: "ifc_docs", description: "Get documentation for an ifcopenshell.api function, 'module.function'.", - parameters: { type: "object", properties: { function_path: { type: "string" } }, required: ["function_path"], additionalProperties: false } + type: "function", function: { name: "ifc_docs", description: "Get documentation for an ifcopenshell.api function, 'module.function'.", + parameters: { type: "object", properties: { function_path: { type: "string" } }, required: ["function_path"], additionalProperties: false } } }, { - type: "function", name: "ifc_edit", description: "Execute an ifcopenshell.api mutation; params is a JSON string of stringly-typed kwargs.", - parameters: { type: "object", properties: { function_path: { type: "string" }, params: { type: "string" } }, required: ["function_path"], additionalProperties: false } + type: "function", function: { name: "ifc_edit", description: "Execute an ifcopenshell.api mutation; params is a JSON string of stringly-typed kwargs.", + parameters: { type: "object", properties: { function_path: { type: "string" }, params: { type: "string" } }, required: ["function_path"], additionalProperties: false } } }, { - type: "function", name: "ifc_validate", description: "Validate the loaded model. Returns valid bool and list of issues.", - parameters: { type: "object", properties: { express_rules: { type: "boolean" } }, required: [], additionalProperties: false } + type: "function", function: { name: "ifc_validate", description: "Validate the loaded model. Returns valid bool and list of issues.", + parameters: { type: "object", properties: { express_rules: { type: "boolean" } }, required: [], additionalProperties: false } } }, { - type: "function", name: "ifc_schedule", description: "List work schedules and nested tasks. Use max_depth=1 for top-level phases only on large projects.", - parameters: { type: "object", properties: { max_depth: { type: "integer" } }, required: [], additionalProperties: false } + type: "function", function: { name: "ifc_schedule", description: "List work schedules and nested tasks. Use max_depth=1 for top-level phases only on large projects.", + parameters: { type: "object", properties: { max_depth: { type: "integer" } }, required: [], additionalProperties: false } } }, { - type: "function", name: "ifc_cost", description: "List cost schedules and nested cost items. Use max_depth=1 for top-level sections only on large BoQs.", - parameters: { type: "object", properties: { max_depth: { type: "integer" } }, required: [], additionalProperties: false } + type: "function", function: { name: "ifc_cost", description: "List cost schedules and nested cost items. Use max_depth=1 for top-level sections only on large BoQs.", + parameters: { type: "object", properties: { max_depth: { type: "integer" } }, required: [], additionalProperties: false } } }, { - type: "function", name: "ifc_schema", description: "Return IFC class documentation for an entity type.", - parameters: { type: "object", properties: { entity_type: { type: "string" } }, required: ["entity_type"], additionalProperties: false } + type: "function", function: { name: "ifc_schema", description: "Return IFC class documentation for an entity type.", + parameters: { type: "object", properties: { entity_type: { type: "string" } }, required: ["entity_type"], additionalProperties: false } } }, { - type: "function", name: "ifc_quantify", description: "Run quantity take-off (QTO) on the model. Modifies model in-place; call ifc_save() after.", - parameters: { type: "object", properties: { rule: { type: "string" }, selector: { type: "string" } }, required: ["rule"], additionalProperties: false } + type: "function", function: { name: "ifc_quantify", description: "Run quantity take-off (QTO) on the model. Modifies model in-place; call ifc_save() after.", + parameters: { type: "object", properties: { rule: { type: "string" }, selector: { type: "string" } }, required: ["rule"], additionalProperties: false } } }, ]; @@ -156,85 +186,50 @@ Rules: Be concise. Avoid dumping huge trees unless asked. `; -let inputItems = []; // running conversation state (Responses API style) - -async function openAIResponsesCreate({ apiKey, model, input, tools }) { - const res = await fetch("https://api.openai.com/v1/responses", { - method: "POST", - headers: { - "Content-Type": "application/json", - "Authorization": `Bearer ${apiKey}`, - }, - body: JSON.stringify({ - model, - instructions: SYSTEM_INSTRUCTIONS, - tools, - input, - }), - }); - - if (!res.ok) { - const text = await res.text(); - throw new Error(`OpenAI error ${res.status}: ${text}`); - } - return await res.json(); -} - -function extractAssistantText(response) { - const out = []; - for (const item of response.output ?? []) { - if (item.type === "message" && item.role === "assistant") { - for (const c of item.content ?? []) { - if (c.type === "output_text") out.push(c.text); - } - } - } - return out.join("\n").trim(); -} +let messages = []; // running conversation state (Chat Completions style) async function runAgentTurn(userText) { const apiKey = apiKeyEl.value.trim(); if (!apiKey) throw new Error("Missing API key"); - // Add user message - inputItems.push({ role: "user", content: userText }); + const { chat } = PROVIDERS[providerEl.value].api; + + messages.push({ role: "user", content: userText }); - // Tool-calling loop (Responses API): append response.output, execute function_call items, append function_call_output. for (let i = 0; i < 64; i++) { - const response = await openAIResponsesCreate({ + const response = await chat({ apiKey, model: modelEl.value, - input: inputItems, + messages: [{ role: "system", content: SYSTEM_INSTRUCTIONS }, ...messages], tools, }); - // Keep ALL output items (incl reasoning/tool calls) in the running state. - inputItems.push(...(response.output ?? [])); + const message = response.choices?.[0]?.message; + if (!message) throw new Error("No message in response"); - // Show any assistant text immediately - const text = extractAssistantText(response); - if (text) addMessage("assistant", text); + messages.push(message); - const calls = (response.output ?? []).filter((x) => x.type === "function_call"); + if (message.content) addMessage("assistant", message.content); + + const calls = message.tool_calls ?? []; if (calls.length === 0) return; for (const call of calls) { let args = {}; - try { args = call.arguments ? JSON.parse(call.arguments) : {}; } + try { args = call.function.arguments ? JSON.parse(call.function.arguments) : {}; } catch { args = {}; } - addMessage("tool", `→ ${call.name}(${JSON.stringify(args)})`); + addMessage("tool", `→ ${call.function.name}(${JSON.stringify(args)})`); - const toolRes = await callWorker("toolCall", { name: call.name, args }); + const toolRes = await callWorker("toolCall", { name: call.function.name, args }); - // Feed tool result back to the model - inputItems.push({ - type: "function_call_output", - call_id: call.call_id, - output: JSON.stringify(toolRes.result), + messages.push({ + role: "tool", + tool_call_id: call.id, + content: JSON.stringify(toolRes.result), }); - addMessage("tool", `← ${call.name}: ${JSON.stringify(toolRes.result, null, 2)}`); + addMessage("tool", `← ${call.function.name}: ${JSON.stringify(toolRes.result, null, 2)}`); } } From fcbec74521088b5295493de475d83fb50f4628fa Mon Sep 17 00:00:00 2001 From: geronimi73 Date: Thu, 2 Apr 2026 13:27:54 +0200 Subject: [PATCH 09/25] spinner --- src/ifcchat/app.js | 4 ++++ src/ifcchat/index.html | 13 ++++++++++++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/src/ifcchat/app.js b/src/ifcchat/app.js index 642ef275df..d9c6d6bcd6 100644 --- a/src/ifcchat/app.js +++ b/src/ifcchat/app.js @@ -61,6 +61,10 @@ function setBusy(isBusy, reason = "") { browseBtn.tabIndex = isBusy ? -1 : 0; } + sendBtn.innerHTML = isBusy + ? `` + : `Send send`; + setStatus(isBusy ? (reason || "Working…") : "Ready"); } diff --git a/src/ifcchat/index.html b/src/ifcchat/index.html index 2bf0b6c4f1..77a57c07a8 100644 --- a/src/ifcchat/index.html +++ b/src/ifcchat/index.html @@ -113,7 +113,18 @@ border-radius: 20px; padding: 10px; display: flex; - width: 50%; + width: 100%; + } + + @keyframes spin { to { transform: rotate(360deg); } } + .spinner { + width: 18px; + height: 18px; + border: 2px solid #aaa; + border-top-color: #333; + border-radius: 50%; + animation: spin 0.7s linear infinite; + display: inline-block; } .status { From 3b28c92414d53780f5f651cea263504075b9fb86 Mon Sep 17 00:00:00 2001 From: geronimi73 Date: Thu, 2 Apr 2026 13:29:35 +0200 Subject: [PATCH 10/25] html too big -> styles into sep. file --- src/ifcchat/index.html | 239 +---------------------------------------- src/ifcchat/style.css | 232 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 233 insertions(+), 238 deletions(-) create mode 100644 src/ifcchat/style.css diff --git a/src/ifcchat/index.html b/src/ifcchat/index.html index 77a57c07a8..686dce5663 100644 --- a/src/ifcchat/index.html +++ b/src/ifcchat/index.html @@ -5,244 +5,7 @@ IfcOpenShell AI Assistant - + diff --git a/src/ifcchat/style.css b/src/ifcchat/style.css new file mode 100644 index 0000000000..6c3af2f56e --- /dev/null +++ b/src/ifcchat/style.css @@ -0,0 +1,232 @@ +* { + box-sizing: border-box; +} + +body { + font-family: system-ui, sans-serif; + margin: 0; +} + +button, +input, +select, +textarea { + font: inherit; +} + +header { + padding: 12px 16px; + border-bottom: 1px solid #ddd; +} + +header input, +header select { + padding: 8px; +} + +main { + display: grid; + grid-template-columns: 320px 1fr; + height: 100vh; +} + +.side { + border-right: 1px solid #ddd; + padding: 100px 12px 12px 12px; + overflow: auto; + background: #f9f9f9; +} + +.chat { + display: flex; + flex-direction: column; + height: 100%; +} + +.msgs { + flex: 1; + overflow: auto; + padding: 16px; +} + +.msg { + margin: 10px 0; +} + +.msg .role { + font-size: 12px; + opacity: 0.7; + margin: 12px 0 4px 0; +} + +.role.user { + text-align: right; +} + +.msg .bubble { + padding: 0; + border-radius: 10px; + white-space: pre-wrap; +} + +.msg.user .bubble { + padding: 10px 20px; + background: #eee; + width: 50%; + margin-left: auto; + border: solid 1px #ddd; +} + +.msg.tool .bubble { + font-family: ui-monospace, SFMono-Regular, Menlo, monospace; + font-size: 60%; + max-height: 100px; + overflow: hidden; +} + +.composer { + display: flex; + gap: 8px; + padding: 12px; + justify-content: center; +} + +.composer textarea { + flex: 1; + resize: none; + height: 88px; + border: none; +} + +.composer button { + align-self: center; +} + +.composer .inner { + border: solid 1px #ddd; + border-radius: 20px; + padding: 10px; + display: flex; + width: 100%; +} + +@keyframes spin { to { transform: rotate(360deg); } } +.spinner { + width: 18px; + height: 18px; + border: 2px solid #aaa; + border-top-color: #333; + border-radius: 50%; + animation: spin 0.7s linear infinite; + display: inline-block; +} + +.status { + font-size: 12px; + opacity: 0.7; +} + +section > .row > label { + display: block; + font-size: 12px; + opacity: 0.75; + margin-bottom: 6px; + font-weight: bold; +} + +section > .row > label .small { + font-weight: normal; + display: block; + font-size: 80%; +} + +.side .row { + margin-bottom: 32px; +} + +.row button { + padding: 8px 10px; +} + +#model { + background: white; + color: gray; + border: solid 1px #eee; + border-radius: 6px; +} + +hr { + border: dashed 1px #ddd; +} + +.btn-row { + display: flex; + gap: 10px; +} + +.btn-row > .btn, +.btn-row > button.btn { + flex: 1 1 0; + min-width: 0; +} + +.btn { + display: inline-flex; + align-items: center; + justify-content: center; + gap: 8px; + font-size: 14px; + + padding: 10px 12px; + border-radius: 10px; + border: 1px solid #d0d0d0; + background: #eee; + + cursor: pointer; + user-select: none; + text-decoration: none; +} + +.btn:hover { + background: #ddd; +} + +.btn:active { + transform: translateY(1px); +} + +.btn-wide { + width: 100%; +} + +.material-icons { + font-size: 18px; + line-height: 1; +} + +.btn.disabled, +.btn:disabled { + opacity: 0.55; + cursor: not-allowed; + pointer-events: none; +} + +#input { + border: none; + outline: none; +} + +#input:focus, +#input:focus-visible { + outline: none; + box-shadow: none; +} + +.code { + font-family: 'Courier New', Courier, monospace; + font-size: 90%; + background-color: #eee; + border: solid 1px #ddd; + padding: 2px; + display: inline-block; +} From 252bd6f4f6a1a171ed4630d22111b69fca555a09 Mon Sep 17 00:00:00 2001 From: geronimi73 Date: Thu, 2 Apr 2026 21:33:54 +0200 Subject: [PATCH 11/25] openai by default --- src/ifcchat/index.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ifcchat/index.html b/src/ifcchat/index.html index 686dce5663..9c89d15715 100644 --- a/src/ifcchat/index.html +++ b/src/ifcchat/index.html @@ -15,8 +15,8 @@
From d7de4f8df07905cb9d6fac239b6b7019f83c4186 Mon Sep 17 00:00:00 2001 From: geronimi73 Date: Thu, 2 Apr 2026 21:35:43 +0200 Subject: [PATCH 12/25] dont freeze UI on error --- src/ifcchat/app.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ifcchat/app.js b/src/ifcchat/app.js index d9c6d6bcd6..ef0e9bce5b 100644 --- a/src/ifcchat/app.js +++ b/src/ifcchat/app.js @@ -250,7 +250,7 @@ sendBtn.onclick = async () => { await runAgentTurn(text); setBusy(false, "Ready"); } catch (e) { - setBusy(true, "Error"); + setBusy(false, "Error"); addMessage("assistant", `Error: ${e.message}`); } }; From 29e5e0fd1ad1689f20e830c1f4eccaae2ef80582 Mon Sep 17 00:00:00 2001 From: geronimi73 Date: Thu, 2 Apr 2026 21:51:45 +0200 Subject: [PATCH 13/25] chevrons for tool result expansion --- src/ifcchat/app.js | 8 +++++--- src/ifcchat/style.css | 9 +++++++++ 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/src/ifcchat/app.js b/src/ifcchat/app.js index ef0e9bce5b..8f2b1f3b97 100644 --- a/src/ifcchat/app.js +++ b/src/ifcchat/app.js @@ -75,14 +75,16 @@ function addMessage(role, text) { const wrap = document.createElement("div"); wrap.className = `msg ${role}`; wrap.innerHTML = ` -
${role}
+
${role}${role === "tool" ? '▶' : ''}
`; const bubble = wrap.querySelector(".bubble"); bubble.textContent = text; bubble.onclick = function () { if (bubble.scrollHeight > 100 && role === "tool") { - bubble.style.maxHeight = bubble.style.maxHeight == 'none' ? '' : 'none'; - bubble.style.borderBottom = bubble.style.borderBottom == '' ? 'dotted 2px gray' : ''; + const expanded = bubble.style.maxHeight === 'none'; + bubble.style.maxHeight = expanded ? '' : 'none'; + bubble.style.borderBottom = expanded ? '' : 'dotted 2px gray'; + wrap.querySelector(".chevron").style.transform = expanded ? '' : 'rotate(90deg)'; } } msgsEl.appendChild(wrap); diff --git a/src/ifcchat/style.css b/src/ifcchat/style.css index 6c3af2f56e..90fd417501 100644 --- a/src/ifcchat/style.css +++ b/src/ifcchat/style.css @@ -82,6 +82,15 @@ main { font-size: 60%; max-height: 100px; overflow: hidden; + cursor: pointer; +} + +.chevron { + display: inline-block; + font-size: 10px; + margin-left: 6px; + transition: transform 0.2s; + vertical-align: middle; } .composer { From 7af0ec13e51139e64dd7c6b1f5f844a9549459b1 Mon Sep 17 00:00:00 2001 From: geronimi73 Date: Thu, 2 Apr 2026 21:52:59 +0200 Subject: [PATCH 14/25] move model to sidebar --- src/ifcchat/index.html | 8 ++++++-- src/ifcchat/style.css | 6 ------ 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/src/ifcchat/index.html b/src/ifcchat/index.html index 9c89d15715..f4e922dca5 100644 --- a/src/ifcchat/index.html +++ b/src/ifcchat/index.html @@ -17,7 +17,12 @@ + +
+ +
+ +
@@ -77,7 +82,6 @@ IfcOpenShell AI Assistant -
diff --git a/src/ifcchat/style.css b/src/ifcchat/style.css index 90fd417501..ef5f6add8f 100644 --- a/src/ifcchat/style.css +++ b/src/ifcchat/style.css @@ -157,12 +157,6 @@ section > .row > label .small { padding: 8px 10px; } -#model { - background: white; - color: gray; - border: solid 1px #eee; - border-radius: 6px; -} hr { border: dashed 1px #ddd; From fbf2946b698d5f762beb2470a3fd7d71ef5291b6 Mon Sep 17 00:00:00 2001 From: geronimi73 Date: Thu, 2 Apr 2026 21:53:44 +0200 Subject: [PATCH 15/25] Update index.html --- src/ifcchat/index.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ifcchat/index.html b/src/ifcchat/index.html index f4e922dca5..20129d1937 100644 --- a/src/ifcchat/index.html +++ b/src/ifcchat/index.html @@ -26,7 +26,7 @@
- +
From 434ea74f22f7d3a930544893d53afc2b2e951336 Mon Sep 17 00:00:00 2001 From: geronimi73 Date: Thu, 2 Apr 2026 21:56:55 +0200 Subject: [PATCH 16/25] format this mess --- src/ifcchat/app.js | 30 ++++++++++++++++++++++++------ 1 file changed, 24 insertions(+), 6 deletions(-) diff --git a/src/ifcchat/app.js b/src/ifcchat/app.js index 8f2b1f3b97..1833cc2e9d 100644 --- a/src/ifcchat/app.js +++ b/src/ifcchat/app.js @@ -6,17 +6,35 @@ const PROVIDERS = { openai: { api: openaiApi, models: [ - { value: "gpt-5", label: "gpt-5" }, - { value: "gpt-4.1", label: "gpt-4.1" }, + { + value: "gpt-5", + label: "gpt-5" + }, + { + value: "gpt-4.1", + label: "gpt-4.1" + }, ], }, openrouter: { api: openrouterApi, models: [ - { value: "openai/gpt-oss-20b", label: "gpt-oss-20b" }, - { value: "anthropic/claude-sonnet-4-5", label: "claude-sonnet-4-5" }, - { value: "openai/gpt-4.1", label: "gpt-4.1" }, - { value: "google/gemini-2.5-pro-preview", label: "gemini-2.5-pro" }, + { + value: "openai/gpt-oss-20b", + label: "gpt-oss-20b" + }, + { + value: "anthropic/claude-sonnet-4-5", + label: "claude-sonnet-4-5" + }, + { + value: "openai/gpt-4.1", + label: "gpt-4.1" + }, + { + value: "google/gemini-2.5-pro-preview", + label: "gemini-2.5-pro" + }, ], }, }; From 8f64f75abb15886e5292939fef8f89643f8a67c8 Mon Sep 17 00:00:00 2001 From: geronimi73 Date: Thu, 2 Apr 2026 22:06:19 +0200 Subject: [PATCH 17/25] add favourite models --- src/ifcchat/app.js | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/ifcchat/app.js b/src/ifcchat/app.js index 1833cc2e9d..b367359516 100644 --- a/src/ifcchat/app.js +++ b/src/ifcchat/app.js @@ -20,17 +20,25 @@ const PROVIDERS = { api: openrouterApi, models: [ { - value: "openai/gpt-oss-20b", + value: "openai/gpt-oss-20b", label: "gpt-oss-20b" }, { - value: "anthropic/claude-sonnet-4-5", - label: "claude-sonnet-4-5" + value: "openai/gpt-oss-120b", + label: "gpt-oss-120b" + }, + { + value: "mistralai/mistral-small-3.2-24b-instruct", + label: "mistral-small-3.2" }, { value: "openai/gpt-4.1", label: "gpt-4.1" }, + { + value: "anthropic/claude-sonnet-4-5", + label: "claude-sonnet-4-5" + }, { value: "google/gemini-2.5-pro-preview", label: "gemini-2.5-pro" From 5d748c5b048ba959fe6aa14dabdf49a8785646b9 Mon Sep 17 00:00:00 2001 From: Thomas Krijnen Date: Fri, 3 Apr 2026 10:49:36 +0200 Subject: [PATCH 18/25] Add Gemini option --- src/ifcchat/README.md | 2 +- src/ifcchat/api_openai.js | 8 +++++-- src/ifcchat/api_openrouter.js | 8 +++++-- src/ifcchat/app.js | 45 ++++++++++++++++++++++++++++++++++- src/ifcchat/index.html | 6 +++++ 5 files changed, 63 insertions(+), 6 deletions(-) diff --git a/src/ifcchat/README.md b/src/ifcchat/README.md index b2cceecc52..5dc434143a 100644 --- a/src/ifcchat/README.md +++ b/src/ifcchat/README.md @@ -1,7 +1,7 @@ IfcOpenShell AI Assistant ========================= -A web-based client-side (pyodide + OpenAI, Anthropic, or OpenRouter API) model interrogation and generation API based on: ifcedit, ifcquery and ifcmcp (ifcopenshell-mcp) packaged in a HTML+JS application. +A web-based client-side (pyodide + OpenAI, Anthropic, Gemini, or OpenRouter API) model interrogation and generation API based on: ifcedit, ifcquery and ifcmcp (ifcopenshell-mcp) packaged in a HTML+JS application. ### Setup instructions diff --git a/src/ifcchat/api_openai.js b/src/ifcchat/api_openai.js index 0fa6d7a88f..8903f897e4 100644 --- a/src/ifcchat/api_openai.js +++ b/src/ifcchat/api_openai.js @@ -1,6 +1,10 @@ +function getChatCompletionsUrl(baseURL) { + const root = (baseURL || "https://api.openai.com/v1").replace(/\/+$/, ""); + return `${root}/chat/completions`; +} -export async function chat({ apiKey, model, messages, tools }) { - const res = await fetch("https://api.openai.com/v1/chat/completions", { +export async function chat({ apiKey, baseURL, model, messages, tools }) { + const res = await fetch(getChatCompletionsUrl(baseURL), { method: "POST", headers: { "Content-Type": "application/json", diff --git a/src/ifcchat/api_openrouter.js b/src/ifcchat/api_openrouter.js index 9a02ee3025..38c11e36e8 100644 --- a/src/ifcchat/api_openrouter.js +++ b/src/ifcchat/api_openrouter.js @@ -1,6 +1,10 @@ +function getChatCompletionsUrl(baseURL) { + const root = (baseURL || "https://openrouter.ai/api/v1").replace(/\/+$/, ""); + return `${root}/chat/completions`; +} -export async function chat({ apiKey, model, messages, tools }) { - const res = await fetch("https://openrouter.ai/api/v1/chat/completions", { +export async function chat({ apiKey, baseURL, model, messages, tools }) { + const res = await fetch(getChatCompletionsUrl(baseURL), { method: "POST", headers: { "Content-Type": "application/json", diff --git a/src/ifcchat/app.js b/src/ifcchat/app.js index 22a9b0a055..28a20f1cb7 100644 --- a/src/ifcchat/app.js +++ b/src/ifcchat/app.js @@ -8,6 +8,9 @@ const PROVIDERS = { api: openaiApi, apiKeyLabel: "OpenAI API key", apiKeyPlaceholder: "sk-...", + baseUrlLabel: "Base URL", + baseUrlPlaceholder: "https://api.openai.com/v1", + baseUrlDefault: "https://api.openai.com/v1", models: [ { value: "gpt-5", @@ -38,10 +41,35 @@ const PROVIDERS = { }, ], }, + gemini: { + api: openaiApi, + apiKeyLabel: "Gemini API key", + apiKeyPlaceholder: "AIza...", + baseUrlLabel: "Base URL", + baseUrlPlaceholder: "https://generativelanguage.googleapis.com/v1beta/openai/", + baseUrlDefault: "https://generativelanguage.googleapis.com/v1beta/openai/", + models: [ + { + value: "gemini-3-flash-preview", + label: "gemini-3-flash-preview" + }, + { + value: "gemini-2.5-flash", + label: "gemini-2.5-flash" + }, + { + value: "gemini-2.5-pro", + label: "gemini-2.5-pro" + }, + ], + }, openrouter: { api: openrouterApi, apiKeyLabel: "OpenRouter API key", apiKeyPlaceholder: "sk-or-v1-...", + baseUrlLabel: "Base URL", + baseUrlPlaceholder: "https://openrouter.ai/api/v1", + baseUrlDefault: "https://openrouter.ai/api/v1", models: [ { value: "openai/gpt-oss-20b", @@ -79,6 +107,9 @@ const sendBtn = $("send"); const inputEl = $("input"); const apiKeyEl = $("apiKey"); const apiKeyLabelEl = $("apiKeyLabel"); +const baseUrlRowEl = $("baseUrlRow"); +const baseUrlLabelEl = $("baseUrlLabel"); +const baseUrlEl = $("baseUrl"); const modelEl = $("model"); const providerEl = $("provider"); const ifcFileEl = $("ifcFile"); @@ -89,6 +120,15 @@ function onProviderChange() { const provider = PROVIDERS[providerEl.value]; apiKeyLabelEl.innerHTML = `${provider.apiKeyLabel}stored in browser memory; only sent to provider servers`; apiKeyEl.placeholder = provider.apiKeyPlaceholder; + baseUrlRowEl.hidden = !provider.baseUrlDefault; + if (provider.baseUrlDefault) { + baseUrlLabelEl.innerHTML = `${provider.baseUrlLabel}override the API endpoint for OpenAI-compatible providers`; + baseUrlEl.placeholder = provider.baseUrlPlaceholder; + baseUrlEl.value = provider.baseUrlDefault; + } else { + baseUrlEl.value = ""; + baseUrlEl.placeholder = ""; + } modelEl.innerHTML = provider.models.map(m => ``).join(""); } @@ -251,13 +291,16 @@ async function runAgentTurn(userText) { const apiKey = apiKeyEl.value.trim(); if (!apiKey) throw new Error("Missing API key"); - const { chat } = PROVIDERS[providerEl.value].api; + const provider = PROVIDERS[providerEl.value]; + const { chat } = provider.api; + const baseURL = provider.baseUrlDefault ? baseUrlEl.value.trim() : undefined; messages.push({ role: "user", content: userText }); for (let i = 0; i < 64; i++) { const response = await chat({ apiKey, + baseURL, model: modelEl.value, messages: [{ role: "system", content: SYSTEM_INSTRUCTIONS }, ...messages], tools, diff --git a/src/ifcchat/index.html b/src/ifcchat/index.html index 1ad3bc198d..d7f348d282 100644 --- a/src/ifcchat/index.html +++ b/src/ifcchat/index.html @@ -17,6 +17,7 @@
@@ -31,6 +32,11 @@ +
+ + +
+
From 24acfeaf454f0a7ca0318c4741dcd4634f5f88f2 Mon Sep 17 00:00:00 2001 From: Thomas Krijnen Date: Fri, 3 Apr 2026 10:59:09 +0200 Subject: [PATCH 19/25] Thinking indicator under chat --- src/ifcchat/app.js | 5 +- src/ifcchat/index.html | 7 +- src/ifcchat/style.css | 17 + .../mapping/IfcPointByDistanceExpression.cpp | 9 + src/ifcopenshell-python/ifcopenshell/draw.py | 3 +- src/ifcparse/IfcSchema.h | 1 + src/ifcwrap/IfcGeomWrapper.i | 5 +- src/svgfill/src/arrange_polygons.cpp | 1050 +++++++++++++---- src/svgfill/src/svgfill.cpp | 12 + src/svgfill/src/svgfill.h | 21 +- 10 files changed, 889 insertions(+), 241 deletions(-) diff --git a/src/ifcchat/app.js b/src/ifcchat/app.js index 28a20f1cb7..ae1d7b0ae9 100644 --- a/src/ifcchat/app.js +++ b/src/ifcchat/app.js @@ -110,6 +110,7 @@ const apiKeyLabelEl = $("apiKeyLabel"); const baseUrlRowEl = $("baseUrlRow"); const baseUrlLabelEl = $("baseUrlLabel"); const baseUrlEl = $("baseUrl"); +const thinkingIndicatorEl = $("thinkingIndicator"); const modelEl = $("model"); const providerEl = $("provider"); const ifcFileEl = $("ifcFile"); @@ -180,12 +181,14 @@ function addMessage(role, text) { wrap.querySelector(".chevron").style.transform = expanded ? '' : 'rotate(90deg)'; } } - msgsEl.appendChild(wrap); + msgsEl.insertBefore(wrap, thinkingIndicatorEl); msgsEl.scrollTop = msgsEl.scrollHeight; } function setStatus(text) { statusEl.textContent = text; + thinkingIndicatorEl.hidden = text !== "Thinking…"; + msgsEl.scrollTop = msgsEl.scrollHeight; } const worker = new Worker("./ifc_worker.js", { type: "module" }); diff --git a/src/ifcchat/index.html b/src/ifcchat/index.html index d7f348d282..1048831657 100644 --- a/src/ifcchat/index.html +++ b/src/ifcchat/index.html @@ -96,7 +96,12 @@
-
+
+ +
diff --git a/src/ifcchat/style.css b/src/ifcchat/style.css index ef5f6add8f..9d3ba064c9 100644 --- a/src/ifcchat/style.css +++ b/src/ifcchat/style.css @@ -53,6 +53,23 @@ main { margin: 10px 0; } +.thinking-indicator { + display: inline-flex; + align-items: center; + gap: 10px; + color: #555; + font-size: 80%; +} + +.thinking-indicator[hidden] { + display: none; +} + +.thinking-indicator .spinner { + width: 14px; + height: 14px; +} + .msg .role { font-size: 12px; opacity: 0.7; diff --git a/src/ifcgeom/mapping/IfcPointByDistanceExpression.cpp b/src/ifcgeom/mapping/IfcPointByDistanceExpression.cpp index 180226fb82..f07234a8f1 100644 --- a/src/ifcgeom/mapping/IfcPointByDistanceExpression.cpp +++ b/src/ifcgeom/mapping/IfcPointByDistanceExpression.cpp @@ -52,6 +52,15 @@ taxonomy::ptr mapping::map_impl(const IfcSchema::IfcPointByDistanceExpression* i if (inst->OffsetVertical().has_value()) { auto offset_vertical = inst->OffsetVertical().get() * length_unit_; o += offset_vertical * z; + + auto tmp1 = (z * offset_vertical).eval(); + auto tmp2 = (Eigen::Vector3d(0, 0, 1) * offset_vertical).eval(); + auto tmp3 = (tmp1 - tmp2).eval(); + + std::ostringstream oss; + oss << "local z: " << z.x() << "," << z.y() << "," << z.z() << "; delta: " << tmp3.x() << "," << tmp3.y() << "," << tmp3.z(); + auto osss = oss.str(); + std::wcout << osss.c_str() << std::endl; } if (inst->OffsetLongitudinal().has_value()) { diff --git a/src/ifcopenshell-python/ifcopenshell/draw.py b/src/ifcopenshell-python/ifcopenshell/draw.py index 5f6d761ceb..962dbbb34f 100644 --- a/src/ifcopenshell-python/ifcopenshell/draw.py +++ b/src/ifcopenshell-python/ifcopenshell/draw.py @@ -42,6 +42,7 @@ WHITE = numpy.array((1.0, 1.0, 1.0)) DO_NOTHING = lambda *args: None +ARRANGE_POLYGON_SETTINGS = W.arrange_polygon_settings() if hasattr(W, 'arrange_polygon_settings') else None @dataclass class draw_settings: @@ -536,7 +537,7 @@ def main( *(tup for i, tup in enumerate(zip(path_objects, section_polies, polies)) if has_relevant_zone(i)) ) - arranged = W.arrange_polygons(polies) + arranged = W.arrange_polygons(*filter(None, (ARRANGE_POLYGON_SETTINGS,)), polies) svg_data_3 = W.polygons_to_svg(arranged, False) dom3 = parseString(svg_data_3) svg3 = dom3.childNodes[0] diff --git a/src/ifcparse/IfcSchema.h b/src/ifcparse/IfcSchema.h index 3dedd47a8e..349a81532d 100644 --- a/src/ifcparse/IfcSchema.h +++ b/src/ifcparse/IfcSchema.h @@ -358,6 +358,7 @@ class IFC_PARSE_API entity : public declaration { const std::vector& subtypes() const { return subtypes_; } const std::vector& attributes() const { return attributes_; } + const std::vector& inverse_attributes() const { return inverse_attributes_; } const std::vector& derived() const { return derived_; } const std::vector all_attributes() const { diff --git a/src/ifcwrap/IfcGeomWrapper.i b/src/ifcwrap/IfcGeomWrapper.i index e155c2837e..e992e4beab 100644 --- a/src/ifcwrap/IfcGeomWrapper.i +++ b/src/ifcwrap/IfcGeomWrapper.i @@ -1166,6 +1166,7 @@ ifcopenshell::geometry::taxonomy::item::ptr try_upcast(PyObject* obj0, swig_type %ignore svgfill::line_segments_to_polygons; %ignore svgfill::svg_to_polygons; %ignore svgfill::arrange_polygons; +%ignore svgfill::abstract_arrangement; %template(svg_line_segments) std::vector>; %template(svg_groups_of_line_segments) std::vector>>; @@ -1287,9 +1288,9 @@ ifcopenshell::geometry::taxonomy::item::ptr try_upcast(PyObject* obj0, swig_type } } - std::vector arrange_polygons(const std::vector& polygons) { + std::vector arrange_polygons(svgfill::arrange_polygon_settings settings, const std::vector& polygons) { std::vector r; - if (svgfill::arrange_polygons(polygons, r)) { + if (svgfill::arrange_polygons(settings, polygons, r)) { return r; } else { throw std::runtime_error("Failed to arrange polygons"); diff --git a/src/svgfill/src/arrange_polygons.cpp b/src/svgfill/src/arrange_polygons.cpp index 4fa20c68dc..3ce49011f7 100644 --- a/src/svgfill/src/arrange_polygons.cpp +++ b/src/svgfill/src/arrange_polygons.cpp @@ -350,6 +350,8 @@ find_overlaps(const std::vector& polygons) { class DebugWriter { public: + DebugWriter() : enabled_(false) {} + DebugWriter(bool enabled, const std::string& filename_prefix) : enabled_(enabled) { if (enabled_) { @@ -368,6 +370,43 @@ class DebugWriter { } } + DebugWriter(const DebugWriter&) = delete; + + DebugWriter(DebugWriter&& other) noexcept + : obj(std::move(other.obj)), vi(other.vi), svg(std::move(other.svg)), enabled_(other.enabled_), last_segment_name_(std::move(other.last_segment_name_)) + { + other.enabled_ = false; + other.vi = 1; + other.last_segment_name_.clear(); + } + + DebugWriter& operator=(const DebugWriter&) = delete; + + DebugWriter& operator=(DebugWriter&& other) noexcept { + if (this == &other) { + return *this; + } + + if (enabled_) { + svg << "\n"; + obj << std::flush; + obj.close(); + svg.close(); + } + + obj = std::move(other.obj); + svg = std::move(other.svg); + vi = other.vi; + enabled_ = other.enabled_; + last_segment_name_ = std::move(other.last_segment_name_); + + other.enabled_ = false; + other.vi = 1; + other.last_segment_name_.clear(); + + return *this; + } + void write_polygon(const Polygon_2& polygon, const std::string& name) { if (enabled_) { write_polygon_to_obj_(obj, vi, true, polygon, name); @@ -387,7 +426,7 @@ class DebugWriter { obj << "l " << vi++; obj << " " << vi++ << "\n"; - svg << ""; + svg << "\n"; obj << std::flush; } @@ -468,7 +507,7 @@ class DebugWriter { } }; -void eliminate_overlaps(double OVERLAP_RESOLUTION_DISTANCE, std::vector& polygons) { +void eliminate_overlaps(DebugWriter& debug_writer, double OVERLAP_RESOLUTION_DISTANCE, std::vector& polygons) { // solve overlaps by means of subtraction // loop over overlaps and subtract the smaller polygon from the larger one @@ -576,11 +615,40 @@ void eliminate_overlaps(double OVERLAP_RESOLUTION_DISTANCE, std::vectorarea() << " " << poly2->area() << std::endl; + + bool is_ = edge == std::make_pair(25, 27); + bool success = false; if ((mp1 = maybe_take_first_if_single_item(create_and_convert_offset_polygon(OVERLAP_RESOLUTION_DISTANCE, *poly2)))) { + if (is_) { + debug_writer.write_polygon(*mp1, "mp1"); + } + smooth_polygon(OVERLAP_RESOLUTION_DISTANCE / 100., *mp1); + if (is_) { + debug_writer.write_polygon(*mp1, "mp1b"); + } if ((mp2 = subtract_retain_largest(*poly1, *mp1))) { + if (is_) { + debug_writer.write_polygon(*mp2, "mp2"); + } + smooth_polygon(OVERLAP_RESOLUTION_DISTANCE / 100., *mp2); + if (is_) { + debug_writer.write_polygon(*mp2, "mp2b"); + } if ((mp3 = maybe_take_first_if_single_item(create_and_convert_offset_polygon(OVERLAP_RESOLUTION_DISTANCE * 2, *mp2)))) { + if (is_) { + debug_writer.write_polygon(*mp3, "mp3"); + } + smooth_polygon(OVERLAP_RESOLUTION_DISTANCE / 100., *mp3); + if (is_) { + debug_writer.write_polygon(*mp3, "mp3b"); + } if ((mp4 = subtract_retain_largest(*poly2, *mp3))) { + if (is_) { + debug_writer.write_polygon(*mp4, "mp4"); + } *poly1 = *mp2; *poly2 = *mp4; success = true; @@ -589,6 +657,8 @@ void eliminate_overlaps(double OVERLAP_RESOLUTION_DISTANCE, std::vectorarea() << " " << poly2->area() << std::endl; + if (!success) { eliminated_polies.insert(swap ? edge.first : edge.second); continue; @@ -777,14 +847,19 @@ Polygon_with_holes_2 subdivide_polygon(double max_distance, const Polygon_with_h std::tuple< std::map>, std::map>, - std::map, std::vector*>>> -build_line_graph(const std::vector& input_polygons, SegmentLookup& segment_lookup, const std::vector& triangular_polygons) { + std::map, std::vector*>>, + std::map +> +build_line_graph(const std::vector& input_polygons, SegmentLookup& segment_lookup, const std::vector& triangular_polygons) +{ + // Build maps of triangle -> edge and edge -> triangle in order to do traversal on the 'corridor mesh' std::map, std::vector*>> segment_to_facet; std::map, std::vector*>> segment_to_input_facet; std::map, Point_2> segment_to_midpoint; std::map> midpoint_to_segment; std::map*, std::vector>> facet_to_segment; + std::map midpoint_to_edge_length; for (auto& tri : triangular_polygons) { for (size_t i = 0; i < 3; ++i) { @@ -813,6 +888,7 @@ build_line_graph(const std::vector& input_polygons, SegmentLookup& se if (p1index != input_polygons.end() && p2index != input_polygons.end() && p1index != p2index) { segment_to_midpoint[p.first] = center; midpoint_to_segment[center] = p.first; + midpoint_to_edge_length[center] = std::sqrt(CGAL::to_double(CGAL::squared_distance(p.first.first, p.first.second))); } } @@ -832,7 +908,7 @@ build_line_graph(const std::vector& input_polygons, SegmentLookup& se } } - return {line_graph, midpoint_to_segment, segment_to_input_facet}; + return {line_graph, midpoint_to_segment, segment_to_input_facet, midpoint_to_edge_length}; } std::set> find_triangles(const std::map>& line_graph) { @@ -1118,66 +1194,91 @@ std::list> extend_end_vertices_based_on_input( const Graph2D& G, const std::map>& midpoint_to_segment, const std::map, std::vector*>>& segment_to_input_facet, - const Polygon_list& inner_offset, - const SegmentLookup& segment_lookup + const Polygon_list& outer_perimiter, + const SegmentLookup& segment_lookup, + const K::FT& max_projection_distance ){ std::list> constructed_segments; - for (auto it = G.vertices_begin(); it != G.vertices_end(); ++it) { - if (it->second.size() == 1) { - auto& M = it->first; + std::set processed_vertices; - const std::pair* q = nullptr; + while (true) { + // The idea was to peal off 1-degree vertices when projecting them did not result into + // nearby intersections with the outer perimiter. This in case there would be turns near + // the perimeter, which would be eliminated by pealing off the vertices, which would then + // require out of the loop because of invalidated iterators. For now we decided to stick + // to a projection of the vertex onto the perimeter segment when the projection distance + // exceeds a threshold. + bool broke_out = false; - if (midpoint_to_segment.find(M) == midpoint_to_segment.end()) { - typename K::FT min_sq_distance = std::numeric_limits::infinity(); - for (auto& pa : midpoint_to_segment) { - if (CGAL::squared_distance(pa.first, M) < min_sq_distance) { - q = &pa.second; - min_sq_distance = CGAL::squared_distance(pa.first, M); - } + for (auto it = G.vertices_begin(); it != G.vertices_end(); ++it) { + if (it->second.size() == 1) { + auto& M = it->first; + + if (processed_vertices.find(M) != processed_vertices.end()) { + continue; } - } else { - q = &midpoint_to_segment.find(M)->second; - } - if (q == nullptr) { - continue; - } + const std::pair* q = nullptr; - bool handled_as_graph_path = false; + if (midpoint_to_segment.find(M) == midpoint_to_segment.end()) { + typename K::FT min_sq_distance = std::numeric_limits::infinity(); + for (auto& pa : midpoint_to_segment) { + if (CGAL::squared_distance(pa.first, M) < min_sq_distance) { + q = &pa.second; + min_sq_distance = CGAL::squared_distance(pa.first, M); + } + } + } else { + q = &midpoint_to_segment.find(M)->second; + } - // distance from unioned - shoot ray? - if (segment_to_input_facet.find(*q)->second.size() == 2) { - for (auto& bnd : inner_offset) { - // if point M is contained in bnd interior: - // if (!bnd.has_on_unbounded_side(M)) { - if (bnd.has_on_bounded_side(M)) { - auto& incoming = *it->second.begin(); - // create ray incoming -> M - CGAL::Ray_2 ray(incoming, M - incoming); - // intersect ray with boundary - boost::optional> closest_segment; - boost::optional> closest_intersection_point; - K::FT sq_distance_along_ray = std::numeric_limits::infinity(); - for (auto jt = bnd.edges_begin(); jt != bnd.edges_end(); ++jt) { - const auto& seg = *jt; - auto x = CGAL::intersection(ray, seg); - if (x) { - if (auto* xp = variant_get>(&*x)) { - auto dist = ((*xp) - M).squared_length(); - if (dist < sq_distance_along_ray) { - closest_segment = seg; - closest_intersection_point = *xp; - sq_distance_along_ray = dist; + if (q == nullptr) { + continue; + } + + bool handled_as_graph_path = false; + + // distance from unioned - shoot ray? + if (segment_to_input_facet.find(*q)->second.size() == 2) { + for (auto& bnd : outer_perimiter) { + // if point M is contained in bnd interior: + // if (!bnd.has_on_unbounded_side(M)) { + if (bnd.has_on_bounded_side(M)) { + auto& incoming = *it->second.begin(); + // create ray incoming -> M + CGAL::Ray_2 ray(incoming, M - incoming); + + std::cerr << "Extending end vertex " << M << " along ray " << ray << " to boundary of input polygon" << std::endl; + + // intersect ray with boundary + boost::optional> closest_segment; + boost::optional> closest_intersection_point; + K::FT sq_distance_along_ray = std::numeric_limits::infinity(); + for (auto jt = bnd.edges_begin(); jt != bnd.edges_end(); ++jt) { + const auto& seg = *jt; + auto x = CGAL::intersection(ray, seg); + if (x) { + if (auto* xp = variant_get>(&*x)) { + auto dist = ((*xp) - M).squared_length(); + std::cerr << " - found " << *xp << " on segment " << seg << " with distance " << std::sqrt(CGAL::to_double(dist)) << std::endl; + if (dist < sq_distance_along_ray) { + if (dist < (max_projection_distance * max_projection_distance)) { + closest_segment = seg; + closest_intersection_point = *xp; + sq_distance_along_ray = dist; + } else { + + } + } } } } - } - if (closest_intersection_point) { - constructed_segments.push_front({M, *closest_intersection_point}); - break; + if (closest_intersection_point) { + constructed_segments.push_front({M, *closest_intersection_point}); + processed_vertices.insert(M); + break; #if 0 Graph2D GGG(bnd); GGG.refine(*GGG.query(*closest_intersection_point, 0.01), *closest_intersection_point); @@ -1217,12 +1318,33 @@ std::list> extend_end_vertices_based_on_input( break; } #endif - } else { - std::cerr << "Warning: no intersection found when extending end vertex, this will likely result in invalid topology" << std::endl; + } else { + + // Loop over boundary segments, and project point onto it, take the closest + K::FT closest_distance = std::numeric_limits::infinity(); + boost::optional> closest_point; + for (auto& poly : outer_perimiter) { + for (auto jt = poly.edges_begin(); jt != poly.edges_end(); ++jt) { + auto seg = *jt; + auto Pp = seg.supporting_line().projection(M); + if (seg.has_on(Pp)) { + auto d = CGAL::squared_distance(Pp, M); + if (d < closest_distance) { + closest_distance = d; + closest_point = Pp; + } + } + } + } + + if (closest_point) { + constructed_segments.push_front({M, *closest_point}); + processed_vertices.insert(M); + } + } } } } - } #if 0 if (!handled_as_graph_path) { @@ -1267,6 +1389,11 @@ std::list> extend_end_vertices_based_on_input( constructed_segments.push_front({avg, R}); } #endif + } + } + + if (!broke_out) { + break; } } @@ -1355,8 +1482,6 @@ void fuse_corridor_halves_with_input(Arrangement_2& arr, Graph2D& G, SegmentL } } -#include - class Segment_2_less { public: bool operator()(const Segment_2& a, const Segment_2& b) const { @@ -1367,7 +1492,112 @@ class Segment_2_less { } }; -void clean_noisy_paths(Arrangement_2& arr, SegmentLookup& segment_lookup) { +std::vector arrangement_cell_iou(Arrangement_2& left, Arrangement_2& right) { + + using Walk_pl = CGAL::Arr_walk_along_line_point_location; + Walk_pl walk_pl(right); + + std::set visited_faces_on_right; + + std::vector return_values; + + for (auto it = left.faces_begin(); it != left.faces_end(); ++it) { + if (!it->is_unbounded()) { + // convert arr facet to polygon with holes + auto polygon_exterior = circ_to_poly(it->outer_ccb()); + Polygon_with_holes_2 pwh(polygon_exterior); + for (auto hit = it->inner_ccbs_begin(); hit != it->inner_ccbs_end(); ++hit) { + pwh.add_hole(circ_to_poly(*hit)); + } + + CGAL::Polygon_triangulation_decomposition_2 decompositor; + std::vector temp; + decompositor(pwh, std::back_inserter(temp)); + + std::set visited_points; + + while (true) { + // select triangle edge that has largest squared edge length times distance from polygon exterior + K::FT max_score = -std::numeric_limits::infinity(); + Point_2 best_point; + for (auto& tri : temp) { + for (size_t i = 0; i < 3; ++i) { + size_t j = (i + 1) % 3; + auto& pi = tri.vertex(i); + auto& pj = tri.vertex(j); + + auto center_point = CGAL::ORIGIN + (((pi - CGAL::ORIGIN) + (pj - CGAL::ORIGIN)) / 2); + + K::FT min_dist = std::numeric_limits::infinity(); + for (auto eit = polygon_exterior.edges_begin(); eit != polygon_exterior.edges_end(); ++eit) { + auto ep = eit->source(); + auto eq = eit->target(); + Segment_2 seg(ep, eq); + auto dist = CGAL::squared_distance(center_point, seg); + if (dist < min_dist) { + min_dist = dist; + } + } + + auto sq_length = CGAL::squared_distance(pi, pj); + + auto score = sq_length * min_dist; + if (score > max_score && visited_points.count(center_point) == 0) { + max_score = score; + best_point = center_point; + } + } + } + + auto res = walk_pl.locate(best_point); + if (auto* v = boost::get(&res)) { + if (visited_faces_on_right.count(*v) > 0) { + return_values.push_back(0); + } else { + // convert arr facet to polygon with holes + auto polygon_exterior = circ_to_poly((*v)->outer_ccb()); + Polygon_with_holes_2 pwh_right(polygon_exterior); + for (auto hit = (*v)->inner_ccbs_begin(); hit != (*v)->inner_ccbs_end(); ++hit) { + pwh_right.add_hole(circ_to_poly(*hit)); + } + + // compute intersection over union of pwh and the original polygon + if (CGAL::do_intersect(pwh, pwh_right)) { + std::vector result; + CGAL::intersection(pwh, pwh_right, std::back_inserter(result)); + typename K::FT intersection_area = 0; + for (auto& r : result) { + auto poly_area = r.outer_boundary().area(); + for (auto& h : r.holes()) { + poly_area -= h.area(); + } + intersection_area += poly_area; + } + CGAL::Polygon_with_holes_2 poly12; + CGAL::join(pwh, pwh_right, poly12); + typename K::FT union_area = poly12.outer_boundary().area(); + for (auto& h : poly12.holes()) { + union_area -= h.area(); + } + return_values.push_back(intersection_area / union_area); + } else { + return_values.push_back(0); + } + } + visited_faces_on_right.insert(*v); + break; + } else { + // Not in facet on right, retry another point + continue; + } + } + } + } + + return return_values; +} + +void clean_noisy_paths(DebugWriter& debug_output, Arrangement_2& arr, SegmentLookup& segment_lookup, double& threshold) { using SK = CGAL::Simple_cartesian; CGAL::Cartesian_converter C{}; @@ -1418,15 +1648,17 @@ void clean_noisy_paths(Arrangement_2& arr, SegmentLookup& segment_lookup) { auto [dv, dl] = get_dir(s); best = std::min(best, angle(dv)); } - return (best + 0.1) / own_length; + return (best + 0.01) / own_length; }; + std::cerr << "badnesses:"; std::map badnesses; for (auto& e : edges) { badnesses[e] = edge_badness(e); + std::cerr << " (" << e.source().x() << "," << e.source().y() << ") - (" << e.target().x() << "," << e.target().y() << "): " << badnesses[e] << ";"; } + std::cerr << std::endl; - double thr; { std::vector tmp; tmp.reserve(badnesses.size()); @@ -1435,12 +1667,14 @@ void clean_noisy_paths(Arrangement_2& arr, SegmentLookup& segment_lookup) { } std::nth_element(tmp.begin(), tmp.begin() + tmp.size() / 2, tmp.end()); double med = tmp[tmp.size() / 2]; - thr = 10.0 * med; + threshold = 4.0 * med; } + std::cerr << "badness threshold: " << threshold << std::endl; + std::set bad_edges; for (auto& p : badnesses) { - if (p.second > thr) { + if (p.second > threshold) { bad_edges.insert(p.first); } } @@ -1568,10 +1802,57 @@ void clean_noisy_paths(Arrangement_2& arr, SegmentLookup& segment_lookup) { return best_x; }; + auto process_modifications = [&]( + Arrangement_2& arr_, + const std::set>& to_remove_, + const std::vector>& to_insert_) { + for (auto& e : to_remove_) { + bool removed = false; + for (auto he = arr_.edges_begin(); he != arr_.edges_end(); ++he) { + auto a = he->source()->point(); + auto b = he->target()->point(); + if ((a == e.first && b == e.second) || (a == e.second && b == e.first)) { + CGAL::remove_edge(arr_, he); + removed = true; + break; + } + } + if (!removed) { + std::cerr << "Warning: unable to locate edge for removal, skipping" << std::endl; + } + } + + for (auto& pq : to_insert_) { + if (pq.first == pq.second) { + continue; + } + CGAL::insert(arr_, Segment_2(pq.first, pq.second)); + } + }; + + size_t path_index = 0; for (auto& path : bad_paths) { + decltype(to_remove) to_remove_this_path; + decltype(to_insert) to_insert_this_path; + + std::cerr << "Processing bad path:"; + for (size_t i = 0; i < path.size() - 1; ++i) { + auto& a = path[i]; + auto& b = path[i + 1]; + std::cerr << " (" << a.x() << "," << a.y() << ") - (" << b.x() << "," << b.y() << ");"; + } + std::cerr << std::endl; + + for (size_t i = 0; i < path.size() - 1; ++i) { + auto& a = path[i]; + auto& b = path[i + 1]; + + debug_output.write_segment(a, b, "arr_bad_path path_nr_" + std::to_string(path_index)); + } + auto x = collapse_path(path); if (!x) { - // std::cerr << "Unable to collapse path, skipping" << std::endl; + std::cerr << "Unable to collapse path, skipping" << std::endl; continue; } @@ -1585,7 +1866,7 @@ void clean_noisy_paths(Arrangement_2& arr, SegmentLookup& segment_lookup) { double new_length = std::sqrt(CGAL::to_double((path.front() - *x).squared_length())) + std::sqrt(CGAL::to_double((path.back() - *x).squared_length())); if (new_length > orig_length * 2 || orig_length > new_length * 2) { - // std::cerr << "Collapsing path would increase length too much, skipping" << std::endl; + std::cerr << "Collapsing path would increase length too much, skipping" << std::endl; continue; } @@ -1604,75 +1885,326 @@ void clean_noisy_paths(Arrangement_2& arr, SegmentLookup& segment_lookup) { auto& b = path[i + 1]; if (a < b) { to_remove.insert({a, b}); + to_remove_this_path.insert({a, b}); } else { to_remove.insert({b, a}); + to_remove_this_path.insert({b, a}); } } auto s = path.front(); auto t = path.back(); if (s != *x) { to_insert.push_back({s, *x}); + to_insert_this_path.push_back({s, *x}); + + debug_output.write_segment(s, *x, "corrected_path path_nr_" + std::to_string(path_index)); } if (t != *x) { to_insert.push_back({t, *x}); + to_insert_this_path.push_back({t, *x}); + + debug_output.write_segment(t, *x, "corrected_path path_nr_" + std::to_string(path_index)); } + + path_index += 1; + +#if 1 + process_modifications(arr, to_remove_this_path, to_insert_this_path); +#else + auto arr_copy = arr; + process_modifications(arr_copy, to_remove_this_path, to_insert_this_path); + auto ious = arrangement_cell_iou(arr, arr_copy); + for (auto& iou : ious) { + std::cerr << " - cell iou: " << CGAL::to_double(iou) << std::endl; + } + std::swap(arr_copy, arr); +#endif } - /* - using Walk_pl = CGAL::Arr_walk_along_line_point_location; - Walk_pl walk_pl(arr); + process_modifications(arr, to_remove, to_insert); +} - for (auto& e : to_remove) { - // debug_output.write_segment(e->source()->point(), e->target()->point(), "arr_bad_remove"); - auto res = walk_pl.locate(e.first); - if (auto* v = boost::get(&res)) { - Arrangement_2::Halfedge_around_vertex_circulator first, curr; - first = curr = (*v)->incident_halfedges(); - size_t i = 0; - std::array pts; - std::array hes; - do { - Arrangement_2::Vertex_const_handle u = curr->source(); - hes[i] = curr; - pts[i++] = u->point(); - } while (++curr != first); +template +void next_circular(typename Vec::const_iterator& it, const Vec& vec) { + std::advance(it, 1); + if (it == vec.end()) { + it = vec.begin(); + } +} +template +void previous_circular(typename Vec::const_iterator& it, const Vec& vec) { + if (it == vec.begin()) { + it = vec.end(); + } + std::advance(it, -1); +} - if ((*v)->point() != e.first) { - std::cerr << "Warning: unable to locate vertex for edge removal, skipping" << std::endl; - continue; +template +std::size_t circular_distance(typename Vec::const_iterator first, + typename Vec::const_iterator last, + const Vec& vec) { + if (first <= last) { + return static_cast(last - first); + } + return static_cast(vec.end() - first) + static_cast(last - vec.begin()); +} + +template +std::pair +longest_wrapping_true_run(const Vec& v, Pred pred) { + using It = typename Vec::const_iterator; + + const auto n = v.size(); + if (n == 0) { + return {v.end(), v.end()}; + } + + // Find best non-wrapping run + std::size_t best_len = 0; + std::size_t best_start = 0; + + std::size_t curr_len = 0; + std::size_t curr_start = 0; + + for (std::size_t i = 0; i < n; ++i) { + if (pred(v[i])) { + if (curr_len == 0) { + curr_start = i; + } + ++curr_len; + if (curr_len > best_len) { + best_len = curr_len; + best_start = curr_start; } } else { - std::cerr << "Warning: unable to locate vertex for edge removal, skipping" << std::endl; - continue; + curr_len = 0; } } - */ - for (auto& e : to_remove) { - bool removed = false; - for (auto he = arr.edges_begin(); he != arr.edges_end(); ++he) { - auto a = he->source()->point(); - auto b = he->target()->point(); - if ((a == e.first && b == e.second) || (a == e.second && b == e.first)) { - CGAL::remove_edge(arr, he); - removed = true; - break; + // Count leading true + std::size_t leading = 0; + while (leading < n && pred(v[leading])) { + ++leading; + } + + // All true + if (leading == n) { + return {v.begin(), v.end()}; + } + + // Count trailing true + std::size_t trailing = 0; + while (trailing < n && pred(v[n - 1 - trailing])) { + ++trailing; + } + + // Wrapped run = [n - trailing, n) + [0, leading) + const std::size_t wrapped_len = leading + trailing; + + if (wrapped_len > best_len) { + It first = v.begin() + static_cast(n - trailing); + It last = v.begin() + static_cast(leading); + return {first, last}; + } + + It first = v.begin() + static_cast(best_start); + It last = first + static_cast(best_len); + return {first, last}; +} + +void clean_noisy_bounds(DebugWriter& debug_output, Arrangement_2& arr, SegmentLookup& segment_lookup, double threshold) { + using SK = CGAL::Simple_cartesian; + CGAL::Cartesian_converter C{}; + + auto other = [](const Segment_2& e, const Point_2& v) { + return (e.source() == v) ? e.target() : e.source(); + }; + + auto edge_badness = [&](const Segment_2& e) -> double { + auto closest = segment_lookup.n_closest_input_segments(e, 2); + if (closest.size() != 2) { + throw std::runtime_error("Unable to locate two nearby edges"); + } + + auto get_dir = [&](const Segment_2& s) { + auto a = C(s.source()); + auto b = C(s.target()); + SK::Vector_2 v = b - a; + double l = std::sqrt(v.squared_length()); + if (l <= 1e-12) { + return std::make_pair(SK::Vector_2(0, 0), 0.); + } + return std::make_pair(v / l, l); + }; + + auto [own_dir, own_length] = get_dir(e); + + auto angle = [&](const SK::Vector_2& ov) { + double d = std::abs(own_dir * ov); + if (d > 1.0) { + d = 1.0; + } + return std::acos(d); + }; + + double best = std::numeric_limits::infinity(); + for (auto& s : closest) { + auto [dv, dl] = get_dir(s); + best = std::min(best, angle(dv)); + } + return (best + 0.01) / own_length; + }; + + size_t facet_index = 0; + for (auto it = arr.faces_begin(); it != arr.faces_end(); ++it, ++facet_index) { + std::cout << "facet_index " << facet_index << std::endl; + if (!it->is_unbounded()) { + std::set> to_remove; + std::vector> to_insert; + + std::vector segs; + std::vector vertices; + std::vector halfedges; + + auto circ = it->outer_ccb(); + do { + auto a = circ->source()->point(); + auto b = circ->target()->point(); + segs.emplace_back(a, b); + vertices.push_back(circ->source()); + halfedges.push_back(circ); + ++circ; + } while (circ != it->outer_ccb()); + + std::cerr << "badnesses:"; + std::vector badnesses; + for (auto& e : segs) { + badnesses.push_back(edge_badness(e)); + std::cerr << " (" << e.source().x() << "," << e.source().y() << ") - (" << e.target().x() << "," << e.target().y() << "): " << badnesses.back() << ";"; + } + std::cerr << std::endl; + + auto bit = std::min_element(badnesses.begin(), badnesses.end()); + if (*bit > threshold) { + std::cerr << "All edges are good, skipping" << std::endl; + continue; + } + + auto it_pair = longest_wrapping_true_run(badnesses, [&](double d) { return d > threshold; }); + auto N = circular_distance(it_pair.first, it_pair.second, badnesses); + + if (N == 0) { + std::cerr << "Unable to find run of bad edges, skipping" << std::endl; + continue; + } + + std::vector> incoming_paths; + + std::cout << "range " << std::distance(badnesses.cbegin(), it_pair.first) << " to " << std::distance(badnesses.cbegin(), it_pair.second) << " length " << N << std::endl; + + auto jt = it_pair.first; + for (std::size_t k = 0; k < N; ++k, next_circular(jt, badnesses)) { + + std::cout << " at " << std::distance(badnesses.cbegin(), jt) << " badness: " << *jt << std::endl; + + auto he = halfedges[std::distance(badnesses.cbegin(), jt)]; + to_remove.insert({he->source()->point(), he->target()->point()}); + debug_output.write_segment(he->source()->point(), he->target()->point(), "arr_bad_bound facet_" + std::to_string(facet_index)); + + Arrangement_2::Vertex_handle v = he->source(); + + // circle around other edges onto v + Arrangement_2::Halfedge_around_vertex_circulator first, curr; + first = curr = v->incident_halfedges(); + do { + Arrangement_2::Vertex_handle u = curr->source(); + if (curr->face() != it && curr->twin()->face() != it) { + + // loop until we find a 3-degree vertex, or we come back to the start + std::vector path{v->point(), u->point()}; + auto he = curr; + + while (u->degree() == 2 && u != v && path.size() < 10) { + std::vector hes; + + { + Arrangement_2::Halfedge_around_vertex_circulator first, curr; + first = curr = u->incident_halfedges(); + do { + hes.push_back(curr); + curr++; + } while (curr != first); + } + + auto next_he = hes.front() != he && hes.front() != he->twin() ? hes.front() : hes.back(); + auto next_v = next_he->target() != u ? next_he->target() : next_he->source(); + + path.push_back(next_v->point()); + u = next_v; + } + incoming_paths.push_back(std::move(path)); + } + } while (++curr != first); + } + + const std::size_t start = + static_cast(std::distance(badnesses.cbegin(), it_pair.first)); + + auto n = badnesses.size(); + + auto wrap = [n](std::ptrdiff_t i) -> std::size_t { + i %= static_cast(n); + if (i < 0) { + i += static_cast(n); + } + return static_cast(i); + }; + + const std::size_t ib = start; + const std::size_t ia = wrap(static_cast(start) - 1); + const std::size_t ic = wrap(static_cast(start + N)); + const std::size_t id = wrap(static_cast(start + N + 1)); + + auto a = vertices.begin() + static_cast(ia); + auto b = vertices.begin() + static_cast(ib); + auto c = vertices.begin() + static_cast(ic); + auto d = vertices.begin() + static_cast(id); + + CGAL::Ray_2 r1((*a)->point(), (*b)->point()); + CGAL::Ray_2 r2((*d)->point(), (*c)->point()); + + std::cout << "a: (" << (*a)->point().x() << "," << (*a)->point().y() << ") b: (" << (*b)->point().x() << "," << (*b)->point().y() << ") c: (" << (*c)->point().x() << "," << (*c)->point().y() << ") d: (" << (*d)->point().x() << "," << (*d)->point().y() << ")" << std::endl; + + auto x = CGAL::intersection(r1, r2); + if (x) { + if (auto* xp = variant_get>(&*x)) { + std::cout << "ray xp: (" << xp->x() << "," << xp->y() << ")" << std::endl; + to_insert.emplace_back((*b)->point(), *xp); + to_insert.emplace_back((*c)->point(), *xp); + + debug_output.write_segment((*b)->point(), *xp, "corrected_bound facet_" + std::to_string(facet_index)); + debug_output.write_segment((*c)->point(), *xp, "corrected_bound facet_" + std::to_string(facet_index)); + } + } else { + CGAL::Line_2 r1((*a)->point(), (*b)->point()); + CGAL::Line_2 r2((*d)->point(), (*c)->point()); + + auto x = CGAL::intersection(r1, r2); + if (x) { + if (auto* xp = variant_get>(&*x)) { + std::cout << "line xp: (" << xp->x() << "," << xp->y() << ")" << std::endl; + to_insert.emplace_back((*b)->point(), *xp); + to_insert.emplace_back((*c)->point(), *xp); + + debug_output.write_segment((*b)->point(), *xp, "corrected_bound facet_" + std::to_string(facet_index)); + debug_output.write_segment((*c)->point(), *xp, "corrected_bound facet_" + std::to_string(facet_index)); + } + } } } - if (!removed) { - std::cerr << "Warning: unable to locate edge for removal, skipping" << std::endl; - } } - - for (auto& pq : to_insert) { - if (pq.first == pq.second) { - continue; - } - CGAL::insert(arr, Segment_2(pq.first, pq.second)); - // debug_output.write_segment(pq.first, pq.second, "arr_bad_insert"); - } - } void remove_colinear_vertices(Arrangement_2& arr) { @@ -1725,20 +2257,31 @@ class timer { public: class entry { public: + entry() {} + entry(std::map::const_iterator start_it) : start_it(start_it) {} + void stop() { - auto end = std::chrono::high_resolution_clock::now(); - auto duration = std::chrono::duration(end - start_it->second).count(); - std::cerr << "Timing for " << start_it->first << ": " << duration << " ms" << std::endl; + if (start_it) { + auto end = std::chrono::high_resolution_clock::now(); + auto duration = std::chrono::duration(end - start_it.value()->second).count(); + std::cerr << "Timing for " << start_it.value()->first << ": " << duration << " ms" << std::endl; + } } private: - std::map::const_iterator start_it; + std::optional::const_iterator> start_it; }; + timer(bool enabled = true) : enabled_(enabled) {} + entry start(const std::string& name) { - return entry(timings_.insert({name, std::chrono::high_resolution_clock::now()}).first); + if (enabled_) { + return entry(timings_.insert({name, std::chrono::high_resolution_clock::now()}).first); + } else { + return entry(); + } } private: @@ -1746,27 +2289,30 @@ class timer { std::string, std::chrono::high_resolution_clock::time_point> timings_; + + bool enabled_; }; -void arrange_cgal_polygons(const std::vector& input_polygons_, std::vector& output_polygons, double polygon_offset_distance = -1.) { +void arrange_cgal_polygons(svgfill::arrange_polygon_settings settings, const std::vector& input_polygons_, std::vector& output_polygons, double polygon_offset_distance = -1.) { static const double OVERLAP_RESOLUTION_DISTANCE = 1.e-1; // even larger amount of inset so that outer perimeter is safely within all input polygons even when overlap resolution is applied // no, `1.e-2 + 1.e-5` creates issues with the outer perimeter, are there other tolerances in play? static const double OUTER_PERIMITER_ADDITIONAL_INSET_AMOUNT = 1.e-5; -#ifdef SVGFILL_DEBUG - auto t = std::time(nullptr); - auto tm = *std::localtime(&t); + DebugWriter debug_output; + if (settings.debug_output) { + auto t = std::time(nullptr); + auto tm = *std::localtime(&t); - std::ostringstream oss; - oss << std::put_time(&tm, "arrangement_%Y%m%d%H%M%S"); - auto now = oss.str(); - DebugWriter debug_output(true, now); -#else - DebugWriter debug_output(false, ""); -#endif + std::ostringstream oss; + oss << std::put_time(&tm, "arrangement_%Y%m%d%H%M%S"); + auto now = oss.str(); + debug_output = DebugWriter(true, now); + } else { + debug_output = DebugWriter(false, ""); + } - timer timer; + timer timer(settings.debug_output); auto t0 = timer.start("input"); @@ -1794,7 +2340,7 @@ void arrange_cgal_polygons(const std::vector& input_polygons_, std::v t0.stop(); t0 = timer.start("overlap elimination"); - eliminate_overlaps(OVERLAP_RESOLUTION_DISTANCE, input_polygons); + eliminate_overlaps(debug_output, OVERLAP_RESOLUTION_DISTANCE, input_polygons); t0.stop(); @@ -1813,79 +2359,79 @@ void arrange_cgal_polygons(const std::vector& input_polygons_, std::v debug_output.write_polygons(input_polygons, "processed_input"); -#if 1 - t0 = timer.start("outer perimeter"); - - // Find the outer perimeter using offset - union - negative offset - std::vector offset_polygons; - for (auto& r : input_polygons) { - auto R = r; - if (!R.is_counterclockwise_oriented()) { - R.reverse_orientation(); - } - - // Overlap removal can also result in close points causing problems when converted into non-exact nt - remove_close_points(R); - - auto ps = create_and_convert_offset_polygon(polygon_offset_distance, R); - for (auto& p : ps) { - if (!p.is_simple()) { - throw std::runtime_error("Complex polygon originated from offset"); - } - } - offset_polygons.insert(offset_polygons.end(), ps.begin(), ps.end()); - } - - debug_output.write_polygons(offset_polygons, "offset_input"); - - // Perform Boolean union on the offset polygons - std::vector unioned_polygons; - CGAL::join(offset_polygons.begin(), offset_polygons.end(), std::back_inserter(unioned_polygons)); - - if (unioned_polygons.size() > 1) { - // @todo this is currently one of the major limitations in the code that still can be eliminated - // by grouping the input polygons by their perimiter polygon in unioned_polygons - std::sort(unioned_polygons.begin(), unioned_polygons.end(), [](auto& p, auto& q) { return p.outer_boundary().area() > q.outer_boundary().area(); }); - } - - debug_output.write_polygon(unioned_polygons.front().outer_boundary(), "offset_joined"); - - Polygon_2 fused_removed_close_points = unioned_polygons.front().outer_boundary(); - remove_close_points(fused_removed_close_points, 1.e-4); - - // Apply negative offset to get the outer perimeter polygon - auto outer_perimiter = create_and_convert_offset_polygon( - // Because polygon_offset is inexact, make sure our inset distance is slightly larger - // std::nexttoward(-polygon_offset_distance, -std::numeric_limits::infinity()), - - // 1.e-8 even was too little and still resulted in slivers of triangle around the perimeter - -polygon_offset_distance - OUTER_PERIMITER_ADDITIONAL_INSET_AMOUNT, - fused_removed_close_points); - - debug_output.write_polygons(outer_perimiter, "outer_perimiter"); -#else - std::map> neighbour_map; - build_radial_neighbour_map(input_polygons, polygon_offset_distance, neighbour_map); - - auto start_vertex = neighbour_map.rbegin()->first; - auto next_vertex = neighbour_map.rbegin()->second.front(); - - std::vector cycle = {start_vertex, next_vertex}; - while (cycle.back() != cycle.front()) { - const auto& incoming_from = *(cycle.rbegin() + 1); - const auto& nb = neighbour_map[cycle.back()]; - auto it = std::find(nb.begin(), nb.end(), incoming_from); - // cycle it -1 around nb - if (it == nb.begin()) { - it == nb.end() - 1; - } else { - --it; - } - cycle.push_back(*it); - } std::vector outer_perimiter; - outer_perimiter.emplace_back(cycle.begin(), cycle.end()); -#endif + if (settings.outer_perimiter_algo == 0) { + t0 = timer.start("outer perimeter"); + + // Find the outer perimeter using offset - union - negative offset + std::vector offset_polygons; + for (auto& r : input_polygons) { + auto R = r; + if (!R.is_counterclockwise_oriented()) { + R.reverse_orientation(); + } + + // Overlap removal can also result in close points causing problems when converted into non-exact nt + remove_close_points(R); + + auto ps = create_and_convert_offset_polygon(polygon_offset_distance, R); + for (auto& p : ps) { + if (!p.is_simple()) { + throw std::runtime_error("Complex polygon originated from offset"); + } + } + offset_polygons.insert(offset_polygons.end(), ps.begin(), ps.end()); + } + + debug_output.write_polygons(offset_polygons, "offset_input"); + + // Perform Boolean union on the offset polygons + std::vector unioned_polygons; + CGAL::join(offset_polygons.begin(), offset_polygons.end(), std::back_inserter(unioned_polygons)); + + if (unioned_polygons.size() > 1) { + // @todo this is currently one of the major limitations in the code that still can be eliminated + // by grouping the input polygons by their perimiter polygon in unioned_polygons + std::sort(unioned_polygons.begin(), unioned_polygons.end(), [](auto& p, auto& q) { return p.outer_boundary().area() > q.outer_boundary().area(); }); + } + + debug_output.write_polygon(unioned_polygons.front().outer_boundary(), "offset_joined"); + + Polygon_2 fused_removed_close_points = unioned_polygons.front().outer_boundary(); + remove_close_points(fused_removed_close_points, 1.e-4); + + // Apply negative offset to get the outer perimeter polygon + outer_perimiter = create_and_convert_offset_polygon( + // Because polygon_offset is inexact, make sure our inset distance is slightly larger + // std::nexttoward(-polygon_offset_distance, -std::numeric_limits::infinity()), + + // 1.e-8 even was too little and still resulted in slivers of triangle around the perimeter + -polygon_offset_distance - OUTER_PERIMITER_ADDITIONAL_INSET_AMOUNT, + fused_removed_close_points); + + debug_output.write_polygons(outer_perimiter, "outer_perimiter"); + } else { + std::map> neighbour_map; + build_radial_neighbour_map(input_polygons, polygon_offset_distance, neighbour_map); + + auto start_vertex = neighbour_map.rbegin()->first; + auto next_vertex = neighbour_map.rbegin()->second.front(); + + std::vector cycle = {start_vertex, next_vertex}; + while (cycle.back() != cycle.front()) { + const auto& incoming_from = *(cycle.rbegin() + 1); + const auto& nb = neighbour_map[cycle.back()]; + auto it = std::find(nb.begin(), nb.end(), incoming_from); + // cycle it -1 around nb + if (it == nb.begin()) { + it = nb.end() - 1; + } else { + --it; + } + cycle.push_back(*it); + } + outer_perimiter.emplace_back(cycle.begin(), cycle.end()); + } t0.stop(); t0 = timer.start("corridor creation"); @@ -1911,8 +2457,10 @@ void arrange_cgal_polygons(const std::vector& input_polygons_, std::v // subdivide difference_result to have better more detailed triangulation and therefore less-pronounced artefacts in midpoint network + auto subdivision_length = polygon_offset_distance / settings.subdivision_factor; + for (auto& pwh : difference_result) { - difference_result_subdivided.push_back(subdivide_polygon(polygon_offset_distance / 8., pwh)); + difference_result_subdivided.push_back(subdivide_polygon(subdivision_length, pwh)); // difference_result_subdivided.push_back(subdivide_polygon(polygon_offset_distance / 64., pwh)); } @@ -1940,13 +2488,43 @@ void arrange_cgal_polygons(const std::vector& input_polygons_, std::v SegmentLookup segment_lookup(input_polygons); - auto [line_graph, midpoint_to_segment, segment_to_input_facet] = build_line_graph(input_polygons, segment_lookup, triangular_polygons); + auto [line_graph, midpoint_to_segment, segment_to_input_facet, midpoint_to_edge_length] = build_line_graph(input_polygons, segment_lookup, triangular_polygons); for (auto& p : line_graph) { for (auto& q : p.second) { debug_output.write_segment(p.first, q, "network_1"); } } + // Write a JSON structure with center line topology with the midpoint_to_edge_length as per-point data + { + std::ofstream ofs("center_line_topology.json"); + ofs << "{\n"; + ofs << " \"vertices\": [\n"; + bool first_vertex = true; + for (auto& p : line_graph) { + if (!first_vertex) { + ofs << ",\n"; + } + first_vertex = false; + ofs << " {\n"; + ofs << " \"point\": [" << p.first.x() << ", " << p.first.y() << "],\n"; + ofs << " \"width\":" << midpoint_to_edge_length.find(p.first)->second << ",\n"; + ofs << " \"connected_to\": [\n"; + bool first_connected = true; + for (auto& q : p.second) { + if (!first_connected) { + ofs << ",\n"; + } + first_connected = false; + ofs << " [" << q.x() << ", " << q.y() << "]"; + } + ofs << "\n ]\n"; + ofs << " }"; + } + ofs << "\n ]\n"; + ofs << "}\n"; + } + t0.stop(); t0 = timer.start("center line cleaning"); @@ -1981,7 +2559,7 @@ void arrange_cgal_polygons(const std::vector& input_polygons_, std::v t0 = timer.start("topology"); - auto segments = extend_end_vertices_based_on_input(G, midpoint_to_segment, segment_to_input_facet, outer_perimiter, segment_lookup); + auto segments = extend_end_vertices_based_on_input(G, midpoint_to_segment, segment_to_input_facet, outer_perimiter, segment_lookup, subdivision_length * 4); // Now plot the edges on an arrangement in order to find planar cycles // and merge the corridor-halves with their neighbouring input polygon @@ -1997,31 +2575,31 @@ void arrange_cgal_polygons(const std::vector& input_polygons_, std::v debug_output.write_segment(pq.first, pq.second, "extended_segments"); } -#if 0 - // Write input polygons to arrangement_2 - // We no longer do this because we add the outer perimiter now, subdivided by the corridor network which is extended and intersected with the outer perimiter - for (auto& poly : input_polygons) { - for (size_t i = 0; i != poly.size(); ++i) { - auto j = (i + 1) % poly.size(); - if (poly.vertex(i) == poly.vertex(j)) { - continue; + if (settings.topology_reconstruction_algo != 0) { + // Write input polygons to arrangement_2 + // We no longer do this because we add the outer perimiter now, subdivided by the corridor network which is extended and intersected with the outer perimiter + for (auto& poly : input_polygons) { + for (size_t i = 0; i != poly.size(); ++i) { + auto j = (i + 1) % poly.size(); + if (poly.vertex(i) == poly.vertex(j)) { + continue; + } + CGAL::insert(arr, Segment_2(poly.vertex(i), poly.vertex(j))); + } + } + } else { + // Write outer perimeter to arrangement_2 + for (auto& p : outer_perimiter) { + for (auto it = p.edges_begin(); it != p.edges_end(); ++it) { + auto source = it->source(); + auto target = it->target(); + if (source == target) { + continue; + } + CGAL::insert(arr, Segment_2(source, target)); } - CGAL::insert(arr, Segment_2(poly.vertex(i), poly.vertex(j))); } } -#else - // Write outer perimeter to arrangement_2 - for (auto& p : outer_perimiter) { - for (auto it = p.edges_begin(); it != p.edges_end(); ++it) { - auto source = it->source(); - auto target = it->target(); - if (source == target) { - continue; - } - CGAL::insert(arr, Segment_2(source, target)); - } - } -#endif // Just for the automatic numbering, create a full vector std::vector temp; @@ -2047,12 +2625,17 @@ void arrange_cgal_polygons(const std::vector& input_polygons_, std::v // corridor network we know it needs to be joined with an input polygon. In that // case the edges need to be eliminated that correspond to original geometry. -#if 0 - fuse_corridor_halves_with_input(arr, G, segment_lookup, input_polygons, debug_output); -#else - remove_colinear_vertices(arr); - clean_noisy_paths(arr, segment_lookup); -#endif + if (settings.topology_reconstruction_algo != 0) { + fuse_corridor_halves_with_input(arr, G, segment_lookup, input_polygons, debug_output); + } + + if (settings.perform_cleanup) { + remove_colinear_vertices(arr); + double threshold; + clean_noisy_paths(debug_output, arr, segment_lookup, threshold); + remove_colinear_vertices(arr); + clean_noisy_bounds(debug_output, arr, segment_lookup, threshold); + } t0.stop(); @@ -2068,8 +2651,7 @@ void arrange_cgal_polygons(const std::vector& input_polygons_, std::v #ifndef SVGFILL_MAIN -bool svgfill::arrange_polygons(const std::vector& polygons, std::vector& arranged) -{ +bool svgfill::arrange_polygons(arrange_polygon_settings settings, const std::vector& polygons, std::vector& arranged) { std::vector cgal_polygons, cgal_polygons_out; std::transform(polygons.begin(), polygons.end(), std::back_inserter(cgal_polygons), [](auto& poly) { Polygon_2 result; @@ -2078,7 +2660,7 @@ bool svgfill::arrange_polygons(const std::vector& polygons, }); return result; }); - arrange_cgal_polygons(cgal_polygons, cgal_polygons_out); + arrange_cgal_polygons(settings, cgal_polygons, cgal_polygons_out); std::transform(cgal_polygons_out.begin(), cgal_polygons_out.end(), std::back_inserter(arranged), [](auto& poly) { svgfill::polygon_2 result; std::transform(poly.begin(), poly.end(), std::back_inserter(result.boundary), [](auto& pt) { @@ -2128,7 +2710,7 @@ int main(int argc, char** argv) { input_polygons.back().push_back(CGAL::Point_2(x, y)); } } - arrange_cgal_polygons(input_polygons, output); + arrange_cgal_polygons(arrange_polygon_settings{}, input_polygons, output); break; } return 0; @@ -2141,7 +2723,7 @@ int main(int argc, char** argv) { input_polygons = { rect1, rect2, rect3, rect4, rect5 }; } - arrange_cgal_polygons(input_polygons, output); + arrange_cgal_polygons(arrange_polygon_settings{}, input_polygons, output); return 0; } diff --git a/src/svgfill/src/svgfill.cpp b/src/svgfill/src/svgfill.cpp index 8a2a1bb008..cf45881c93 100644 --- a/src/svgfill/src/svgfill.cpp +++ b/src/svgfill/src/svgfill.cpp @@ -483,6 +483,18 @@ public: return ps; } + size_t delete_same_facet_edge_pairs() { + size_t n_deleted = 0; + for (auto it = arr.edges_begin(); it != arr.edges_end();) { + decltype(it) current = it++; + if (current->face() == current->twin()->face()) { + arr.remove_edge(current); + n_deleted++; + } + } + return n_deleted; + } + void merge(const std::vector& edge_indices) { if (edge_indices.empty()) { return; diff --git a/src/svgfill/src/svgfill.h b/src/svgfill/src/svgfill.h index 396fc9c924..fab4dd0142 100644 --- a/src/svgfill/src/svgfill.h +++ b/src/svgfill/src/svgfill.h @@ -67,6 +67,7 @@ namespace svgfill { virtual std::vector get_face_pairs() = 0; virtual size_t num_edges() = 0; virtual size_t num_faces() = 0; + virtual size_t delete_same_facet_edge_pairs() = 0; }; class SVGFILL_API context { @@ -101,6 +102,7 @@ namespace svgfill { void write(std::vector>&); size_t num_edges() { return arr_->num_edges(); } size_t num_faces() { return arr_->num_faces(); } + size_t delete_same_facet_edge_pairs() { return arr_->delete_same_facet_edge_pairs(); } ~context() { delete arr_; @@ -113,7 +115,22 @@ namespace svgfill { SVGFILL_API std::string polygons_to_svg(const std::vector>& polygons, bool random_color=false); SVGFILL_API std::string polygons_to_svg(const std::vector& polygons, bool random_color = false); SVGFILL_API bool svg_to_polygons(const std::string& data, const boost::optional& class_name, std::vector& polygons); - SVGFILL_API bool arrange_polygons(const std::vector& polygons, std::vector& arranged); -} + + struct SVGFILL_API arrange_polygon_settings { + bool debug_output = false; + // -1: compute from average edge length + double polygon_offset_distance = -1.; + // 0: use offset - union - negative offset to find the outer perimeter + // 1: radial walk along vertices; exact, but can only reuse vertices, not create new positions by means of intersections + int outer_perimiter_algo = 0; + // 0: outer perimiter and corridor center lines + // 1: input polygons, corridor center lines and segments connecting corridor center lines to input polygons + int topology_reconstruction_algo = 0; + bool perform_cleanup = true; + double subdivision_factor = 16.; + }; + + SVGFILL_API bool arrange_polygons(arrange_polygon_settings settings, const std::vector& polygons, std::vector& arranged); + } #endif From c1f146966cc281986b36d5db6749d233dc6c69c5 Mon Sep 17 00:00:00 2001 From: Thomas Krijnen Date: Fri, 3 Apr 2026 11:03:44 +0200 Subject: [PATCH 20/25] The end of open source? Just regurgitate some markdown parsing code. --- src/ifcchat/app.js | 175 +++++++++++++++++++++++++++++++++++++++++- src/ifcchat/style.css | 67 ++++++++++++++++ 2 files changed, 241 insertions(+), 1 deletion(-) diff --git a/src/ifcchat/app.js b/src/ifcchat/app.js index ae1d7b0ae9..c0046491b5 100644 --- a/src/ifcchat/app.js +++ b/src/ifcchat/app.js @@ -162,17 +162,190 @@ function setBusy(isBusy, reason = "") { setStatus(isBusy ? (reason || "Working…") : "Ready"); } +function escapeHtml(text) { + return text + .replaceAll("&", "&") + .replaceAll("<", "<") + .replaceAll(">", ">") + .replaceAll('"', """) + .replaceAll("'", "'"); +} + +function sanitizeUrl(url) { + try { + const parsed = new URL(url, window.location.href); + if (["http:", "https:", "mailto:"].includes(parsed.protocol)) { + return parsed.href; + } + } catch { + } + return null; +} + +function renderInlineMarkdown(text) { + const placeholders = []; + const addPlaceholder = (html) => { + const token = `@@MD${placeholders.length}@@`; + placeholders.push({ token, html }); + return token; + }; + + let rendered = text; + + rendered = rendered.replace(/`([^`]+)`/g, (_, code) => addPlaceholder(`${escapeHtml(code)}`)); + rendered = rendered.replace(/\[([^\]]+)\]\(([^)\s]+)\)/g, (_, label, url) => { + const href = sanitizeUrl(url); + if (!href) { + return `${label} (${url})`; + } + return addPlaceholder( + `${escapeHtml(label)}` + ); + }); + + rendered = escapeHtml(rendered); + rendered = rendered.replace(/\*\*([^*]+)\*\*/g, "$1"); + rendered = rendered.replace(/\*([^*]+)\*/g, "$1"); + rendered = rendered.replace(/_([^_]+)_/g, "$1"); + + for (const placeholder of placeholders) { + rendered = rendered.replaceAll(placeholder.token, placeholder.html); + } + + return rendered; +} + +function renderMarkdown(text) { + const lines = String(text).replace(/\r\n?/g, "\n").split("\n"); + const html = []; + let paragraphLines = []; + let quoteLines = []; + let listType = null; + let listItems = []; + + const flushParagraph = () => { + if (!paragraphLines.length) return; + html.push(`

${renderInlineMarkdown(paragraphLines.join(" "))}

`); + paragraphLines = []; + }; + + const flushQuote = () => { + if (!quoteLines.length) return; + const quoteBody = quoteLines.map((line) => renderInlineMarkdown(line)).join("
"); + html.push(`

${quoteBody}

`); + quoteLines = []; + }; + + const flushList = () => { + if (!listItems.length || !listType) return; + const items = listItems.map((item) => `
  • ${renderInlineMarkdown(item)}
    • `).join(""); + html.push(`<${listType}>${items}`); + listType = null; + listItems = []; + }; + + const flushAll = () => { + flushParagraph(); + flushQuote(); + flushList(); + }; + + for (let index = 0; index < lines.length; index++) { + const line = lines[index]; + const trimmed = line.trim(); + + if (trimmed.startsWith("```")) { + flushAll(); + const language = trimmed.slice(3).trim(); + const codeLines = []; + index += 1; + while (index < lines.length && !lines[index].trim().startsWith("```")) { + codeLines.push(lines[index]); + index += 1; + } + const languageClass = language ? ` class="language-${escapeHtml(language)}"` : ""; + html.push(`
    ${escapeHtml(codeLines.join("\n"))}
    `); + continue; + } + + if (!trimmed) { + flushAll(); + continue; + } + + const headingMatch = trimmed.match(/^(#{1,6})\s+(.+)$/); + if (headingMatch) { + flushAll(); + const level = headingMatch[1].length; + html.push(`${renderInlineMarkdown(headingMatch[2])}`); + continue; + } + + const quoteMatch = trimmed.match(/^>\s?(.*)$/); + if (quoteMatch) { + flushParagraph(); + flushList(); + quoteLines.push(quoteMatch[1]); + continue; + } + + if (quoteLines.length) { + flushQuote(); + } + + const unorderedListMatch = trimmed.match(/^[-*]\s+(.+)$/); + if (unorderedListMatch) { + flushParagraph(); + if (listType && listType !== "ul") { + flushList(); + } + listType = "ul"; + listItems.push(unorderedListMatch[1]); + continue; + } + + const orderedListMatch = trimmed.match(/^\d+\.\s+(.+)$/); + if (orderedListMatch) { + flushParagraph(); + if (listType && listType !== "ol") { + flushList(); + } + listType = "ol"; + listItems.push(orderedListMatch[1]); + continue; + } + + if (listItems.length) { + flushList(); + } + + paragraphLines.push(trimmed); + } + + flushAll(); + + return html.join(""); +} + function addMessage(role, text) { if (text.ok) { text = text.data; } + if (typeof text !== "string") { + text = JSON.stringify(text, null, 2); + } const wrap = document.createElement("div"); wrap.className = `msg ${role}`; wrap.innerHTML = `
    ${role}${role === "tool" ? '▶' : ''}
    `; const bubble = wrap.querySelector(".bubble"); - bubble.textContent = text; + if (role === "assistant") { + bubble.classList.add("markdown-content"); + bubble.innerHTML = renderMarkdown(text); + } else { + bubble.textContent = text; + } bubble.onclick = function () { if (bubble.scrollHeight > 100 && role === "tool") { const expanded = bubble.style.maxHeight === 'none'; diff --git a/src/ifcchat/style.css b/src/ifcchat/style.css index 9d3ba064c9..1dfd5335b8 100644 --- a/src/ifcchat/style.css +++ b/src/ifcchat/style.css @@ -86,6 +86,73 @@ main { white-space: pre-wrap; } +.msg.assistant .bubble { + padding: 10px 14px; + line-height: 1.5; +} + +.markdown-content > :first-child { + margin-top: 0; +} + +.markdown-content > :last-child { + margin-bottom: 0; +} + +.markdown-content p, +.markdown-content ul, +.markdown-content ol, +.markdown-content blockquote, +.markdown-content pre { + margin: 0 0 12px 0; +} + +.markdown-content h1, +.markdown-content h2, +.markdown-content h3, +.markdown-content h4, +.markdown-content h5, +.markdown-content h6 { + margin: 0 0 12px 0; + line-height: 1.25; +} + +.markdown-content ul, +.markdown-content ol { + padding-left: 24px; +} + +.markdown-content blockquote { + margin-left: 0; + padding-left: 12px; + border-left: 3px solid #ddd; + color: #555; +} + +.markdown-content code { + padding: 1px 4px; + border-radius: 4px; + background: #f2f2f2; + font-family: ui-monospace, SFMono-Regular, Menlo, monospace; + font-size: 90%; +} + +.markdown-content pre { + overflow-x: auto; + padding: 12px; + border-radius: 10px; + background: #f4f4f4; +} + +.markdown-content pre code { + padding: 0; + background: transparent; +} + +.markdown-content a { + color: inherit; +} + .msg.user .bubble { padding: 10px 20px; background: #eee; From 7350ccd25e71d73162846ced432abaac75d4e2cd Mon Sep 17 00:00:00 2001 From: Thomas Krijnen Date: Fri, 3 Apr 2026 11:14:40 +0200 Subject: [PATCH 21/25] Tweak header padding --- src/ifcchat/index.html | 4 +--- src/ifcchat/style.css | 4 ++++ 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/ifcchat/index.html b/src/ifcchat/index.html index 1048831657..e919b1ae8f 100644 --- a/src/ifcchat/index.html +++ b/src/ifcchat/index.html @@ -84,10 +84,8 @@
    +
    - - - IfcOpenShell AI Assistant
    diff --git a/src/ifcchat/style.css b/src/ifcchat/style.css index 1dfd5335b8..b84fb1fb30 100644 --- a/src/ifcchat/style.css +++ b/src/ifcchat/style.css @@ -317,3 +317,7 @@ hr { padding: 2px; display: inline-block; } + +header .row:nth-child(2) { + padding: 15px 0 0 0; +} \ No newline at end of file From 918cc65a0da33ddc20cb97d6c19a42051b959b38 Mon Sep 17 00:00:00 2001 From: Thomas Krijnen Date: Fri, 3 Apr 2026 11:22:43 +0200 Subject: [PATCH 22/25] Provider selection as tabs --- src/ifcchat/app.js | 14 ++++++++---- src/ifcchat/index.html | 24 +++++++++++++++----- src/ifcchat/style.css | 51 ++++++++++++++++++++++++++++++++++++++++-- 3 files changed, 77 insertions(+), 12 deletions(-) diff --git a/src/ifcchat/app.js b/src/ifcchat/app.js index c0046491b5..7a7eb1adab 100644 --- a/src/ifcchat/app.js +++ b/src/ifcchat/app.js @@ -112,13 +112,17 @@ const baseUrlLabelEl = $("baseUrlLabel"); const baseUrlEl = $("baseUrl"); const thinkingIndicatorEl = $("thinkingIndicator"); const modelEl = $("model"); -const providerEl = $("provider"); +const providerEls = document.querySelectorAll('input[name="provider"]'); const ifcFileEl = $("ifcFile"); const newBtn = $("newModel"); const downloadBtn = $("downloadIfc"); +function getProviderValue() { + return document.querySelector('input[name="provider"]:checked')?.value || "openai"; +} + function onProviderChange() { - const provider = PROVIDERS[providerEl.value]; + const provider = PROVIDERS[getProviderValue()]; apiKeyLabelEl.innerHTML = `${provider.apiKeyLabel}stored in browser memory; only sent to provider servers`; apiKeyEl.placeholder = provider.apiKeyPlaceholder; baseUrlRowEl.hidden = !provider.baseUrlDefault; @@ -133,7 +137,9 @@ function onProviderChange() { modelEl.innerHTML = provider.models.map(m => ``).join(""); } -providerEl.addEventListener("change", onProviderChange); +for (const providerEl of providerEls) { + providerEl.addEventListener("change", onProviderChange); +} onProviderChange(); function setBusy(isBusy, reason = "") { @@ -467,7 +473,7 @@ async function runAgentTurn(userText) { const apiKey = apiKeyEl.value.trim(); if (!apiKey) throw new Error("Missing API key"); - const provider = PROVIDERS[providerEl.value]; + const provider = PROVIDERS[getProviderValue()]; const { chat } = provider.api; const baseURL = provider.baseUrlDefault ? baseUrlEl.value.trim() : undefined; diff --git a/src/ifcchat/index.html b/src/ifcchat/index.html index e919b1ae8f..daf6146126 100644 --- a/src/ifcchat/index.html +++ b/src/ifcchat/index.html @@ -14,12 +14,24 @@
    - +
    + + + + +
    diff --git a/src/ifcchat/style.css b/src/ifcchat/style.css index b84fb1fb30..105e0d574f 100644 --- a/src/ifcchat/style.css +++ b/src/ifcchat/style.css @@ -32,7 +32,7 @@ main { .side { border-right: 1px solid #ddd; - padding: 100px 12px 12px 12px; + padding: 12px; overflow: auto; background: #f9f9f9; } @@ -234,7 +234,54 @@ section > .row > label .small { } .side .row { - margin-bottom: 32px; + margin-bottom: 20px; +} + +.provider-tabs { + display: flex; + flex-wrap: wrap; + gap: 6px; + background: #00000010; + padding: 6px 0 0 6px; +} + +.provider-tab { + position: relative; + display: inline-flex; +} + +.provider-tab input { + position: absolute; + opacity: 0; + pointer-events: none; +} + +.provider-tab span { + display: inline-flex; + align-items: center; + justify-content: center; + min-height: 38px; + padding: 8px; + border: 1px solid #d0d0d0; + border-radius: 2px 2px 0 0; + background: #e8e8e8; + color: #555; + cursor: pointer; + user-select: none; + transition: background 0.15s, border-color 0.15s, color 0.15s; + font-size: 75%; +} + +.provider-tab input:checked + span { + background: #f9f9f9; + border-color: #999; + color: #111; + border-bottom: none; +} + +.provider-tab input:focus-visible + span { + outline: 2px solid #666; + outline-offset: 2px; } .row button { From 9bc0588d211fc70d384a1b33adf354af64f45387 Mon Sep 17 00:00:00 2001 From: Thomas Krijnen Date: Fri, 3 Apr 2026 11:30:23 +0200 Subject: [PATCH 23/25] Update openai model list --- src/ifcchat/app.js | 44 ++++++++++++++++++++++++++++++++++++++------ 1 file changed, 38 insertions(+), 6 deletions(-) diff --git a/src/ifcchat/app.js b/src/ifcchat/app.js index 7a7eb1adab..971ff9e4e6 100644 --- a/src/ifcchat/app.js +++ b/src/ifcchat/app.js @@ -12,13 +12,45 @@ const PROVIDERS = { baseUrlPlaceholder: "https://api.openai.com/v1", baseUrlDefault: "https://api.openai.com/v1", models: [ - { - value: "gpt-5", - label: "gpt-5" + { + value: "gpt-5.2", + label: "gpt-5.2" }, - { - value: "gpt-4.1", - label: "gpt-4.1" + { + value: "gpt-5.2-chat-latest", + label: "gpt-5.2-chat-latest" + }, + { + value: "gpt-5.2-pro", + label: "gpt-5.2-pro" + }, + { + value: "gpt-5", + label: "gpt-5" + }, + { + value: "gpt-5-chat-latest", + label: "gpt-5-chat-latest" + }, + { + value: "gpt-5-mini", + label: "gpt-5-mini" + }, + { + value: "gpt-5-nano", + label: "gpt-5-nano" + }, + { + value: "gpt-4.1", + label: "gpt-4.1" + }, + { + value: "gpt-4.1-mini", + label: "gpt-4.1-mini" + }, + { + value: "gpt-4.1-nano", + label: "gpt-4.1-nano" }, ], }, From c28251a1b15bee2bdd828b144eeec5e4f5a23e48 Mon Sep 17 00:00:00 2001 From: Thomas Krijnen Date: Fri, 3 Apr 2026 11:30:33 +0200 Subject: [PATCH 24/25] Add CNAME file --- src/ifcchat/CNAME | 1 + 1 file changed, 1 insertion(+) create mode 100644 src/ifcchat/CNAME diff --git a/src/ifcchat/CNAME b/src/ifcchat/CNAME new file mode 100644 index 0000000000..6fbcc54661 --- /dev/null +++ b/src/ifcchat/CNAME @@ -0,0 +1 @@ +ai-chat.ifcopenshell.org \ No newline at end of file From c478da5257d4418a5ebc49863e81d3b4cec7b4e0 Mon Sep 17 00:00:00 2001 From: Thomas Krijnen Date: Fri, 3 Apr 2026 11:36:56 +0200 Subject: [PATCH 25/25] Remove pro --- src/ifcchat/app.js | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/ifcchat/app.js b/src/ifcchat/app.js index 971ff9e4e6..b268bf8256 100644 --- a/src/ifcchat/app.js +++ b/src/ifcchat/app.js @@ -20,10 +20,6 @@ const PROVIDERS = { value: "gpt-5.2-chat-latest", label: "gpt-5.2-chat-latest" }, - { - value: "gpt-5.2-pro", - label: "gpt-5.2-pro" - }, { value: "gpt-5", label: "gpt-5"