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

feat(web): implementation of modipress, integration with multitap 🐵 #9876

Merged
merged 22 commits into from
Nov 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
1a3d7a2
feat(web): implementation of modipress, compatibility with multitap
jahorton Oct 27, 2023
d2ba7e2
fix(web): unneeded source-completion check
jahorton Oct 27, 2023
86e217a
fix(web): cleanup for cleanup code
jahorton Oct 27, 2023
3b28dd2
fix(web): swift longpress's early modipress cancellation
jahorton Oct 27, 2023
441600d
fix(web): modipressing of a multitap subkey
jahorton Oct 27, 2023
facafe3
feat(web): modipress detection heuristic
jahorton Oct 27, 2023
0ae48cc
fix(web): niche-case handling - subkey from released modipress
jahorton Oct 27, 2023
96dae39
chore(web): a bit of polish, re-enables a check
jahorton Oct 30, 2023
d5e704f
chore(web): squashes warning when attempting to correct modipresses
jahorton Oct 30, 2023
dd52337
chore(web): Merge branch 'feat/web/generalized-multitap' into feat/we…
jahorton Nov 1, 2023
b9d02cb
fix(web): automated-test patchup
jahorton Nov 2, 2023
44c1a71
chore(web): now uses common/web/types enum for button classes
jahorton Nov 2, 2023
c1436c6
fix(web): fixes modipress issues discovered in personal tests
jahorton Nov 3, 2023
6955f90
chore(web): addresses most review concerns
jahorton Nov 3, 2023
0897a28
chore(web): semi-verbose check for modipress button-class critierion
jahorton Nov 3, 2023
2189f1e
chore(web): Merge branch 'feat/web/generalized-multitap' into feat/we…
jahorton Nov 3, 2023
3ef5519
fix(web): modipress+longpress next-layer handling
jahorton Nov 3, 2023
93c1317
fix(web): modipress on first tap of multitappable
jahorton Nov 6, 2023
d227550
fix(web): path with undefined return value in matcherSelector
jahorton Nov 6, 2023
8e87e66
chore(web): gesture-engine integration null-guards
jahorton Nov 6, 2023
eb74bb9
chore(web): taggedContext null guard in lm-worker
jahorton Nov 6, 2023
cc9de9d
chore(web): Merge branch 'feature-gestures' into feat/web/modipress
jahorton Nov 13, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ export interface PredecessorMatch<Type, StateToken> {
readonly result: MatchResult<Type>;
readonly model?: GestureModel<Type, any>;
readonly baseItem: Type;
readonly predecessor?: PredecessorMatch<Type, StateToken>;
}

export interface MatchResult<Type> {
Expand Down Expand Up @@ -56,7 +57,7 @@ export class GestureMatcher<Type, StateToken = any> implements PredecessorMatch<

private _isCancelled: boolean = false;

private readonly predecessor?: PredecessorMatch<Type, StateToken>;
readonly predecessor?: PredecessorMatch<Type, StateToken>;

private readonly publishedPromise: ManagedPromise<MatchResult<Type>>; // unsure on the actual typing at the moment.
private _result: MatchResult<Type>;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -230,8 +230,9 @@ export class GestureSequence<Type, StateToken = any> extends EventEmitter<EventM
// In some automated tests, `this.touchpointCoordinator` may be `null`.
let selectorNotCurrent = false;
if(this.touchpointCoordinator) {
selectorNotCurrent = ![this.selector, this.pushedSelector].find((sel) => sel == this.touchpointCoordinator.currentSelector);
selectorNotCurrent = !this.touchpointCoordinator.selectorStackIncludes(this.selector);
}

let nextModels = modelSetForAction(selection.result.action, this.gestureConfig, this.baseGestureSetId);
if(selectorNotCurrent) {
// If this sequence's selector isn't current, we're in an unrooted state; the parent, base gesture
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -193,9 +193,22 @@ export class MatcherSelector<Type, StateToken = any> extends EventEmitter<EventM
* 'detached', a state not compatible with the needs of this method.
*/
const sourceNotYetStaged = source instanceof GestureSource;

const determinePredecessorSources = (source: PredecessorMatch<Type, StateToken>) => {
const directSources = (source.sources as GestureSourceSubview<Type>[]).map((source => source.baseSource));

if(directSources && directSources.length > 0) {
return directSources;
} else if(!source.predecessor) {
return [];
} else {
return determinePredecessorSources(source.predecessor);
}
}

const sources = sourceNotYetStaged
? [source instanceof GestureSourceSubview ? source.baseSource : source]
: (source.sources as GestureSourceSubview<Type>[]).map((source) => source.baseSource);
: determinePredecessorSources(source);

// Defining these as locals helps the TS type-checker better infer types within
// this method; a later assignment to `source` will remove its ability to infer
Expand Down Expand Up @@ -310,6 +323,33 @@ export class MatcherSelector<Type, StateToken = any> extends EventEmitter<EventM
currentSample.item = source.currentRecognizerConfig.itemIdentifier(currentSample, null);
unmatchedSource.baseItem = currentSample.item;
}

const newlyMatched = extendableMatcherSet.find((entry) => entry.result);

// If the incoming Source triggered a match AND is included in the model,
// do not build new independent models for it.
if(newlyMatched && newlyMatched.allSourceIds.includes(source.identifier)) {
matchPromise.resolve({
matcher: null,
result: {
matched: false,
action: {
type: 'complete',
item: null
}
}
});

return {
selectionPromise: Promise.resolve({
result: {
action: null,
matched: false
},
matcher: null
})
};
}
}
}

Expand Down Expand Up @@ -451,9 +491,6 @@ export class MatcherSelector<Type, StateToken = any> extends EventEmitter<EventM
const matchedContactIds = matcher.allSourceIds;

const sourceMetadata = matchedContactIds.map((id) => {
// It is fine if there is no match; we build tracking only so far as the most recent
// predecessor for disambiguation, so the initial tap/source for a third tap of a
// multitap will not be accessible here.
return this._sourceSelector.find((metadata) => metadata.source.identifier == id);
}).filter((entry) => !!entry); // remove `undefined` entries, as they're irrelevant.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,8 @@ export class PathMatcher<Type, StateToken = any> {
this.source = source;

if(model.timer) {
this.timerPromise = new TimeoutPromise(model.timer.duration);
const offset = model.timer.inheritElapsed ? Math.min(source.path.stats.duration, model.timer.duration) : 0;
this.timerPromise = new TimeoutPromise(model.timer.duration - offset);

this.publishedPromise.then(() => {
this.timerPromise.resolve(false);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,15 @@ export interface ContactModel<Type, StateToken = any> {
*/
timer?: {
duration: number,
expectedResult: boolean;
expectedResult: boolean,
/**
* If not defined or set to `false`, any already-elapsed time for the source will be
* not be considered; the matching timer will start fresh.
*
* If `true`, the timer will use the inherited `path.stats.duration` stat as an
* offset that has already elapsed, counting it against the timer.
*/
inheritElapsed?: boolean
}

// This field is primarly used at the `GestureMatcher` level, rather than the
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,10 @@ export class TouchpointCoordinator<HoveredItemType, StateToken=any> extends Even
this.currentSelector.stateToken = this.stateToken;
}

public selectorStackIncludes(selector: MatcherSelector<HoveredItemType, StateToken>): boolean {
return this.selectorStack.includes(selector);
}

public get currentSelector() {
return this.selectorStack[this.selectorStack.length-1];
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -542,7 +542,7 @@ export class ContextTracker extends CircularArray<TrackedContextState> {
if(doublecheckContext.left != context.left) {
continue;
}
} else if(priorMatchState.taggedContext.left != context.left) {
} else if(priorMatchState.taggedContext?.left != context.left) {
continue;
}

Expand Down
15 changes: 9 additions & 6 deletions web/src/engine/main/src/keymanEngine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,12 +54,7 @@ export default class KeymanEngine<
}
}

//... probably only applies for physical keystrokes.
if(!event.isSynthetic) {
if(this.osk?.vkbd?.keyPending) {
this.osk.vkbd.keyPending = null;
}
} else if(this.keyEventRefocus) { // && event.isSynthetic // as in, is from the OSK.
if(this.keyEventRefocus) {
// Do anything needed to guarantee that the outputTarget stays active (`app/browser`: maintains focus).
// (Interaction with the OSK may have de-focused the element providing active context;
// we want to restore it in case the user swaps back to the hardware keyboard afterward.)
Expand All @@ -72,6 +67,14 @@ export default class KeymanEngine<
// Deleting matched deadkeys here seems to correct some of the issues. (JD 6/6/14)
outputTarget.deadkeys().deleteMatched(); // Delete any matched deadkeys before continuing

if(event.isSynthetic) {
const oskLayer = this.osk.vkbd.layerId;

// In case of modipresses.
if(oskLayer && oskLayer != this.core.keyboardProcessor.layerId) {
this.core.keyboardProcessor.layerId = oskLayer;
}
}
const result = this.core.processKeyEvent(event, outputTarget);

if(result && result.transcription?.transform) {
Expand Down
80 changes: 80 additions & 0 deletions web/src/engine/osk/src/input/gestures/browser/modipress.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
import { type KeyElement } from '../../../keyElement.js';
import VisualKeyboard from '../../../visualKeyboard.js';

import { KeyDistribution, ActiveKeyBase, ManagedPromise } from '@keymanapp/keyboard-processor';
import { GestureSequence } from '@keymanapp/gesture-recognizer';
import { GestureHandler } from '../gestureHandler.js';

/**
* Represents a potential modipress gesture's implementation within KeymanWeb, including
* modipresses generated at the end of multitap sequences.
*
* This involves "locking" the current layer in place until the modipress is complete.
*/
export default class Modipress implements GestureHandler {
private completionCallback: () => void;
private originalLayer: string;
private shouldRestore: boolean = false;
private source: GestureSequence<KeyElement, string>;

constructor(
source: GestureSequence<KeyElement, string>,
vkbd: VisualKeyboard,
completionCallback: () => void,
) {
const initialStage = source.stageReports[0];
this.originalLayer = initialStage.sources[0].stateToken;
this.source = source;

this.completionCallback = () => {
vkbd.lockLayer(false);
if(this.shouldRestore) {
vkbd.layerId = this.originalLayer;
vkbd.updateState();
}
completionCallback?.();
};

vkbd.lockLayer(true);

source.on('stage', (stage) => {
const stageName = stage.matchedId;
if(stageName.includes('modipress') && stageName.includes('-end')) {
this.clear();
} else if(stageName.includes('modipress') && stageName.includes('-hold')) {
this.shouldRestore = true;
}
});

source.on('complete', () => this.cancel());
}

get isLocked(): boolean {
return this.shouldRestore;
}

setLocked() {
this.shouldRestore = true;
}

get completed(): boolean {
return this.completionCallback === null;
}

clear() {
const callback = this.completionCallback;
this.completionCallback = null;
callback?.();
}

cancel() {
this.clear();
this.source.cancel();
}

readonly hasModalVisualization = false;

currentStageKeyDistribution(baseDistMap: Map<ActiveKeyBase, number>): KeyDistribution {
return null;
}
}
55 changes: 45 additions & 10 deletions web/src/engine/osk/src/input/gestures/browser/multitap.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,11 @@ import { type KeyElement } from '../../../keyElement.js';
import VisualKeyboard from '../../../visualKeyboard.js';

import { DeviceSpec, KeyEvent, ActiveSubKey, ActiveKey, KeyDistribution, ActiveKeyBase } from '@keymanapp/keyboard-processor';
import { GestureSequence } from '@keymanapp/gesture-recognizer';
import { GestureSequence, GestureStageReport } from '@keymanapp/gesture-recognizer';
import { GestureHandler } from '../gestureHandler.js';
import { distributionFromDistanceMaps } from '@keymanapp/input-processor';
import Modipress from './modipress.js';
import { keySupportsModipress } from '../specsForLayout.js';

/**
* Represents a potential multitap gesture's implementation within KeymanWeb.
Expand All @@ -23,6 +25,7 @@ export default class Multitap implements GestureHandler {

private readonly multitaps: ActiveSubKey[];
private tapIndex = 0;
private modipress: Modipress;

private sequence: GestureSequence<KeyElement, string>;

Expand All @@ -35,21 +38,38 @@ export default class Multitap implements GestureHandler {
this.baseKey = e;
this.baseContextToken = contextToken;
this.multitaps = [e.key.spec].concat(e.key.spec.multitap);
this.sequence = source;

this.originalLayer = vkbd.layerId;
const startModipress = (tap) => {
// In case of a previous modipress that somehow wasn't cleared.
this.modipress?.clear();

const modipressHandler = new Modipress(source, vkbd, () => {
this.modipress = vkbd.activeModipress = null;
});
this.modipress = vkbd.activeModipress = modipressHandler;
}

// // For multitaps, keeping the key highlighted makes sense. I think.
// this.baseKey.key.highlight(true);
this.originalLayer = vkbd.layerId;

source.on('complete', () => {
if(source.stageReports.length > 1) {
}
// this.currentSelection?.key.highlight(false);

this.modipress?.cancel();
this.clear();
});

source.on('stage', (tap) => {
const stageHandler = (tap: GestureStageReport<KeyElement, string>) => {
switch(tap.matchedId) {
// In the case that a modifier key supports multitap, reaching this stage
// indicates that the multitapping is over. Not the modipressing, though.
case 'modipress-hold':
this.clear();
// We'll let the co-existing modipress handler continue.
source.off('stage', stageHandler);
return;
case 'modipress-end-multitap-transition':
case 'modipress-multitap-end':
case 'modipress-end':
case 'multitap-end':
Expand All @@ -58,8 +78,8 @@ export default class Multitap implements GestureHandler {
// Once a multitap starts, it's better to emit keys on keydown; that way,
// if a user holds long, they get what they see if they decide to stop,
// but also have time to decide if they want to continue to what's next.
case 'multitap-start':
case 'modipress-multitap-start':
case 'multitap-start':
break;
default:
throw new Error(`Unsupported gesture state encountered during multitap: ${tap.matchedId}`);
Expand All @@ -74,7 +94,7 @@ export default class Multitap implements GestureHandler {

const coord = tap.sources[0].currentSample;
const baseDistances = vkbd.getSimpleTapCorrectionDistances(coord, this.baseKey.key.spec as ActiveKey);
if(coord.stateToken != vkbd.layerId) {
if(coord.stateToken != vkbd.layerId && !tap.matchedId.includes('modipress')) {
const matchKey = vkbd.layerGroup.findNearestKey({...coord, stateToken: vkbd.layerId});

// Replace the key at the current location for the current layer key
Expand All @@ -94,7 +114,19 @@ export default class Multitap implements GestureHandler {
keyEvent.kNextLayer ||= this.originalLayer;

vkbd.raiseKeyEvent(keyEvent, null);
});

// Now that the key has been processed, with a layer possibly changed as a result...
if(tap.matchedId == 'modipress-multitap-start') {
startModipress(tap);
}
};

source.on('stage', stageHandler);

const initialTap = source.stageReports[0];
if(initialTap.matchedId == 'modipress-start') {
startModipress(source.stageReports[0]);
}

/* In theory, setting up a specialized recognizer config limited to the base key's surface area
* would be pretty ideal - it'd provide automatic cancellation if anywhere else were touched.
Expand All @@ -117,7 +149,10 @@ export default class Multitap implements GestureHandler {
const keyIndex = baseDistribution.findIndex((entry) => entry.keySpec == this.baseKey.key.spec);

if(keyIndex == -1) { // also covers undefined, but does not include 0.
console.warn("Could not find base key's probability for multitap correction");
// Modipress keys generally get left out of the key-correction calculations.
if(!keySupportsModipress(this.baseKey)) {
console.warn("Could not find base key's probability for multitap correction");
}

// Decently recoverable; just use the simple-tap distances instead.
return baseDistribution;
Expand Down
6 changes: 6 additions & 0 deletions web/src/engine/osk/src/input/gestures/browser/subkeyPopup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ export default class SubkeyPopup implements GestureHandler {
private source: GestureSequence<KeyElement>;
private readonly gestureParams: GestureParams;

readonly shouldLockLayer: boolean = false;

constructor(
source: GestureSequence<KeyElement, string>,
configChanger: ConfigChangeClosure<KeyElement>,
Expand All @@ -46,6 +48,10 @@ export default class SubkeyPopup implements GestureHandler {
this.source = source;
this.gestureParams = gestureParams;

if(vkbd.layerLocked) {
this.shouldLockLayer = true;
}

source.on('complete', () => {
this.currentSelection?.key.highlight(false);
this.clear();
Expand Down
Loading