Make press hold and enabled togglable

[Folcon]

Hey @tonsky

I've been using HumbleUI to build a game and I just finished wiring up camera panning and realised that kPressAndHoldEnabled is on by default

JWM/macos/cc/JWMMainView.mm

Line 197 in dc1b898

kPressAndHoldEnabled = nullptr == [userDefaults objectForKey:@"ApplePressAndHoldEnabled"] ? YES : [userDefaults boolForKey:@"ApplePressAndHoldEnabled"];

This is useful so I'd like to propose adding a toggle so when keyboard input is required I can turn it on and when it's not like when I'm just wanting more raw keyboard input, I can turn it off

Feel free to correct me, I'm not familiar with calling native code from java and I've tried to fit your calling conventions

From what I can see this would involve modifying WindowMac.java and adding a method like:

public static void setPressAndHoldEnabled(boolean enabled) {
    assert _onUIThread() : "Should be run on UI thread";
    _nSetPressAndHoldEnabled(enabled);
}

As well as modifying WindowMac.mm to add:

extern "C" JNIEXPORT void JNICALL Java_io_github_humbleui_jwm_WindowMac__1nSetPressAndHoldEnabled
  (JNIEnv *env, jclass cls, jboolean enabled) {
    jwm::kPressAndHoldEnabled = enabled;
}

Let me know if you'd prefer me to do it some other way of if you don't think this is a good way to do it =)

Otherwise happy to whip up a PR

From what I can tell I'll also need to do some work with HumbleUI to update it to get it in line with the work you've been doing to update JMW so I can use this, I don't mind putting some time in for that as well

Really want to get back into doing some gamedev on HumbleUI again, your sand example was a good reminder that there's some fun stuff that can be done here!

[tonsky]

Yes, all sounds good, please make a PR. It’s quite handy that we already have the flag :)

[Folcon]

Hi @tonsky

I've done a bit more digging, because surprisingly it didn't 100% fix the problem

It looks like it doesn't respect the setting, so JWM is sending to macos to process the events

JWM/macos/cc/JWMMainView.mm

Lines 517 to 520 in dc1b898

	// Allow TSM to look at the event and potentially send back NSTextInputClient messages.
	[self interpretKeyEvents:[NSArray arrayWithObject:event]];

So I tried guarding against that

// Allow TSM to look at the event and potentially send back NSTextInputClient messages.
if (jwm::kPressAndHoldEnabled) {
    [self interpretKeyEvents:[NSArray arrayWithObject:event]];
}

And this works from an input processing perspective, but it doesn't prevent this from happening

Digging into it[0][1], from what I can tell you're supposed to do this:

[[NSUserDefaults standardUserDefaults] setBool:(BOOL)enabled forKey:@"ApplePressAndHoldEnabled"];

I tried that by doing this in WindowMac.mm:

extern "C" JNIEXPORT void JNICALL Java_io_github_humbleui_jwm_WindowMac__1nSetPressAndHoldEnabled
  (JNIEnv *env, jclass cls, jboolean enabled) {
    jwm::kPressAndHoldEnabled = enabled;
    [[NSUserDefaults standardUserDefaults] setBool:(BOOL)enabled forKey:@"ApplePressAndHoldEnabled"];
}

However it only works when I added this at the bottom of initKeyTable() in JWMMainView.mm:

kPressAndHoldEnabled = nullptr == [userDefaults objectForKey:@"ApplePressAndHoldEnabled"] ? YES : [userDefaults boolForKey:@"ApplePressAndHoldEnabled"];

kPressAndHoldEnabled = NO;

So I suggest we swap it to this and use the toggle to turn it on for fields:

kPressAndHoldEnabled = nullptr == [userDefaults objectForKey:@"ApplePressAndHoldEnabled"] ? NO : [userDefaults boolForKey:@"ApplePressAndHoldEnabled"];

I tried that and it both doesn't show the popup any more when I key repeat in my game and if you start typing in the Text Field example it still works, which is my preferred behaviour, all the benefit of richer text input while still having functioning key repeat =)

As you can see, key entry and the popup

Would you like to swap to this? IE default kPressAndHoldEnabled is off, however it's turned on for specific panels etc?

Is there an alternative way you'd prefer this to work?

-[0]: https://macos-defaults.com/keyboard/applepressandholdenabled.html
-[1]: https://gist.github.com/DaniruKun/ad47501af0996b80cb8752687f8146ad

[Folcon]

A little bit more experimentation shows this is unnecessary

extern "C" JNIEXPORT void JNICALL Java_io_github_humbleui_jwm_WindowMac__1nSetPressAndHoldEnabled
  (JNIEnv *env, jclass cls, jboolean enabled) {
    jwm::kPressAndHoldEnabled = enabled;
    [[NSUserDefaults standardUserDefaults] setBool:(BOOL)enabled forKey:@"ApplePressAndHoldEnabled"];
}

So I'm reverting that:

extern "C" JNIEXPORT void JNICALL Java_io_github_humbleui_jwm_WindowMac__1nSetPressAndHoldEnabled
  (JNIEnv *env, jclass cls, jboolean enabled) {
    jwm::kPressAndHoldEnabled = enabled;
}

Everything else works, there's just some bookkeeping in terms of turning it on via the toggle afterwards

If you know of another way of having the setting respected that would be great =)

I'd much rather prefer turning on keyrepeat when it's needed, I've just not figured out how to do that yet

[Folcon]

Ok, caching was making this fiddly to test, so I've also created a UI demo in HumbleUI to check this works (HumbleUI/HumbleUI#95)

I kept debugging it because I was unsatisfied at needing to change the default and I think I've fixed it

I've reduced the changeset down to:

The original change to WindowMac.mm

extern "C" JNIEXPORT void JNICALL Java_io_github_humbleui_jwm_WindowMac__1nSetPressAndHoldEnabled
  (JNIEnv *env, jclass cls, jboolean enabled) {
    jwm::kPressAndHoldEnabled = enabled;
}

and WindowMac.java

public static void setPressAndHoldEnabled(boolean enabled) {
    assert _onUIThread() : "Should be run on UI thread";
    _nSetPressAndHoldEnabled(enabled);
}

...

@ApiStatus.Internal public static native void _nSetPressAndHoldEnabled(boolean enabled);

And this change to JWMMainView.mm

// Allow TSM to look at the event and potentially send back NSTextInputClient messages.
if (jwm::kPressAndHoldEnabled) {
  [self interpretKeyEvents:[NSArray arrayWithObject:event]];
} else {
  // Bypass text input system to avoid press and hold popup,
  //   but still deliver typed characters
  NSString *characters = [event characters];
  if ([characters length] > 0) {
      [self insertText:characters replacementRange:NSMakeRange(NSNotFound,
0)];
  }
}

The existing default behaviour doesn't change, accent character works and key repeat works, the only gap is that the signal is not initialised in the demo based on the current state of kPressAndHoldEnabled as there isn't a get method for it, I wasn't sure if it was worthwhile adding one

I'll pause here, let me know what you think =)

[tonsky]

Hey, I took a look, looks great. I think this is the move, basically we say “enable special mode when inside text field”, which sounds fine. But here’s what I am thinking:

• You want no pressAndHold when outside but you also want default behavior when inside. And default can be either with pressAndHold or without. So it’s not just enable/disable, we also need a way to determine system value.
• I added isPressAndHoldEnabled to match setPressAndHoldEnabled
• I think we need something like isPressAndHoldEnabledGlobally which will just check the value of OS setting. Then we can use this + set to match system behavior in text fields, not just “enable always”
• In your implementation, backspace stops working. It now produces unreadable character that it deletes instead of deleting a last character before it. You can check it in demo that I added (toggle PressAndHold with Cmd+A). We definitely need this resolved
• It looks like I borrowed implementation from JetBrains, can you look what they are doing and do the same maybe? https://github.com/JetBrains/JetBrainsRuntime/blob/725d9778dd299206127c8538c3174f18b9111fe0/src/java.desktop/macosx/native/libawt_lwawt/awt/AWTView.m#L373

Very excited for this!

[tonsky]

Here’s the patch with isPressAndHoldEnabled and dashboard example supporting Cmd+A. Feel free to incorporate it into your branch (I think it’s the easiest this way, or I can make a branch for you in HumbleUI/JWM if you prefer)

example.patch

[Folcon]

Thanks @tonsky, I've got some time this weekend so I'll try and get it in =)

Question for you, I'm assuming there is one, but any reason why JWM uses LayerMetalSkija here instead of OpenGL other than it's deprecated?

JWM/examples/dashboard/java/PanelRendering.java

Lines 31 to 32 in dc1b898

	if (Platform.CURRENT == Platform.MACOS)
	layers = new String[] { "LayerMetalSkija", "LayerGLSkija" };

I'm asking as I'm trying to do 3d render and from what I can see using LWJGL I've got to hack running an opengl context headlessly and then copy the pixels over, which I've got working, however it would be better if they could be on the same thread

Not sure how big a step trying to move to metal would be, but it seems like it might be big and that's why I'm putting up with this hack for the moment

[tonsky]

Maybe I don’t understand what you are asking. You can use both layers, they should work more interchangeably, and Skija code should stay the same. JWM dashboard app (which is just a sample app) uses Metal by default but you can switch to OpenGL too (Cmd+L IIRC). It demonstrates that you can use either