search

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

Adding new spots fails after deleting all spots #294

Open maarzt opened 3 months ago

maarzt commented 3 months ago

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:

Other solutions:

  1. Remove outdated spots from the many maps in the BranchGraphImp
  2. Alternative: Completely rework the BranchGraphImp
tinevez commented 3 months ago

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

stefanhahmann commented 3 months ago

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

GIF 13 03 2024 17-16-50

tinevez commented 3 months ago

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

stefanhahmann commented 3 months ago

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

tinevez commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

@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 commented 3 months ago

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

@tinevez what do you think?

tinevez commented 3 months ago

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 commented 1 month ago

Same issue than #226 ?

stefanhahmann commented 1 month ago

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.