Merge pull request #9059 from keymanapp/chore/developer/cleanup-wasm-…

…data-passing

chore(developer): cleanup data passing from wasm 🗜

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();

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 CompilerResultExtra 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;
     }

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}));

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,7 +24,6 @@ if(!await kmnCompiler.init(callbacks)) {
   process.exit(1);
 }
 
-
 let result = kmnCompiler.runCompiler(infile, outfile, {
   shouldAddCompilerVersion: false,
   saveDebug: true,  // TODO: we should probably use passed debug flag
@@ -36,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();
 

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,15 +36,14 @@ 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
     });
 
     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');
 

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
 */

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;

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:

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;
 }

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_<KMCMP_COMPILER_RESULT_EXTRA>("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);

developer/src/kmcmplib/src/compfile.h

@@ -21,6 +21,7 @@
 #define _COMPFILE_H
 
 #include "kmcompx.h"
+#include "kmcmplibapi.h"
 #include <kmx_file.h>
 #include <string>
 
@@ -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;