Skip to content

Conversation

@Licini
Copy link
Contributor

@Licini Licini commented Dec 12, 2023

Still in the work. This is to add the mechanism to track guids in scene objects. So the draw function should now always returns a list of GUIDs. Question: I currently wrote item_guids, but not sure it is the best name, any other suggestions @tomvanmele ? For instance in GH the draw function will be returning the instances of rhino geometry like Box, Sphere instead of the doc objects. And in blender it will be a list of bpy.types.Object.

----------
ITEM_SCENEOBJECT : dict[str, dict[Type[:class:`~compas.data.Data`], Type[:class:`~compas.scene.SceneObject`]]]
Dictionary mapping data types to the corresponding scene objects types per visualization context.
item_guids : list[object]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or perhaps this attribute goes to SceneObject implementation of each contexts, then the name varies depends on what it is actually tracking. In compas_rhino we call it item_guids, in GH item_gemetries and in blender item_objects?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not just guids

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That was my initial attention too. But won't guids creates the confusion whether it means guids for itself or the guids for drawn entities of underlying item? I understand a possible distinction could be guid singular refers to self, guids plural refers to drawn entities. Another discussion I didn't want to go there yet is whether we should have guid for SceneObject as well. Anyway, maybe easier to discuss all this in tomorrow's meeting

@Licini Licini requested a review from tomvanmele December 12, 2023 12:50
@Licini Licini marked this pull request as ready for review December 14, 2023 12:15
@Licini
Copy link
Contributor Author

Licini commented Dec 14, 2023

@tomvanmele Ok this is ready for review. The tracking for guids is now added to all scene objects. There are also some fixes / implementations added now all types of objects works in all 3 platform

@staticmethod
def draw_collection(collection):
"""Drawing method for drawing an entire collection of objects."""
@abstractmethod
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should the default not be to call the pluggable and have the pluggable throw the error?

Comment on lines 49 to +58
def clear(self):
clear()
guids = []
for sceneobject in self.sceneobjects:
guids += sceneobject.guids
sceneobject._guids = None
clear(guids=guids)

def redraw(self):
self.clear()

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tomvanmele How about now? we collect the all guids of scene then clear all together. Also we add guids as argument for clear, so it does not delete the other stuff in context that are not managed by the scene.

Comment on lines -55 to +62
@staticmethod
def draw_collection(collection):
"""Drawing method for drawing an entire collection of objects."""
raise NotImplementedError
def clear(self):
"""The main clearing method."""
clear(guids=self.guids)
self._guids = None
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then we also use same same clear function for individual object. In this way the clear function of the object doesn't need to be re-implemented anymore

@Licini Licini merged commit 0c8f79e into main Dec 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants