Adding new spots fails after deleting all spots

[maarzt]

Steps to reproduce

1. Open a mastodon project with some spots already in there.
2. Open branch graph and BDV windows.
3. Delete all spots in BDV. (Ctrl + A and Shift + Del in BDV window)
4. Try to add a new spot in BDV, e.g. by pressing a
5. Produces an exception:

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:137)
                at org.mastodon.model.branch.BranchGraphSelectionAdapter.isSelected(BranchGraphSelectionAdapter.java:95)
                at org.mastodon.model.branch.BranchGraphSelectionAdapter.getSelectedVertices(BranchGraphSelectionAdapter.java:277)
                at org.mastodon.views.table.TableViewFrameBuilder$GraphTableBundle.selectionChanged(TableViewFrameBuilder.java:547)
                at org.mastodon.model.DefaultSelectionModel.resumeListeners(DefaultSelectionModel.java:382)
                at org.mastodon.adapter.SelectionModelAdapter.resumeListeners(SelectionModelAdapter.java:157)
                at org.mastodon.views.bdv.overlay.EditBehaviours$AddSpotBehaviour.click(EditBehaviours.java:344)
                at org.scijava.ui.behaviour.MouseAndKeyHandler.handleKeyPressed(MouseAndKeyHandler.java:459)
                at org.scijava.ui.behaviour.MouseAndKeyHandler.access$000(MouseAndKeyHandler.java:51)
                at org.scijava.ui.behaviour.MouseAndKeyHandler$1.handleKeyPressed(MouseAndKeyHandler.java:403)
                at org.scijava.ui.behaviour.KeyPressedManager.handleKeyPressed(KeyPressedManager.java:58)
                at org.scijava.ui.behaviour.MouseAndKeyHandler.keyPressed(MouseAndKeyHandler.java:365)
                at java.awt.AWTEventMulticaster.keyPressed(AWTEventMulticaster.java:249)
                at java.awt.Component.processKeyEvent(Component.java:6497)
                at javax.swing.JComponent.processKeyEvent(JComponent.java:2832)
                at java.awt.Component.processEvent(Component.java:6316)
                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.KeyboardFocusManager.redispatchEvent(KeyboardFocusManager.java:1954)
                at java.awt.DefaultKeyboardFocusManager.preDispatchKeyEvent(DefaultKeyboardFocusManager.java:1102)
                at java.awt.DefaultKeyboardFocusManager.typeAheadAssertions(DefaultKeyboardFocusManager.java:973)
                at java.awt.DefaultKeyboardFocusManager.dispatchEvent(DefaultKeyboardFocusManager.java:799)
                at java.awt.Component.dispatchEventImpl(Component.java:4760)
                at java.awt.Container.dispatchEventImpl(Container.java:2297)
                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)

Reason for the exception

The ModelBranchGraph tries to reference ModelGraph spots that no longer exist. Results in a call to DefaultSelectionModel.isSelected(...) with a spot poolindex -1.

Potential solutions

Preferred solution:

1. It could be patched it by adding a 'regenerate' call to the remove spot(s) action.

To make the branch graph consistent with the model graph one could also add automatic branch graph rebuild after:

• Spot added
• Edge added
• Edge removed

Other solutions:

2. Remove outdated spots from the many maps in the BranchGraphImp
3. Alternative: Completely rework the BranchGraphImp

[tinevez]

I could not reproduce the bug with the beta-29 on Mac. Am i missing a step?

[stefanhahmann]

I could not reproduce the bug with the beta-29 on Mac. Am i missing a step?

This is strange. We could reproduce this bug on Ubuntu and Windows with the beta-29. I used this dataset for testing:
https://github.com/mastodon-sc/mastodon-example-data/tree/master/tgmm-mini

[tinevez]

Do the same, but click 'Regenerate' before adding a new spot.

[stefanhahmann]

That would work, but users would need to be aware of this.

[tinevez]

No I think this is a bug. Your finding confirms that it caused by the branch graph becoming seriously out of sync with the core graph. We could patch it by adding a 'regenerate' call to the remove action. What do you think?

[stefanhahmann]

No I think this is a bug. Your finding confirms that it caused by the branch graph becoming seriously out of sync with the core graph. We could patch it by adding a 'regenerate' call to the remove action. What do you think?

I had the same idea, but since this is different from the solutions @maarzt suggested, I would also like to have his opinion on this.

[maarzt]

@tinevez wrote:

No I think this is a bug. Your finding confirms that it caused by the branch graph becoming seriously out of sync with the core graph. We could patch it by adding a 'regenerate' call to the remove action. What do you think?

That's a very good idea. And much simpler than what I had in mind.

[stefanhahmann]

To make the branch graph consistent with the model graph one could also add automatic branch graph rebuild after:

• Spot added
• Edge added
• Edge removed
• (and Spot removed, as already said)

@tinevez what do you think?

[tinevez]

Hum I am not in favor of this. Initially we had the 'regenerate' button to avoid triggering branch graph recalculation at every event. I am not sure yet how to reconcile robustness (not crashing) with not calling the regenerate routine too often.

[tinevez]

Same issue than #226 ?

[stefanhahmann]

Same issue than #226 ?

Yes, they are very likely related.

I would still not close this issue, since, I think, it is good to have multiple test scenarios for a potential fix at hand.