root
/
aseprite
mirror of https://github.com/aseprite/aseprite.git
1
0
Fork 0
Code Issues Actions 6 Packages Projects Releases Wiki Activity

Refactor key context matching (fix #5390) 

* Added AppShortcut::fitsBetterThan() to compare shortcuts and decide
  which one is better depending on the current key context and the ones
  where shortcuts are defined
* Added Key::fitsContext() to known if this key definition makes sense
  for a given current key context, e.g. Normal context can be used in
  SelectionTool context, and SelectionTool can be used in Transformation
* Added KeyboardShortcuts::findBestKeyFromMessage() to centralize the
  keyboard shortcuts matching moving some code from
  CustomizedGuiManager::processKey()
* Added more keyboard shortcuts tests (including some for #5390)
This commit is contained in:
David Capello 2025-09-03 18:17:32 -03:00
parent df5dcdc1d9
commit 71e680e05a
7 changed files with 245 additions and 53 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
src/app/modules/gui.cpp
View File

@ -680,27 +680,13 @@ void CustomizedGuiManager::onNewDisplayConfiguration(Display* display)
bool CustomizedGuiManager::processKey(Message* msg)
{
App* app = App::instance();
const KeyContext currentCtx = KeyboardShortcuts::getCurrentKeyContext();
const KeyboardShortcuts* keys = KeyboardShortcuts::instance();
const KeyContext contexts[] = { currentCtx, KeyContext::Normal };
int n = (contexts[0] != contexts[1] ? 2 : 1);
// Find best match (prefer the shortcut that matches the context first)
KeyPtr key = nullptr;
for (int i = 0; i < n; ++i) {
for (const KeyPtr& k : *keys) {
if (k->isPressed(msg, contexts[i]) &&
(!key ||
(key->keycontext() != currentCtx && match_key_context(k->keycontext(), currentCtx)))) {
key = k;
}
}
}
const KeyPtr key = keys->findBestKeyFromMessage(msg);
if (!key)
return false;
// Cancel menu-bar loops (to close any popup menu)
App* app = App::instance();
app->mainWindow()->getMenuBar()->cancelMenuLoop();
switch (key->type()) {

83
src/app/ui/key.cpp
View File

@ -295,6 +295,32 @@ namespace app {
using namespace ui;
bool AppShortcut::fitsBetterThan(const KeyContext currentContext,
const KeyContext thisShortcutContext,
const KeyContext otherShortcutContext,
const AppShortcut& otherShortcut) const
{
// Better context in the same source
if (otherShortcut.source == this->source && otherShortcutContext != currentContext &&
thisShortcutContext == currentContext)
return true;
// Better key source/level: User-defined > Extension-defined > App-defined
if (int(this->source) > int(otherShortcut.source) && (thisShortcutContext == currentContext ||
// User-defined "Any" context overwrites all
// app-defined context
thisShortcutContext == KeyContext::Any))
return true;
// Normal > SelectionTool > Transformation
if ((currentContext == KeyContext::Transformation &&
otherShortcutContext != KeyContext::Transformation &&
thisShortcutContext == KeyContext::SelectionTool))
return true;
return false;
}
Key::Key(const Key& k)
: m_type(k.m_type)
, m_adds(k.m_adds)
@ -437,31 +463,58 @@ void Key::add(const ui::Shortcut& shortcut, const KeySource source, KeyboardShor
}
}
bool Key::fitsContext(const KeyContext keyContext) const
{
// This key is for any context
if (m_keycontext == KeyContext::Any)
return true;
// This key is for the same context
if (m_keycontext == keyContext)
return true;
// Use Normal or SelectionTool keys in Transformation context
if (keyContext == KeyContext::Transformation &&
(m_keycontext == KeyContext::SelectionTool || m_keycontext == KeyContext::Normal))
return true;
// Use Normal keys in SelectionTool or FramesSelection contexts
if ((keyContext == KeyContext::SelectionTool || keyContext == KeyContext::FramesSelection) &&
(m_keycontext == KeyContext::Normal))
return true;
return false;
}
const AppShortcut* Key::isPressed(const Message* msg, const KeyContext keyContext) const
{
const AppShortcut* best = nullptr;
if (const auto* keyMsg = dynamic_cast<const KeyMessage*>(msg)) {
for (const AppShortcut& shortcut : shortcuts()) {
if (shortcut.shortcut.isPressed(keyMsg->modifiers(),
keyMsg->scancode(),
keyMsg->unicodeChar()) &&
(m_keycontext == KeyContext::Any || match_key_context(m_keycontext, keyContext))) {
return &shortcut;
if (fitsContext(keyContext)) {
for (const AppShortcut& shortcut : shortcuts()) {
if (shortcut.shortcut.isPressed(keyMsg->modifiers(),
keyMsg->scancode(),
keyMsg->unicodeChar()) &&
(!best || shortcut.fitsBetterThan(keyContext, keycontext(), keycontext(), *best))) {
best = &shortcut;
}
}
}
}
else if (const auto* mouseMsg = dynamic_cast<const MouseMessage*>(msg)) {
for (const AppShortcut& shortcut : shortcuts()) {
if ((shortcut.shortcut.modifiers() == mouseMsg->modifiers()) &&
(m_keycontext == KeyContext::Any ||
// TODO we could have multiple mouse wheel key-context,
// like "sprite editor" context, or "timeline" context,
// etc.
m_keycontext == KeyContext::MouseWheel)) {
return &shortcut;
if (m_keycontext == KeyContext::Any ||
// TODO we could have multiple mouse wheel key-context,
// like "sprite editor" context, or "timeline" context,
// etc.
m_keycontext == KeyContext::MouseWheel) {
for (const AppShortcut& shortcut : shortcuts()) {
if (shortcut.shortcut.modifiers() == mouseMsg->modifiers())
return &shortcut;
}
}
}
return nullptr;
return best;
}
const AppShortcut* Key::isPressed(const Message* msg) const

13
src/app/ui/key.h
View File

@ -103,6 +103,8 @@ inline KeyAction operator&(KeyAction a, KeyAction b)
return KeyAction(int(a) & int(b));
}
// This is a ui::Shortcut wrapper (just one key shortcut) defined by
// the app, an extension, or the user (KeySource).
struct AppShortcut {
KeySource source;
ui::Shortcut shortcut;
@ -124,6 +126,13 @@ struct AppShortcut {
ui::KeyModifiers modifiers() const { return shortcut.modifiers(); }
ui::KeyScancode scancode() const { return shortcut.scancode(); }
int unicodeChar() const { return shortcut.unicodeChar(); }
// Returns true if this AppShortcut is better for the current
// context, compared to another shortcut.
bool fitsBetterThan(KeyContext currentContext,
KeyContext thisShortcutContext,
KeyContext otherShortcutContext,
const AppShortcut& otherShortcut) const;
};
using AppShortcuts = ui::ShortcutsT<AppShortcut>;
@ -133,6 +142,8 @@ using KeyPtr = std::shared_ptr<Key>;
using Keys = std::vector<KeyPtr>;
using DragVector = base::Vector2d<double>;
// A set of key shortcuts (AppShortcuts) associated to one command,
// tool, or specific action.
class Key {
public:
Key(const Key& key);
@ -148,6 +159,8 @@ public:
const AppShortcuts& delsKeys() const { return m_dels; }
void add(const ui::Shortcut& shortcut, KeySource source, KeyboardShortcuts& globalKeys);
bool fitsContext(KeyContext keyContext) const;
const AppShortcut* isPressed(const ui::Message* msg, KeyContext keyContext) const;
const AppShortcut* isPressed(const ui::Message* msg) const;
bool isPressed() const;

47
src/app/ui/key_context.h
View File

@ -11,28 +11,61 @@
namespace app {
// Specifies a possible App state/context where the user can press a
// key and associate different actions/commands in that specific
// context. Useful to associate the same key to different actions
// depending on the current context.
//
// This list should be sorted in such a way that more specific
// contexts are below:
//
// Normal > SelectionTool > Transformation
//
// This means that keys defined in SelectionTool are still valid in
// Transformation context.
//
// Other key context are just temporary in the Editor, like a mouse
// drag operation when we are transforming the
// selection. E.g. TranslatingSelection, ScalingSelection, etc.
enum class KeyContext {
// Special context to define keys. This is not used as a "current
// key context", but just to define keys for the whole range of key
// contexts.
Any,
// Regular context in a standby user state, the user is not dragging
// or clicking the mouse.
Normal,
// The user has a selection on the canvas and a selection-like tool
// (selection ink) selected.
SelectionTool,
// Special context to modify a specific action when the mouse is
// above the handles to translate/scale/rotate or is already
// translating/scaling/rotating the selection.
TranslatingSelection,
ScalingSelection,
RotatingSelection,
// Special context when the user has a specific tool selected to
// modify the tool behavior with a key:
MoveTool,
FreehandTool,
ShapeTool,
// When the mouse wheel is used above the editor.
MouseWheel,
// The user has a range of frames/cels selected in the timeline.
FramesSelection,
// The user has moved the selection and has yet to drop the pixels
// (the user can drop or undo the transformation).
Transformation,
};
inline bool match_key_context(const KeyContext a, const KeyContext b)
{
return (a == b) || (a == KeyContext::Any || b == KeyContext::Any) ||
((a == KeyContext::SelectionTool && b == KeyContext::Transformation) ||
(a == KeyContext::Transformation && b == KeyContext::SelectionTool));
}
} // namespace app
#endif

47
src/app/ui/keyboard_shortcuts.cpp
View File

@ -576,23 +576,46 @@ KeyContext KeyboardShortcuts::getCurrentKeyContext()
return KeyContext::Normal;
}
KeyPtr KeyboardShortcuts::findBestKeyFromMessage(const ui::Message* msg,
KeyContext currentKeyContext,
std::optional<KeyType> filterByType) const
{
const KeyContext contexts[] = { currentKeyContext, KeyContext::Normal };
int n = (contexts[0] != contexts[1] ? 2 : 1);
KeyPtr bestKey = nullptr;
const AppShortcut* bestShortcut = nullptr;
for (int i = 0; i < n; ++i) {
for (const KeyPtr& key : m_keys) {
// Skip keys that are not for the specific KeyType (e.g. only for commands).
if (filterByType.has_value() && key->type() != *filterByType)
continue;
const AppShortcut* shortcut = key->isPressed(msg, contexts[i]);
if (shortcut && (!bestKey || shortcut->fitsBetterThan(currentKeyContext,
key->keycontext(),
bestKey->keycontext(),
*bestShortcut))) {
bestKey = key;
bestShortcut = shortcut;
}
}
}
return bestKey;
}
bool KeyboardShortcuts::getCommandFromKeyMessage(const ui::Message* msg,
Command** command,
Params* params,
KeyContext currentKeyContext)
{
const KeyContext contexts[] = { currentKeyContext, KeyContext::Normal };
int n = (contexts[0] != contexts[1] ? 2 : 1);
for (int i = 0; i < n; ++i) {
for (KeyPtr& key : m_keys) {
if (key->type() == KeyType::Command && key->isPressed(msg, contexts[i])) {
if (command)
*command = key->command();
if (params)
*params = key->params();
return true;
}
}
KeyPtr key = findBestKeyFromMessage(msg, currentKeyContext, std::make_optional(KeyType::Command));
if (key) {
ASSERT(key->type() == KeyType::Command);
if (command)
*command = key->command();
if (params)
*params = key->params();
return true;
}
return false;
}

7
src/app/ui/keyboard_shortcuts.h
View File

@ -12,6 +12,8 @@
#include "app/ui/key.h"
#include "obs/signal.h"
#include <optional>
namespace tinyxml2 {
class XMLElement;
}
@ -60,6 +62,11 @@ public:
static KeyContext getCurrentKeyContext();
KeyPtr findBestKeyFromMessage(
const ui::Message* msg,
KeyContext currentKeyContext = KeyboardShortcuts::getCurrentKeyContext(),
std::optional<KeyType> filterByType = std::nullopt) const;
bool getCommandFromKeyMessage(
const ui::Message* msg,
Command** command,

83
src/app/ui/keyboard_shortcuts_tests.cpp
View File

@ -41,12 +41,18 @@ static KeyboardShortcuts* ks = nullptr;
{ \
KeyMessage msg(kKeyDownMessage, scancode, kKeyNoneModifier, 0, 0); \
Command* cmd = nullptr; \
Params params; \
EXPECT_TRUE(ks->getCommandFromKeyMessage(&msg, &cmd, &params, keycontext)); \
EXPECT_TRUE(ks->getCommandFromKeyMessage(&msg, &cmd, nullptr, keycontext)); \
ASSERT_TRUE(cmd != nullptr) << "command not found for key"; \
EXPECT_EQ(CommandId::commandId(), cmd->id()) << "other command found: " << cmd->id(); \
}
#define NO_COMMAND_FOR_KEY(scancode, keycontext) \
{ \
KeyMessage msg(kKeyDownMessage, scancode, kKeyNoneModifier, 0, 0); \
Command* cmd = nullptr; \
EXPECT_FALSE(ks->getCommandFromKeyMessage(&msg, &cmd, nullptr, keycontext)); \
ASSERT_TRUE(cmd == nullptr) << "command found for key: " << cmd->id(); \
}
TEST(KeyboardShortcuts, Basic)
{
ks->clear();
@ -73,14 +79,28 @@ TEST(KeyboardShortcuts, KeyContexts)
DEFINE_KEY(Cancel, kKeyEsc, KeyContext::Any);
DEFINE_KEY(GotoPreviousFrame, kKeyLeft, KeyContext::Normal);
DEFINE_KEY(PlayAnimation, kKeyEnter, KeyContext::Normal);
DEFINE_KEY(Clear, kKeyBackspace, KeyContext::Normal);
DEFINE_KEY(MoveMask, kKeyLeft, KeyContext::SelectionTool);
DEFINE_KEY(Apply, kKeyEnter, KeyContext::Transformation);
DEFINE_KEY(Cut, kKeyX, KeyContext::SelectionTool);
EXPECT_COMMAND_FOR_KEY(Cancel, kKeyEsc, KeyContext::Normal);
EXPECT_COMMAND_FOR_KEY(GotoPreviousFrame, kKeyLeft, KeyContext::Normal);
EXPECT_COMMAND_FOR_KEY(MoveMask, kKeyLeft, KeyContext::SelectionTool);
EXPECT_COMMAND_FOR_KEY(PlayAnimation, kKeyEnter, KeyContext::Normal);
EXPECT_COMMAND_FOR_KEY(PlayAnimation, kKeyEnter, KeyContext::SelectionTool);
EXPECT_COMMAND_FOR_KEY(Apply, kKeyEnter, KeyContext::Transformation);
EXPECT_COMMAND_FOR_KEY(Clear, kKeyBackspace, KeyContext::Normal);
EXPECT_COMMAND_FOR_KEY(Clear, kKeyBackspace, KeyContext::SelectionTool);
EXPECT_COMMAND_FOR_KEY(Clear, kKeyBackspace, KeyContext::Transformation);
NO_COMMAND_FOR_KEY(kKeyX, KeyContext::Normal);
EXPECT_COMMAND_FOR_KEY(Cut, kKeyX, KeyContext::SelectionTool);
EXPECT_COMMAND_FOR_KEY(Cut, kKeyX, KeyContext::Transformation);
}
TEST(KeyboardShortcuts, UserDefined)
TEST(KeyboardShortcuts, UserDefinedPriority)
{
ks->clear();
@ -89,6 +109,63 @@ TEST(KeyboardShortcuts, UserDefined)
DEFINE_USER_KEY(Redo, kKeyZ, KeyContext::Normal);
EXPECT_COMMAND_FOR_KEY(Redo, kKeyZ, KeyContext::Normal);
DEFINE_KEY(MoveMask, kKeyLeft, KeyContext::SelectionTool);
DEFINE_USER_KEY(GotoPreviousFrame, kKeyLeft, KeyContext::Normal);
EXPECT_COMMAND_FOR_KEY(MoveMask, kKeyLeft, KeyContext::SelectionTool);
DEFINE_USER_KEY(GotoPreviousFrame, kKeyLeft, KeyContext::Any);
EXPECT_COMMAND_FOR_KEY(GotoPreviousFrame, kKeyLeft, KeyContext::SelectionTool);
}
TEST(KeyboardShortcuts, SpecificContextHasMorePriorityButNotIfItsUserDefined)
{
ks->clear();
DEFINE_KEY(Cancel, kKeyEsc, KeyContext::Any);
DEFINE_KEY(Undo, kKeyEsc, KeyContext::Transformation);
// Pressing "Esc" in "Transformation" context should run "Undo",
// although "Cancel" is defined for "Any" context, a more specific
// context should have more priority.
EXPECT_COMMAND_FOR_KEY(Undo, kKeyEsc, KeyContext::Transformation);
// But an user-defined key, even for Any context, will overwrite the
// app-defined shortcut in all contexts.
DEFINE_USER_KEY(Zoom, kKeyEsc, KeyContext::Any);
EXPECT_COMMAND_FOR_KEY(Zoom, kKeyEsc, KeyContext::Transformation);
}
// Test that we can configure the Left key to always Undo when the
// default configuration says that the Left key does other actions in
// different contexts.
//
// Related issue: https://github.com/aseprite/aseprite/issues/5390
TEST(KeyboardShortcuts, UndoWithLeftAndRight)
{
ks->clear();
DEFINE_KEY(GotoPreviousFrame, kKeyLeft, KeyContext::Normal);
DEFINE_KEY(MoveMask, kKeyLeft, KeyContext::SelectionTool);
EXPECT_COMMAND_FOR_KEY(GotoPreviousFrame, kKeyLeft, KeyContext::Normal);
EXPECT_COMMAND_FOR_KEY(MoveMask, kKeyLeft, KeyContext::SelectionTool);
// "Transformation" is a sub-context of "Selection" context
EXPECT_COMMAND_FOR_KEY(MoveMask, kKeyLeft, KeyContext::Transformation);
// Now we try defining the "Left" key for "Any" context overwriting it in all contexts.
DEFINE_USER_KEY(Undo, kKeyLeft, KeyContext::Any);
EXPECT_COMMAND_FOR_KEY(Undo, kKeyLeft, KeyContext::Normal);
EXPECT_COMMAND_FOR_KEY(Undo, kKeyLeft, KeyContext::SelectionTool);
EXPECT_COMMAND_FOR_KEY(Undo, kKeyLeft, KeyContext::Transformation);
}
TEST(KeyboardShortcuts, FramesSelection)
{
ks->clear();
DEFINE_KEY(LayerProperties, kKeyF2, KeyContext::Normal);
DEFINE_KEY(SetLoopSection, kKeyF2, KeyContext::FramesSelection);
EXPECT_COMMAND_FOR_KEY(LayerProperties, kKeyF2, KeyContext::Normal);
EXPECT_COMMAND_FOR_KEY(SetLoopSection, kKeyF2, KeyContext::FramesSelection);
}
int app_main(int argc, char* argv[])