From 24910fb25a7e8bc7ffea823e22d260bafb4d812c Mon Sep 17 00:00:00 2001 From: Marc Durdin Date: Wed, 21 Jun 2023 15:33:09 +0700 Subject: [PATCH 1/3] chore(developer): cleanup data passing from wasm Reorganizes the data structures passed out of kmcmplib via WASM to make it easier to work with the extra metadata and reduce the number of places we have to touch when we add extra metadata fields. Preparation for supporting the metadata that the kmw compiler requires. --- .../src/kmc-kmn/src/compiler/compiler.ts | 54 ++++++++++++------- .../kmw-compiler/write-compiled-keyboard.ts | 7 +-- .../kmc-kmn/test/kmw/test-compiler-manual.ts | 5 +- .../src/kmc-kmn/test/kmw/test-compiler.ts | 4 +- developer/src/kmcmplib/include/kmcmplibapi.h | 11 ++-- .../kmcmplib/src/CompileKeyboardBuffer.cpp | 4 +- developer/src/kmcmplib/src/Compiler.cpp | 4 +- .../src/kmcmplib/src/CompilerInterfaces.cpp | 7 +-- .../kmcmplib/src/CompilerInterfacesWasm.cpp | 32 +++++------ developer/src/kmcmplib/src/compfile.h | 15 +----- 10 files changed, 68 insertions(+), 75 deletions(-) diff --git a/developer/src/kmc-kmn/src/compiler/compiler.ts b/developer/src/kmc-kmn/src/compiler/compiler.ts index 5976a82eb14..9e6bfc2e664 100644 --- a/developer/src/kmc-kmn/src/compiler/compiler.ts +++ b/developer/src/kmc-kmn/src/compiler/compiler.ts @@ -21,21 +21,24 @@ export const COMPILETARGETS_KMX = 0x01; export const COMPILETARGETS_JS = 0x02; export const COMPILETARGETS__MASK = 0x03; -export interface CompilerResultMetadata { +/** + * Data in CompilerResultMetadata comes from kmcmplib + */ +export interface CompilerResultExtra { /** * A bitmask, consisting of COMPILETARGETS_KMX and/or COMPILETARGETS_JS */ targets: number; kvksFilename?: string; displayMapFilename?: string; - displayMap?: Osk.PuaMap; }; export interface CompilerResult { kmx?: CompilerResultFile; kvk?: CompilerResultFile; js?: CompilerResultFile; - data: CompilerResultMetadata; + extra: CompilerResultExtra; + displayMap?: Osk.PuaMap; }; export interface KmnCompilerOptions extends CompilerOptions { @@ -173,7 +176,6 @@ export class KmnCompiler implements UnicodeSetParser { loadFile: this.loadFileCallback }; - let result: CompilerResult = {data:{targets:0}}; let wasm_interface = new Module.CompilerInterface(); let wasm_options = new Module.CompilerOptions(); let wasm_result = null; @@ -189,7 +191,17 @@ export class KmnCompiler implements UnicodeSetParser { return null; } - if(wasm_result.targets & COMPILETARGETS_KMX) { + const result: CompilerResult = { + // We cannot Object.assign or {...} on a wasm-defined object, so... + extra: { + targets: wasm_result.extra.targets, + displayMapFilename: wasm_result.extra.displayMapFilename, + kvksFilename: wasm_result.extra.kvksFilename, + }, + displayMap: null + }; + + if(result.extra.targets & COMPILETARGETS_KMX) { result.kmx = { filename: outfile, data: new Uint8Array(Module.HEAP8.buffer, wasm_result.kmx, wasm_result.kmxSize) @@ -200,16 +212,13 @@ export class KmnCompiler implements UnicodeSetParser { // Visual Keyboard transform // - result.data.kvksFilename = wasm_result.kvksFilename; - result.data.displayMapFilename = wasm_result.displayMapFilename; - result.data.displayMap = null; - if(wasm_result.displayMapFilename) { - result.data.displayMap = this.loadDisplayMapping(infile, result.data.displayMapFilename) + if(result.extra.displayMapFilename) { + result.displayMap = this.loadDisplayMapping(infile, result.extra.displayMapFilename) } - if(result.data.kvksFilename) { - result.kvk = this.runKvkCompiler(result.data.kvksFilename, infile, outfile, result.data.displayMap); + if(result.extra.kvksFilename) { + result.kvk = this.runKvkCompiler(result.extra.kvksFilename, infile, outfile, result.displayMap); if(!result.kvk) { return null; } @@ -219,19 +228,24 @@ export class KmnCompiler implements UnicodeSetParser { // KeymanWeb compiler // - if(wasm_result.targets & COMPILETARGETS_JS) { + if(wasm_result.extra.targets & COMPILETARGETS_JS) { wasm_options.target = 1; // CKF_KEYMANWEB TODO use COMPILETARGETS_JS wasm_result = Module.kmcmp_compile(infile, wasm_options, wasm_interface); if(!wasm_result.result) { return null; } - - let web_kmx = { - filename: outfile, - data: new Uint8Array(Module.HEAP8.buffer, wasm_result.kmx, wasm_result.kmxSize) + const kmw_result: CompilerResult = { + extra: { + // We cannot Object.assign or {...} on a wasm-defined object, so... + targets: wasm_result.extra.targets, + displayMapFilename: wasm_result.extra.displayMapFilename, + kvksFilename: wasm_result.extra.kvksFilename, + }, + displayMap: result.displayMap // we can safely re-use the kmx compile displayMap }; - result.js = this.runWebCompiler(infile, outfile, web_kmx.data, result.kvk?.data, result.data.displayMap, options); + let web_kmx = new Uint8Array(Module.HEAP8.buffer, wasm_result.kmx, wasm_result.kmxSize) + result.js = this.runWebCompiler(infile, outfile, web_kmx, result.kvk?.data, kmw_result, options); if(!result.js) { return null; } @@ -257,10 +271,10 @@ export class KmnCompiler implements UnicodeSetParser { kmxFilename: string, web_kmx: Uint8Array, kvk: Uint8Array, - displayMap: Osk.PuaMap, + kmxResult: CompilerResult, options: CompilerOptions ): CompilerResultFile { - const data = WriteCompiledKeyboard(this.callbacks, kmnFilename, web_kmx, kvk, displayMap, options.saveDebug); + const data = WriteCompiledKeyboard(this.callbacks, kmnFilename, web_kmx, kvk, kmxResult, options.saveDebug); if(!data) { return null; } diff --git a/developer/src/kmc-kmn/src/kmw-compiler/write-compiled-keyboard.ts b/developer/src/kmc-kmn/src/kmw-compiler/write-compiled-keyboard.ts index 27c26e4edc3..b0665f5c585 100644 --- a/developer/src/kmc-kmn/src/kmw-compiler/write-compiled-keyboard.ts +++ b/developer/src/kmc-kmn/src/kmw-compiler/write-compiled-keyboard.ts @@ -1,10 +1,11 @@ -import { KMX, CompilerOptions, CompilerCallbacks, KvkFileReader, VisualKeyboard, KmxFileReader, Osk, KvkFile } from "@keymanapp/common-types"; +import { KMX, CompilerOptions, CompilerCallbacks, KvkFileReader, VisualKeyboard, KmxFileReader, KvkFile } from "@keymanapp/common-types"; import { ExpandSentinel, incxstr, xstrlen } from "./util.js"; import { options, nl, FTabStop, setupGlobals, IsKeyboardVersion10OrLater, callbacks, FFix183_LadderLength } from "./compiler-globals.js"; import { JavaScript_ContextMatch, JavaScript_KeyAsString, JavaScript_Name, JavaScript_OutputString, JavaScript_Rules, JavaScript_Shift, JavaScript_ShiftAsString, JavaScript_Store, zeroPadHex } from './javascript-strings.js'; import { KmwCompilerMessages } from "./messages.js"; import { ValidateLayoutFile } from "./validate-layout-file.js"; import { VisualKeyboardFromFile } from "./visual-keyboard-compiler.js"; +import { CompilerResult } from "src/compiler/compiler.js"; function requote(s: string): string { return "'" + s.replaceAll(/(['\\])/, "\\$1") + "'"; @@ -33,7 +34,7 @@ export function RequotedString(s: string, RequoteSingleQuotes: boolean = false): return s; } -export function WriteCompiledKeyboard(callbacks: CompilerCallbacks, kmnfile: string, keyboardData: Uint8Array, kvkData: Uint8Array, displayMap: Osk.PuaMap, FDebug: boolean = false): string { +export function WriteCompiledKeyboard(callbacks: CompilerCallbacks, kmnfile: string, keyboardData: Uint8Array, kvkData: Uint8Array, kmxResult: CompilerResult, FDebug: boolean = false): string { let opts: CompilerOptions = { shouldAddCompilerVersion: false, saveDebug: FDebug @@ -153,7 +154,7 @@ export function WriteCompiledKeyboard(callbacks: CompilerCallbacks, kmnfile: str if (sLayoutFilename != '') { // I3483 sLayoutFilename = callbacks.resolveFilename(kmnfile, sLayoutFilename); - let result = ValidateLayoutFile(keyboard, options.saveDebug, sLayoutFilename, sVKDictionary, displayMap); + let result = ValidateLayoutFile(keyboard, options.saveDebug, sLayoutFilename, sVKDictionary, kmxResult.displayMap); if(!result.result) { sLayoutFile = ''; callbacks.reportMessage(KmwCompilerMessages.Error_TouchLayoutFileInvalid({filename:sLayoutFilename})); diff --git a/developer/src/kmc-kmn/test/kmw/test-compiler-manual.ts b/developer/src/kmc-kmn/test/kmw/test-compiler-manual.ts index 6d7653ccc61..f934b972317 100644 --- a/developer/src/kmc-kmn/test/kmw/test-compiler-manual.ts +++ b/developer/src/kmc-kmn/test/kmw/test-compiler-manual.ts @@ -3,7 +3,6 @@ import { fileURLToPath } from 'url'; import fs from 'fs'; import { TestCompilerCallbacks } from '@keymanapp/developer-test-helpers'; import { KmnCompiler } from '../../src/compiler/compiler.js'; -import { WriteCompiledKeyboard } from '../../src/kmw-compiler/write-compiled-keyboard.js'; import { extractTouchLayout } from './util.js'; const __dirname = dirname(fileURLToPath(import.meta.url)).replace(/\\/g, '/'); @@ -25,8 +24,6 @@ if(!await kmnCompiler.init(callbacks)) { process.exit(1); } -// TODO: this needs rewrite due to circular deps - let result = kmnCompiler.runCompiler(infile, outfile, { shouldAddCompilerVersion: false, saveDebug: true, // TODO: we should probably use passed debug flag @@ -37,7 +34,7 @@ if(!result) { process.exit(1); } -const js = WriteCompiledKeyboard(callbacks, infile, result.kmx.data, result.kvk.data, null, true); +const js = new TextDecoder().decode(result.js.data); callbacks.printMessages(); diff --git a/developer/src/kmc-kmn/test/kmw/test-compiler.ts b/developer/src/kmc-kmn/test/kmw/test-compiler.ts index 4d5e80b0120..d0d059ef2ca 100644 --- a/developer/src/kmc-kmn/test/kmw/test-compiler.ts +++ b/developer/src/kmc-kmn/test/kmw/test-compiler.ts @@ -1,7 +1,6 @@ import 'mocha'; import { assert } from 'chai'; // import sinonChai from 'sinon-chai'; -import { WriteCompiledKeyboard } from '../../src/kmw-compiler/write-compiled-keyboard.js'; import { dirname } from 'path'; import { fileURLToPath } from 'url'; import fs from 'fs'; @@ -37,7 +36,6 @@ describe('Compiler class', function() { const kmnCompiler = new KmnCompiler(); assert.isTrue(await kmnCompiler.init(callbacks)); - // TODO: runToMemory, add option to kmxCompiler to store debug-data for conversion to .js (e.g. store metadata, group readonly metadata, visual keyboard source filename, etc) let result = kmnCompiler.runCompiler(infile, outfile, { shouldAddCompilerVersion: false, saveDebug: true, // TODO: we should probably use passed debug flag @@ -45,7 +43,7 @@ describe('Compiler class', function() { assert.isNotNull(result); - const js = WriteCompiledKeyboard(callbacks, infile, result.kmx?.data, result.kvk?.data, null, true); + const js = new TextDecoder().decode(result.js.data); const fjs = fs.readFileSync(fixtureName, 'utf8'); diff --git a/developer/src/kmcmplib/include/kmcmplibapi.h b/developer/src/kmcmplib/include/kmcmplibapi.h index 0720f4aa831..ee9b6cccc79 100644 --- a/developer/src/kmcmplib/include/kmcmplibapi.h +++ b/developer/src/kmcmplib/include/kmcmplibapi.h @@ -34,14 +34,19 @@ struct KMCMP_COMPILER_OPTIONS { #define COMPILETARGETS_JS 0x02 #define COMPILETARGETS__MASK 0x03 -struct KMCMP_COMPILER_RESULT { - void* kmx; - size_t kmxSize; +struct KMCMP_COMPILER_RESULT_EXTRA { int targets; /// COMPILETARGETS__MASK = COMPILETARGETS_KMX | COMPILETARGETS_JS + std::string kmnFilename; std::string kvksFilename; std::string displayMapFilename; }; +struct KMCMP_COMPILER_RESULT { + void* kmx; + size_t kmxSize; + struct KMCMP_COMPILER_RESULT_EXTRA extra; +}; + /** * @param szText UTF-8 string */ diff --git a/developer/src/kmcmplib/src/CompileKeyboardBuffer.cpp b/developer/src/kmcmplib/src/CompileKeyboardBuffer.cpp index 48479bcb4f5..fdbed181bca 100644 --- a/developer/src/kmcmplib/src/CompileKeyboardBuffer.cpp +++ b/developer/src/kmcmplib/src/CompileKeyboardBuffer.cpp @@ -43,8 +43,8 @@ bool CompileKeyboardBuffer(KMX_BYTE* infile, int sz, PFILE_KEYBOARD fk) fk->cxVKDictionary = 0; // I3438 fk->dpVKDictionary = NULL; // I3438 fk->extra->targets = COMPILETARGETS_KMX; - fk->extra->kvksFilename = u""; - fk->extra->displayMapFilename = u""; + fk->extra->kvksFilename = ""; + fk->extra->displayMapFilename = ""; /* fk->szMessage[0] = 0; fk->szLanguageName[0] = 0;*/ fk->dwBitmapSize = 0; diff --git a/developer/src/kmcmplib/src/Compiler.cpp b/developer/src/kmcmplib/src/Compiler.cpp index 6e5ad857752..0b5d81589ee 100644 --- a/developer/src/kmcmplib/src/Compiler.cpp +++ b/developer/src/kmcmplib/src/Compiler.cpp @@ -1026,7 +1026,7 @@ KMX_DWORD ProcessSystemStore(PFILE_KEYBOARD fk, KMX_DWORD SystemID, PFILE_STORE { // Store extra metadata for callers as we mutate this store during // compilation - fk->extra->kvksFilename = sp->dpString; + fk->extra->kvksFilename = string_from_u16string(sp->dpString); // Strip path from the store, leaving bare filename only p = sp->dpString; @@ -1140,7 +1140,7 @@ KMX_DWORD ProcessSystemStore(PFILE_KEYBOARD fk, KMX_DWORD SystemID, PFILE_STORE // This store is allowed in older versions of Keyman, as it is a // compile-time only feature. Implemented only in kmc-kmn, not in // the legacy compilers. - fk->extra->displayMapFilename = sp->dpString; + fk->extra->displayMapFilename = string_from_u16string(sp->dpString); break; default: diff --git a/developer/src/kmcmplib/src/CompilerInterfaces.cpp b/developer/src/kmcmplib/src/CompilerInterfaces.cpp index 0cb32d2cb2d..17968590001 100644 --- a/developer/src/kmcmplib/src/CompilerInterfaces.cpp +++ b/developer/src/kmcmplib/src/CompilerInterfaces.cpp @@ -15,7 +15,7 @@ EXTERN bool kmcmp_CompileKeyboard( ) { FILE_KEYBOARD fk; - fk.extra = new FILE_KEYBOARD_EXTRA; + fk.extra = new KMCMP_COMPILER_RESULT_EXTRA; fk.extra->kmnFilename = pszInfile; kmcmp::FSaveDebug = options.saveDebug; // I3681 @@ -103,10 +103,7 @@ EXTERN bool kmcmp_CompileKeyboard( result.kmx = data; result.kmxSize = dataSize; - // TODO: can we eliminate this intermediate structure? - result.targets = fk.extra->targets; - result.kvksFilename = string_from_u16string(fk.extra->kvksFilename); // convert to UTF8 - result.displayMapFilename = string_from_u16string(fk.extra->displayMapFilename); // convert to UTF8 + result.extra = *fk.extra; return TRUE; } diff --git a/developer/src/kmcmplib/src/CompilerInterfacesWasm.cpp b/developer/src/kmcmplib/src/CompilerInterfacesWasm.cpp index 764e67d7483..805796db3bf 100644 --- a/developer/src/kmcmplib/src/CompilerInterfacesWasm.cpp +++ b/developer/src/kmcmplib/src/CompilerInterfacesWasm.cpp @@ -53,25 +53,13 @@ struct WASM_COMPILER_RESULT { // Following are pointer offsets in heap + buffer size int kmx; int kmxSize; - // Following are compiler side-channel data, required for - // follow-on transform - int targets; /* COMPILETARGETS_KMX | COMPILETARGETS_JS */ - std::string kvksFilename; - std::string displayMapFilename; - // TODO: additional data to be passed back + KMCMP_COMPILER_RESULT_EXTRA extra; }; WASM_COMPILER_RESULT kmcmp_wasm_compile(std::string pszInfile, const KMCMP_COMPILER_OPTIONS options, const WASM_COMPILER_INTERFACE intf) { - WASM_COMPILER_RESULT r; + WASM_COMPILER_RESULT r = {false}; KMCMP_COMPILER_RESULT kr; - r.result = false; - r.kmx = 0; - r.kmxSize = 0; - r.targets = 0; - r.kvksFilename = ""; - r.displayMapFilename = ""; - r.result = kmcmp_CompileKeyboard( pszInfile.c_str(), options, @@ -85,9 +73,7 @@ WASM_COMPILER_RESULT kmcmp_wasm_compile(std::string pszInfile, const KMCMP_COMPI // TODO: additional data as required by kmc_kmw r.kmx = (int) kr.kmx; r.kmxSize = (int) kr.kmxSize; - r.kvksFilename = kr.kvksFilename; - r.displayMapFilename = kr.displayMapFilename; - r.targets = kr.targets; + r.extra = kr.extra; } return r; @@ -114,9 +100,15 @@ EMSCRIPTEN_BINDINGS(compiler_interface) { .property("result", &WASM_COMPILER_RESULT::result) .property("kmx", &WASM_COMPILER_RESULT::kmx) .property("kmxSize", &WASM_COMPILER_RESULT::kmxSize) - .property("targets", &WASM_COMPILER_RESULT::targets) - .property("kvksFilename", &WASM_COMPILER_RESULT::kvksFilename) - .property("displayMapFilename", &WASM_COMPILER_RESULT::displayMapFilename) + .property("extra", &WASM_COMPILER_RESULT::extra) + ; + + emscripten::class_("CompilerResultExtra") + .constructor<>() + .property("targets", &KMCMP_COMPILER_RESULT_EXTRA::targets) + .property("kmnFilename", &KMCMP_COMPILER_RESULT_EXTRA::kmnFilename) + .property("kvksFilename", &KMCMP_COMPILER_RESULT_EXTRA::kvksFilename) + .property("displayMapFilename", &KMCMP_COMPILER_RESULT_EXTRA::displayMapFilename) ; emscripten::function("kmcmp_compile", &kmcmp_wasm_compile); diff --git a/developer/src/kmcmplib/src/compfile.h b/developer/src/kmcmplib/src/compfile.h index 0143aff9261..f45cb3ea348 100644 --- a/developer/src/kmcmplib/src/compfile.h +++ b/developer/src/kmcmplib/src/compfile.h @@ -21,6 +21,7 @@ #define _COMPFILE_H #include "kmcompx.h" +#include "kmcmplibapi.h" #include #include @@ -119,18 +120,6 @@ struct FILE_VKDICTIONARY { }; typedef FILE_VKDICTIONARY *PFILE_VKDICTIONARY; -/** - * Extra metadata for API consumers - */ -struct FILE_KEYBOARD_EXTRA { - int targets; - std::string kmnFilename; // utf-8 - std::u16string kvksFilename; // utf-16, original TSS_VISUALKEYBOARD value - std::u16string displayMapFilename; // utf-16, original TSS_DISPLAY_MAP value -}; - -typedef struct FILE_KEYBOARD_EXTRA* PFILE_KEYBOARD_EXTRA; - struct FILE_KEYBOARD { KMX_DWORD KeyboardID; // deprecated, unused @@ -160,7 +149,7 @@ struct FILE_KEYBOARD { KMX_DWORD cxVKDictionary; PFILE_VKDICTIONARY dpVKDictionary; // temp - virtual key dictionary - PFILE_KEYBOARD_EXTRA extra; + KMCMP_COMPILER_RESULT_EXTRA* extra; // extra metadata passed back from the compiler }; typedef FILE_KEYBOARD *PFILE_KEYBOARD; From 8cd28bdc977d1269c411ddd101a5f01fc9f671ac Mon Sep 17 00:00:00 2001 From: Marc Durdin Date: Thu, 22 Jun 2023 07:47:39 +0700 Subject: [PATCH 2/3] chore(developer): rename data to extra --- developer/src/kmc-analyze/src/util/get-osk-from-kmn-file.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/developer/src/kmc-analyze/src/util/get-osk-from-kmn-file.ts b/developer/src/kmc-analyze/src/util/get-osk-from-kmn-file.ts index e790f17c539..686dcbd4f90 100644 --- a/developer/src/kmc-analyze/src/util/get-osk-from-kmn-file.ts +++ b/developer/src/kmc-analyze/src/util/get-osk-from-kmn-file.ts @@ -26,8 +26,8 @@ export async function getOskFromKmnFile(callbacks: CompilerCallbacks, filename: return null; } - if(result.data.kvksFilename) { - kvksFilename = callbacks.resolveFilename(filename, result.data.kvksFilename); + if(result.extra.kvksFilename) { + kvksFilename = callbacks.resolveFilename(filename, result.extra.kvksFilename); } const reader = new KmxFileReader(); From 9216a69636e5160f0dc88ac474bb8b6cea130f18 Mon Sep 17 00:00:00 2001 From: Marc Durdin Date: Fri, 23 Jun 2023 13:23:29 +1000 Subject: [PATCH 3/3] chore: Update developer/src/kmc-kmn/src/compiler/compiler.ts Co-authored-by: Eberhard Beilharz --- developer/src/kmc-kmn/src/compiler/compiler.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/developer/src/kmc-kmn/src/compiler/compiler.ts b/developer/src/kmc-kmn/src/compiler/compiler.ts index 9e6bfc2e664..86e371f5327 100644 --- a/developer/src/kmc-kmn/src/compiler/compiler.ts +++ b/developer/src/kmc-kmn/src/compiler/compiler.ts @@ -22,7 +22,7 @@ export const COMPILETARGETS_JS = 0x02; export const COMPILETARGETS__MASK = 0x03; /** - * Data in CompilerResultMetadata comes from kmcmplib + * Data in CompilerResultExtra comes from kmcmplib */ export interface CompilerResultExtra { /**