scenerygraphics / sciview

sciview is a tool for visualization and interaction with ND image and mesh data
BSD 2-Clause "Simplified" License
62 stars 17 forks source link

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

Closed odinsbane closed 3 weeks ago

odinsbane commented 2 months ago

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.

kephale commented 1 month ago

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

odinsbane commented 1 month ago

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?

kephale commented 1 month ago

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:

kephale commented 1 month ago

@odinsbane would you consider this closed if we get this merged: https://github.com/scenerygraphics/sciview/pull/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.

odinsbane commented 1 month ago

Yes! I am not sure where this flag is, but when you told me to use sciview to remove the node. That flag works ok too.

On Mon, Jun 3, 2024, 4:14 PM Kyle I S Harrington @.***> wrote:

@odinsbane https://github.com/odinsbane would you consider this closed if we get this merged: #594 https://github.com/scenerygraphics/sciview/pull/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.

— Reply to this email directly, view it on GitHub https://github.com/scenerygraphics/sciview/issues/578#issuecomment-2145313812, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA2NNEPH32KDBDVRYMQOUGLZFR25BAVCNFSM6AAAAABGNKQXP6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNBVGMYTGOBRGI . You are receiving this because you were mentioned.Message ID: @.***>

kephale commented 1 month ago

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