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

Adds Emscripten platform #1423

Merged
merged 13 commits into from
Jan 30, 2022
Merged

Conversation

ericoporto
Copy link
Member

@ericoporto ericoporto commented Sep 30, 2021

This is for the Web Assembly + HTML + JS version of the engine for the web. Redoing of #1346

Just updating the Emscripten PR, adding CI.

It's using the latest Emscripten tools, but we can always use 2.0.16, which is "old", but may be easier to work with.

Following bugs to fix in upstream, using workaround for now:

Adding a build target on the Editor is something I would prefer to do in a later PR. Here's the preview: https://github.com/ericoporto/ags/tree/feature-webbuild

Edit: I also found a bug in Chrome for Android, it's been tracked here: https://bugs.chromium.org/p/chromium/issues/detail?id=1285389 (I can only reproduce the bug in one of my phones though, and a particular emulator config)

Fix has been commited: https://chromium.googlesource.com/v8/v8/+/ed1cd8f10505c2d8da0d9f896e7589b1299dc95b

@ericoporto ericoporto changed the title Adds Emscripten Adds Emscripten platform Sep 30, 2021
@ericoporto ericoporto force-pushed the emscripten branch 6 times, most recently from bf96636 to f278ae8 Compare October 1, 2021 01:28
@ericoporto ericoporto force-pushed the emscripten branch 15 times, most recently from 2d1f68a to 464e1e2 Compare January 1, 2022 11:40
@ericoporto ericoporto marked this pull request as ready for review January 1, 2022 11:41
@ericoporto
Copy link
Member Author

ericoporto commented Jan 1, 2022

This PR adds a new build_emscripten task which uses latest Emscripten to build AGS source code. This new task has it's own artifacts, which contain:

  • ags.html
  • ags.js
  • ags.wasm

by running ags.html on webserver and loading it in a browser, you should be able to play AGS games on it. I updated the version below with the result of the previous latest build from here:

https://ericoporto.github.io/agsjs/

Only thing missing now is probably a README in the Emscripten directory, but not sure what to write on it.

@ivan-mogilko
Copy link
Contributor

ivan-mogilko commented Jan 2, 2022

IMHO it would be nice to have all the description from the old PR here, because I had a number of questions and only found explanation in the old PR's description. So I have to refer to it too.

@@ -82,19 +87,25 @@ void WaitForNextFrame()

auto frame_time_remaining = next_frame_timestamp - now;
if (frame_time_remaining > std::chrono::milliseconds::zero()) {
#if AGS_PLATFORM_OS_EMSCRIPTEN
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question: should not this logically check for AGS_DISABLE_THREADS?
if not, what is the reason that Emscripten can work with SDL_Delay but not std:: thread sleep?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Theoretically both should work, Asyncify should work with either of them - this is the spot Asyncify should unroll the stack, allow the browser to do it's work and come back. But for some reason the result with std::thread currently is the browser getting unresponsive - which is weird since it's written by Emscripten people. SDL_Delay appears to use some JS wait function when running with Emscripten, and it's working.

Copy link
Member Author

@ericoporto ericoporto Jan 2, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, I forgot, you can enable threads which will run the Sound thread through a JS Worker (Emscripten pthreads), but the "main" thread runs on the browser and still requires the browser to have some time to draw and process inputs. So the SDL_Delay is just because it's Emscripten, but the sounds parts are the difference between with thread and without threads.

@ericoporto
Copy link
Member Author

@ivan-mogilko did you understand the building process? I can add a bat script too if you want - with the lines in the README.

@ivan-mogilko
Copy link
Contributor

ivan-mogilko commented Jan 9, 2022

So, I tried this, and soon got into the first problem: emsdk requires Python 3.9, which apparently does not support Windows 7 (which i'm still using). I will try on Windows 10 VM, unless there's a way to use lower version of Python with emsdk?

In any case it would be nice to mention system limitations of this building process.

@ericoporto
Copy link
Member Author

ericoporto commented Jan 9, 2022

I think anything above python 3.6 should be good (due to a change in SSL on GitHub/ old OpenSSL in python), but Windows specifically requires python above 3.9.2 it appears, which is only compatible with Windows 10 and 11 - in Windows 10, you just type python on the terminal once and it will prompt an install of python through the Windows package mangement system through the MS Store.

Edit: I added a bit more info in the README, i can squash that and the previous commit in the other commits that create the files they edith in this branch before merge.

@ivan-mogilko
Copy link
Contributor

Hmm, maybe it will be easier to do this on Linux, as I have git and repositories installed there.

@ivan-mogilko
Copy link
Contributor

ivan-mogilko commented Jan 10, 2022

Ok, sorry for delay (i got distracted again), I managed to install and activate emsdk on my Windows 10 virtual machine, so will continue from there.

in Windows 10, you just type python on the terminal once and it will prompt an install of python through the Windows package mangement system

This does not work for me btw, I am using a stripped down version of OS, it does not have much set up; so anyway I just install everything by hand.

@ivan-mogilko
Copy link
Contributor

ivan-mogilko commented Jan 10, 2022

Alright, the success! There were some hiccups, but eventually i've got a random ags game working in a browser.
For some reason I thought that I have to package each game on its own, but this is simply a universal launcher now. Or will there be an individual game packaging afterwards?

Few notes in regards to the building process.

  1. pip - does it refer to Python's package manager? In any case, I did not need to install it, or maybe it got installed along with the python itself, not sure, nothing complained about it missing.
  2. The first step of the building process, emcmake cmake ../.. -DCMAKE_BUILD_TYPE=Release, really took a while as was warned, and there is no progress counter or anything. I just wondered, is it something that we may adjust and add some messages to notify user that the process did not hang; or this is how emsdk works internally and nothing may be done?

On running.

  1. python3 -m http.server command is likely python -m http.server on Windows 10, at least this is what I had to run.
  2. On launch the client in browser loads full CPU core to 100%!! even without any game. It stays like that for 20-30 seconds, then goes down. Do you know why that may be happening? May that depend on a browser too?

In regards to this PR, the Emscripten readme link need to be added to the root readme, where we have all the ports listed.

@ericoporto
Copy link
Member Author

really took a while as was warned, and there is no progress counter or anything.

The emcmake cmake ... will download AGS dependencies, I am not sure if it's the things that I download from Emscripten ports (ogg, freetype, ...) but they take a while the first time, once you type ninja it downloads more things, like the dependencies of the dependencies, from emscripten itself now. After the first setup it's cached somewhere in the emsdk dir, because you can delete the build for contents and run everything again and things should go faster.

there be an individual game packaging afterwards?

The engine is the same, just the html ui is different, so I added it in the same directory, you can just use the other after build instead - no need to change the command line parameters. I will add a note in the readme to clarify this.

python3 -m http.server command is likely python -m http.server

Uhm, there may be differences depending on how python is installed, but python should always work on Windows 10, so I will amend that in the readme.

On launch the client in browser loads full CPU core to 100%!! even without any game. It stays like that for 20-30 seconds, then goes down. Do you know why that may be happening? May that depend on a browser too?

I don't experience this, but right at launch it downloads the ags.wasm and does the initial allocation for it - doesn't "run" it since that requires a game. I wonder if it's because you are using the browser in a VM. :/ Did you experience this in your Windows 7 machine too?

Ah, yes this reminds, the port is not perfect, it has a different FreeType version and some fonts show up slightly different because of it. I could not manage porting the specific version we use. Emscripten ports has a different FreeType version and current main FreeType branch is also compatible with Emscripten (it can be built and linked directly). The newer FreeType mostly works fine.

@ivan-mogilko
Copy link
Contributor

ivan-mogilko commented Jan 12, 2022

My last nitpick in this PR here is this code: https://github.com/ericoporto/ags/blob/emscripten/Engine/main/graphics_mode.cpp#L542-L548

The function graphics_mode_update_render_frame is supposed to be called as a reaction to window resize, after the new display mode is already set; so resizing screen again here seems like a reverse logic. Also, you are setting screen_size to something different from the display mode of the gfx driver, which may mean that there will be inconsistency between these?
That, and it's not good to do actions depending on the name of the gfx driver.

When we receive SDL event SDL_WINDOWEVENT_SIZE_CHANGED, we first of all call gfxDriver->UpdateDeviceScreen, this is where graphics driver must update its display mode. Here's the Software renderer's implementation:https://github.com/ericoporto/ags/blob/emscripten/Engine/gfx/ali3dsw.cpp#L162.
Would it be possible to place this code there instead?

I.e. do something like:

void SDLRendererGraphicsDriver::UpdateDeviceScreen(const Size &screen_sz)
{
    // ...... other code
#if AGS_PLATFORM_OS_EMSCRIPTEN
    if(_mode.IsWindowed())
    {
        _mode.Width = get_canvas_width();
        _mode.Height = get_canvas_height();
        do_resize_after_time();
    }
#endif
}

Is this action actually necessary only for the software renderer, or is it just that only software renderer works for Emscripten now?

@ericoporto
Copy link
Member Author

ericoporto commented Jan 12, 2022

Hey, I will look into this, if I remember correctly it's actually just other renderers except software, I think I pulled the hack from SDL renderer source - basically the browser has a delay between changing the size of the canvas and the resize of the screen/browser.

I will introduce an additional change in the CI for Emscripten have it's own docker image so the dependencies, emsdk stdc++ and other stuff is prebuilt once to avoid it breaking if some upstream server is not working for whatever reason or upstream breaks something (e.g.: current fix is emscripten-core/emscripten#15985)

Edit: my fix was merged upstream, once there's a release I will pin it :)

@ericoporto
Copy link
Member Author

ericoporto commented Jan 23, 2022

I dropped the resize change commit, it appears it's not needed for it work with current version of Emscripten and SDL2. I also rebased the build. Updated the build result in here: https://ericoporto.github.io/agsjs/ (but not the single game example hosted there, that one is attached to a Google Chrome bug report :/)

Should we need, the original PR I kept in a different branch here: https://github.com/ericoporto/ags/tree/emscripten-orig-pr (including ericoporto@b4728de)

Also, if this is merged before the sound changes PR I think that may have merge conflicts, so merge that before, rebasing and adjusting this one is much simpler.

@ivan-mogilko
Copy link
Contributor

ivan-mogilko commented Jan 30, 2022

Sorry, looks like i missed the last comment again.

This seem fine now, the changes to the engine code are minimal, and mostly under the AGS_DISABLE_THREADS macro.

EDIT:

Also, if this is merged before the sound changes PR I think that may have merge conflicts, so merge that before, rebasing and adjusting this one is much simpler.

I was not certain, what PR does this refer to...

@ivan-mogilko ivan-mogilko merged commit eed6c2c into adventuregamestudio:master Jan 30, 2022
@ericoporto ericoporto deleted the emscripten branch January 31, 2022 23:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants