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 54e5b47954c..f40b42e5afd 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 @@ -115,9 +115,6 @@ export class GestureMatcher { } private finalize(matched: boolean, cause: FulfillmentCause) { - // Problem: on first path completion, the first registered model seems to execute first, regardless of - // 'priority'; we're seeing longpress reject on path end before simple tap, despite higher simple-tap - // priority. if(this.publishedPromise.isFulfilled) { return this._result; } @@ -309,16 +306,6 @@ export class GestureMatcher { this.finalize(resolution.type == 'resolve', resolution.cause); }); - // As this class manages its own internal subview, we must ensure that - // we update when it - our internal subview - is updated. It's difficult - // to ensure from outside this class, so we do it here, inside. - simpleSource.path.on('invalidated', () => { - this.update(); - }); - simpleSource.path.on('complete', () => { - this.update(); - }); - this.pathMatchers.push(contactModel); } 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 a31399665b6..f4510198ba9 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 @@ -21,20 +21,20 @@ interface EventMap { 'rejectionwithaction': (selection: MatcherSelection, replaceModelWith: (replacementModel: GestureModel) => void) => void; } -// /** -// * This class is responsible for interpreting the output of the various input-engine types -// * and facilitating the detection of related gestures. Its role is to serve as a headless -// * version of the main `GestureRecognizer` class, avoiding its DOM and DOM-event dependencies. -// * -// * Of particular note: when a gesture involves multiple touchpoints - like a multitap - this class -// * is responsible for linking related touchpoints together for the detection of that gesture. -// */ - /** - * TBD + * This class is used to "select" successfully-matched gesture models from among an + * active set of potential GestureMatchers. There may be multiple GestureSources / + * contact-points active; it is able to resolve when they are correlated and how + * resolution should proceed based upon the "selected" gesture model. + * + * When at least one "match" for a gesture model occurs, this engine ensures that the + * highest-priority one that matched is selected. It will be returned via Promise along + * with the specified match "action". If, instead, no model ends up matching a + * GestureSource, the Promise will resolve when the last potential model is rejected, + * providing values indicating match failure and the action to be taken. */ export class MatcherSelector extends EventEmitter> { - private _activeSources: GestureSourceTracker[] = []; + private _sourceSelector: GestureSourceTracker[] = []; private potentialMatchers: GestureMatcher[] = []; private readonly promisePrioritizer = new QueuedPromisePrioritizer(); @@ -48,23 +48,7 @@ export class MatcherSelector extends EventEmitter> { public matchGesture( source: GestureSource, gestureModelSet: GestureModel[] - ): Promise>; // FIXME: can't return reset requests (directly) - - // ---------------------- SEE NEXT COMMENT ----------------- - - /* - * Current ideas for above point: - * - * - EventEmitter to the rescue: - * - could have an event for _any_ rejected matcher - * - SHOULD have an event for rejected ones with actions - * - 'rejectionwithaction' - * - pass the corresponding 'MatcherSelection'. - * - also, a func (w validation) for establishing that followup gesture. - * - That way, we: - * - maintain the original promise structure - * - can sync filtering for the new matcher with the VERY SAME PROMISE as originally used - */ + ): Promise>; /** * Facilitates matching a new stage in an ongoing gesture-stage sequence based on a previously- @@ -104,18 +88,17 @@ export class MatcherSelector extends EventEmitter> { // to handle gesture stages that start without active sources - such as multitap stages after // the initial tap. - // Sets up source trackers. - const sourceMetadata: GestureSourceTracker = { + // Sets up source selectors - the object that matches a source against its Promise. + // Promises only resolve once, after all - once called, a "selection" has been made. + const sourceSelectors: GestureSourceTracker = { source: src, matchPromise: matchPromise }; - this._activeSources.push(sourceMetadata); - this.addSourceUpdateHooks(src); + this._sourceSelector.push(sourceSelectors); - return sourceMetadata; - }); //.reduce((cleansedArray, entry) => entry ? cleansedArray.concat(entry) : cleansedArray, [] as GestureSourceTracker[]) + return sourceSelectors; + }); - //const synchronizationSet = sourceTrackers.length == 0 ? [matchPromise] : sourceTrackers.map((track) => track.matchPromise); const synchronizationSet = sourceTrackers.map((track) => track.matchPromise); /** @@ -130,6 +113,9 @@ export class MatcherSelector extends EventEmitter> { const extendableMatcherSet = this.potentialMatchers.filter((matcher) => matcher.mayAddContact()); extendableMatcherSet.forEach((matcher) => { // TODO: do we alter the resolution priority in any way, now that there's an extra touchpoint? + // Answer is not yet clear; perhaps work on gesture-staging will help indicate if this would + // be useful... and how it should act, if so. + matcher.addContact(source); matcher.promise.then(this.matcherSelectionFilter(matcher, synchronizationSet)); }); @@ -156,25 +142,44 @@ export class MatcherSelector extends EventEmitter> { */ this.potentialMatchers.sort((a, b) => b.model.resolutionPriority - a.model.resolutionPriority); + // Now that all GestureMatchers are built, reset ALL of our sync-update-check hooks. + this.resetSourceHooks(); + return matchPromise.corePromise; } - private addSourceUpdateHooks(gestureSource: GestureSource) { - const _this = this; - const synchronizedPathUpdateHandler = () => { - _this.potentialMatchers.forEach(() => {}); - const sourceCurrentTimestamps = this._activeSources.map((tracker) => tracker.source.currentSample.t); - const t = sourceCurrentTimestamps[0]; + private readonly attemptSynchronousUpdate = () => { + const sourceCurrentTimestamps = this._sourceSelector.map((tracker) => tracker.source.isPathComplete ? null : tracker.source.currentSample.t); + const t = sourceCurrentTimestamps[0]; - if(sourceCurrentTimestamps.find((t2) => t != t2)) { - return; - } + // Ignore timestamps from already-terminated paths; they should not block synchronicity checks. + if(sourceCurrentTimestamps.find((t2) => (t2 !== null) && (t != t2))) { + return; + } + + this.potentialMatchers.forEach((matcher) => matcher.update()); + }; - this.potentialMatchers.forEach((matcher) => matcher.update()); + private resetSourceHooks() { + const resetHooks = (gestureSource: GestureSourceSubview) => { + // GestureSourceSubviews stay synchronized with their 'base' via event handlers. + // We want GestureMatchers to receive all updates before we attempt a sync'd update. + const baseSource = gestureSource.baseSource; + + // So, a resetHooks call says to remove the old handler... + baseSource.path.off('step', this.attemptSynchronousUpdate); + baseSource.path.off('complete', this.attemptSynchronousUpdate); + baseSource.path.off('invalidated', this.attemptSynchronousUpdate); + + // And re-add it, but at the end of the handler list. + baseSource.path.on('step', this.attemptSynchronousUpdate); + baseSource.path.on('complete', this.attemptSynchronousUpdate); + baseSource.path.on('invalidated', this.attemptSynchronousUpdate); } - // WARNING: may not be synchronized on deeper-level subviews held within each GestureMatcher! - gestureSource.path.on('step', synchronizedPathUpdateHandler); + // Make sure our source-watching hooks are the last handler for the event; + // matcher-handlers should go first. (Due to how subview synchronization works) + this._sourceSelector.forEach((entry) => resetHooks(entry.source)); } private matchersForSource(source: GestureSource) { @@ -183,11 +188,12 @@ export class MatcherSelector extends EventEmitter> { }); } - // NOTE: is called by resolving Promises. private matcherSelectionFilter(matcher: GestureMatcher, matchSynchronizers: ManagedPromise[]) { // Returns a closure-captured Promise-resolution handler used by individual GestureMatchers managed // by this class instance. return async (result: MatchResult) => { + // Note: is only called by GestureMatcher Promises that are resolving. + // Ensure that any essentially-synchronous resolving GestureMatchers resolve in order of // their specified `resolutionPriority`, with larger values first. await this.promisePrioritizer.queueWithPriority(matcher.model.resolutionPriority); @@ -209,11 +215,13 @@ export class MatcherSelector extends EventEmitter> { const _this = this; const sourceMetadata = matchedContactIds.map((id) => { - const match = this._activeSources.find((metadata) => metadata.source.identifier == id); + const match = this._sourceSelector.find((metadata) => metadata.source.identifier == id); + /* c8 ignore start */ if(!match) { - _this._activeSources.map(() => {}); + _this._sourceSelector.map(() => {}); throw Error(`Could not find original tracker-object for source with id ${id}`); } + /* c8 ignore end */ return match; }); @@ -270,6 +278,7 @@ export class MatcherSelector extends EventEmitter> { replacementMatcher.promise.then(this.matcherSelectionFilter(replacementMatcher, sourceMetadata.map((entry) => entry.matchPromise))); this.potentialMatchers.push(replacementMatcher); + this.resetSourceHooks(); }; // So we emit an event to signal the rejection & allow its replacement via the closure above. @@ -285,7 +294,7 @@ export class MatcherSelector extends EventEmitter> { }); // Drop all trackers for the matched sources. - this._activeSources = this._activeSources.filter((a) => !sourceMetadata.find((b) => a == b)); + this._sourceSelector = this._sourceSelector.filter((a) => !sourceMetadata.find((b) => a == b)); // And now for one line with some "heavy lifting":