From 9f3ccffe2b06bbcbfe75e059fbe1b4d60991400e Mon Sep 17 00:00:00 2001 From: jahorton Date: Thu, 9 Apr 2020 12:14:27 +0700 Subject: [PATCH] refactor(web/engine): changes model load + mayPredict interactions --- web/source/osk/bannerManager.ts | 7 +-- web/source/osk/preProcessor.ts | 2 +- web/source/text/inputProcessor.ts | 4 +- .../text/prediction/languageProcessor.ts | 31 ++++++++++--- web/source/text/prediction/modelManager.ts | 43 ------------------- 5 files changed, 29 insertions(+), 58 deletions(-) diff --git a/web/source/osk/bannerManager.ts b/web/source/osk/bannerManager.ts index 69a75f6075..fc1a444998 100644 --- a/web/source/osk/bannerManager.ts +++ b/web/source/osk/bannerManager.ts @@ -211,11 +211,8 @@ namespace com.keyman.osk { private selectBanner(state?: text.prediction.ModelChangeEnum) { let keyman = com.keyman.singleton; - // Only display a SuggestionBanner when the current - // language has an active predictive model. - // ModelManager will never have an active model - // when predictions are disabled. - if(keyman.core.activeModel) { + // Only display a SuggestionBanner when LanguageProcessor states it is active.s + if(keyman.core.languageProcessor.isActive) { this.setBanner('suggestion'); } else if(this.alwaysShow) { this.setBanner('image'); diff --git a/web/source/osk/preProcessor.ts b/web/source/osk/preProcessor.ts index 72a687166f..4659e47329 100644 --- a/web/source/osk/preProcessor.ts +++ b/web/source/osk/preProcessor.ts @@ -49,7 +49,7 @@ namespace com.keyman.osk { outputTarget.deadkeys().deleteMatched(); // Delete any matched deadkeys before continuing let Lkc = PreProcessor._GetClickEventProperties(e['key'].spec as keyboards.ActiveKey, Lelem); - if(keyman.core.languageProcessor.enabled) { + if(keyman.core.languageProcessor.isActive) { Lkc.source = touch; Lkc.keyDistribution = keyDistribution; } diff --git a/web/source/text/inputProcessor.ts b/web/source/text/inputProcessor.ts index 82f55a8662..7c87a565b0 100644 --- a/web/source/text/inputProcessor.ts +++ b/web/source/text/inputProcessor.ts @@ -82,7 +82,7 @@ namespace com.keyman.text { // If suggestions exist AND space is pressed, accept the suggestion and do not process the keystroke. // If a suggestion was just accepted AND backspace is pressed, revert the change and do not process the backspace. // We check the first condition here, while the prediction UI handles the second through the try__() methods below. - if(this.languageProcessor.enabled) { + if(this.languageProcessor.isActive) { // The following code relies on JS's logical operator "short-circuit" properties to prevent unwanted triggering of the second condition. // Can the suggestion UI revert a recent suggestion? If so, do that and swallow the backspace. @@ -110,7 +110,7 @@ namespace com.keyman.text { // If we're performing a 'default command', it's not a standard 'typing' event - don't do fat-finger stuff. // Also, don't do fat-finger stuff if predictive text isn't enabled. - if(this.languageProcessor.enabled && !ruleBehavior.triggersDefaultCommand) { + if(this.languageProcessor.isActive && !ruleBehavior.triggersDefaultCommand) { // Note - we don't yet do fat-fingering with longpress keys. if(keyEvent.keyDistribution && keyEvent.kbdLayer) { let activeLayout = this.activeKeyboard.layout(keyEvent.device.formFactor); diff --git a/web/source/text/prediction/languageProcessor.ts b/web/source/text/prediction/languageProcessor.ts index 77fb51117c..40d66a5473 100644 --- a/web/source/text/prediction/languageProcessor.ts +++ b/web/source/text/prediction/languageProcessor.ts @@ -36,6 +36,9 @@ namespace com.keyman.text.prediction { this.lmEngine.unloadModel(); delete this.currentModel; delete this.configuration; + + let keyman = com.keyman.singleton; + keyman.util.callEvent(ModelManager.EVENT_PREFIX + 'modelchange', 'unloaded'); } loadModel(model: ModelSpec): Promise { @@ -50,6 +53,14 @@ namespace com.keyman.text.prediction { return this.lmEngine.loadModel(file).then(function(config: Configuration) { mm.currentModel = model; mm.configuration = config; + + try { + let keyman = com.keyman.singleton; + keyman.util.callEvent(ModelManager.EVENT_PREFIX + 'modelchange', 'loaded'); + } catch (err) { + // Does this provide enough logging information? + console.error("Could not load model '" + model.id + "': " + (err as Error).message); + } }); } @@ -69,7 +80,7 @@ namespace com.keyman.text.prediction { } public wordbreak(target: OutputTarget): Promise { - if(!this.enabled) { + if(!this.isActive) { return null; } @@ -78,7 +89,7 @@ namespace com.keyman.text.prediction { } public predict(transcription?: Transcription) { - if(!this.enabled) { + if(!this.isActive) { return; } @@ -157,7 +168,11 @@ namespace com.keyman.text.prediction { this.lmEngine.shutdown(); } - public get enabled(): boolean { + public get isActive(): boolean { + if(!this.canEnable()) { + this._mayPredict = false; + return false; + } return this.activeModel && this._mayPredict; } @@ -171,15 +186,17 @@ namespace com.keyman.text.prediction { } public set mayPredict(flag: boolean) { - let enabled = this.enabled; - if(!this.canEnable()) { return; } + let oldVal = this._mayPredict; this._mayPredict = flag; - if(enabled != this.enabled || flag) { - com.keyman.singleton.modelManager.doEnable(flag); + + // 'modelchange' always did signify more of a 'is active' flag. + if(oldVal != flag) { + let keyman = com.keyman.singleton; + keyman.util.callEvent(ModelManager.EVENT_PREFIX + 'modelchange', flag ? 'loaded' : 'unloaded'); } } diff --git a/web/source/text/prediction/modelManager.ts b/web/source/text/prediction/modelManager.ts index 7d1cfdde0b..45d83e954c 100644 --- a/web/source/text/prediction/modelManager.ts +++ b/web/source/text/prediction/modelManager.ts @@ -103,10 +103,6 @@ namespace com.keyman.text.prediction { let keyman = com.keyman.singleton; let core = keyman.core; - if(!core.languageProcessor.mayPredict) { - return Promise.resolve(); - } - if(typeof kbdInfo == 'string') { // This case refers to the active language code. kbdInfo = { ['internalName']: keyman.keyboardManager.getActiveKeyboardName(), @@ -122,30 +118,11 @@ namespace com.keyman.text.prediction { if(core.activeModel !== model) { if(core.activeModel) { core.languageProcessor.unloadModel(); - keyman.util.callEvent(ModelManager.EVENT_PREFIX + 'modelchange', 'unloaded'); } if(model) { loadPromise = core.languageProcessor.loadModel(model); } - - // If we're loading a model, we need to defer until its completion before we report a change of state. - if(loadPromise) { - let mm = this; - loadPromise.then(function() { - // Because this is executed from a Promise, it's possible to have a race condition - // where the 'loaded' event triggers after an 'unloaded' event meant to disable the model. - // (Especially in the embedded apps.) This will catch these cases. - if(core.languageProcessor.mayPredict) { - keyman.util.callEvent(ModelManager.EVENT_PREFIX + 'modelchange', 'loaded'); - } else { - core.languageProcessor.unloadModel(); - } - }).catch(function(failReason: any) { - // Does this provide enough logging information? - console.error("Could not load model '" + model.id + "': " + failReason); - }); - } } } @@ -185,7 +162,6 @@ namespace com.keyman.text.prediction { // Is it the active model? if(core.activeModel && core.activeModel.id == modelId) { core.languageProcessor.unloadModel(); - keyman.util.callEvent(ModelManager.EVENT_PREFIX + 'modelchange', 'unloaded'); } // Ensure the model is deregistered for each targeted language code variant. @@ -201,25 +177,6 @@ namespace com.keyman.text.prediction { return !! this.registeredModels[model.id]; } - doEnable(flag: boolean) { - let keyman = com.keyman.singleton; - - if(flag) { - let lgCode = keyman.keyboardManager.getActiveLanguage(); - if(keyman.modelManager.languageModelMap[lgCode]) { - // Just reuse the existing model-change trigger code. - keyman.modelManager.onKeyboardChange(lgCode); - } - } else { - if(keyman.core.activeModel) { // We only need to unload a model when one is actually loaded. - keyman.core.languageProcessor.unloadModel(); - } - - // Ensure that the banner is unloaded. - keyman.util.callEvent(ModelManager.EVENT_PREFIX + 'modelchange', 'unloaded'); - } - } - /** * Function addEventListener * Scope Public