From 1e331c12751017e0731d90314d55b1d10dc1ca61 Mon Sep 17 00:00:00 2001 From: Licini Date: Sat, 2 Mar 2024 10:39:52 +0100 Subject: [PATCH 01/13] remove SceneObjectNode --- src/compas/datastructures/tree/tree.py | 2 +- src/compas/scene/__init__.py | 2 - src/compas/scene/scene.py | 163 ++----------------------- src/compas/scene/sceneobject.py | 53 ++++---- 4 files changed, 38 insertions(+), 182 deletions(-) diff --git a/src/compas/datastructures/tree/tree.py b/src/compas/datastructures/tree/tree.py index 7b9218d70a1a..aaff66dcc23c 100644 --- a/src/compas/datastructures/tree/tree.py +++ b/src/compas/datastructures/tree/tree.py @@ -71,7 +71,7 @@ def __from_data__(cls, data): return node def __init__(self, name=None, **kwargs): - super(TreeNode, self).__init__(name=name, **kwargs) + super(TreeNode, self).__init__(name=name) self.attributes = kwargs self._parent = None self._children = [] diff --git a/src/compas/scene/__init__.py b/src/compas/scene/__init__.py index e200201ec78a..03195d396c73 100644 --- a/src/compas/scene/__init__.py +++ b/src/compas/scene/__init__.py @@ -23,7 +23,6 @@ from .context import register from .scene import Scene -from .scene import SceneObjectNode from .scene import SceneTree from compas.plugins import plugin @@ -49,7 +48,6 @@ def register_scene_objects_base(): "GeometryObject", "VolMeshObject", "Scene", - "SceneObjectNode", "SceneTree", "clear", "before_draw", diff --git a/src/compas/scene/scene.py b/src/compas/scene/scene.py index 2c4c427c1b2a..a2d5efa1c1de 100644 --- a/src/compas/scene/scene.py +++ b/src/compas/scene/scene.py @@ -9,81 +9,6 @@ from .sceneobject import SceneObject -class SceneObjectNode(TreeNode): - """A node representing a scene object in a scene tree. The SceneObjectNode should only be used internally by the SceneTree. - - Parameters - ---------- - object : :class:`compas.scene.SceneObject` - The scene object associated with the node. - - Attributes - ---------- - name : str - The name of the node, same as the underlying scene object. - object : :class:`compas.scene.SceneObject` - The scene object associated with the node. - parentobject : :class:`compas.scene.SceneObject` - The scene object associated with the parent node. - childobjects : list[:class:`compas.scene.SceneObject`] - The scene objects associated with the child nodes. - - """ - - @property - def __data__(self): - return { - "item": str(self.object.item.guid), - "settings": self.object.settings, - "children": [child.__data__ for child in self.children], - } - - @classmethod - def __from_data__(cls, data): - raise TypeError("SceneObjectNode cannot be created from data. Use Scene.__from_data__ instead.") - - def __init__(self, sceneobject, name=None): - super(SceneObjectNode, self).__init__(name=name) - self.object = sceneobject - - @property - def name(self): - if self.object: - return self.object.name - - @property - def parentobject(self): - if self.parent and isinstance(self.parent, SceneObjectNode): - return self.parent.object - return None - - @property - def childobjects(self): - return [child.object for child in self.children] - - def add_item(self, item, **kwargs): - """Add an child item to the node. - - Parameters - ---------- - item : :class:`compas.data.Data` - The item to add. - **kwargs : dict - Additional keyword arguments to create the scene object for the item. - - Returns - ------- - :class:`compas.scene.SceneObject` - The scene object associated with the item. - - """ - sceneobject = SceneObject(item, **kwargs) - node = SceneObjectNode(sceneobject) - self.add(node) - sceneobject._node = node # type: ignore - return sceneobject - - class SceneTree(Tree): """A tree structure for storing the hierarchy of scene objects in a scene. The SceneTree should only be used internally by the Scene. @@ -92,11 +17,6 @@ class SceneTree(Tree): name : str, optional The name of the tree. - Attributes - ---------- - objects : list[:class:`compas.scene.SceneObject`] - All scene objects in the scene tree. - """ @classmethod @@ -105,76 +25,9 @@ def __from_data__(cls, data): def __init__(self, name=None): super(SceneTree, self).__init__(name=name) - root = TreeNode(name="root") + root = TreeNode(name="ROOT") self.add(root) - @property - def objects(self): - return [node.object for node in self.nodes if isinstance(node, SceneObjectNode)] - - def add_object(self, sceneobject, parent=None): - """Add a scene object to the tree. - - Parameters - ---------- - sceneobject : :class:`compas.scene.SceneObject` - The scene object to add. - parent : :class:`compas.scene.SceneObject`, optional - The parent scene object. - - Returns - ------- - :class:`compas.scene.SceneObjectNode` - The node associated with the scene object. - - """ - node = SceneObjectNode(sceneobject) - if parent is None: - self.add(node, parent=self.root) - else: - parent_node = self.get_node_from_object(parent) - self.add(node, parent=parent_node) - - sceneobject._node = node - return node - - def remove_object(self, sceneobject): - """Remove a scene object from the tree. - - Parameters - ---------- - sceneobject : :class:`compas.scene.SceneObject` - The scene object to remove. - - """ - node = self.get_node_from_object(sceneobject) - self.remove(node) - - def get_node_from_object(self, sceneobject): - """Get the node associated with a scene object. - - Parameters - ---------- - sceneobject : :class:`compas.scene.SceneObject` - The scene object. - - Returns - ------- - :class:`compas.scene.SceneObjectNode` - The node associated with the scene object. - - Raises - ------ - ValueError - If the scene object is not in the scene tree. - - """ - for node in self.nodes: - if isinstance(node, SceneObjectNode): - if node.object is sceneobject: - return node - raise ValueError("Scene object not in scene tree") - class Scene(Data): """A scene is a container for hierarchical scene objects which are to be visualised in a given context. @@ -242,7 +95,7 @@ def tree(self): @property def objects(self): - return self.tree.objects + return [node for node in self.tree.nodes if not node.is_root] def add(self, item, parent=None, **kwargs): """Add an item to the scene. @@ -261,8 +114,14 @@ def add(self, item, parent=None, **kwargs): :class:`compas.scene.SceneObject` The scene object associated with the item. """ - sceneobject = SceneObject(item, context=self.context, **kwargs) - self.tree.add_object(sceneobject, parent=parent) + + parent = parent or self.tree.root + + if isinstance(item, SceneObject): + sceneobject = item + else: + sceneobject = SceneObject(item, context=self.context, **kwargs) + self.tree.add(sceneobject, parent=parent) return sceneobject def remove(self, sceneobject): @@ -274,7 +133,7 @@ def remove(self, sceneobject): The scene object to remove. """ - self.tree.remove_object(sceneobject) + self.tree.remove(sceneobject) def clear(self): """Clear the current context of the scene.""" diff --git a/src/compas/scene/sceneobject.py b/src/compas/scene/sceneobject.py index 914ce8b354d7..0f6bf58f5410 100644 --- a/src/compas/scene/sceneobject.py +++ b/src/compas/scene/sceneobject.py @@ -7,13 +7,14 @@ from .descriptors.color import ColorAttribute from .context import clear from .context import get_sceneobject_cls +from compas.datastructures import TreeNode from compas.colors import Color from compas.geometry import Transformation from functools import reduce from operator import mul -class SceneObject(object): +class SceneObject(TreeNode): """Base class for all scene objects. Parameters @@ -33,10 +34,6 @@ class SceneObject(object): The node in the scene tree which represents the scene object. guids : list[object] The GUIDs of the items drawn in the visualization context. - parent : :class:`compas.scene.SceneObject` - The parent scene object. - children : list[:class:`compas.scene.SceneObject`] - The child scene objects. frame : :class:`compas.geometry.Frame` The local frame of the scene object, in relation to its parent frame. transformation : :class:`compas.geometry.Transformation` @@ -66,41 +63,41 @@ def __new__(cls, item, **kwargs): sceneobject_cls = get_sceneobject_cls(item, **kwargs) return super(SceneObject, cls).__new__(sceneobject_cls) - def __init__(self, item, **kwargs): + def __init__(self, item, name=None, **kwargs): + name = name or item.name + super(SceneObject, self).__init__(name=name, **kwargs) self._item = item self._guids = None self._node = None self._frame = kwargs.get("frame", None) self._transformation = kwargs.get("transformation", None) self._contrastcolor = None - self.name = kwargs.get("name", item.name or item.__class__.__name__) self.color = kwargs.get("color", self.color) self.opacity = kwargs.get("opacity", 1.0) self.show = kwargs.get("show", True) @property - def item(self): - return self._item + def __data__(self): + return { + "item": str(self.item.guid), + "settings": self.settings, + "children": [child.__data__ for child in self.children], + } - @property - def guids(self): - return self._guids or [] + @classmethod + def __from_data__(cls, data): + raise TypeError("Serialisation outside Scene not allowed.") - @property - def node(self): - return self._node + def __repr__(self): + return "<{}: {}>".format(self.__class__.__name__, self.name) @property - def parent(self): - if self.node: - return self.node.parentobject + def item(self): + return self._item @property - def children(self): - if self.node: - return self.node.childobjects - else: - return [] + def guids(self): + return self._guids or [] @property def frame(self): @@ -122,7 +119,7 @@ def transformation(self, transformation): def worldtransformation(self): frame_stack = [] parent = self.parent - while parent: + while parent and not parent.is_root: if parent.frame: frame_stack.append(parent.frame) parent = parent.parent @@ -170,10 +167,12 @@ def add(self, item, **kwargs): ValueError If the scene object does not have an associated scene node. """ - if self.node: - return self.node.add_item(item, **kwargs) + if isinstance(item, SceneObject): + sceneobject = item else: - raise ValueError("Cannot add items to a scene object without a node.") + sceneobject = SceneObject(item, **kwargs) + super().add(sceneobject) + return sceneobject @property def settings(self): From 051b094277c999034ce14d61961b2b1259295268 Mon Sep 17 00:00:00 2001 From: Licini Date: Sat, 2 Mar 2024 10:41:08 +0100 Subject: [PATCH 02/13] changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6a09944bc01a..057b0ef8256e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Removed +* Removed `compas.scene.SceneObjectNode`, functionalities merged into `compas.scene.SceneObject`. + ## [2.1.0] 2024-03-01 From d6ef7f839077ca40d534c05c6c6bd1f2c38e82c6 Mon Sep 17 00:00:00 2001 From: tomvanmele Date: Mon, 4 Mar 2024 20:14:30 +0100 Subject: [PATCH 03/13] with these changes the scene can be used to recursively add objects --- src/compas/scene/scene.py | 23 ++++++++++--- src/compas/scene/sceneobject.py | 59 +++++++++++++++++++++++---------- 2 files changed, 59 insertions(+), 23 deletions(-) diff --git a/src/compas/scene/scene.py b/src/compas/scene/scene.py index a2d5efa1c1de..5fc0abff2177 100644 --- a/src/compas/scene/scene.py +++ b/src/compas/scene/scene.py @@ -1,3 +1,5 @@ +import compas.datastructures # noqa: F401 +import compas.geometry # noqa: F401 from compas.data import Data from compas.datastructures import Tree from compas.datastructures import TreeNode @@ -21,9 +23,11 @@ class SceneTree(Tree): @classmethod def __from_data__(cls, data): + # type: (dict) -> SceneTree raise TypeError("SceneTree cannot be created from data. Use Scene.__from_data__ instead.") def __init__(self, name=None): + # type: (str | None) -> None super(SceneTree, self).__init__(name=name) root = TreeNode(name="ROOT") self.add(root) @@ -57,10 +61,9 @@ class Scene(Data): """ - viewerinstance = None - @property def __data__(self): + # type: () -> dict items = {str(object.item.guid): object.item for object in self.objects} return { "name": self.name, @@ -70,6 +73,7 @@ def __data__(self): @classmethod def __from_data__(cls, data): + # type: (dict) -> Scene scene = cls(data["name"]) items = {str(item.guid): item for item in data["items"]} @@ -85,19 +89,25 @@ def add(node, parent, items): return scene def __init__(self, name=None, context=None): + # type: (str | None, str | None) -> None super(Scene, self).__init__(name) - self._tree = SceneTree("Scene") + self._tree = SceneTree(name="Scene") self.context = context or detect_current_context() @property def tree(self): + # type: () -> SceneTree return self._tree @property def objects(self): - return [node for node in self.tree.nodes if not node.is_root] + # type: () -> list[SceneObject] + # this is flagged by the type checker + # because the tree returns nodes of type TreeNode + return [node for node in self.tree.nodes if not node.is_root] # type: ignore def add(self, item, parent=None, **kwargs): + # type: (compas.geometry.Geometry | compas.datastructures.Datastructure, SceneObject | TreeNode | None, dict) -> SceneObject """Add an item to the scene. Parameters @@ -120,11 +130,12 @@ def add(self, item, parent=None, **kwargs): if isinstance(item, SceneObject): sceneobject = item else: - sceneobject = SceneObject(item, context=self.context, **kwargs) + sceneobject = SceneObject(item, context=self.context, **kwargs) # type: ignore self.tree.add(sceneobject, parent=parent) return sceneobject def remove(self, sceneobject): + # type: (SceneObject) -> None """Remove a scene object from the scene. Parameters @@ -136,10 +147,12 @@ def remove(self, sceneobject): self.tree.remove(sceneobject) def clear(self): + # type: () -> None """Clear the current context of the scene.""" clear() def clear_objects(self): + # type: () -> None """Clear all objects inside the scene.""" guids = [] for sceneobject in self.objects: diff --git a/src/compas/scene/sceneobject.py b/src/compas/scene/sceneobject.py index 0f6bf58f5410..3928d1fe798c 100644 --- a/src/compas/scene/sceneobject.py +++ b/src/compas/scene/sceneobject.py @@ -7,6 +7,11 @@ from .descriptors.color import ColorAttribute from .context import clear from .context import get_sceneobject_cls + +import compas.colors # noqa: F401 +import compas.datastructures # noqa: F401 +import compas.geometry # noqa: F401 + from compas.datastructures import TreeNode from compas.colors import Color from compas.geometry import Transformation @@ -64,8 +69,11 @@ def __new__(cls, item, **kwargs): return super(SceneObject, cls).__new__(sceneobject_cls) def __init__(self, item, name=None, **kwargs): + # type: (compas.geometry.Geometry | compas.datastructures.Datastructure, str | None, dict) -> None + name = name or item.name super(SceneObject, self).__init__(name=name, **kwargs) + self.context = kwargs.get("context") self._item = item self._guids = None self._node = None @@ -78,6 +86,7 @@ def __init__(self, item, name=None, **kwargs): @property def __data__(self): + # type: () -> dict return { "item": str(self.item.guid), "settings": self.settings, @@ -86,37 +95,46 @@ def __data__(self): @classmethod def __from_data__(cls, data): + # type: (dict) -> None raise TypeError("Serialisation outside Scene not allowed.") def __repr__(self): + # type: () -> str return "<{}: {}>".format(self.__class__.__name__, self.name) @property def item(self): + # type: () -> compas.geometry.Geometry | compas.datastructures.Datastructure return self._item @property def guids(self): + # type: () -> list[str] return self._guids or [] @property def frame(self): + # type: () -> compas.geometry.Frame | None return self._frame @frame.setter def frame(self, frame): + # type: (compas.geometry.Frame) -> None self._frame = frame @property def transformation(self): + # type: () -> compas.geometry.Transformation | None return self._transformation @transformation.setter def transformation(self, transformation): + # type: (compas.geometry.Transformation) -> None self._transformation = transformation @property def worldtransformation(self): + # type: () -> compas.geometry.Transformation frame_stack = [] parent = self.parent while parent and not parent.is_root: @@ -136,6 +154,7 @@ def worldtransformation(self): @property def contrastcolor(self): + # type: () -> compas.colors.Color if not self._contrastcolor: if self.color.is_light: self._contrastcolor = self.color.darkened(50) @@ -145,9 +164,28 @@ def contrastcolor(self): @contrastcolor.setter def contrastcolor(self, color): + # type: (compas.colors.Color) -> None self._contrastcolor = Color.coerce(color) + @property + def settings(self): + # type: () -> dict + settings = { + "name": self.name, + "color": self.color, + "opacity": self.opacity, + "show": self.show, + } + + if self.frame: + settings["frame"] = self.frame + if self.transformation: + settings["transformation"] = self.transformation + + return settings + def add(self, item, **kwargs): + # type: (compas.geometry.Geometry | compas.datastructures.Datastructure, dict) -> SceneObject """Add a child item to the scene object. Parameters @@ -170,25 +208,10 @@ def add(self, item, **kwargs): if isinstance(item, SceneObject): sceneobject = item else: - sceneobject = SceneObject(item, **kwargs) - super().add(sceneobject) - return sceneobject - - @property - def settings(self): - settings = { - "name": self.name, - "color": self.color, - "opacity": self.opacity, - "show": self.show, - } + sceneobject = SceneObject(item, context=self.context, **kwargs) # type: ignore - if self.frame: - settings["frame"] = self.frame - if self.transformation: - settings["transformation"] = self.transformation - - return settings + super(SceneObject, self).add(sceneobject) + return sceneobject @abstractmethod def draw(self): From d3790d705119874709ff147ffc515f57fe75080c Mon Sep 17 00:00:00 2001 From: tomvanmele Date: Mon, 4 Mar 2024 20:16:58 +0100 Subject: [PATCH 04/13] remove viewer detection --- src/compas/scene/context.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/compas/scene/context.py b/src/compas/scene/context.py index 7b197f34ab66..f10ea4faab93 100644 --- a/src/compas/scene/context.py +++ b/src/compas/scene/context.py @@ -92,17 +92,17 @@ def register(item_type, sceneobject_type, context=None): ITEM_SCENEOBJECT[context][item_type] = sceneobject_type -def is_viewer_open(): - """Returns True if an instance of the compas_view2 App is available. +# def is_viewer_open(): +# """Returns True if an instance of the compas_view2 App is available. - Returns - ------- - bool +# Returns +# ------- +# bool - """ - from compas.scene import Scene +# """ +# from compas.scene import Scene - return Scene.viewerinstance is not None +# return Scene.viewerinstance is not None def detect_current_context(): @@ -119,8 +119,8 @@ def detect_current_context(): """ - if is_viewer_open(): - return "Viewer" + # if is_viewer_open(): + # return "Viewer" if compas.is_grasshopper(): return "Grasshopper" if compas.is_rhino(): From 2f19d35359af9875cb7836c73a29a6fdb63a912b Mon Sep 17 00:00:00 2001 From: tomvanmele Date: Mon, 4 Mar 2024 20:19:18 +0100 Subject: [PATCH 05/13] update tests --- tests/compas/scene/test_scene.py | 34 ++++++++++++++++---------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/tests/compas/scene/test_scene.py b/tests/compas/scene/test_scene.py index 122e7bb68d37..06c0d463f1c1 100644 --- a/tests/compas/scene/test_scene.py +++ b/tests/compas/scene/test_scene.py @@ -80,31 +80,31 @@ def test_sceneobject_auto_context_discovery(mocker): assert isinstance(sceneobject, FakeSceneObject) - def test_sceneobject_auto_context_discovery_viewer(mocker): - mocker.patch("compas.scene.context.is_viewer_open", return_value=True) - context.ITEM_SCENEOBJECT["Viewer"] = {FakeItem: FakeSceneObject} + # def test_sceneobject_auto_context_discovery_viewer(mocker): + # mocker.patch("compas.scene.context.is_viewer_open", return_value=True) + # context.ITEM_SCENEOBJECT["Viewer"] = {FakeItem: FakeSceneObject} - item = FakeSubItem() - sceneobject = SceneObject(item) + # item = FakeSubItem() + # sceneobject = SceneObject(item) - assert isinstance(sceneobject, FakeSceneObject) + # assert isinstance(sceneobject, FakeSceneObject) - def test_sceneobject_auto_context_discovery_viewer_priority(mocker): - mocker.patch("compas.scene.context.is_viewer_open", return_value=True) + # def test_sceneobject_auto_context_discovery_viewer_priority(mocker): + # mocker.patch("compas.scene.context.is_viewer_open", return_value=True) - class FakeViewerSceneObject(FakeSceneObject): - pass + # class FakeViewerSceneObject(FakeSceneObject): + # pass - class FakePlotterSceneObject(FakeSceneObject): - pass + # class FakePlotterSceneObject(FakeSceneObject): + # pass - context.ITEM_SCENEOBJECT["Viewer"] = {FakeItem: FakeViewerSceneObject} - context.ITEM_SCENEOBJECT["Plotter"] = {FakeItem: FakePlotterSceneObject} + # context.ITEM_SCENEOBJECT["Viewer"] = {FakeItem: FakeViewerSceneObject} + # context.ITEM_SCENEOBJECT["Plotter"] = {FakeItem: FakePlotterSceneObject} - item = FakeSubItem() - sceneobject = SceneObject(item) + # item = FakeSubItem() + # sceneobject = SceneObject(item) - assert isinstance(sceneobject, FakeViewerSceneObject) + # assert isinstance(sceneobject, FakeViewerSceneObject) def test_sceneobject_auto_context_discovery_no_context(mocker): mocker.patch("compas.scene.context.is_viewer_open", return_value=False) From 75d99d6a33285162897c273df1261f0b643c0e05 Mon Sep 17 00:00:00 2001 From: tomvanmele Date: Mon, 4 Mar 2024 20:47:25 +0100 Subject: [PATCH 06/13] proper handling of context in kwargs --- src/compas/scene/scene.py | 8 +++++++ src/compas/scene/sceneobject.py | 39 ++++++++++++++++++++++++--------- 2 files changed, 37 insertions(+), 10 deletions(-) diff --git a/src/compas/scene/scene.py b/src/compas/scene/scene.py index 5fc0abff2177..289e871bc05e 100644 --- a/src/compas/scene/scene.py +++ b/src/compas/scene/scene.py @@ -130,6 +130,14 @@ def add(self, item, parent=None, **kwargs): if isinstance(item, SceneObject): sceneobject = item else: + if "context" in kwargs: + if kwargs["context"] != self.context: + raise Exception( + "Object context should be the same as scene context: {} != {}".format( + kwargs["context"], self.context + ) + ) + del kwargs["context"] # otherwist the SceneObject receives "context" twice, which results in an error sceneobject = SceneObject(item, context=self.context, **kwargs) # type: ignore self.tree.add(sceneobject, parent=parent) return sceneobject diff --git a/src/compas/scene/sceneobject.py b/src/compas/scene/sceneobject.py index 3928d1fe798c..7bb4500c5fff 100644 --- a/src/compas/scene/sceneobject.py +++ b/src/compas/scene/sceneobject.py @@ -68,21 +68,33 @@ def __new__(cls, item, **kwargs): sceneobject_cls = get_sceneobject_cls(item, **kwargs) return super(SceneObject, cls).__new__(sceneobject_cls) - def __init__(self, item, name=None, **kwargs): - # type: (compas.geometry.Geometry | compas.datastructures.Datastructure, str | None, dict) -> None - + def __init__( + self, + item, # type: compas.geometry.Geometry | compas.datastructures.Datastructure + name=None, # type: str | None + color=None, # type: compas.colors.Color | None + opacity=1.0, # type: float + show=True, # type: bool + frame=None, # type: compas.geometry.Frame | None + transformation=None, # type: compas.geometry.Transformation | None + context=None, # type: str | None + **kwargs # type: dict + ): # type: (...) -> None name = name or item.name super(SceneObject, self).__init__(name=name, **kwargs) - self.context = kwargs.get("context") + # the scene object needs to store the context + # because it has no access to the tree and/or the scene before it is added + # which means that adding child objects will be added in context "None" + self.context = context self._item = item self._guids = None self._node = None - self._frame = kwargs.get("frame", None) - self._transformation = kwargs.get("transformation", None) + self._frame = frame + self._transformation = transformation self._contrastcolor = None - self.color = kwargs.get("color", self.color) - self.opacity = kwargs.get("opacity", 1.0) - self.show = kwargs.get("show", True) + self.color = color or self.color + self.opacity = opacity + self.show = show @property def __data__(self): @@ -185,7 +197,6 @@ def settings(self): return settings def add(self, item, **kwargs): - # type: (compas.geometry.Geometry | compas.datastructures.Datastructure, dict) -> SceneObject """Add a child item to the scene object. Parameters @@ -208,6 +219,14 @@ def add(self, item, **kwargs): if isinstance(item, SceneObject): sceneobject = item else: + if "context" in kwargs: + if kwargs["context"] != self.context: + raise Exception( + "Child context should be the same as parent context: {} != {}".format( + kwargs["context"], self.context + ) + ) + del kwargs["context"] # otherwist the SceneObject receives "context" twice, which results in an error sceneobject = SceneObject(item, context=self.context, **kwargs) # type: ignore super(SceneObject, self).add(sceneobject) From a31060d2792cc6ba2bd67e0187576c718f3e7abb Mon Sep 17 00:00:00 2001 From: tomvanmele Date: Mon, 4 Mar 2024 20:50:00 +0100 Subject: [PATCH 07/13] some notes --- src/compas/scene/sceneobject.py | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/src/compas/scene/sceneobject.py b/src/compas/scene/sceneobject.py index 7bb4500c5fff..df88eb31f412 100644 --- a/src/compas/scene/sceneobject.py +++ b/src/compas/scene/sceneobject.py @@ -19,6 +19,13 @@ from operator import mul +# if we know the parameters +# we should document them +# and not just make everything kwargs +# not only is this not very helpful for the user +# it does not allow for proper propagation of parameters across the inheritance chain + + class SceneObject(TreeNode): """Base class for all scene objects. @@ -26,8 +33,14 @@ class SceneObject(TreeNode): ---------- item : Any The item which should be visualized using the created SceneObject. - **kwargs : dict - Additional keyword arguments for constructing SceneObject. + name + color + opacity + show + frame + transformation + context + **kwargs Attributes ---------- From 8887e73fda42d1d38630e85092f5432e47859d4b Mon Sep 17 00:00:00 2001 From: tomvanmele Date: Mon, 4 Mar 2024 20:52:21 +0100 Subject: [PATCH 08/13] fix test --- tests/compas/scene/test_scene.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/compas/scene/test_scene.py b/tests/compas/scene/test_scene.py index 06c0d463f1c1..cffc134c08dd 100644 --- a/tests/compas/scene/test_scene.py +++ b/tests/compas/scene/test_scene.py @@ -107,7 +107,7 @@ def test_sceneobject_auto_context_discovery(mocker): # assert isinstance(sceneobject, FakeViewerSceneObject) def test_sceneobject_auto_context_discovery_no_context(mocker): - mocker.patch("compas.scene.context.is_viewer_open", return_value=False) + # mocker.patch("compas.scene.context.is_viewer_open", return_value=False) mocker.patch("compas.scene.context.compas.is_grasshopper", return_value=False) mocker.patch("compas.scene.context.compas.is_rhino", return_value=False) From 1c83067da60a8934d273ff9803cb1b7eec11d7a8 Mon Sep 17 00:00:00 2001 From: Licini Date: Tue, 12 Mar 2024 14:18:53 +0100 Subject: [PATCH 09/13] Scene inherent from Tree --- src/compas/scene/__init__.py | 2 -- src/compas/scene/scene.py | 62 ++++++------------------------------ 2 files changed, 10 insertions(+), 54 deletions(-) diff --git a/src/compas/scene/__init__.py b/src/compas/scene/__init__.py index 03195d396c73..6f1c1eb64f3d 100644 --- a/src/compas/scene/__init__.py +++ b/src/compas/scene/__init__.py @@ -23,7 +23,6 @@ from .context import register from .scene import Scene -from .scene import SceneTree from compas.plugins import plugin from compas.geometry import Geometry @@ -48,7 +47,6 @@ def register_scene_objects_base(): "GeometryObject", "VolMeshObject", "Scene", - "SceneTree", "clear", "before_draw", "after_draw", diff --git a/src/compas/scene/scene.py b/src/compas/scene/scene.py index 289e871bc05e..e0c50a7c316c 100644 --- a/src/compas/scene/scene.py +++ b/src/compas/scene/scene.py @@ -11,29 +11,7 @@ from .sceneobject import SceneObject -class SceneTree(Tree): - """A tree structure for storing the hierarchy of scene objects in a scene. The SceneTree should only be used internally by the Scene. - - Parameters - ---------- - name : str, optional - The name of the tree. - - """ - - @classmethod - def __from_data__(cls, data): - # type: (dict) -> SceneTree - raise TypeError("SceneTree cannot be created from data. Use Scene.__from_data__ instead.") - - def __init__(self, name=None): - # type: (str | None) -> None - super(SceneTree, self).__init__(name=name) - root = TreeNode(name="ROOT") - self.add(root) - - -class Scene(Data): +class Scene(Tree): """A scene is a container for hierarchical scene objects which are to be visualised in a given context. Parameters @@ -67,7 +45,7 @@ def __data__(self): items = {str(object.item.guid): object.item for object in self.objects} return { "name": self.name, - "tree": self.tree.__data__, + "root": self.root.__data__, # type: ignore "items": list(items.values()), } @@ -84,27 +62,22 @@ def add(node, parent, items): sceneobject = parent.add(items[guid], **settings) add(child_node, sceneobject, items) - add(data["tree"]["root"], scene, items) + add(data["root"], scene, items) return scene - def __init__(self, name=None, context=None): - # type: (str | None, str | None) -> None - super(Scene, self).__init__(name) - self._tree = SceneTree(name="Scene") + def __init__(self, name="Scene", context=None): + # type: (str | "Scene", str | None) -> None + super(Scene, self).__init__(name=name) + super(Scene, self).add(TreeNode(name="ROOT")) self.context = context or detect_current_context() - @property - def tree(self): - # type: () -> SceneTree - return self._tree - @property def objects(self): # type: () -> list[SceneObject] # this is flagged by the type checker # because the tree returns nodes of type TreeNode - return [node for node in self.tree.nodes if not node.is_root] # type: ignore + return [node for node in self.nodes if not node.is_root] # type: ignore def add(self, item, parent=None, **kwargs): # type: (compas.geometry.Geometry | compas.datastructures.Datastructure, SceneObject | TreeNode | None, dict) -> SceneObject @@ -125,7 +98,7 @@ def add(self, item, parent=None, **kwargs): The scene object associated with the item. """ - parent = parent or self.tree.root + parent = parent or self.root if isinstance(item, SceneObject): sceneobject = item @@ -139,21 +112,9 @@ def add(self, item, parent=None, **kwargs): ) del kwargs["context"] # otherwist the SceneObject receives "context" twice, which results in an error sceneobject = SceneObject(item, context=self.context, **kwargs) # type: ignore - self.tree.add(sceneobject, parent=parent) + super(Scene, self).add(sceneobject, parent=parent) return sceneobject - def remove(self, sceneobject): - # type: (SceneObject) -> None - """Remove a scene object from the scene. - - Parameters - ---------- - sceneobject : :class:`compas.scene.SceneObject` - The scene object to remove. - - """ - self.tree.remove(sceneobject) - def clear(self): # type: () -> None """Clear the current context of the scene.""" @@ -187,6 +148,3 @@ def draw(self): return drawn_objects - def print_hierarchy(self): - """Print the hierarchy of the scene.""" - self.tree.print_hierarchy() From 45f5e524d4c03ac5a2c6a706ca94cd8e199c6021 Mon Sep 17 00:00:00 2001 From: Licini Date: Tue, 12 Mar 2024 14:22:14 +0100 Subject: [PATCH 10/13] lint --- src/compas/scene/scene.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/compas/scene/scene.py b/src/compas/scene/scene.py index e0c50a7c316c..93e2551e15c2 100644 --- a/src/compas/scene/scene.py +++ b/src/compas/scene/scene.py @@ -1,6 +1,5 @@ import compas.datastructures # noqa: F401 import compas.geometry # noqa: F401 -from compas.data import Data from compas.datastructures import Tree from compas.datastructures import TreeNode @@ -147,4 +146,3 @@ def draw(self): after_draw(drawn_objects) return drawn_objects - From 9d20f9cc811e9e7e728894bf80f0b4b3371b092e Mon Sep 17 00:00:00 2001 From: Licini Date: Fri, 15 Mar 2024 13:08:32 +0100 Subject: [PATCH 11/13] clear up docs string and commented outs --- src/compas/scene/sceneobject.py | 36 ++++++++++++++++++-------------- tests/compas/scene/test_scene.py | 27 ------------------------ 2 files changed, 20 insertions(+), 43 deletions(-) diff --git a/src/compas/scene/sceneobject.py b/src/compas/scene/sceneobject.py index df88eb31f412..3c8607e36da8 100644 --- a/src/compas/scene/sceneobject.py +++ b/src/compas/scene/sceneobject.py @@ -19,13 +19,6 @@ from operator import mul -# if we know the parameters -# we should document them -# and not just make everything kwargs -# not only is this not very helpful for the user -# it does not allow for proper propagation of parameters across the inheritance chain - - class SceneObject(TreeNode): """Base class for all scene objects. @@ -33,14 +26,22 @@ class SceneObject(TreeNode): ---------- item : Any The item which should be visualized using the created SceneObject. - name - color - opacity - show - frame - transformation - context - **kwargs + name : str, optional + The name of the scene object. Note that is is not the same as the name of underlying data item, since different scene objects can refer to the same data item. + color : :class:`compas.colors.Color`, optional + The color of the object. + opacity : float, optional + The opacity of the object. + show : bool, optional + Flag for showing or hiding the object. Default is ``True``. + frame : :class:`compas.geometry.Frame`, optional + The local frame of the scene object, in relation to its parent frame. + transformation : :class:`compas.geometry.Transformation`, optional + The local transformation of the scene object in relation to its frame. + context : str, optional + The context in which the scene object is created. + **kwargs : dict + Additional keyword arguments to create the scene object for the item. Attributes ---------- @@ -68,7 +69,9 @@ class SceneObject(TreeNode): show : bool Flag for showing or hiding the object. Default is ``True``. settings : dict - The settings including necessary attributes for reconstructing the scene object. + The settings including necessary attributes for reconstructing the scene object besides the Data item. + context : str + The context in which the scene object is created. """ @@ -195,6 +198,7 @@ def contrastcolor(self, color): @property def settings(self): # type: () -> dict + # The settings are all the nessessary attributes to reconstruct the scene object besides the Data item. settings = { "name": self.name, "color": self.color, diff --git a/tests/compas/scene/test_scene.py b/tests/compas/scene/test_scene.py index cffc134c08dd..ba1a1c6c5653 100644 --- a/tests/compas/scene/test_scene.py +++ b/tests/compas/scene/test_scene.py @@ -80,34 +80,7 @@ def test_sceneobject_auto_context_discovery(mocker): assert isinstance(sceneobject, FakeSceneObject) - # def test_sceneobject_auto_context_discovery_viewer(mocker): - # mocker.patch("compas.scene.context.is_viewer_open", return_value=True) - # context.ITEM_SCENEOBJECT["Viewer"] = {FakeItem: FakeSceneObject} - - # item = FakeSubItem() - # sceneobject = SceneObject(item) - - # assert isinstance(sceneobject, FakeSceneObject) - - # def test_sceneobject_auto_context_discovery_viewer_priority(mocker): - # mocker.patch("compas.scene.context.is_viewer_open", return_value=True) - - # class FakeViewerSceneObject(FakeSceneObject): - # pass - - # class FakePlotterSceneObject(FakeSceneObject): - # pass - - # context.ITEM_SCENEOBJECT["Viewer"] = {FakeItem: FakeViewerSceneObject} - # context.ITEM_SCENEOBJECT["Plotter"] = {FakeItem: FakePlotterSceneObject} - - # item = FakeSubItem() - # sceneobject = SceneObject(item) - - # assert isinstance(sceneobject, FakeViewerSceneObject) - def test_sceneobject_auto_context_discovery_no_context(mocker): - # mocker.patch("compas.scene.context.is_viewer_open", return_value=False) mocker.patch("compas.scene.context.compas.is_grasshopper", return_value=False) mocker.patch("compas.scene.context.compas.is_rhino", return_value=False) From 8826190b9fd4fa9cb1cbb0c34f1c5dca52fd735e Mon Sep 17 00:00:00 2001 From: Licini Date: Fri, 15 Mar 2024 13:34:00 +0100 Subject: [PATCH 12/13] changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 057b0ef8256e..00db7f2e78b9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,9 +11,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +* Changed `compas.scene.Scene` to inherent from `compas.datastructrues.Tree`. +* Changed `compas.scene.SceneObject` to inherent from `compas.datastructrues.TreeNode`. + ### Removed * Removed `compas.scene.SceneObjectNode`, functionalities merged into `compas.scene.SceneObject`. +* Removed `compas.scene.SceneTree`, functionalities merged into `compas.scene.Scene`. ## [2.1.0] 2024-03-01 From 96dca2aa2159d40409488d0d585961dd74e45d56 Mon Sep 17 00:00:00 2001 From: Li Date: Mon, 25 Mar 2024 16:38:19 +0100 Subject: [PATCH 13/13] format --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d01acec2cecc..cf9dcdaab078 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,8 +10,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added ### Changed -* Changed and update the `compas_view2` examples into `compas_viewer`. +* Changed and updated the `compas_view2` examples into `compas_viewer`. * Changed `compas.scene.Scene` to inherent from `compas.datastructrues.Tree`. * Changed `compas.scene.SceneObject` to inherent from `compas.datastructrues.TreeNode`.