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

DISTS: Initial Emscripten Demo / Proof of Concept #3046

Merged
merged 14 commits into from
Aug 20, 2021

Conversation

chkuendig
Copy link
Member

@chkuendig chkuendig commented Jun 4, 2021

This is an initial version of my Emscripten/Webassembly target as mentioned on Discord a few days ago (see video https://discord.com/channels/581224060529148060/581224061091446795/849353211877523536)

There's still a lot to do depending on how such a build would be deployed. I summarized quick build instructions and a summary of what's done and what's still missing at ./dists/emscripten/README.md.

I built a demo based on this (running ./dists/emscripten/build.sh all) and uploaded it to http://scummvm.kuendig.io/scummvm.html. This includes some freeware games, some demos and the ScummVM testbed.

I only tested this on macOS, but at least in theory this should also build on Linux and WSL (or anything else that can run shell scripts and is supported by emsdk).

Update 7th June: I fixed a few more minor issues and updated the demo at http://scummvm.kuendig.io/scummvm.html - feel free to retest. Let me know what you think. I'll keep improving this in the coming days and address any further review comments.

Update 21st July: I didn't have much time in the last few weeks to work on this. I still plan though to get this merged this summer/asap. But it might take a few more weeks until I'll be able to address the remaining reviews.

engines/grim/grim.cpp Outdated Show resolved Hide resolved
graphics/opengl/framebuffer.cpp Outdated Show resolved Hide resolved
graphics/opengl/shader.cpp Outdated Show resolved Hide resolved
@chkuendig chkuendig force-pushed the emscripten branch 4 times, most recently from 8d3300d to 40ef4a2 Compare June 4, 2021 23:08
@chkuendig
Copy link
Member Author

chkuendig commented Jun 4, 2021

I tried to fix most codacy errors. I'm not sure if the remaining once have any relevance.

Update 6/6: I fixed them all 🙂

configure Show resolved Hide resolved
- too verbose logging slows down everything
- datadir / causes the whole FS to be scanned at launch, including files which are loaded lazily via XHR
- corrupt default data in local storage
- double-slash in path of VORBIS_LIBS
@chkuendig
Copy link
Member Author

chkuendig commented Jun 8, 2021

I pushed a few more minor improvements and bug fixes today. There's still a few minor bugs and quite a lot potential improvements and I could probably work on this branch for the foreseeable future (and mark this PR as a draft).

OTOH I was wondering what would be a good point to get this initial state merged from the projects perspective? I could then focus on this and postpone a few of the less important improvements.

Copy link
Member

@sev- sev- left a comment

Choose a reason for hiding this comment

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

I made a couple more minor comments.

I think we may merge it as soon as those are addressed. Would you like to join the team as an official porter?

A question, is it possible to somehow create a standalone version from it? E.g. basically a release build?

I love this port, and while of course, it is a resource hog, I was able to just open it and play on my non-jailbroken iPad. Yay!

dists/emscripten/build.sh Show resolved Hide resolved
dists/emscripten/build.sh Show resolved Hide resolved
dists/emscripten/build.sh Outdated Show resolved Hide resolved
@chkuendig
Copy link
Member Author

I made a couple more minor comments.

I think we may merge it as soon as those are addressed. Would you like to join the team as an official porter?

I'll address these as soon as possible. Would love to join. Does this involve anything I should know about that?

A question, is it possible to somehow create a standalone version from it? E.g. basically a release build?

Yes, my idea was to get this to build some type of bare bone minimal release. I'll see that I'll add this. At the moment this is a bit hobbled together with the bundling of the demos.

@neuromancer
Copy link
Contributor

I tested the live demo from http://scummvm.kuendig.io/scummvm.html in the Switch browser and it doesn't work. Nintendo probably disabled some HTML5 APIs to avoid people playing games there 😞

@sev-
Copy link
Member

sev- commented Jul 8, 2021

Not much is involved in being a porter except continue to show love for your port, address blocker issues (if any) and be around the release time which is coordinated via scummvm-devel mailing list.

Looking forward to the review issues addressed and the port merged.

@sev-
Copy link
Member

sev- commented Jul 27, 2021

@chkuendig any update?

@chkuendig
Copy link
Member Author

Sorry, I didn't have much free time in the last few weeks to work on this. I still plan though to get this merged asap. But it might take a few more weeks until I'll be able to address everything.

@chkuendig chkuendig force-pushed the emscripten branch 2 times, most recently from 9aa4768 to f3c2018 Compare August 1, 2021 14:41
@chkuendig
Copy link
Member Author

Fixed the last minor issues. There's three larger topics I plan to tackle later:

  • How do best bundle demos / scope of an official release build
  • Better file loading in general (range requests, async loading etc.)
  • Dynamic plugin support (so we can have one build supporting all games without blowing up build and load times)

Happy to join the team as a porter.

@chkuendig chkuendig requested a review from sev- August 19, 2021 15:47
@@ -1117,6 +1117,13 @@ void GrimEngine::mainLoop() {
g_imuseState = -1;
}

#if defined(__EMSCRIPTEN__)
Copy link
Member

Choose a reason for hiding this comment

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

The preprocessor directives must have zero indentation, e.g. start from the column 1 per our code formatting giudelines.

glRenderbufferStorage(GL_RENDERBUFFER, GL_STENCIL_INDEX8, _texWidth, _texHeight);
glFramebufferRenderbuffer(GL_FRAMEBUFFER, GL_STENCIL_ATTACHMENT, GL_RENDERBUFFER, _renderBuffers[1]);
const char *glVersion = (const char *)glGetString(GL_VERSION);
if (strstr(glVersion,"WebGL 1.0") != NULL) {
Copy link
Member

Choose a reason for hiding this comment

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

Please, add space after the comma

if (strstr(glVersion,"WebGL 1.0") != NULL) {
// See https://www.khronos.org/registry/webgl/specs/latest/1.0/#FBO_ATTACHMENTS
// and https://github.com/emscripten-core/emscripten/issues/4832
#ifndef GL_DEPTH_STENCIL
Copy link
Member

Choose a reason for hiding this comment

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

Same thing regarding the preprocessor directives

@sev-
Copy link
Member

sev- commented Aug 20, 2021

Just minor formatting issues and we're fine to merge.

@sev-
Copy link
Member

sev- commented Aug 20, 2021

Actually, let me merge it and then fix after by myself.

@sev- sev- merged commit dd3b93b into scummvm:master Aug 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants