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

chore(developer): cleanup data passing from wasm 🗜 #9059

Merged
merged 4 commits into from
Jun 23, 2023
Merged
Show file tree
Hide file tree
Changes from 2 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
4 changes: 2 additions & 2 deletions developer/src/kmc-analyze/src/util/get-osk-from-kmn-file.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
54 changes: 34 additions & 20 deletions developer/src/kmc-kmn/src/compiler/compiler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
mcdurdin marked this conversation as resolved.
Show resolved Hide resolved
*/
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 {
Expand Down Expand Up @@ -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;
Expand All @@ -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)
Expand All @@ -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;
}
Expand All @@ -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;
}
Expand All @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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") + "'";
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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}));
Expand Down
5 changes: 1 addition & 4 deletions developer/src/kmc-kmn/test/kmw/test-compiler-manual.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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, '/');
Expand All @@ -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
Expand All @@ -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();

Expand Down
4 changes: 1 addition & 3 deletions developer/src/kmc-kmn/test/kmw/test-compiler.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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');

Expand Down
11 changes: 8 additions & 3 deletions developer/src/kmcmplib/include/kmcmplibapi.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand Down
4 changes: 2 additions & 2 deletions developer/src/kmcmplib/src/CompileKeyboardBuffer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions developer/src/kmcmplib/src/Compiler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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:
Expand Down
7 changes: 2 additions & 5 deletions developer/src/kmcmplib/src/CompilerInterfaces.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
}
Expand Down
32 changes: 12 additions & 20 deletions developer/src/kmcmplib/src/CompilerInterfacesWasm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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;
Expand All @@ -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);
Expand Down
15 changes: 2 additions & 13 deletions developer/src/kmcmplib/src/compfile.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#define _COMPFILE_H

#include "kmcompx.h"
#include "kmcmplibapi.h"
#include <kmx_file.h>
#include <string>

Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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;
Expand Down