-
Notifications
You must be signed in to change notification settings - Fork 20
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Rework the application model #253
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This branch reflects an effort to rework the main class of the Mamut app: the MamutAppModel. The main goal is to make the app model the core of a Mastodon session, and have all of its fields final, and have them containing everything we need to run all featuures of the app. This is a WIP but so far we have: - The windowManager is now a field of the app model. - The app model is immutable: it cannot change under the user feet. When a user loads a project, it gets another instance, but cannot modify an existing one. - And to do this, the main work of this PR is the rework of the project loading and saving, detailed a bit below. The ProjectManager class is gone, replaced by 4 classes with only static methods, in the package org.mastodon.mamut.io. Each one takes care about loading, saving, importing or exporting. The loading methods all produces a new MamutAppModel instance. This instance receives the MamutProject it was created on (handy to save it later). The app model is properly initiatialized ONCE with everything. Then a consumer can create a MainWindow to pilot it. I have updated all the classes so that they compile. The tests all run (except for the garbage collection from Mathias, which requires changing two deps). But I am anxious to see if we cover all the use cases below: - [x] open a project - [ ] save a modified project - [x] 'save as' a project - [ ] test that we can reload a saved project - [ ] test that we can reload a 'saved as' project The importers and exporters work, as seen by the tests, but I would like to test them and rework their UI. Then I will rework again the app model, rename it etc...
Also make more explicit their respective role in the javadoc.
We rely on MamutAppModel for non-view related fields. Also, do not test for appModel == null anymore.
They will generate an error in v2.0.
This commit removes mention of the deprecated SetupAssignments and BrightnessDialog classes. The actions that brought the corresponding dialogs are removed as well. There is some code to still make it possible to save/load bdv settings and use ones that are generated 'elsewhere'.
Mainly as an effort to reduce the number of fields and methods in the window manager class. We began with just a manager for BDV and TrackScheme styles but now we have 5 of them. For now. Also this will make it easier to extend Mastodon with new views in the future.
The view lists are stored in a map, indexed by their class, like for the manager. This diminish the number of fields and methods in the window manager class. This also allows for treating views in a general manner without having to change too much code when a new view will be added. For this I shall find a way to create view instances in a generic manner, probably with a factory pattern.
We started with just 2 views (BDV and TrackScheme) but now there are enough of them is that it is worthwhile having some generic way of handling them. The next commits will be about coming with a sensible way of doing it. Ideally, we want to make it possible for 3rd parties to extends Mastodon with new views without having to recompile it.
- Get rid of IMastodonView and IMastodonFrameView - An interface for views that have a ViewerFrame, that MastodonFrameView implements. - An interface for all Mamut views (branch and core graphs) that extend from it, and add methods for GroupHandle on onClose(). - MamutView and MamutBranchView both implement this interface.
So that now we can have one factory type for both branch and core graph views. Tune the bdv factory accordingly.
The FocusModel interface had an unneeded type. It does not need to know the type of edges in a graph, because it does not work with a graph, but with a simple collection. Removing this superficious type triggered a lot of changes in the code because we touched the signature of a core interface. But not impact otherwise.
Imperfect because a full saving requires have a BDV view opened, which is not necessarily our case. When we don't have a view opened, save everything we can but no more. Fixes #230
It listens to programmatic changes in the color bar overlay, which ensures the menu is in sync with what the colorbar is configured with. This is important when restoring the GUI state. Concomittantly: we have one class more, but we diminish the duplicated code between MamutView and MamutBranchView.
Because we build it before regenerating the GUI state, the TrackScheme transforms of the branch trackschemes are now properly restored. With the previous commits, fixes #243
Before this commits, all the sessions launched with this class failed to terminate properly.
If we don't wait a little bit, the BDV state, notably the transform, is not restored properly. It is not enough to just display the frame before restoring the transform. We put the delay 0.1s in a separate thread not to block the loading if we have many BDV views. Fixes #251
Doing otherwise causes a very weird bug, making Mastodon crash when deployed in Fiji but not in Eclipse. The reason for the crash is the following: When creating the FeatureSpecsService, it immediately runs Specs discovery. For this it needs the PluginService to be ALREADY initialized. This is the case when running Mastodon from Eclipse, but not when running it from Fiji. Setting the priority to extremely low makes sure FeatureSpecsService is initialized among the last, well after the services it depends on.
…to rework-app-model
So that we don't see warnings everytime we change a setting.
To separate pages made to configure views, from pages made to control plugins.
I want to move and rename style files, without breaking things for the user. This will simplofy this process.
- Number of incoming and outgoing links on a spot. Useful to fish for cells that have a division or a merge, and debug unwanted linking events. - Oriented time-difference for links. Useful to detect buggy links introduced by improper semi-auto tracking. Debugging without this feature is really a pain.
when creating the table header and collecting values. This is to protect against a bug that appear with features that are not careful to always return the same set of projections when the projections() method is called. This happend for the link target feature and generated the bug below: #217 One can fix the bug by imposing the same order on features, but it is also a plus to make the table rendering independent of this order being preserved.
This feature relies oncode part of Mobie that have moved from there recently. It will need to be reimplemented, using the new changes in the N5 library (and friends) by the Saalfeld lab (and friends). Remote opening of BDV data on Big-Data-Servers still work.
- Remove dep imagej-legacy - It generated errors with the legacy injector when trying to run Mastodon from Eclipse. - Remove mobie-io, jackson-core - We have to re-engineer N5 and S3 support. - Remove scijava-common, IO_ - Needed for DnD support, which needs to be rewritten as well. - Remove custom specification of bdv-core - It was ignored anyway and we now rely on the parent ponm specification.
It is helpful if a plugin has access to a list of views. Use case: https://github.com/elephant-track/elephant-client/blob/951a2c60115d30d9debae453436ddfbd40ac8042/src/main/java/org/elephant/actions/mixins/WindowManagerMixin.java#L55
The purpose of the getViewList() method now changed. It is meant to be used by 3rd party consumers, not by the internal logic.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR relates to a large amount of changes focused on reworking the application model. That is: the classes and logic that control the end-user application specific to the Mamut application (Spot and Link on a ModelGraph). It contains major API breaking changes which will prompt for a new beta version, hopefully the last one. This PR serves as a holder for the description of the changes as they are committed. Ideally a documentation will be extracted from the text appended here.
These changes try to address old and rather difficult TODOs that sometime appeared rather early in the project. For instance: In the beginning Mastodon had only two views and everything could be hardcoded. But now there are 7 of them, and new ones are coming, yielding a very long window manager class with many similar hard coded methods and many fields. The plugin system was built along the way while the app model was being elaborated. Loading and saving were handled by a component of the window manager (the ProjectManager), that could set, change or nullify the app model within one window manager instance, and dependents of the app model had to deal individually with these possibilities. And Mastodon had a lot of UI-related classes (the Keymaps, CommandDescriptions, SettingsPage frameworks) that Tobi moved recently to upstream packages (BDV-core and ui-behaviour).
This PR tries to address them, adding or tweaking features along the way.
Almost all the changes are invisible to the user. There are no changes in the file format. But developers can expect a lot of compile errors when they will depend on this PR.
The new
ProjectModel
class.The
MamutAppModel
class have been renamed asProjectModel
and is now the central class that manages everything related to one 'session'. That is: when the user loads or creates a project, a newProjectModel
instance is created that has everything needed.The plugins receive directly a
ProjectModel
instance.There is no need to make a separate class for the model provided to the plugins. They receive the
ProjectModel
, and stressing again: which is never null and never changes. So there is no need to check whether its null or to update the commands the plugin provides.The interface for Mamut plugins is now:
Loading and saving.
The
ProjectManager
class is gone, replaced by 4 classes with only static methods, in the packageorg.mastodon.mamut.io
. Each one takes care about loading, saving, importing or exporting.The loading methods all produces a new
ProjectModel
instance. This instance receives theMamutProject
it was created on (handy to save it later). The project model is properly initiatialized ONCE with everything. Then a consumer can create a MainWindow to pilot it. Example:The
MamutProject
and related classes have also been moved.MamutProjectIO
has now only static methods.Generic views for Mastodon in the window manager.
The view (
MamutViewBDV
,MamutViewTrackScheme
, ...) aer now created each by a specific factory, implementing the interfaceorg.mastodon.mamut.views.MamutViewFactory
. It is a discoverableSciJavaPlugin
, that defines how to create a specific view using aProjectModel
instance, and how to de/serialize its GUI state.The window manager has now a
MamutViews
field that is in charge of detecting and managing a collection of these view factories. Each factory is then used by the window manager to make one action that creates a view, registers it in the WM, deals whether it has a context provider or a context chooser, notifies listeners of its creation (see below) and finally shows it.To create a specific view, you simply have to provide its class. For instance to create and show a BDV view:
the new view will be properly registered and its context provider will be made available to all other context chooser views.
New views are created, registered and managed only via these factories now. Which means that that window manager does not know of any concrete view class. It just manages all the ones it found. This will make it easier to extends Mastodon with new views, without having to touch or even recompile the core.
This also makes the window manager class much shorter. There are much less methods and much less fields, since all views are managed in a generic manner.
The view factory are also nice because they contain in one place all the info related to an action: command name, menu text, description.
For ELEPHANT, Ko requires that there is a view creation listeners list for BDV window. I kept this, but made it generic. There is now one listener list per view type. So you can register a listener for the creation of any type of view, and get it before it is shown.
The factories also handle GUI state serialization. The
ProjectLoader
andProjectSaver
classes use these factories when loading / saving for restoring / saving the GUI state. There are no specific hard-coded code to handle serialization of GUI state in the io package. This de/serialization also now takes much less code than before, as there were a lot of duplicated code, which is now present once in the mother abstract class for view factories.Also: the views are now decoupled from saving or restoring the GUI state. The factory does it, which is cleaner.
All the view classes have been moved to subpackages, organized as follow:
Style managers and Settings pages are also managed by a factory pattern.
The various managers (
RenderSettingsManager
, ...) are managed in a similar way in the window manager. There is a factory interface for each (org.mastodon.mamut.managers.StyleManagerFactory
), and it can return whether the style manager has a Settings page. If it is the case, it is automatically added to thePreferencesDialog
.Replace
KeyMap
and associated classes by upstream implementations.The
Keymap
,KeymapManager
,CommandDescriptions
,SettingsPage
and all associated classes have been migrated in upstream repos, mainly the BDV-core and ui-behaviour. Their initial implementation have been removed from Mastodon, which now use the common ones.There are two adaptations specific for Mastodon:
1/ The
KeymapManager
class is extended by a specific one for Mastodon so that we can load and save keymaps in the Mastodon folder, and so that we load the builtin keymaps we had before.2/ The new
CommandDescription
requires specifying a context AND now a scope. I made two scopes: one for the actions that are generic to Mastodon (everything in views, that do not know of the Mamut model), and one specific for the Mamut application (everything underorg.mastodon.mamut
, which is the one users have).I could not replace one class:
KeymapSettingsPage
: There is the equivalent in BDV-core, but it does not 'see' the builtin keymaps we have in Mastodon.Better error messages and failing gracefully when opening projects.
I also worked on having the launcher not hang when something wrong happens. It should now show a meaningful error message to the user and allows them to open another project.
The views and dialogs show the current project name.
And it is updated when the user saves the project under a different name.