Skip to content
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

Configurable keymap prototype. #109

Draft
wants to merge 12 commits into
base: master
Choose a base branch
from
Draft

Conversation

tinevez
Copy link

@tinevez tinevez commented Sep 11, 2023

This PR is a prototype for a version of LabKit that ships configurable keymaps.

Only a subset of the actions in LabKit have configurable shortcuts for now. The configurability was introduced by changing as little code as possible, and retaining the original shortcuts in full. Supporting all actions and replacing the old keymap system with a new one involve quite some changes I would like to submit to @maarzt judgement first. So here is the PR for it.

There is now a Preferences dialog that can be opened from the Help menu.
It contains only one page, allowing to configure keymaps. This is the same framework used in BDV and Mastodon:

Screenshot 2023-09-11 at 10 19 56

In the code, I added several Actions and Behaviors to key UI classes, that are linked to a common KeymapManager, special to LabKit (it can discover key bindings for the LabKit context and for the BDV context).

For instance, to support editing keys for BDV navigation, the initBdv() method of the BasicLabellingComponent class is now the following:

private void initBdv(boolean is2D) {
  final BdvOptions options = BdvOptions.options();
  if (is2D) options.is2D();
  if (keymapManager != null) options.keymapManager(keymapManager);
  
  bdvHandle = new BdvHandlePanel(dialogBoxOwner, options);
  bdvHandle.getViewerPanel().setDisplayMode(DisplayMode.FUSED);
  
  if (keymapManager != null) {
	  ViewerPanel viewer = bdvHandle.getViewerPanel();
  
	  InputActionBindings keybindings = bdvHandle.getKeybindings();
	  TriggerBehaviourBindings triggerbindings = bdvHandle.getTriggerbindings();
  
	  Keymap keymap = keymapManager.getForwardSelectedKeymap();
  
	  final Actions actions = new Actions(keymap.getConfig(), "bdv");
	  actions.install(keybindings, "view");
  
	  Behaviours behaviours = new Behaviours(keymap.getConfig(), "bdv");
	  behaviours.install(triggerbindings, "view");
  
	  viewer.getTransformEventHandler().install(behaviours);
	  NavigationActions.install(actions, viewer, is2D);
  
	  keymap.updateListeners().add(() -> {
		  actions.updateKeyConfig(keymap.getConfig());
		  behaviours.updateKeyConfig(keymap.getConfig());
	  });
	}
  }

Ultimately, if we go this way, we will have to resolve duplicate keybindings (in the KeymapManager and in the ACCELERATOR of the buttons). There is also a custom class in LabKit ActionsAndBehaviors that also uses Actions and Behaviors, but are not linked to a keymap manager. It controls the painting actions and I could not find a way to have it listen to keymap changes properly yet.

- Defines a scope and key config context for LabKit.
- Load keymaps from a user config dir specific to LabKit.
- Collect command descriptions specific to LabKit.
Containing only a page to set the keymap.
Also added to the Help menu using the original menu system of
LabKit.
This is just a toy example: It *adds* an Actions object, set to
the LabKit context, to the existing shortcuts. The actions added
to the Actions are linked to the keymap manager, and the keybindings
can be edited in the preferences dialog.

The keys that the user edits in the preferences dialog are saved
to the labkit config dir ( home / .labkit / keymaps ).

Right now there are just 3 actions:
- show the preferences dialog
- close it
- the dummy example action that prints a useless message.
To change as little code as possible we simply make an action that
toggles the planar mode button. Then we register an item listener
to this button, so that it is notified when it is toggled
programmatically.

Also, the BasicLabelingComponent has now a 2nd set of actions and
behaviours, if it is provided with a KeymapManager, distinct from
the ones for the BDV.
Again, we simply use named actions that perform 'doClick' on
the existing toggle buttons.
Copy link
Collaborator

@maarzt maarzt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see problems with this PR. The key problem is that I don't get the scijava behaviour API. It just feels wrong. I look at it for an hour. Then I understand maybe 30% of it. And when I look at it the next time I start at zero again.

  • The KeyConfigManager cannot be used to configure short cuts that a visible in a Menu. Sorry I was incorrect it works
  • The DoClickButtonAction is a hack.
  • To much boiler plate code

There are also problems with the existing Labkit code. I would like to get rid of the BasicLabelingComponent. I would in general want to improve the structure of the code that wires the UI together.

One approach to succeed with this PR would be to build a very simple MockUp (Labkit similar UI) and demo how to use Scijava behavoirs there. @tinevez I don't expect you to do it. It's a task on my side.

@maarzt
Copy link
Collaborator

maarzt commented Sep 25, 2023

Here is my preliminary cheat sheet to help me understand SciJava behavoirs:

InputMap - A map: short cuts -> action ID
ActionMap - A map: action ID -> actions
InputTriggerMap - A map: short cuts -> behaviour ID
BehaviourMap - A map: behaviour ID -> behaviours

Actions - wraps around InputMap and ActionMap
Behaviours - wraps around InputTriggerMap and BehaviourMap
InputTriggerConfig - contains configuration
SwingUtilities.replaceUIActionMap
SwingUtilities.replaceUIInputMap

MouseAndKeyHandler - binds InputTriggerMap and BehaviourMap to a JComponent

InputActionBindings - chain multiple InputMaps and ActionMaps
TriggerBehaviourBindings - chain multiple InputTriggerMaps and BehaviorsMaps

@tinevez
Copy link
Author

tinevez commented Sep 29, 2023

If I can help with anything don't hesitate to ring my bell.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants