Redundant entities managment in pyvista plotter

[gbene]

When updating an entity (adding, removing or visualizing a given property) it is not necessary to remove the actor and adding a new one. Pyvista already takes care of this.

For example for DEMs and DOMs when we add a new property, _dom_data_keys_modified_updateviews is called. Here to "refresh" the entity we do the following:

...
[1] self.remove_actor_in_view(uid=uid)
[2] this_actor = self.show_actor_with_property(uid=uid, collection='dom_coll', show_property=None,visible=show)
...

where [1] self.remove_actor_in_view executes:

if not self.actors_df.loc[self.actors_df['uid'] == uid].empty:
    this_actor = self.actors_df.loc[self.actors_df['uid'] == uid, 'actor'].values[0]
    success = self.plotter.remove_actor(this_actor)
    self.actors_df.drop(self.actors_df[self.actors_df['uid'] == uid].index, inplace=True)

while [2] self.show_actor_with_property essentially executes:

self.plotter.add_mesh(plot_entity, many_args, name=uid, other_args)

Here lies the redundancy. If we don't need to remove the entity from the plotter (i.e. from the entire project), there is no need to call self.plotter.remove_actor(this_actor). Since we use _self.plotter.addmesh with the name=uid argument, Pyvista searches the actor with the same name and replaces it with the new one. This also takes care of the "blinking" effect that happens when a visualized property is changed.

We should separate the actual removing of the entity in the plotter (calling success = self.plotter.remove_actor(this_actor)) from the updating (not calling success = self.plotter.remove_actor(this_actor)). This can be done by adding a flag in _remove_actor_inview such as update=True and then introduce an if statement:

def remove_actor_in_view(self, uid=None, redraw=False,update=True):
    """"Remove actor from plotter"""
    """plotter.remove_actor can remove a single entity or a list of entities as actors -> here we remove a single entity"""
    if not self.actors_df.loc[self.actors_df['uid'] == uid].empty:
        this_actor = self.actors_df.loc[self.actors_df['uid'] == uid, 'actor'].values[0]
        if update == False: #<---- New statement
          success = self.plotter.remove_actor(this_actor)
        self.actors_df.drop(self.actors_df[self.actors_df['uid'] == uid].index, inplace=True)

or by creating a new function (e.g. update_actor_in_view).

The first is quicker to implement because there is no need to update all of the codebase to adapt to the change and only the x_removed_update_views signals need to be slightly adjusted:

def dom_removed_update_views(self, updated_list=None):
    """This is called when a DOM is removed from the dom collection.
    Disconnect signals to dom list, if they are set, then they are
    reconnected when the list is rebuilt"""
    self.DOMsTableWidget.itemChanged.disconnect()
    for uid in updated_list:
        self.remove_actor_in_view(uid=uid,update=False) #<---- Adjustment
        self.update_dom_list_removed(removed_list=updated_list)
    """Re-connect signals."""
    self.DOMsTableWidget.itemChanged.connect(self.toggle_dom_visibility)

Let me know what you think!

[andrea-bistacchi]

I think creating a new function for update in the end will result in code that is more easily understood.

[andrea-bistacchi]

Both self.plotter.add_mesh and pyvista.Plotter.add_volume with name=uid are confirmed to replace the old actor with the new one.

https://docs.pyvista.org/version/stable/api/plotting/_autosummary/pyvista.Plotter.add_mesh.html#pyvista.Plotter.add_mesh

https://docs.pyvista.org/version/stable/api/plotting/_autosummary/pyvista.plotter.add_volume#pyvista.Plotter.add_volume

So I'm reimplementing all the update functions using add_mesh or add_volume to replace the actor, and something similar to the following to update the actors dataframe.

this_actor = self.show_actor_with_property(uid=uid, collection=collection, show_property=show_property, visible=show)
self.actors_df.loc[self.actors_df["uid"] == uid, ["show_prop"]] = show_property

[andrea-bistacchi]

There are still two cases where self.remove_actor_in_view is used, in stereoplots. Must see if this is necessary with Matplotlib.