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): load .kmx keyboard from blob 🎼 #12823

Open
wants to merge 1 commit into
base: fix/core/alignment
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 4 additions & 1 deletion core/include/keyman/keyman_core_api.h
Original file line number Diff line number Diff line change
Expand Up @@ -302,11 +302,14 @@ typedef uint8_t (*km_core_keyboard_imx_platform)(km_core_state*, uint32_t, void*

## Description

An error code mechanism similar to COMs `HRESULT` scheme (unlike COM, any
An error code mechanism similar to COM's `HRESULT` scheme (unlike COM, any
non-zero value is an error).

## Specification

-->
// keep in sync with web/src/engine/core-processor/src/core-factory.ts
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

<!--
```c */
enum km_core_status_codes {
KM_CORE_STATUS_OK = 0,
Expand Down
3 changes: 3 additions & 0 deletions web/build.sh
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,9 @@ build_action() {

tsc --project "${KEYMAN_ROOT}/web/src/test/auto/tsconfig.json"

mkdir -p "${KEYMAN_ROOT}/web/build/test/dom/cases/core-processor/import/core/"
cp "${KEYMAN_ROOT}/web/src/engine/core-processor/src/import/core/keymancore.d.ts" "${KEYMAN_ROOT}/web/build/test/dom/cases/core-processor/import/core/"

for dir in \
"${KEYMAN_ROOT}/web/build/test/dom/cases"/*/ \
"${KEYMAN_ROOT}/web/build/test/integrated/" \
Expand Down
38 changes: 38 additions & 0 deletions web/src/engine/core-processor/src/core-factory.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
// Keyman is copyright (C) SIL International. MIT License.

import { type MainModule } from './import/core/keymancore.js';

// Unfortunately embind has an open issue with enums and typescript where it
// only generates a type for the enum, but not the values in a usable way.
// So we have to re-define the enum here.
// See https://github.com/emscripten-core/emscripten/issues/18585
// NOTE: Keep in sync with core/include/keyman/keyman_core_api.h#L311
export enum KM_CORE_STATUS {
OK = 0,
NO_MEM = 1,
IO_ERROR = 2,
INVALID_ARGUMENT = 3,
KEY_ERROR = 4,
INSUFFICENT_BUFFER = 5,
INVALID_UTF = 6,
INVALID_KEYBOARD = 7,
NOT_IMPLEMENTED = 8,
OS_ERROR = 0x80000000
}

export class CoreFactory {
public static async createCoreProcessor(baseurl: string): Promise<MainModule> {
try {
const module = await import(baseurl + '/km-core.js');
const createCoreProcessor = module.default;
return await createCoreProcessor({
locateFile: function (path: string, scriptDirectory: string) {
return baseurl + '/' + path;
}
});
} catch (e: any) {
console.log('got execption in CoreFactory.createCoreProcessor', e);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
console.log('got execption in CoreFactory.createCoreProcessor', e);
console.log('got exception in CoreFactory.createCoreProcessor', e);

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need to ensure that exceptions here aren't masked. Console log is not available in many contexts e.g. System keyboard. Console.error is a start but not enough maybe?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you suggest?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason the exception needs to be masked here? Could it just change to propagate?

return null;
}
}
}
30 changes: 0 additions & 30 deletions web/src/engine/core-processor/src/core-processor.ts

This file was deleted.

4 changes: 3 additions & 1 deletion web/src/engine/core-processor/src/index.ts
Original file line number Diff line number Diff line change
@@ -1 +1,3 @@
export * from './core-processor.js';
export { CoreFactory, KM_CORE_STATUS } from './core-factory.js';
import { type MainModule, type km_core_keyboard, type CoreKeyboardReturn } from './import/core/keymancore.js';
export { MainModule, km_core_keyboard, CoreKeyboardReturn };
12 changes: 8 additions & 4 deletions web/src/engine/main/src/headless/inputProcessor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ import { LanguageProcessor } from "./languageProcessor.js";
import type { ModelSpec, PathConfiguration } from "keyman/engine/interfaces";
import { globalObject, DeviceSpec } from "@keymanapp/web-utils";

import { CoreProcessor } from "keyman/engine/core-processor";
import { CoreFactory, MainModule as KmCoreModule } from 'keyman/engine/core-processor';

import { Codes, type Keyboard, type KeyEvent } from "keyman/engine/keyboard";
import {
type Alternate,
Expand Down Expand Up @@ -35,7 +36,7 @@ export class InputProcessor {
private contextDevice: DeviceSpec;
private kbdProcessor: KeyboardProcessor;
private lngProcessor: LanguageProcessor;
private coreProcessor: CoreProcessor;
private km_core: KmCoreModule;

private readonly contextCache = new TranscriptionCache();

Expand All @@ -51,11 +52,10 @@ export class InputProcessor {
this.contextDevice = device;
this.kbdProcessor = new KeyboardProcessor(device, options);
this.lngProcessor = new LanguageProcessor(predictiveTextWorker, this.contextCache);
this.coreProcessor = new CoreProcessor();
}

public async init(paths: PathConfiguration) {
this.coreProcessor.init(paths.basePath);
this.km_core = await CoreFactory.createCoreProcessor(paths.basePath);
}

public get languageProcessor(): LanguageProcessor {
Expand All @@ -70,6 +70,10 @@ export class InputProcessor {
return this.keyboardProcessor.keyboardInterface;
}

public get keymanCore(): KmCoreModule {
return this.km_core;
}

public get activeKeyboard(): Keyboard {
return this.keyboardInterface.activeKeyboard;
}
Expand Down
31 changes: 24 additions & 7 deletions web/src/test/auto/dom/cases/core-processor/basic.tests.ts
Original file line number Diff line number Diff line change
@@ -1,21 +1,38 @@
import { assert } from 'chai';
import { CoreProcessor } from 'keyman/engine/core-processor';
import { CoreFactory, KM_CORE_STATUS } from 'keyman/engine/core-processor';

const coreurl = '/web/build/engine/core-processor/obj/import/core';
const coreurl = '/build/engine/core-processor/obj/import/core';

// Test the CoreProcessor interface.
describe('CoreProcessor', function () {
async function loadKeyboardBlob(uri: string) {
const response = await fetch(uri);
if (!response.ok) {
throw new Error(`HTTP ${response.status} ${response.statusText}`);
}

const buffer = await response.arrayBuffer();
return new Uint8Array(buffer);
}

it('can initialize without errors', async function () {
const kp = new CoreProcessor();
assert.isTrue(await kp.init(coreurl));
assert.isNotNull(await CoreFactory.createCoreProcessor(coreurl));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here for example would fail if there's an err.

});

it('can call temp function', async function () {
const kp = new CoreProcessor();
await kp.init(coreurl);
const a = kp.tmp_wasm_attributes();
const km_core = await CoreFactory.createCoreProcessor(coreurl);
const a = km_core.tmp_wasm_attributes();
assert.isNotNull(a);
assert.isNumber(a.max_context);
console.dir(a);
});

it('can load a keyboard from blob', async function () {
const km_core = await CoreFactory.createCoreProcessor(coreurl);
const blob = await loadKeyboardBlob('/common/test/resources/keyboards/test_8568_deadkeys.kmx')
const result = km_core.keyboard_load_from_blob('test', blob);
assert.equal(result.status, KM_CORE_STATUS.OK);
assert.isNotNull(result.object);
result.delete();
});
});
2 changes: 2 additions & 0 deletions web/src/test/auto/dom/web-test-runner.config.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,8 @@ export default {
function rewriteResourcePath(context, next) {
if(context.url.startsWith('/resources/')) {
context.url = '/web/src/test/auto' + context.url;
} else if (context.url.startsWith('/build/')) {
context.url = '/web' + context.url;
}

return next();
Expand Down
Loading