mastodon-sc / mastodon

Mastodon – a large-scale tracking and track-editing framework for large, multi-view images.
BSD 2-Clause "Simplified" License
66 stars 20 forks source link

TrackScheme Branch and Hierarchy Windows Become Buggy After Removing Spots #226

Open maarzt opened 1 year ago

maarzt commented 1 year ago

Problem Description

They problem can be reproduced like this:

  1. Open a Mastodon project
  2. Open the TrackScheme Branch window
  3. Select any branch
  4. Delete the branch by clicking SHIFT+delete
  5. Select the branch again
  6. The branch won't get selected, but a exception gets printed:
Exception in thread "AWT-EventQueue-0" java.lang.IndexOutOfBoundsException: bitIndex < 0: -1
    at java.util.BitSet.get(BitSet.java:623)
    at org.mastodon.model.DefaultSelectionModel.isSelected(DefaultSelectionModel.java:124)
    at org.mastodon.model.DefaultSelectionModel.setSelected(DefaultSelectionModel.java:157)
    at org.mastodon.model.branch.BranchGraphSelectionAdapter.setVertexSelected(BranchGraphSelectionAdapter.java:142)
    at org.mastodon.model.branch.BranchGraphSelectionAdapter.setSelected(BranchGraphSelectionAdapter.java:127)
    at org.mastodon.adapter.SelectionModelAdapter.setSelected(SelectionModelAdapter.java:91)
    at org.mastodon.views.trackscheme.display.TrackSchemeNavigationBehaviours.select(TrackSchemeNavigationBehaviours.java:252)
    at org.mastodon.views.trackscheme.display.TrackSchemeNavigationBehaviours.access$1100(TrackSchemeNavigationBehaviours.java:63)
    at org.mastodon.views.trackscheme.display.TrackSchemeNavigationBehaviours$ClickSelectionBehaviour.click(TrackSchemeNavigationBehaviours.java:393)
    at org.scijava.ui.behaviour.MouseAndKeyHandler.mouseClicked(MouseAndKeyHandler.java:265)
    at java.awt.AWTEventMulticaster.mouseClicked(AWTEventMulticaster.java:270)
    at java.awt.Component.processMouseEvent(Component.java:6542)
    at javax.swing.JComponent.processMouseEvent(JComponent.java:3324)
    at java.awt.Component.processEvent(Component.java:6304)
    at java.awt.Container.processEvent(Container.java:2239)
    at java.awt.Component.dispatchEventImpl(Component.java:4889)
    at java.awt.Container.dispatchEventImpl(Container.java:2297)
    at java.awt.Component.dispatchEvent(Component.java:4711)
    at java.awt.LightweightDispatcher.retargetMouseEvent(Container.java:4904)
    at java.awt.LightweightDispatcher.processMouseEvent(Container.java:4544)
    at java.awt.LightweightDispatcher.dispatchEvent(Container.java:4476)
    at java.awt.Container.dispatchEventImpl(Container.java:2283)
    at java.awt.Window.dispatchEventImpl(Window.java:2746)
    at java.awt.Component.dispatchEvent(Component.java:4711)
    at java.awt.EventQueue.dispatchEventImpl(EventQueue.java:760)
    at java.awt.EventQueue.access$500(EventQueue.java:97)
    at java.awt.EventQueue$3.run(EventQueue.java:709)
    at java.awt.EventQueue$3.run(EventQueue.java:703)
    at java.security.AccessController.doPrivileged(Native Method)
    at java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:74)
    at java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:84)
    at java.awt.EventQueue$4.run(EventQueue.java:733)
    at java.awt.EventQueue$4.run(EventQueue.java:731)
    at java.security.AccessController.doPrivileged(Native Method)
    at java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:74)
    at java.awt.EventQueue.dispatchEvent(EventQueue.java:730)
    at java.awt.EventDispatchThread.pumpOneEventForFilters(EventDispatchThread.java:205)
    at java.awt.EventDispatchThread.pumpEventsForFilter(EventDispatchThread.java:116)
    at java.awt.EventDispatchThread.pumpEventsForHierarchy(EventDispatchThread.java:105)
    at java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:101)
    at java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:93)
    at java.awt.EventDispatchThread.run(EventDispatchThread.java:82)

Workaround

Update the branch graph after deleting spots or links: Click the "Regen. branch-graph" button.

Insights from investigating the problem

The ModelBranchGraph gets outdated when making changes to the ModelGraph. After deleting spots the ModelBranchGraph methods getFirstLinkedVertex, getLastLinkedVertex and getLinkedEdge will return references to no longer existing spots and links. The VertexBranchIterator and EdgeBranchIterator return wrong spot and edge references and might not terminate.

The exception listed above is triggered because the EdgeBranchIteration returns wrong link references.

Ideas how to fix this issue

Here is an example that demonstrates how the branchGraph.edgeBranchIterator(...) gives wrong results after deleting a spot in the ModelGraph:

package org.mastodon;

import static org.junit.Assert.assertEquals;

import java.util.Iterator;

import org.junit.Test;
import org.mastodon.mamut.model.Link;
import org.mastodon.mamut.model.Model;
import org.mastodon.mamut.model.ModelGraph;
import org.mastodon.mamut.model.Spot;
import org.mastodon.mamut.model.branch.BranchSpot;
import org.mastodon.mamut.model.branch.ModelBranchGraph;

public class BranchGraphProblem
{

    @Test
    public void test() {
        // create simple example graph
        final Model model = new Model();
        final ModelGraph graph = model.getGraph();
        final Spot a = graph.addVertex().init( 0, new double[] { 1, 2, 3 }, 1 );
        final Spot b = graph.addVertex().init( 1, new double[] { 1, 2, 3 }, 1 );
        final Spot c = graph.addVertex().init( 2, new double[] { 1, 2, 3 }, 1 );
        graph.addEdge( a, b ).init();
        graph.addEdge( b, c ).init();

        // rebuilt branch graph
        ModelBranchGraph branchGraph = model.getBranchGraph();
        branchGraph.graphRebuilt();

        // remove spot
        graph.remove( c );
        final Spot d = graph.addVertex().init( 2, new double[] { 1, 2, 3 }, 1 );

        // test
        BranchSpot bvA = branchGraph.getBranchVertex( a, branchGraph.vertexRef() );
        Spot bvA_end = branchGraph.getLastLinkedVertex( bvA, graph.vertexRef() ); // BUG: returns a spot that is no longer part of the graph
        System.out.println( bvA_end );
        assertEquals( d, bvA_end );
        Iterator< Link > edges = branchGraph.edgeBranchIterator( bvA );
        System.out.println( "link pool index: " + edges.next().getInternalPoolIndex() );
        System.out.println( "link pool index: " + edges.next().getInternalPoolIndex() ); // BUG: returns -1, this edge does not exist
    }

}

TODOs

Acceptance Criteria

tinevez commented 1 year ago

The incremental solution is very hard to build with mastodon data structures. It was our initial attempt and we failed to make it work against 100% cases. This failure is the main reason why we have this design currently in mastodon.

But it is also really not great when it comes to performance, and has a measurable cost in responsiveness, even if the user plans not to use the branch graph.

I would be in favor with solutions like "it is the responsibility of the caller to ensure blablabla". Now we must find a way not too clunky to have this in the UI. Maybe gracefully fail when we have invalid objects?