From e686dbe67d20814880af8e55549218ff08871921 Mon Sep 17 00:00:00 2001 From: "Joshua A. Horton" Date: Fri, 26 Apr 2024 10:15:32 +0700 Subject: [PATCH 1/4] fix(web): longpress shortcut activation should be based on purely-northward component change(web): stricter threshold for up-flick shortcut --- .../engine/osk/src/input/gestures/specsForLayout.ts | 5 ++++- web/src/engine/osk/src/visualKeyboard.ts | 12 ++++++++++-- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/web/src/engine/osk/src/input/gestures/specsForLayout.ts b/web/src/engine/osk/src/input/gestures/specsForLayout.ts index 4fb8806e267..f9bc02f4614 100644 --- a/web/src/engine/osk/src/input/gestures/specsForLayout.ts +++ b/web/src/engine/osk/src/input/gestures/specsForLayout.ts @@ -464,7 +464,10 @@ export function longpressContactModel(params: GestureParams, enabledFlicks: bool * each side of due N in total. */ if((enabledFlicks && spec.permitsFlick(stats.lastSample.item)) && (stats.cardinalDirection?.indexOf('n') != -1 ?? false)) { - if(stats.netDistance > spec.flickDist) { + const baseDistance = stats.netDistance; + const angle = stats.angle; // from <0, -1> (straight up) going clockwise. + const verticalDistance = baseDistance * Math.cos(angle); + if(verticalDistance > spec.flickDist) { return 'resolve'; } } else if(resetForRoaming) { diff --git a/web/src/engine/osk/src/visualKeyboard.ts b/web/src/engine/osk/src/visualKeyboard.ts index 6b8af2e0c17..2edfc9fb374 100644 --- a/web/src/engine/osk/src/visualKeyboard.ts +++ b/web/src/engine/osk/src/visualKeyboard.ts @@ -1298,9 +1298,17 @@ export default class VisualKeyboard extends EventEmitter implements Ke Note: longpress.flickDist needs to be no greater than flick.startDist. Otherwise, the longpress up-flick shortcut will not work on keys that support flick gestures. (Such as sil_euro_latin 3.0+) + + Since it's also based on the purely northward component, it's best to + have it be slightly lower. 80% of flick.startDist gives a range of + about 37 degrees to each side before a flick-start would win, while + 70.7% gives 45 degrees. + + (The range _will_ be notably tighter on keys with both longpresses and + flicks as a result.) */ - this.gestureParams.longpress.flickDist = 0.25 * this.currentLayer.rowHeight; - this.gestureParams.flick.startDist = 0.25 * this.currentLayer.rowHeight; + this.gestureParams.longpress.flickDist = 0.24 * this.currentLayer.rowHeight; + this.gestureParams.flick.startDist = 0.30 * this.currentLayer.rowHeight; this.gestureParams.flick.dirLockDist = 0.35 * this.currentLayer.rowHeight; this.gestureParams.flick.triggerDist = 0.75 * this.currentLayer.rowHeight; } From 8d2a1e8b93b5e2dc3e8382800ff01bad7a709544 Mon Sep 17 00:00:00 2001 From: "Joshua A. Horton" Date: Mon, 29 Apr 2024 08:53:32 +0700 Subject: [PATCH 2/4] change(web): longpress up-flick now has two components, requires greater distance --- .../osk/src/input/gestures/specsForLayout.ts | 40 ++++++++++++++++--- web/src/engine/osk/src/visualKeyboard.ts | 9 +++-- 2 files changed, 40 insertions(+), 9 deletions(-) diff --git a/web/src/engine/osk/src/input/gestures/specsForLayout.ts b/web/src/engine/osk/src/input/gestures/specsForLayout.ts index f9bc02f4614..213278277e4 100644 --- a/web/src/engine/osk/src/input/gestures/specsForLayout.ts +++ b/web/src/engine/osk/src/input/gestures/specsForLayout.ts @@ -31,10 +31,16 @@ export interface GestureParams { */ permitsFlick: (item?: Item) => boolean, + /** + * The minimum _net_ distance traveled before a longpress flick-shortcut will cancel any + * conflicting flick models. + */ + flickDistStart: number, + /** * The minimum _net_ distance traveled before a longpress flick-shortcut will trigger. */ - flickDist: number, + flickDistFinal: number, /** * The maximum amount of raw-distance movement allowed for a longpress before it is @@ -104,7 +110,8 @@ export const DEFAULT_GESTURE_PARAMS: GestureParams = { permitsFlick: () => true, // Note: actual runtime value is determined at runtime based upon row height. // See `VisualKeyboard.refreshLayout`, CTRL-F "Step 3". - flickDist: 5, + flickDistStart: 8, + flickDistFinal: 40, waitLength: 500, noiseTolerance: 10 }, @@ -349,11 +356,34 @@ export function instantContactResolutionModel(): ContactModel { }; } -export function flickStartContactModel(params: GestureParams): ContactModel { +export function flickStartContactModel(params: GestureParams): gestures.specs.ContactModel { + const flickParams = params.flick; + return { itemPriority: 1, pathModel: { - evaluate: (path) => path.stats.netDistance > params.flick.startDist ? 'resolve' : null + evaluate: (path, _, item) => { + const stats = path.stats; + const keySpec = item?.key.spec; + + if(keySpec && keySpec.sk) { + const flickSpec = keySpec.flick; + const hasUpFlick = flickSpec.nw || flickSpec.n || flickSpec.ne; + + if(!hasUpFlick) { + // Check for possible conflict with the longpress up-flick shortcut; + // it's supported on this key, as there is no true northish flick. + const baseDistance = stats.netDistance; + const angle = stats.angle; // from <0, -1> (straight up) going clockwise. + const verticalDistance = baseDistance * Math.cos(angle); + if(verticalDistance > params.longpress.flickDistStart) { + return 'reject'; + } + } + } + + return stats.netDistance > flickParams.startDist ? 'resolve' : null; + } }, pathResolutionAction: 'resolve', pathInheritance: 'partial' @@ -467,7 +497,7 @@ export function longpressContactModel(params: GestureParams, enabledFlicks: bool const baseDistance = stats.netDistance; const angle = stats.angle; // from <0, -1> (straight up) going clockwise. const verticalDistance = baseDistance * Math.cos(angle); - if(verticalDistance > spec.flickDist) { + if(verticalDistance > spec.flickDistFinal) { return 'resolve'; } } else if(resetForRoaming) { diff --git a/web/src/engine/osk/src/visualKeyboard.ts b/web/src/engine/osk/src/visualKeyboard.ts index 2edfc9fb374..97afe3352d8 100644 --- a/web/src/engine/osk/src/visualKeyboard.ts +++ b/web/src/engine/osk/src/visualKeyboard.ts @@ -1307,10 +1307,11 @@ export default class VisualKeyboard extends EventEmitter implements Ke (The range _will_ be notably tighter on keys with both longpresses and flicks as a result.) */ - this.gestureParams.longpress.flickDist = 0.24 * this.currentLayer.rowHeight; - this.gestureParams.flick.startDist = 0.30 * this.currentLayer.rowHeight; - this.gestureParams.flick.dirLockDist = 0.35 * this.currentLayer.rowHeight; - this.gestureParams.flick.triggerDist = 0.75 * this.currentLayer.rowHeight; + this.gestureParams.longpress.flickDistStart = 0.24 * this.currentLayer.rowHeight; + this.gestureParams.flick.startDist = 0.30 * this.currentLayer.rowHeight; + this.gestureParams.flick.dirLockDist = 0.35 * this.currentLayer.rowHeight; + this.gestureParams.flick.triggerDist = 0.75 * this.currentLayer.rowHeight; + this.gestureParams.longpress.flickDistFinal = 0.75 * this.currentLayer.rowHeight; } // Phase 4: Refresh the layout of the layer-group and active layer. From 8ddd2eb04a17340ed950159f5fb0a2865bb1db42 Mon Sep 17 00:00:00 2001 From: "Joshua A. Horton" Date: Tue, 30 Apr 2024 09:04:06 +0700 Subject: [PATCH 3/4] fix(web): failed gesture-model spinup should not lock the gesture engine --- .../gestures/matchers/matcherSelector.ts | 26 ++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/common/web/gesture-recognizer/src/engine/headless/gestures/matchers/matcherSelector.ts b/common/web/gesture-recognizer/src/engine/headless/gestures/matchers/matcherSelector.ts index c8a2cefa1a1..8e7efc73886 100644 --- a/common/web/gesture-recognizer/src/engine/headless/gestures/matchers/matcherSelector.ts +++ b/common/web/gesture-recognizer/src/engine/headless/gestures/matchers/matcherSelector.ts @@ -409,10 +409,30 @@ export class MatcherSelector extends EventEmitter new GestureMatcher(model, unmatchedSource || priorMatcher)); + let newMatchers = gestureModelSet.map((model) => { + try { + /* + Spinning up a new gesture model means running code for that model and + path, which are defined outside of the engine. We should not allow + errors from engine-external code to prevent us from continuing with + unaffected models. + + It's also important to keep the overall flow going; this code is run + during touch-start spinup. An abrupt stop due to an unhandled error + here can lock up the AsyncDispatchQueue for touch events, locking up + the engine! + */ + return new GestureMatcher(model, unmatchedSource || priorMatcher) + } catch (err) { + console.error(err); + return null; + } + // Filter out any models that failed to 'spin-up' due to exceptions. + }).filter((entry) => !!entry); // If any newly-activating models are disqualified due to initial conditions, don't add them. newMatchers = newMatchers.filter((matcher) => !matcher.result || matcher.result.matched !== false); From 3ca69066d08f65adfa3b9ab2746ec8692ccc2395 Mon Sep 17 00:00:00 2001 From: "Joshua A. Horton" Date: Tue, 30 Apr 2024 09:03:33 +0700 Subject: [PATCH 4/4] fix(web): invalid initial-state aborts further model processing --- .../engine/headless/gestures/matchers/gestureMatcher.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/common/web/gesture-recognizer/src/engine/headless/gestures/matchers/gestureMatcher.ts b/common/web/gesture-recognizer/src/engine/headless/gestures/matchers/gestureMatcher.ts index 9f909631ac8..a311548f0ec 100644 --- a/common/web/gesture-recognizer/src/engine/headless/gestures/matchers/gestureMatcher.ts +++ b/common/web/gesture-recognizer/src/engine/headless/gestures/matchers/gestureMatcher.ts @@ -477,6 +477,13 @@ export class GestureMatcher implements PredecessorMatch< here, as the decision is made due to a validation check against the initial item. */ this.finalize(false, 'cancelled'); + + /* + * There's no need to process the gesture-model any further... and the + * invalid state may correspond to assumptions in the path-model that + * will be invalidated if we continue. + */ + return; } } @@ -507,6 +514,7 @@ export class GestureMatcher implements PredecessorMatch< instantly fail and thus cancel. */ this.finalize(false, whileInitializing ? 'cancelled' : 'path'); + return; } // Standard path: trigger either resolution or rejection when the contact model signals either.