-
Notifications
You must be signed in to change notification settings - Fork 11
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鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add C++ as a language option #65
Add C++ as a language option #65
Conversation
- new WASM library compilation targets (for JS and C++) - language definitions that control which scripts are loaded - C++ compiler backend - javascript 'compiler' wrapper (needs refactoring later) - C++ runtime based on asynchronous execution via WebWorker (inside Execution Environment iFrame) - service worker to handle communication between C++ runtime and Execution Environment - project initializer for C++ - small improvements to notifications - fallible messages extension (can now resolve/reject like promises) - terminal styling improvement (monospace + retain whitespace) - error handling for programRun/Pause etc - added favicon
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is obviously still a WIP feature so I have tried not to note things you are likely already aware of. The compiler and runtime code seem rather clean, and I have not noticed anything erroneous on a line-by-line basis. Nothing has regressed, with one exception.
I encountered a bug while testing that is somewhat difficult to reproduce. If while the user's program is running, the execution of the page is paused with the browser's debugger for long enough, the page will lock up entirely upon unpausing. My guess is that this is somehow related to the workers, as while the page is frozen, I am still able to see requests for programEvents.js
being sent. Perhaps when the page unpauses, it is suddenly inundated with thousands of yet unhandled requests? I cannot think of any conditions under which this would trigger for a normal user, but it would make development more difficult.
This is less immediately relevant but: I find the notifications slightly cluttered. Immediate feedback is an important part of a tool like SplashKit Online, but now if the user restarts the program more than 3~4 times in short succession, the entire terminal and canvas are obscured by notifications. Some of these messages might be better suited to a status bar; one is already carved out at the bottom of the page, though goes unused.
Browser_IDE/stylesheet.css
Outdated
.sk-terminal { | ||
overflow-y: scroll; | ||
height:200px; | ||
border:0; | ||
padding-left:1em; | ||
resize: none; | ||
font-family:monospace; | ||
font-size:14px; | ||
white-space: pre; | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Browser_IDE/languageDefinitions.js
Outdated
compilerFiles: [ | ||
"compilers/javascript/javascriptCompiler.js", | ||
], | ||
compilerCommand: "JavaScript Patch", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm unclear on the naming conventions here. These 'compiler commands' seem more like 'compiler IDs'. They identify an entire source transformation tool, rather than one part of a source transformer, and don't take any kind of arguments. More subjectively, it seems better for the strings to be uniform, e.g. 'javascriptPatch' (or even simply 'javascript') rather than the plain English 'JavaScript Patch'. I don't believe they are ever presented to the user.
Browser_IDE/languageDefinitions.js
Outdated
// Some synonyms to make development easier | ||
SplashKitOnlineLanguageDefinitions["CXX"] = SplashKitOnlineLanguageDefinitions["C++"]; | ||
SplashKitOnlineLanguageDefinitions["C"] = SplashKitOnlineLanguageDefinitions["C++"]; | ||
SplashKitOnlineLanguageDefinitions["JS"] = SplashKitOnlineLanguageDefinitions["JavaScript"]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Somewhat forward-looking: This will make it difficult to get a list of all languages defined, which we will need in order to later give the user a choice of language for their project. It's rather minor, though, so it doesn't particularly matter; it would not be difficult for a future developer to address.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah that's a good point - the aliases probably belong inside the language definitions themselves anyway (this was a bit of a hack really...)
function findGetParameter(parameterName, _default=null) { | ||
location.search.substr(1).split("&").forEach(function (piece) { | ||
let tmp = piece.split("="); | ||
if (tmp[0] === parameterName) | ||
return decodeURIComponent(tmp[1]); | ||
}); | ||
|
||
return _default; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't find any place in which this function is actually used. Am I missing one?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was used...at some point... You can probably tell that splashKitOnlineEnvParams.js
was a bit rushed 馃槄 - will remove!
|
||
// global object that can be used to configure the IDE | ||
let SKO = { | ||
language: page_url.searchParams.get("language") ?? "JavaScript", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
URLSearchParams (searchParams) converts plus characters to spaces when interpreting the URL query string, so 'C++' will not actually work as a parameter value, and the error is somewhat abstruse: 'Unable to switch to language C , defaulting to JavaScript.'
Thanks so much for reviewing this! Really glad to know someone else was able to get it setup and working - did the instructions cover everything or were there issues? If they were alright I'll add a tidied up version of these instructions to the README too as part of the PR.
Ah that's a good one, I'll have a look and see if I can reproduce it.
Yes I was wondering about this too. As a stop-gap for this PR I might just make the notifications vanish after 1.5 seconds rather than 5, since this will largely mitigate the problem - does this seem alright? As another task, I was thinking of either adding a status bar like you suggest, or perhaps making the notifications themselves update (so a single notification might appear for 'Compiling...', perhaps with a progress bar, and then once compilation completes it would update to say 'Running project!', and then vanish after the usual few seconds). |
I didn't face any issues during set-up, no. The instructions are perfectly clear. Making the notifications disappear more quickly seems like a fine fix for now. |
# Conflicts: # Browser_IDE/editorMain.js
- SplashKitOnlineLanguageDefinitions is now an array - aliases are defined inside each definition - SplashKitOnlineLanguageAliasMap is generated from the names/aliases maps name -> language definition object - SKO.language now uses the undecoded URL query string, so 'C++' in URL works - editorMain now displays the list of languages available when it can't find the chosen language
- use font-family 'monospace, serif' instead of just 'monospace' to avoid browser adjusting font size from normal fonts. No need to set it ourselves now. - re-enable word wrapping (disabling this was accidental...) - darken background
- mane page sends signal every 0.5 seconds - worker expects it by at most 1 second since last one - this is mainly to prevent a build-up of messages to the main page when its paused for debugging - I _think_ it might also improve behaviour when a tab is unfocussed
- in the future perhaps it would be better if these were instead functions that return their respective compiler?
@jessbdeakin Alright I think I've addressed all the issues - fixed the debugging problem too 馃槃 Just going to update the README as well and then if it's okay with you I'll go ahead and merge it! |
- new paths for javascript files - info on how to setup c++ side
Alright README is updated too, so should be good to go hopefully. |
Though this is already merged, I nevertheless wanted to note that these changes all look great. I am excited to build on this feature. |
Description
This PR adds support for compiling and running C++ inside SplashKit Online. It is not feature complete - audio and filesystem access does not work. For the most part however it works quite well, and so I think it's still worth merging in now, with follow-up PRs coming for finishing up full feature support.
Testing
Getting the binaries
In order to test this, you'll need binaries for the following:
These can all be found in the branch here - just copy the files into the same spot in your local repo.
Browser_IDE/compilers/cxx/bin/*
compiler.zip
- I just had to compress the files in it to stay within GitHub's file size limit.Browser_IDE/runtimes/cxx/bin/*
Browser_IDE/runtimes/javascript/bin/*
Compiling the binaries
If you want to compile some of this yourself, currently the repository supports building both the C++/JavaScript runtimes, and the Compiler System Root Files (partially). Building the compilers themselves is not yet supported - I intend to make this reproducible soon though. To compile the C++ system root file, you'll need the following file:
SplashKitWasm/prebuilt/sysroot.zip
From there you can follow the instructions on the README, just use the following command instead:
(Compiling the JavaScript backend is enabled by default, while the C++ one currently needs to be enabled with
ENABLE_CPP_BACKEND=ON
. I also recommend the-j8
to speed up compilation, might cause trouble depending on machine specs though)Switching to C++
Once its all setup, you can head to
localhost:8000
, which will bring up the normal JavaScript runtime. To switch to C++, just go tolocalhost:8000/?language=CXX
- you'll probably also want to clickNew Project
to create the default C++ project. You can also uselocalhost:8000/?language=JavaScript
for JavaScript.Brief explanation of how this works
There's now a new file -
languageDefinitions.js
that defines the languages SplashKit supports. The languages are basically just a list of script files to include, either for compilation or the runtime.Compilation
C++ compilation and linking is all just done using Clang compiled to WebAssembly. This is actually pretty straightforward - the user's source files are written into
clang++
's filesystem, and clang is invoked to compile them. These object files are read out, and then written intowasm-ld
(the linker)'s filesystem, where it links them into a.wasm
(WebAssembly), which we can then instantiate and run. High level work is handled inBrowser_IDE/compilers/cxx/cxxCompiler.js
, and more detailed work inBrowser_IDE/compilers/cxx/cxxCompilerClangBackend.js
Couple of tricky things:
main
works, for about 8 tries. Clang internally has a signal array that fills up after 8 runs, afterwhich it starts aborting.Browser_IDE/compilers/cxx/cxxCompilerClangWebWorker.js
) - this way compilation doesn't block the main interface. It also has a positive that I've found WebWorkers clean up their memory better than the main page, so repeated refreshes no longer make the browser crash 馃槃Runtime
The runtime (
Browser_IDE/runtimes/cxx/cxxRuntime.js
) just (ab)uses the Emscripten generated JavaScript to run the user's compiled.wasm
rather than the one Emscripten generated the glue code for. This is a bit of a hack, but it works surprisingly well. To make it work we have to patch Emscripten's generated glue a bit - this is just done with a Python script as a post process (SplashKitWasm/tools/cxx_backend_emscripten_glue_patcher.py
).Tricky things:
SharedArrayBuffer
s, which allow the main page and WebWorker to share an array - then just read from that array in SplashKit'sprocess_events
to check for events. This works well, but it requires special HTTP headers we can't set on GitHub (so we couldn't host it on GitHub anymore), and has very iffy browser support.Browser_IDE/SKOservice-worker.js
) in between the main page and Web Worker, which acts as a proxy. We send events to it for queuing, and then when the WebWorker requests the file '/programEvents.js', we give it a JSON file filled with the latest events! (seeBrowser_IDE/runtimes/cxx/workerEventProcessor.js
) This is what is used in this PR.And that's the gist of it!
List of changes
Type of change
How Has This Been Tested?
C++ compilation and execution with the new default project has been tested on both desktops and mobile, on Firefox, Edge and Opera. Error reporting has been tested by adding random syntax errors - runtime errors have not been tested.
JavaScript execution has also been checked for regressions, with the default project and Cave Escape.
Testing Checklist
Checklist