EsotericSoftware / spine-editor

Issue tracking for the Spine editor.
http://esotericsoftware.com/
29 stars 2 forks source link

There are some hotkeys that do not work when the `cmd` key is specified (macOS) #803

Closed misaki-eymard closed 1 month ago

misaki-eymard commented 3 months ago

Description: There are some hotkeys that don't work properly when the cmd key is specified in the hotkey, as if the key had been pressed multiple times.

--- General ---

--- Playback ---

Here is a video shows the issue: https://github.com/EsotericSoftware/spine-editor/assets/85478846/a0284b08-fdd2-42c0-98a6-1678b728f91a

Also, although it will not behave as if the key has been pressed multiple times, the following hotkeys will no longer be recognized when cmd is specified.

--- Tools ---

Here is a video shows the issue: https://github.com/EsotericSoftware/spine-editor/assets/85478846/b3636e3e-9ab4-4b8f-b29a-3d14cb4fb6de

Expected behavior: Even if you specify the cmd key, the hotkey will still be recognized normally.

Steps to reproduce:

  1. Open Spine 4.2.29 on Mac PC.
  2. Make sure you can drag and pan while holding down the Jkey in the viewport.
  3. Open the hotkeys.txt file and edit Pan Drag: J to Pan Drag: cmd + J.
  4. Reload the hotkeys file and press cmd + J when the viewport is active. Then you will notice that Pan Drag is no longer working.

The version of Spine in which this problem was found: Spine 4.2.29

davidetan commented 3 months ago

The issue here seems to be connected to the fact that while keeping pressed cmd + KEY:

If we keep a single key press, without modifer cmd, in KeyNames.java:


Moreover, if I overwrite the key method in GlfwInputProcessor like this:

public void key (long w, int key, int scancode, int action, int modifiers) {
    switch (action) {
    case GLFW_PRESS -> System.out.println("GLFW_PRESS");
    case GLFW_RELEASE -> System.out.println("GLFW_RELEASE");
    case GLFW_REPEAT -> System.out.println("GLFW_REPEAT");
    }
}

and run a basic example of JGLFW that just set the callback key like the above function, I have different behaviour.

In editor repo if I keep pressed cmd + KEY, I get:

GLFW_PRESS -> cmd
GLFW_PRESS -> j
GLFW_RELEASE -> j
GLFW_PRESS -> j
GLFW_RELEASE -> j
GLFW_PRESS -> j
...
GLFW_RELEASE -> j
GLFW_PRESS -> j
GLFW_RELEASE -> j
GLFW_RELEASE -> cmd

In the basic JGLFW example if I keep pressed cmd + KEY, I get:

GLFW_PRESS -> cmd
GLFW_PRESS -> j
GLFW_REPEAT -> j
GLFW_REPEAT -> j
...
GLFW_REPEAT -> j
GLFW_RELEASE -> j
GLFW_RELEASE -> cmd

The difference is that in the editor we have libgdx in the middle. So, I have to try with a plain libgdx example.

davidetan commented 3 months ago

Further debugging leads me here: https://github.com/badlogic/jglfw/commit/6625d363d9ae273a52b443498de57c7e0d8ac187 That cause a key release, and not repeat, when a key is kept pressed along with cmd modifier.

davidetan commented 3 months ago

If we just keep the else branch of the commit above, we don't have this bug.

In any case, when the code enters that if, it continuously generates GLFW_RELEASE events, this is why we have the strange behaviour showed above, that is:

rather than just GLFW_RELEASE.

The bug that custom sendEvent fixes, does not need of the additional attempt, if the app is not GLFWApplication. Because at least in java, when we call java.awt.Toolkit.getDefaultToolkit() in JglfwApplication#initialize, it initializes Apple AWT that initializes our NSApp, extending it as a NSApplicationAWT. We received the keyUp event because Apple AWT does the sendEvent fix for us: https://github.com/openjdk/jdk/blob/master/src/java.desktop/macosx/native/libosxapp/NSApplicationAWT.m#L373

So, after that I'm confident in removing the if above and just leave the else branch because:

Or, if we want to keep the thing similar as possible, we could change the internal if like this:

if (![NSApp isKindOfClass:[GLFWApplication class]] && ![NSStringFromClass([NSApp class]) isEqualToString:@"NSApplicationAWT"])
davidetan commented 3 months ago

Latest commit (30/06) in this PR seems to be working! Tested with the launcher in a debug build with latest commit of editor repo ecbb72b50.

davidetan commented 2 months ago

The Last Tool issue reported in the OP is not solved by the fix above. However, that is not a mac bug, but a specific behaviour of the Last Tool (it won't work when cmd/ctlr is pressed). On a windows machine the behaviour is the same.

This is because the Last Tool action is not triggered when the Selection Tool is active (for reference: EditMode#toggleTool -> if (tool == selectTool) return false;). The Selection Tool is the rectangle that you can draw to select when ctrl/cmd is pressed. So basically, ctrl/cmd activate the Selection Tool. When Selection Tool is active (ctrl/cmd is pressed), the code prevent the Last Tool action.

davidetan commented 2 months ago

@NathanSweet What do you think about the last tool behaviour?

Should we do anything, or the behaviour is expected and we can close this?

NathanSweet commented 1 month ago

@davidetan Is the jglfw commit ready to merge?

This is because the Last Tool action is not triggered when the Selection Tool is active (for reference: EditMode#toggleTool -> if (tool == selectTool) return false;).

Perfect time for SmartGit's Investigate. That line was added for 3.8.48-beta:

Fixed hold ctrl, right click, let go of ctrl -> right click no longer toggles last tool.

Other changes were made in the same commit and that line alone doesn't actually affect the problem mentioned in the commit message. Instead the line prevents right clicks while holding ctrl (for selection) from toggling the tool. That is OK and we can move the check out.

After doing that, there are some other issues where the main and alternate tools get set to the same tool. I've fixed those in 4.2.36.

davidetan commented 1 month ago

@davidetan Is the https://github.com/badlogic/jglfw/pull/7 ready to merge?

Yes, I'd say it's ready to be merged. No one complained about anything related to that fix 👍

NathanSweet commented 1 month ago

4.2.36