From 04df7f76c8d6dc0b1e1ea115a5db39ca09029d3e Mon Sep 17 00:00:00 2001 From: "Joshua A. Horton" Date: Wed, 3 Apr 2024 11:40:15 +0700 Subject: [PATCH 1/4] feat(web): high-level layout op batching Affects change of keyboards and context resets - particularly when new-context rules trigger a non-default layer --- web/src/engine/main/src/keymanEngine.ts | 40 +++++++++++++++++++----- web/src/engine/osk/src/views/oskView.ts | 19 ++++++++++- web/src/engine/osk/src/visualKeyboard.ts | 15 ++++++++- 3 files changed, 64 insertions(+), 10 deletions(-) diff --git a/web/src/engine/main/src/keymanEngine.ts b/web/src/engine/main/src/keymanEngine.ts index 1400c82e9cd..b7a593d3f8f 100644 --- a/web/src/engine/main/src/keymanEngine.ts +++ b/web/src/engine/main/src/keymanEngine.ts @@ -152,16 +152,32 @@ export default class KeymanEngine< this.osk.startHide(false); } - if(this.osk) { - this.osk.setNeedsLayout(); - this.osk.activeKeyboard = kbd; - this.osk.present(); - } - // Needed to ensure the correct layer is displayed. // Needs to be after the OSK has loaded for the keyboard in case the default // layer should be something other than "default" for the current context. - this.core.resetContext(this.contextManager.activeTarget); + const doContextReset = () => { + this.contextManager.resetContext(); + } + + /* + This pattern is designed to minimize layout reflow during the keyboard-swap process. + The 'default' layer is loaded by default, but some keyboards will start on different + layers depending on the current state of the context. + + If possible, we want to only perform layout operations once the correct layer is + set to active. + */ + if(this.osk) { + this.osk.batchLayoutAfter(() => { + this.osk.activeKeyboard = kbd; + // Note: when embedded within the mobile apps, the keyboard will still be visible + // at this time. + doContextReset(); + this.osk.present(); + }); + } else { + doContextReset(); + } }); this.contextManager.on('keyboardasyncload', (metadata) => { @@ -216,7 +232,15 @@ export default class KeymanEngine< resetContext: (target) => { // Could reset the target's deadkeys here, but it's really more of a 'core' task. // So we delegate that to keyboard-processor. - this.core.resetContext(target); + const doReset = () => this.core.resetContext(target); + + if(this.osk) { + this.osk.batchLayoutAfter(() => { + doReset(); + }) + } else { + doReset(); + } }, predictionContext: new PredictionContext(this.core.languageProcessor, this.core.keyboardProcessor), keyboardCache: this.keyboardRequisitioner.cache diff --git a/web/src/engine/osk/src/views/oskView.ts b/web/src/engine/osk/src/views/oskView.ts index 85b63067ab7..2b67d3d1659 100644 --- a/web/src/engine/osk/src/views/oskView.ts +++ b/web/src/engine/osk/src/views/oskView.ts @@ -159,6 +159,7 @@ export default abstract class OSKView private uiStyleSheetManager: StylesheetManager; private config: Configuration; + private deferLayout: boolean; private _boxBaseMouseDown: (e: MouseEvent) => boolean; private _boxBaseTouchStart: (e: TouchEvent) => boolean; @@ -595,8 +596,24 @@ export default abstract class OSKView this.needsLayout = true; } + public batchLayoutAfter(closure: () => void) { + try { + this.deferLayout = true; + if(this.vkbd) { + this.vkbd.deferLayout = true; + } + closure(); + } finally { + this.deferLayout = false; + if(this.vkbd) { + this.vkbd.deferLayout = false; + } + this.refreshLayout(); + } + } + public refreshLayout(pending?: boolean): void { - if(!this.keyboardView) { + if(!this.keyboardView || this.deferLayout) { return; } diff --git a/web/src/engine/osk/src/visualKeyboard.ts b/web/src/engine/osk/src/visualKeyboard.ts index 34d267805cf..67ae9f48fc0 100644 --- a/web/src/engine/osk/src/visualKeyboard.ts +++ b/web/src/engine/osk/src/visualKeyboard.ts @@ -231,11 +231,20 @@ export default class VisualKeyboard extends EventEmitter implements Ke activeGestures: GestureHandler[] = []; activeModipress: Modipress = null; + private _deferLayout: boolean; // The keyboard object corresponding to this VisualKeyboard. public readonly layoutKeyboard: Keyboard; public readonly layoutKeyboardProperties: KeyboardProperties; + get deferLayout(): boolean { + return this._deferLayout; + } + + set deferLayout(value: boolean) { + this._deferLayout = value; + } + get layerId(): string { return this.layerGroup?.activeLayerId ?? 'default'; } @@ -253,7 +262,7 @@ export default class VisualKeyboard extends EventEmitter implements Ke } } - if(changedLayer) { + if(changedLayer && !this._deferLayout) { this.updateState(); // We changed the active layer, but not any layout property of the keyboard as a whole. this.layerGroup.refreshLayout(this.constructLayoutParams()); @@ -1206,6 +1215,10 @@ export default class VisualKeyboard extends EventEmitter implements Ke * when needed. */ refreshLayout() { + if(this._deferLayout) { + return; + } + /* Phase 1: calculations possible at the start without triggering _any_ additional layout reflow. (A single, initial reflow may happen depending on DOM manipulations before this method..., From 1a59f9b90e153dad6cce3212abade153ae057f2a Mon Sep 17 00:00:00 2001 From: "Joshua A. Horton" Date: Wed, 3 Apr 2024 11:57:53 +0700 Subject: [PATCH 2/4] fix(web): handling of nested layout deferrals --- web/src/engine/osk/src/views/oskView.ts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/web/src/engine/osk/src/views/oskView.ts b/web/src/engine/osk/src/views/oskView.ts index 2b67d3d1659..984ba63d856 100644 --- a/web/src/engine/osk/src/views/oskView.ts +++ b/web/src/engine/osk/src/views/oskView.ts @@ -597,6 +597,15 @@ export default abstract class OSKView } public batchLayoutAfter(closure: () => void) { + /* + Is there already an ongoing batch? If so, just run the closure and don't + adjust the tracking variables. The outermost call will finalize layout. + */ + if(this.deferLayout) { + closure(); + return; + } + try { this.deferLayout = true; if(this.vkbd) { From 8cf0da476e19f34e8db417a6e84944fe3dda71c4 Mon Sep 17 00:00:00 2001 From: "Joshua A. Horton" Date: Thu, 4 Apr 2024 11:30:18 +0700 Subject: [PATCH 3/4] change(web): prevents one small reflow during kbd swap --- web/src/engine/main/src/keymanEngine.ts | 34 ++++++++++++++++--------- 1 file changed, 22 insertions(+), 12 deletions(-) diff --git a/web/src/engine/main/src/keymanEngine.ts b/web/src/engine/main/src/keymanEngine.ts index b7a593d3f8f..e6dfb063148 100644 --- a/web/src/engine/main/src/keymanEngine.ts +++ b/web/src/engine/main/src/keymanEngine.ts @@ -2,7 +2,7 @@ import { type Keyboard, KeyboardKeymanGlobal, ProcessorInitOptions } from "@keym import { DOMKeyboardLoader as KeyboardLoader } from "@keymanapp/keyboard-processor/dom-keyboard-loader"; import { InputProcessor, PredictionContext } from "@keymanapp/input-processor"; import { OSKView } from "keyman/engine/osk"; -import { KeyboardRequisitioner, ModelCache, ModelSpec, toUnprefixedKeyboardId as unprefixed } from "keyman/engine/package-cache"; +import { KeyboardRequisitioner, KeyboardStub, ModelCache, ModelSpec, toUnprefixedKeyboardId as unprefixed } from "keyman/engine/package-cache"; import { EngineConfiguration, InitOptionSpec } from "./engineConfiguration.js"; import KeyboardInterface from "./keyboardInterface.js"; @@ -137,14 +137,6 @@ export default class KeymanEngine< }); this.contextManager.on('keyboardchange', (kbd) => { - this.refreshModel(); - this.core.activeKeyboard = kbd?.keyboard; - - this.legacyAPIEvents.callEvent('keyboardchange', { - internalName: kbd?.metadata.id ?? '', - languageCode: kbd?.metadata.langId ?? '' - }); - // Hide OSK and do not update keyboard list if using internal keyboard (desktops). // Condition will not be met for touch form-factors; they force selection of a // default keyboard. @@ -152,9 +144,25 @@ export default class KeymanEngine< this.osk.startHide(false); } - // Needed to ensure the correct layer is displayed. - // Needs to be after the OSK has loaded for the keyboard in case the default - // layer should be something other than "default" for the current context. + const earlyBatchClosure = () => { + this.refreshModel(); + // Triggers context resets that can trigger layout stuff. + // It's not the final such context-reset, though. + this.core.activeKeyboard = kbd?.keyboard; + + this.legacyAPIEvents.callEvent('keyboardchange', { + internalName: kbd?.metadata.id ?? '', + languageCode: kbd?.metadata.langId ?? '' + }); + } + + /* + Needed to ensure the correct layer is displayed AND that deadkeys from + the old keyboard have been wiped. + + Needs to be after the OSK has loaded for the keyboard in case the default + layer should be something other than "default" for the current context. + */ const doContextReset = () => { this.contextManager.resetContext(); } @@ -169,6 +177,7 @@ export default class KeymanEngine< */ if(this.osk) { this.osk.batchLayoutAfter(() => { + earlyBatchClosure(); this.osk.activeKeyboard = kbd; // Note: when embedded within the mobile apps, the keyboard will still be visible // at this time. @@ -176,6 +185,7 @@ export default class KeymanEngine< this.osk.present(); }); } else { + earlyBatchClosure(); doContextReset(); } }); From c5953036e6fdc7756b2ebc3b7b8152a655c22cfe Mon Sep 17 00:00:00 2001 From: "Joshua A. Horton" Date: Thu, 4 Apr 2024 15:35:56 +0700 Subject: [PATCH 4/4] change(web): eliminates early redundant context reset This one occurred during Promise fulfillment synchronously but outside OSK batching mode... with another call later _within_ OSK batching mode. --- web/src/engine/main/src/contextManagerBase.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/web/src/engine/main/src/contextManagerBase.ts b/web/src/engine/main/src/contextManagerBase.ts index 54cfc7e8dd2..df6542aebcc 100644 --- a/web/src/engine/main/src/contextManagerBase.ts +++ b/web/src/engine/main/src/contextManagerBase.ts @@ -274,8 +274,6 @@ export abstract class ContextManagerBase // (!wasNull || !!keyboard) - blocks events for `null` -> `null` transitions. // (keyman/keymanweb.com#96) if(this.currentKeyboardSrcTarget() == originalKeyboardTarget && (!wasNull || !!keyboard)) { - // Perform standard context-reset ops, including the processing of new-context events. - this.resetContext(); // Will trigger KeymanEngine handler that passes keyboard to the OSK, displays it. this.emit('keyboardchange', this.activeKeyboard); }