change(core): bifurcate normalize_nfd() to use JS native call instead of ICU on wasm 🙀 #11520
Conversation
- does not hit all ICU usage yet #9467
User Test ResultsTest specification and instructions User tests are not required |
| EM_JS(char*, NormalizeNFD, (const char* input), { | ||
| if (!input) return input; // pass through null | ||
| const instr = Module.UTF8ToString(input); | ||
| const nfd = instr.normalize("NFD"); |
There was a problem hiding this comment.
I'm not sure what is happening here. Is ln 21 equivalent to icu::Normalizer2::getNFDInstance(status);
where is the ln 21 normalised defined?
There was a problem hiding this comment.
Yes it's the JS idiom for the same thing. Ends up calling icu..
There was a problem hiding this comment.
So... this line is doing the JS call? I don't see a definition for instr, and I'd like to learn more about whatever interface there is that would allow WASM to make native-JS calls.
There was a problem hiding this comment.
The entire EM_JS block is Javascript.
I don't see a definition for
instr
It's on line 20.
I'd like to learn more about whatever interface there is that would allow WASM to make native-JS calls.
There was a problem hiding this comment.
On Emscripten, beware, it's a deep rabbit hole.
There was a problem hiding this comment.
I don't see a definition for
instrIt's on line 20.
Kinda passes the buck off to Module, though. It's at least referenced through the link you provided - thanks! Kinda odd how it just assumes Module exists and doesn't talk about it, though.
In case I return to this comment in the future, or someone else wants a reference on it... https://emscripten.org/docs/api_reference/module.html#other-methods
I'm still not clear on where exactly UTF8ToString comes from, but I'm willing to handwave that tidbit away for now.
There was a problem hiding this comment.
Yeah, it's all part of the Emscripten tooling. It's a little magical when you first encounter it.
There was a problem hiding this comment.
Module is a JavaScript global at that point.
Yeah it's mostly glue and one call. But it will eventually save us a lot in linked libraries.
…/core/9467-devolve-norm-to-js2-epic-ldml
…/core/9467-devolve-norm-to-js2-epic-ldml
srl295
changed the title
|
updated title after merge 🤦 |
|
Changes in this pull request will be available for download in Keyman version 18.0.45-alpha |
6 similar comments
|
Changes in this pull request will be available for download in Keyman version 18.0.45-alpha |
|
Changes in this pull request will be available for download in Keyman version 18.0.45-alpha |
|
Changes in this pull request will be available for download in Keyman version 18.0.45-alpha |
|
Changes in this pull request will be available for download in Keyman version 18.0.45-alpha |
|
Changes in this pull request will be available for download in Keyman version 18.0.45-alpha |
|
Changes in this pull request will be available for download in Keyman version 18.0.45-alpha |
#9467
@keymanapp-test-bot skip