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

fix(web): megabranch - gesture-engine rapid typing issues #10822

Conversation

jahorton
Copy link
Contributor

@jahorton jahorton commented Feb 23, 2024

Currently includes copious amounts of console logging that helped me narrow down the various issues at play here. I have plans to split this into multiple pieces. In planned order, here's my "rough notes" on what to split & how to do so:

  1. gestureMatcher.ts, addContactInternal
  • there was no check for non-initial-state instant-rejections - that's why longpress-roam had a looping reset!
  • this in itself might actually be a very significant thing to publish as its own PR... and it may have
    noticeable performance impacts during rapid typing.
  • Happened to notice this a few times during development/investigation of the rapid-typing issues... and it became reproducible with ease after a certain level of logging, interestingly.
    // Now that we've done the initial-state check, we can check for instantly-matching and
    // instantly-rejecting path models.
    let result = contactModel.update();
    if(result?.type == 'reject') {
      this.finalize(false, 'cancelled');
    }
  1. matcherSelector: pendingMatchSetup
  • have seen double-stacking on the promise; we need to swap it out & replace to ensure things queue up.
  • also, the early lock should probably only trigger if sourceNotYetStaged; no need to delay multitaps, since they just continue the old gesture, rather than starting afresh.
  1. Oh wow, THIS one was tricky.
  • selection matchGesture() startup for NEW paths:
    • that double timedPromise() wait - so, what happens if any model matches before the second one returns?
    • the "standalone" models won't get a chance to match! (This is the number 1 reason keys were getting outright-skipped!)
    • there are a lot of moving Promises at times across the whole gesture-engine system; we need to ensure that matchGesture() setup acts atomically when setting up both "matching old" and "considering new" behaviors.
  1. Sequentialization Queue
  • Ensures that the promises stay in a logical start-to-end order despite all the promise shenanigans in the selector and matcher phases
  • also remaps gesture timestamps to prevent automatic longpresses that could occur due to long input lag on older devices during heavy rapid-typing bursts. They now "start" whenever they can safely be pulled from the input queue, but time differences within the gesture itself are otherwise preserved.
  • is likely the most complex change, so we should definitely isolate it from the other tidbits mentioned herein.

In my tests since getting "all the ducks in a row", I get the same number of output chars as the number of keys I pressed. Can't 100% say they're the exact intended keys when typing in very rapid bursts, but the keypresses are countable, at least, and that's holding up - even if it takes a certain older test device the better part of a second to catch up (due to excessive console logging, most likely).

@keymanapp-test-bot keymanapp-test-bot bot added the user-test-missing User tests have not yet been defined for the PR label Feb 23, 2024
@jahorton jahorton closed this Feb 26, 2024
@jahorton jahorton deleted the fix/web/gesture-engine-rapid-typing-megabranch branch January 20, 2025 07:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant