scenerygraphics/sciview

Adding a group to a Volume and then removing the group doesn't update the object inspector.

odinsbane opened this issue · 6 comments

I've created a command in that command I 'Mesh's to a group that I add to a volume. Since there are a lot of them, I made a way to remove them from the volume. When I remove the meshes, they remain in the inspect. If I remove one of them from the inspector, it updates and the volume no longer has the groups as children.

The command lives here:

https://github.com/Living-Technologies/SciviewDM3D 

The relevant code from MeshVolumeDemo first adds a bunch of groups to the volume.

        Node active = sciView.getActiveNode();
        Volume v = (Volume)active;
        Group g = new Group();
        g.addChild(mesh);
        sciView.addNode(g, v);

Then in the MeshVolumeNavigator the groups are removed from the volume.

    TreeMap<Integer, Group> meshes;
    meshes.values().forEach(v::removeChild);

Maybe I should be removing the nodes through the sciView interface. I'm sure there are a lot of problems going on here.

Yes, you are correct. To get UI updates you need to use the sciview API to remove them. You would use SciView.deleteNode.

That works, it is slow if I don't control the publish option.

        Iterator<Group> iter = meshes.values().iterator();
        while(iter.hasNext()){
            Node node = iter.next();
            boolean publish = ! iter.hasNext();
            sciView.deleteNode(node, publish);
        }

Is there a publish method?

Highlighting some of the issue:

@skalarproduktraum @ctrueden We've seen issues like this before and with enough regularity that we should probably do something more serious than the activePublish flag. My immediate proposal:

At both of the previously mentioned locations where NodeRemovedEvents trigger updates:

  • change the rebuild/update calls to flag setting logic (e.g. Properties.isValid)
  • then run the corresponding logic in the SciView and Properties classes conditionally on a per-frame basis (e.g. each timestep check if we need to run the rebuilds)

@odinsbane would you consider this closed if we get this merged: #594

Otherwise having deletion work with the removeChild methods would be a bigger ask since that means connecting the scenery methods to the corresponding sciview instance.

Great, and the PR should speed up node deletion for many nodes.