From 7adf27dd845f155785ddcbbfaceb643136fcf38f Mon Sep 17 00:00:00 2001 From: "Joshua A. Horton" <joshua_horton@sil.org> Date: Tue, 17 Oct 2023 13:42:54 +0700 Subject: [PATCH 1/5] feat(web): subkey-menu fat-finger correction --- common/web/input-processor/src/corrections.ts | 15 ++- .../src/text/inputProcessor.ts | 5 +- .../src/keyboards/keyboard.ts | 4 +- .../keyboard-processor/src/text/keyEvent.ts | 7 +- .../src/input/gestures/browser/subkeyPopup.ts | 89 ++++++++++++- .../osk/src/input/gestures/gestureHandler.ts | 4 + .../osk/src/input/gestures/heldRepeater.ts | 5 + web/src/engine/osk/src/visualKeyboard.ts | 125 ++++++------------ 8 files changed, 152 insertions(+), 102 deletions(-) diff --git a/common/web/input-processor/src/corrections.ts b/common/web/input-processor/src/corrections.ts index 0fb2af4a921..bf46a3b6b67 100644 --- a/common/web/input-processor/src/corrections.ts +++ b/common/web/input-processor/src/corrections.ts @@ -1,3 +1,4 @@ +import { ActiveKeyBase, KeyDistribution } from "@keymanapp/keyboard-processor"; import { CorrectionLayout } from "./correctionLayout.js"; /** @@ -9,8 +10,8 @@ import { CorrectionLayout } from "./correctionLayout.js"; * by a correction algorithm, also within <0, 0> to <1, 1>. * @returns A mapping of key IDs to the 'squared pseudo-distance' of the touchpoint to each key. */ -export function keyTouchDistances(touchCoords: {x: number, y: number}, correctiveLayout: CorrectionLayout): Map<string, number> { - let keyDists: Map<string, number> = new Map<string, number>(); +export function keyTouchDistances(touchCoords: {x: number, y: number}, correctiveLayout: CorrectionLayout): Map<ActiveKeyBase, number> { + let keyDists: Map<ActiveKeyBase, number> = new Map<ActiveKeyBase, number>(); // This double-nested loop computes a pseudo-distance for the touch from each key. Quite useful for // generating a probability distribution. @@ -54,7 +55,7 @@ export function keyTouchDistances(touchCoords: {x: number, y: number}, correctiv distY += dy * entry.height; const distance = distX * distX + distY * distY; - keyDists.set(entry.keySpec.coreID, distance); + keyDists.set(entry.keySpec, distance); }); return keyDists; @@ -65,8 +66,8 @@ export function keyTouchDistances(touchCoords: {x: number, y: number}, correctiv * consideration. * @returns */ -export function distributionFromDistanceMap(squaredDistMap: Map<string, number>): {keyId: string, p: number}[] { - const keyProbs = new Map<string, number>(); +export function distributionFromDistanceMap(squaredDistMap: Map<ActiveKeyBase, number>): KeyDistribution { + const keyProbs = new Map<ActiveKeyBase, number>(); let totalMass = 0; // Should we wish to allow multiple different transforms for distance -> probability, use a function parameter in place @@ -81,10 +82,10 @@ export function distributionFromDistanceMap(squaredDistMap: Map<string, number>) keyProbs.set(key, keyProbs.get(key) ?? 0 + entry); } - const list: {keyId: string, p: number}[] = []; + const list: {keySpec: ActiveKeyBase, p: number}[] = []; for(let key of keyProbs.keys()) { - list.push({keyId: key, p: keyProbs.get(key) / totalMass}); + list.push({keySpec: key, p: keyProbs.get(key) / totalMass}); } return list.sort(function(a, b) { diff --git a/common/web/input-processor/src/text/inputProcessor.ts b/common/web/input-processor/src/text/inputProcessor.ts index 51516882a4e..80821bc76d3 100644 --- a/common/web/input-processor/src/text/inputProcessor.ts +++ b/common/web/input-processor/src/text/inputProcessor.ts @@ -295,7 +295,6 @@ export default class InputProcessor { // Sort the distribution into probability-descending order. keyDistribution.sort((a, b) => b.p - a.p); - let activeLayout = this.activeKeyboard.layout(keyEvent.device.formFactor); alternates = []; let totalMass = 0; // Tracks sum of non-error probabilities. @@ -314,9 +313,9 @@ export default class InputProcessor { let mock = Mock.from(windowedMock, false); - let altKey = activeLayout.getLayer(keyEvent.kbdLayer).getKey(pair.keyId); + const altKey = pair.keySpec; if(!altKey) { - console.warn("Potential fat-finger key could not be found in layer!"); + console.warn("Internal error: failed to properly filter set of keys for corrections"); continue; } diff --git a/common/web/keyboard-processor/src/keyboards/keyboard.ts b/common/web/keyboard-processor/src/keyboards/keyboard.ts index f70f30fcb90..9738792cda1 100644 --- a/common/web/keyboard-processor/src/keyboards/keyboard.ts +++ b/common/web/keyboard-processor/src/keyboards/keyboard.ts @@ -1,6 +1,6 @@ import Codes from "../text/codes.js"; import { Layouts, type LayoutFormFactor } from "./defaultLayouts.js"; -import { ActiveKey, ActiveLayout } from "./activeLayout.js"; +import { ActiveKey, ActiveKeyBase, ActiveLayout } from "./activeLayout.js"; import KeyEvent from "../text/keyEvent.js"; import type OutputTarget from "../text/outputTarget.js"; @@ -468,7 +468,7 @@ export default class Keyboard { return keyEvent; } - constructKeyEvent(key: ActiveKey, device: DeviceSpec, stateKeys: StateKeyMap): KeyEvent { + constructKeyEvent(key: ActiveKeyBase, device: DeviceSpec, stateKeys: StateKeyMap): KeyEvent { // Make a deep copy of our preconstructed key event, filling it out from there. const Lkc = key.baseKeyEvent; Lkc.device = device; diff --git a/common/web/keyboard-processor/src/text/keyEvent.ts b/common/web/keyboard-processor/src/text/keyEvent.ts index 719de488b11..502fec8dfbf 100644 --- a/common/web/keyboard-processor/src/text/keyEvent.ts +++ b/common/web/keyboard-processor/src/text/keyEvent.ts @@ -11,10 +11,11 @@ import { type DeviceSpec } from "@keymanapp/web-utils"; import Codes from './codes.js'; import DefaultRules from './defaultRules.js'; +import { ActiveKeyBase } from "../index.js"; // Represents a probability distribution over a keyboard's keys. // Defined here to avoid compilation issues. -export type KeyDistribution = {keyId: string, p: number}[]; +export type KeyDistribution = {keySpec: ActiveKeyBase, p: number}[]; /** * A simple instance of the standard 'default rules' for keystroke processing from the @@ -45,8 +46,8 @@ export interface KeyEventSpec { */ srcKeyboard?: Keyboard; - // Holds relevant event properties leading to construction of this KeyEvent. - source?: any; // Technically, KeyEvent|MouseEvent|Touch - but those are DOM types that must be kept out of headless mode. + // // Holds relevant event properties leading to construction of this KeyEvent. + // source?: any; // Technically, KeyEvent|MouseEvent|Touch - but those are DOM types that must be kept out of headless mode. // Holds a generated fat-finger distribution (when appropriate) keyDistribution?: KeyDistribution; diff --git a/web/src/engine/osk/src/input/gestures/browser/subkeyPopup.ts b/web/src/engine/osk/src/input/gestures/browser/subkeyPopup.ts index 784ebebfd9a..3b742bd3103 100644 --- a/web/src/engine/osk/src/input/gestures/browser/subkeyPopup.ts +++ b/web/src/engine/osk/src/input/gestures/browser/subkeyPopup.ts @@ -3,9 +3,11 @@ import { type KeyElement } from '../../../keyElement.js'; import OSKBaseKey from '../../../keyboard-layout/oskBaseKey.js'; import VisualKeyboard from '../../../visualKeyboard.js'; -import { DeviceSpec, KeyEvent, ActiveSubKey } from '@keymanapp/keyboard-processor'; +import { DeviceSpec, KeyEvent, ActiveSubKey, KeyDistribution } from '@keymanapp/keyboard-processor'; import { ConfigChangeClosure, GestureRecognizerConfiguration, GestureSequence, PaddedZoneSource } from '@keymanapp/gesture-recognizer'; import { GestureHandler } from '../gestureHandler.js'; +import { CorrectionLayout, CorrectionLayoutEntry, distributionFromDistanceMap, keyTouchDistances } from '@keymanapp/input-processor'; +import { GestureParams } from '../specsForLayout.js'; /** * Represents a 'realized' longpress gesture's default implementation @@ -31,15 +33,18 @@ export default class SubkeyPopup implements GestureHandler { public readonly subkeys: KeyElement[]; private source: GestureSequence<KeyElement>; + private readonly gestureParams: GestureParams; constructor( source: GestureSequence<KeyElement>, configChanger: ConfigChangeClosure<KeyElement>, vkbd: VisualKeyboard, - e: KeyElement + e: KeyElement, + gestureParams: GestureParams ) { this.baseKey = e; this.source = source; + this.gestureParams = gestureParams; source.on('complete', () => { this.currentSelection?.key.highlight(false); @@ -337,6 +342,86 @@ export default class SubkeyPopup implements GestureHandler { return this.element.style.visibility == 'visible'; } + buildCorrectiveLayout(): CorrectionLayout { + const baseBounding = this.element.getBoundingClientRect(); + const aspectRatio = baseBounding.width / baseBounding.height; + + const keys = this.subkeys.map((keyElement) => { + const subkeyBounds = keyElement.getBoundingClientRect(); + + // Ensures we have the right typing. + const correctiveData: CorrectionLayoutEntry = { + keySpec: keyElement.key.spec, + centerX: ((subkeyBounds.right - subkeyBounds.width / 2) - baseBounding.left) / baseBounding.width, + centerY: ((subkeyBounds.bottom - subkeyBounds.height / 2) - baseBounding.top) / baseBounding.height, + width: subkeyBounds.width / baseBounding.width, + height: subkeyBounds.height / baseBounding.height + } + + return correctiveData; + }); + + return { + keys: keys, + kbdScaleRatio: aspectRatio + } + } + + currentStageKeyDistances(): KeyDistribution { + const latestStage = this.source.stageReports[this.source.stageReports.length-1]; + const baseStage = this.source.stageReports[0]; + const gestureSource = latestStage.sources[0]; + const lastCoord = gestureSource.currentSample; + + const baseBounding = this.element.getBoundingClientRect(); + const mappedCoord = { + x: lastCoord.targetX / baseBounding.width, + y: lastCoord.targetY / baseBounding.height + } + + // Lock the coordinate within base-element bounds; corrects for the allowed 'popup roaming' zone. + // + // To consider: add a 'clipping' feature to `keyTouchDistances`? It'd make sense for base keys, + // too. + mappedCoord.x = mappedCoord.x < 0 ? 0 : (mappedCoord.x > 1 ? 1: mappedCoord.x); + mappedCoord.y = mappedCoord.y < 0 ? 0 : (mappedCoord.y > 1 ? 1: mappedCoord.y); + + const rawSqDistances = keyTouchDistances(mappedCoord, this.buildCorrectiveLayout()); + const currentKeyDist = rawSqDistances.get(lastCoord.item.key.spec); + + // Include the base key as a corrective option. + if(!this.subkeys.find((entry) => entry.keyId == this.baseKey.keyId)) { + /* + * - how long has the subkey menu been visible? + * - Base key should be less likely if it's been visible a while, + * but reasonably likely if it only just appeared. + * - Especially if up-flicks are allowed. Though, in that case, consider + * base-layer neighbors, and particularly the one directly under the touchpoint? + * - raw distance traveled (since the menu appeared) + * - similarly, short distance = a more likely base key? + */ + + // The concept: how likely is it that the user MEANT to output a subkey? + let timeDistance = Math.min( + // The full path is included by the model - meaning the base wait is included here in + // in the stats; we subtract it to get just the duration of the subkey menu. + gestureSource.path.stats.duration - baseStage.sources[0].path.stats.duration, + this.gestureParams.longpress.waitLength + ) / (2 * this.gestureParams.longpress.waitLength); // normalize: max time distance of 0.5 + + let pathDistance = Math.min( + gestureSource.path.stats.rawDistance, + this.gestureParams.longpress.noiseTolerance*4 + ) / (this.gestureParams.longpress.noiseTolerance * 8); // normalize similarly. + + // We only want to add a single distance 'dimension' - we'll choose the one that affects + // the interpreted distance the least. (This matters for upflick-shortcutting in particular) + const layerDistance = Math.min(timeDistance * timeDistance, pathDistance * pathDistance); + rawSqDistances.set(this.baseKey.key.spec, currentKeyDist + layerDistance); + } + return distributionFromDistanceMap(rawSqDistances); + } + cancel() { this.clear(); this.source.cancel(); diff --git a/web/src/engine/osk/src/input/gestures/gestureHandler.ts b/web/src/engine/osk/src/input/gestures/gestureHandler.ts index 6128e50b91e..991b43742c1 100644 --- a/web/src/engine/osk/src/input/gestures/gestureHandler.ts +++ b/web/src/engine/osk/src/input/gestures/gestureHandler.ts @@ -1,3 +1,5 @@ +import { KeyDistribution } from "@keymanapp/keyboard-processor"; + export interface GestureHandler { /** * Triggers cancellation of any further processing for the gesture being handled. @@ -9,4 +11,6 @@ export interface GestureHandler { * scenario in which key previews (on phones) should be disabled. */ readonly hasModalVisualization: boolean; + + currentStageKeyDistances(): KeyDistribution; } \ No newline at end of file diff --git a/web/src/engine/osk/src/input/gestures/heldRepeater.ts b/web/src/engine/osk/src/input/gestures/heldRepeater.ts index 3b7609c9cef..ef7f3a8c5fc 100644 --- a/web/src/engine/osk/src/input/gestures/heldRepeater.ts +++ b/web/src/engine/osk/src/input/gestures/heldRepeater.ts @@ -1,4 +1,5 @@ import { GestureSequence } from "@keymanapp/gesture-recognizer"; +import { KeyDistribution } from "@keymanapp/keyboard-processor"; import { KeyElement } from "../../keyElement.js"; import { GestureHandler } from './gestureHandler.js'; @@ -35,4 +36,8 @@ export class HeldRepeater implements GestureHandler { this.timerHandle = window.setTimeout(this.deleteRepeater, HeldRepeater.REPEAT_DELAY); } + + currentStageKeyDistances(): KeyDistribution { + return null; + } } \ No newline at end of file diff --git a/web/src/engine/osk/src/visualKeyboard.ts b/web/src/engine/osk/src/visualKeyboard.ts index 6c002f3bcd9..ba8dca256cd 100644 --- a/web/src/engine/osk/src/visualKeyboard.ts +++ b/web/src/engine/osk/src/visualKeyboard.ts @@ -20,6 +20,7 @@ import { buildCorrectiveLayout, distributionFromDistanceMap, keyTouchDistances } import { GestureRecognizer, GestureRecognizerConfiguration, + GestureSequence, GestureSource, InputSample, PaddedZoneSource @@ -385,6 +386,8 @@ export default class VisualKeyboard extends EventEmitter<EventMap> implements Ke key: KeyElement }> = {}; + const gestureHandlerMap = new Map<GestureSequence<KeyElement>, GestureHandler>(); + // Now to set up event-handling links. // This handler should probably vary based on the keyboard: do we allow roaming touches or not? recognizer.on('inputstart', (source) => { @@ -450,6 +453,8 @@ export default class VisualKeyboard extends EventEmitter<EventMap> implements Ke // This should probably vary based on the type of gesture. gestureSequence.on('stage', (gestureStage, configChanger) => { + let handler: GestureHandler = gestureHandlerMap.get(gestureSequence); + // Disable roaming-touch highlighting (and current highlighting) for all // touchpoints included in a gesture, even newly-included ones as they occur. for(let id of gestureStage.allSourceIds) { @@ -478,17 +483,26 @@ export default class VisualKeyboard extends EventEmitter<EventMap> implements Ke } if(gestureKey) { + let correctionKeyDistribution: KeyDistribution; if(gestureStage.matchedId == 'multitap') { // TODO: examine sequence, determine rota-style index to apply; select THAT item instead. } if(gestureStage.matchedId == 'subkey-select') { + if(!handler) { + throw new Error("Invalid state - reference to subkey menu is missing"); + } // TODO: examine subkey menu, determine proper set of fat-finger alternates. + correctionKeyDistribution = handler.currentStageKeyDistances(); + } + + if(!correctionKeyDistribution) { + correctionKeyDistribution = this.getSimpleTapCorrectionProbabilities(coord, gestureKey.key.spec as ActiveKey); } // Once the best coord to use for fat-finger calculations has been determined: - this.modelKeyClick(gestureStage.item, coord); + this.modelKeyClick(gestureStage.item, coord, correctionKeyDistribution); } // Outside of passing keys along... the handling of later stages is delegated @@ -497,8 +511,6 @@ export default class VisualKeyboard extends EventEmitter<EventMap> implements Ke return; } - let handler: GestureHandler = null; - // So, if this is the first stage, this is where we need to perform that delegation. // -- Scratch-space as gestures start becoming integrated -- @@ -517,11 +529,18 @@ export default class VisualKeyboard extends EventEmitter<EventMap> implements Ke } else if(gestureStage.matchedId.indexOf('longpress') > -1) { // Matches: 'longpress', 'longpress-reset'. // Likewise. - handler = new SubkeyPopup(gestureSequence, configChanger, this, gestureSequence.stageReports[0].sources[0].baseItem); + handler = new SubkeyPopup( + gestureSequence, + configChanger, + this, + gestureSequence.stageReports[0].sources[0].baseItem, + DEFAULT_GESTURE_PARAMS + ); } if(handler) { this.activeGestures.push(handler); + gestureHandlerMap.set(gestureSequence, handler); gestureSequence.on('complete', () => { this.activeGestures = this.activeGestures.filter((gesture) => gesture != handler); }); @@ -727,7 +746,7 @@ export default class VisualKeyboard extends EventEmitter<EventMap> implements Ke * @param keySpec The spec of the key directly triggered by the input event. May be for a subkey. * @returns */ - getTouchProbabilities(input: InputSample<KeyElement, string>, keySpec?: ActiveKey): KeyDistribution { + getSimpleTapCorrectionProbabilities(input: InputSample<KeyElement, string>, keySpec?: ActiveKey): KeyDistribution { // TODO: It'd be nice to optimize by keeping these off when unused, but the wiring // necessary would get in the way of modularization at the moment. // let keyman = com.keyman.singleton; @@ -736,82 +755,20 @@ export default class VisualKeyboard extends EventEmitter<EventMap> implements Ke // } // Note: if subkeys are active, they will still be displayed at this time. - // TODO: In such cases, we should build an ActiveLayout (of sorts) for subkey displays, - // update their geometries to the actual display values, and use the results here. let touchKbdPos = this.getTouchCoordinatesOnKeyboard(input); let layerGroup = this.layerGroup.element; // Always has proper dimensions, unlike kbdDiv itself. - let width = layerGroup.offsetWidth, height = this.kbdDiv.offsetHeight; + const width = layerGroup.offsetWidth, height = this.kbdDiv.offsetHeight; + // Prevent NaN breakages. if (!width || !height) { return null; } - let kbdAspectRatio = layerGroup.offsetWidth / this.kbdDiv.offsetHeight; + let kbdAspectRatio = width / height; + const correctiveLayout = buildCorrectiveLayout(this.kbdLayout.getLayer(this.layerId), kbdAspectRatio); const rawSqDistances = keyTouchDistances(touchKbdPos, correctiveLayout); - - let baseKeyProbabilities = distributionFromDistanceMap(rawSqDistances); - - if (!keySpec || !this.subkeyGesture || !this.subkeyGesture.baseKey.key) { - return baseKeyProbabilities; - } else { - // A temp-hack, as this was noted just before 14.0's release. - // Since a more... comprehensive solution would be way too complex this late in the game, - // this provides a half-decent stopgap measure. - // - // Will not correct to nearby subkeys; only includes the selected subkey and its base keys. - // Still, better than ignoring them both for whatever base key is beneath the final cursor location. - let baseMass = 1.0; - - let baseKeyMass = 1.0; - let baseKeyID = this.subkeyGesture.baseKey.key.spec.coreID; - - let popupKeyMass = 0.0; - let popupKeyID: string = null; - - popupKeyMass = 3.0; - popupKeyID = keySpec.coreID; - - // If the base key appears in the subkey array and was selected, merge the probability masses. - if (popupKeyID == baseKeyID) { - baseKeyMass += popupKeyMass; - popupKeyMass = 0; - } else { - // We namespace it so that lookup operations know to find it via its base key. - popupKeyID = `${baseKeyID}::${popupKeyID}`; - } - - // Compute the normalization factor - let totalMass = baseMass + baseKeyMass + popupKeyMass; - let scalar = 1.0 / totalMass; - - // Prevent duplicate entries in the final map & normalize the remaining entries! - for (let i = 0; i < baseKeyProbabilities.length; i++) { - let entry = baseKeyProbabilities[i]; - if (entry.keyId == baseKeyID) { - baseKeyMass += entry.p * scalar; - baseKeyProbabilities.splice(i, 1); - i--; - } else if (entry.keyId == popupKeyID) { - popupKeyMass = + entry.p * scalar; - baseKeyProbabilities.splice(i, 1); - i--; - } else { - entry.p *= scalar; - } - } - - let finalArray: { keyId: string, p: number }[] = []; - - if (popupKeyMass > 0) { - finalArray.push({ keyId: popupKeyID, p: popupKeyMass * scalar }); - } - - finalArray.push({ keyId: baseKeyID, p: baseKeyMass * scalar }); - - finalArray = finalArray.concat(baseKeyProbabilities); - return finalArray; - } + return distributionFromDistanceMap(rawSqDistances); } //#region Input handling start @@ -1068,12 +1025,20 @@ export default class VisualKeyboard extends EventEmitter<EventMap> implements Ke } //#endregion - modelKeyClick(e: KeyElement, input?: InputSample<KeyElement, string>) { - let keyEvent = this.initKeyEvent(e, input); + modelKeyClick(e: KeyElement, input?: InputSample<KeyElement, string>, keyDistribution?: KeyDistribution) { + let keyEvent = this.initKeyEvent(e); + + if (input) { + keyEvent.source = input; + } + if(keyDistribution) { + keyEvent.keyDistribution = keyDistribution; + } + this.raiseKeyEvent(keyEvent, e); } - initKeyEvent(e: KeyElement, input?: InputSample<KeyElement, string>) { + initKeyEvent(e: KeyElement) { // Turn off key highlighting (or preview) this.highlightKey(e, false); @@ -1088,11 +1053,6 @@ export default class VisualKeyboard extends EventEmitter<EventMap> implements Ke return null; } - // Return the event object. - return this.keyEventFromSpec(keySpec, input); - } - - keyEventFromSpec(keySpec: ActiveKey, input?: InputSample<KeyElement, string>) { //let core = com.keyman.singleton.core; // only singleton-based ref currently needed here. // Start: mirrors _GetKeyEventProperties @@ -1113,11 +1073,6 @@ export default class VisualKeyboard extends EventEmitter<EventMap> implements Ke // End - mirrors _GetKeyEventProperties - if (input) { - Lkc.source = input; - Lkc.keyDistribution = this.getTouchProbabilities(input, keySpec); - } - // Return the event object. return Lkc; } From 215c6b8f1bdf0e8c8d471313448d02a72e1610fb Mon Sep 17 00:00:00 2001 From: "Joshua A. Horton" <joshua_horton@sil.org> Date: Tue, 17 Oct 2023 13:58:05 +0700 Subject: [PATCH 2/5] feat(web): notable TODO --- .../src/input/gestures/browser/subkeyPopup.ts | 63 +++++++++++-------- 1 file changed, 36 insertions(+), 27 deletions(-) diff --git a/web/src/engine/osk/src/input/gestures/browser/subkeyPopup.ts b/web/src/engine/osk/src/input/gestures/browser/subkeyPopup.ts index 3b742bd3103..7932384a71d 100644 --- a/web/src/engine/osk/src/input/gestures/browser/subkeyPopup.ts +++ b/web/src/engine/osk/src/input/gestures/browser/subkeyPopup.ts @@ -389,36 +389,45 @@ export default class SubkeyPopup implements GestureHandler { const rawSqDistances = keyTouchDistances(mappedCoord, this.buildCorrectiveLayout()); const currentKeyDist = rawSqDistances.get(lastCoord.item.key.spec); + /* + * - how long has the subkey menu been visible? + * - Base key should be less likely if it's been visible a while, + * but reasonably likely if it only just appeared. + * - Especially if up-flicks are allowed. Though, in that case, consider + * base-layer neighbors, and particularly the one directly under the touchpoint? + * - raw distance traveled (since the menu appeared) + * - similarly, short distance = a more likely base key? + */ + + // The concept: how likely is it that the user MEANT to output a subkey? + let timeDistance = Math.min( + // The full path is included by the model - meaning the base wait is included here in + // in the stats; we subtract it to get just the duration of the subkey menu. + gestureSource.path.stats.duration - baseStage.sources[0].path.stats.duration, + this.gestureParams.longpress.waitLength + ) / (2 * this.gestureParams.longpress.waitLength); // normalize: max time distance of 0.5 + + let pathDistance = Math.min( + gestureSource.path.stats.rawDistance, + this.gestureParams.longpress.noiseTolerance*4 + ) / (this.gestureParams.longpress.noiseTolerance * 8); // normalize similarly. + + // We only want to add a single distance 'dimension' - we'll choose the one that affects + // the interpreted distance the least. (This matters for upflick-shortcutting in particular) + const layerDistance = Math.min(timeDistance * timeDistance, pathDistance * pathDistance); + const baseKeyDistance = currentKeyDist + layerDistance; + // Include the base key as a corrective option. - if(!this.subkeys.find((entry) => entry.keyId == this.baseKey.keyId)) { - /* - * - how long has the subkey menu been visible? - * - Base key should be less likely if it's been visible a while, - * but reasonably likely if it only just appeared. - * - Especially if up-flicks are allowed. Though, in that case, consider - * base-layer neighbors, and particularly the one directly under the touchpoint? - * - raw distance traveled (since the menu appeared) - * - similarly, short distance = a more likely base key? - */ - - // The concept: how likely is it that the user MEANT to output a subkey? - let timeDistance = Math.min( - // The full path is included by the model - meaning the base wait is included here in - // in the stats; we subtract it to get just the duration of the subkey menu. - gestureSource.path.stats.duration - baseStage.sources[0].path.stats.duration, - this.gestureParams.longpress.waitLength - ) / (2 * this.gestureParams.longpress.waitLength); // normalize: max time distance of 0.5 - - let pathDistance = Math.min( - gestureSource.path.stats.rawDistance, - this.gestureParams.longpress.noiseTolerance*4 - ) / (this.gestureParams.longpress.noiseTolerance * 8); // normalize similarly. - - // We only want to add a single distance 'dimension' - we'll choose the one that affects - // the interpreted distance the least. (This matters for upflick-shortcutting in particular) - const layerDistance = Math.min(timeDistance * timeDistance, pathDistance * pathDistance); + const subkeyMatch = this.subkeys.find((entry) => entry.keyId == this.baseKey.keyId); + if(subkeyMatch) { + const distAsSubkey = rawSqDistances.get(subkeyMatch.key.spec); + if(distAsSubkey > baseKeyDistance) { + rawSqDistances.set(subkeyMatch.key.spec, distAsSubkey); + } // else make no changes + } else { rawSqDistances.set(this.baseKey.key.spec, currentKeyDist + layerDistance); } + // TODO: allow use of multiple maps for distance combining instead! return distributionFromDistanceMap(rawSqDistances); } From e35a741f6588398fd498ea65512b31dfd9fd1d5f Mon Sep 17 00:00:00 2001 From: "Joshua A. Horton" <joshua_horton@sil.org> Date: Wed, 18 Oct 2023 08:24:56 +0700 Subject: [PATCH 3/5] chore(web): better handling for subkeys matching a base key --- common/web/input-processor/src/corrections.ts | 26 ++++++++++++------- .../src/input/gestures/browser/subkeyPopup.ts | 23 ++++++++-------- web/src/engine/osk/src/visualKeyboard.ts | 4 +-- 3 files changed, 30 insertions(+), 23 deletions(-) diff --git a/common/web/input-processor/src/corrections.ts b/common/web/input-processor/src/corrections.ts index bf46a3b6b67..9038cdfa942 100644 --- a/common/web/input-processor/src/corrections.ts +++ b/common/web/input-processor/src/corrections.ts @@ -66,20 +66,26 @@ export function keyTouchDistances(touchCoords: {x: number, y: number}, correctiv * consideration. * @returns */ -export function distributionFromDistanceMap(squaredDistMap: Map<ActiveKeyBase, number>): KeyDistribution { +export function distributionFromDistanceMaps(squaredDistMaps: Map<ActiveKeyBase, number> | Map<ActiveKeyBase, number>[]): KeyDistribution { const keyProbs = new Map<ActiveKeyBase, number>(); let totalMass = 0; - // Should we wish to allow multiple different transforms for distance -> probability, use a function parameter in place - // of the formula in the loop below. - for(let key of squaredDistMap.keys()) { - // We've found that in practice, dist^-4 seems to work pretty well. (Our input has dist^2.) - // (Note: our rule of thumb here has only been tested for layout-based distances.) - const entry = 1 / (Math.pow(squaredDistMap.get(key), 2) + 1e-6); // Prevent div-by-0 errors. - totalMass += entry; + if(!Array.isArray(squaredDistMaps)) { + squaredDistMaps = [squaredDistMaps]; + } + + for(let squaredDistMap of squaredDistMaps) { + // Should we wish to allow multiple different transforms for distance -> probability, use a function parameter in place + // of the formula in the loop below. + for(let key of squaredDistMap.keys()) { + // We've found that in practice, dist^-4 seems to work pretty well. (Our input has dist^2.) + // (Note: our rule of thumb here has only been tested for layout-based distances.) + const entry = 1 / (Math.pow(squaredDistMap.get(key), 2) + 1e-6); // Prevent div-by-0 errors. + totalMass += entry; - // In case of duplicate key IDs. - keyProbs.set(key, keyProbs.get(key) ?? 0 + entry); + // In case of duplicate key IDs; this can occur if multiple sets are specified. + keyProbs.set(key, keyProbs.get(key) ?? 0 + entry); + } } const list: {keySpec: ActiveKeyBase, p: number}[] = []; diff --git a/web/src/engine/osk/src/input/gestures/browser/subkeyPopup.ts b/web/src/engine/osk/src/input/gestures/browser/subkeyPopup.ts index 7932384a71d..1b034ffbedd 100644 --- a/web/src/engine/osk/src/input/gestures/browser/subkeyPopup.ts +++ b/web/src/engine/osk/src/input/gestures/browser/subkeyPopup.ts @@ -3,10 +3,10 @@ import { type KeyElement } from '../../../keyElement.js'; import OSKBaseKey from '../../../keyboard-layout/oskBaseKey.js'; import VisualKeyboard from '../../../visualKeyboard.js'; -import { DeviceSpec, KeyEvent, ActiveSubKey, KeyDistribution } from '@keymanapp/keyboard-processor'; +import { DeviceSpec, KeyEvent, ActiveSubKey, KeyDistribution, ActiveKeyBase } from '@keymanapp/keyboard-processor'; import { ConfigChangeClosure, GestureRecognizerConfiguration, GestureSequence, PaddedZoneSource } from '@keymanapp/gesture-recognizer'; import { GestureHandler } from '../gestureHandler.js'; -import { CorrectionLayout, CorrectionLayoutEntry, distributionFromDistanceMap, keyTouchDistances } from '@keymanapp/input-processor'; +import { CorrectionLayout, CorrectionLayoutEntry, distributionFromDistanceMaps, keyTouchDistances } from '@keymanapp/input-processor'; import { GestureParams } from '../specsForLayout.js'; /** @@ -381,8 +381,9 @@ export default class SubkeyPopup implements GestureHandler { // Lock the coordinate within base-element bounds; corrects for the allowed 'popup roaming' zone. // - // To consider: add a 'clipping' feature to `keyTouchDistances`? It'd make sense for base keys, - // too. + // To consider: add a 'clipping' feature to `keyTouchDistances`? It could make sense for base + // keys, too - especially when emulating a touch OSK via the inline-OSK mode used in the + // Developer host page. mappedCoord.x = mappedCoord.x < 0 ? 0 : (mappedCoord.x > 1 ? 1: mappedCoord.x); mappedCoord.y = mappedCoord.y < 0 ? 0 : (mappedCoord.y > 1 ? 1: mappedCoord.y); @@ -418,17 +419,17 @@ export default class SubkeyPopup implements GestureHandler { const baseKeyDistance = currentKeyDist + layerDistance; // Include the base key as a corrective option. + const baseKeyMap = new Map<ActiveKeyBase, number>(); const subkeyMatch = this.subkeys.find((entry) => entry.keyId == this.baseKey.keyId); if(subkeyMatch) { - const distAsSubkey = rawSqDistances.get(subkeyMatch.key.spec); - if(distAsSubkey > baseKeyDistance) { - rawSqDistances.set(subkeyMatch.key.spec, distAsSubkey); - } // else make no changes + // Ensure that the base key's entry can be merged with that of its subkey. + // (Assuming that always makes sense.) + baseKeyMap.set(subkeyMatch.key.spec, baseKeyDistance); } else { - rawSqDistances.set(this.baseKey.key.spec, currentKeyDist + layerDistance); + baseKeyMap.set(this.baseKey.key.spec, baseKeyDistance); } - // TODO: allow use of multiple maps for distance combining instead! - return distributionFromDistanceMap(rawSqDistances); + + return distributionFromDistanceMaps([rawSqDistances, baseKeyMap]); } cancel() { diff --git a/web/src/engine/osk/src/visualKeyboard.ts b/web/src/engine/osk/src/visualKeyboard.ts index ba8dca256cd..84aaedc9da7 100644 --- a/web/src/engine/osk/src/visualKeyboard.ts +++ b/web/src/engine/osk/src/visualKeyboard.ts @@ -15,7 +15,7 @@ import { LayoutKey } from '@keymanapp/keyboard-processor'; -import { buildCorrectiveLayout, distributionFromDistanceMap, keyTouchDistances } from '@keymanapp/input-processor'; +import { buildCorrectiveLayout, distributionFromDistanceMaps, keyTouchDistances } from '@keymanapp/input-processor'; import { GestureRecognizer, @@ -768,7 +768,7 @@ export default class VisualKeyboard extends EventEmitter<EventMap> implements Ke const correctiveLayout = buildCorrectiveLayout(this.kbdLayout.getLayer(this.layerId), kbdAspectRatio); const rawSqDistances = keyTouchDistances(touchKbdPos, correctiveLayout); - return distributionFromDistanceMap(rawSqDistances); + return distributionFromDistanceMaps(rawSqDistances); } //#region Input handling start From e0abc0f68e453e6e883032e1124719c06ece97dc Mon Sep 17 00:00:00 2001 From: "Joshua A. Horton" <joshua_horton@sil.org> Date: Wed, 18 Oct 2023 14:32:31 +0700 Subject: [PATCH 4/5] chore(web): better method name --- web/src/engine/osk/src/input/gestures/browser/subkeyPopup.ts | 2 +- web/src/engine/osk/src/input/gestures/gestureHandler.ts | 2 +- web/src/engine/osk/src/input/gestures/heldRepeater.ts | 2 +- web/src/engine/osk/src/visualKeyboard.ts | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/web/src/engine/osk/src/input/gestures/browser/subkeyPopup.ts b/web/src/engine/osk/src/input/gestures/browser/subkeyPopup.ts index 1b034ffbedd..536cc1eb8b6 100644 --- a/web/src/engine/osk/src/input/gestures/browser/subkeyPopup.ts +++ b/web/src/engine/osk/src/input/gestures/browser/subkeyPopup.ts @@ -367,7 +367,7 @@ export default class SubkeyPopup implements GestureHandler { } } - currentStageKeyDistances(): KeyDistribution { + currentStageKeyDistribution(): KeyDistribution { const latestStage = this.source.stageReports[this.source.stageReports.length-1]; const baseStage = this.source.stageReports[0]; const gestureSource = latestStage.sources[0]; diff --git a/web/src/engine/osk/src/input/gestures/gestureHandler.ts b/web/src/engine/osk/src/input/gestures/gestureHandler.ts index 991b43742c1..da9b1c2cb0e 100644 --- a/web/src/engine/osk/src/input/gestures/gestureHandler.ts +++ b/web/src/engine/osk/src/input/gestures/gestureHandler.ts @@ -12,5 +12,5 @@ export interface GestureHandler { */ readonly hasModalVisualization: boolean; - currentStageKeyDistances(): KeyDistribution; + currentStageKeyDistribution(): KeyDistribution; } \ No newline at end of file diff --git a/web/src/engine/osk/src/input/gestures/heldRepeater.ts b/web/src/engine/osk/src/input/gestures/heldRepeater.ts index ef7f3a8c5fc..57c2b160b9b 100644 --- a/web/src/engine/osk/src/input/gestures/heldRepeater.ts +++ b/web/src/engine/osk/src/input/gestures/heldRepeater.ts @@ -37,7 +37,7 @@ export class HeldRepeater implements GestureHandler { this.timerHandle = window.setTimeout(this.deleteRepeater, HeldRepeater.REPEAT_DELAY); } - currentStageKeyDistances(): KeyDistribution { + currentStageKeyDistribution(): KeyDistribution { return null; } } \ No newline at end of file diff --git a/web/src/engine/osk/src/visualKeyboard.ts b/web/src/engine/osk/src/visualKeyboard.ts index 84aaedc9da7..2d255f6d7bb 100644 --- a/web/src/engine/osk/src/visualKeyboard.ts +++ b/web/src/engine/osk/src/visualKeyboard.ts @@ -494,7 +494,7 @@ export default class VisualKeyboard extends EventEmitter<EventMap> implements Ke throw new Error("Invalid state - reference to subkey menu is missing"); } // TODO: examine subkey menu, determine proper set of fat-finger alternates. - correctionKeyDistribution = handler.currentStageKeyDistances(); + correctionKeyDistribution = handler.currentStageKeyDistribution(); } if(!correctionKeyDistribution) { From b4767516bf1bfa605fec1ec196cef5ecf3ae3ebe Mon Sep 17 00:00:00 2001 From: "Joshua A. Horton" <joshua_horton@sil.org> Date: Mon, 23 Oct 2023 14:41:40 +0700 Subject: [PATCH 5/5] chore(web): minor cleanup per review comment --- common/web/keyboard-processor/src/text/keyEvent.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/common/web/keyboard-processor/src/text/keyEvent.ts b/common/web/keyboard-processor/src/text/keyEvent.ts index 502fec8dfbf..eb15bc0d1c8 100644 --- a/common/web/keyboard-processor/src/text/keyEvent.ts +++ b/common/web/keyboard-processor/src/text/keyEvent.ts @@ -46,8 +46,6 @@ export interface KeyEventSpec { */ srcKeyboard?: Keyboard; - // // Holds relevant event properties leading to construction of this KeyEvent. - // source?: any; // Technically, KeyEvent|MouseEvent|Touch - but those are DOM types that must be kept out of headless mode. // Holds a generated fat-finger distribution (when appropriate) keyDistribution?: KeyDistribution;