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

Emscripten support #7510

Closed
wants to merge 14 commits into from
Closed

Emscripten support #7510

wants to merge 14 commits into from

Conversation

Milek7
Copy link
Contributor

@Milek7 Milek7 commented Apr 13, 2019

This is currently WIP, but I would appreciate feedback on it.
Libtimidity is restored with mixer output, Emscripten support is added and various fixes for Emscripten quirks are applied.
Dockerfile for build environment is here: https://github.com/Milek7/openttd-emscripten-dockerfile/tree/sdl1

It currently works on SDL1 on 32bpp blitter (8bpp blitter doesn't work, I suspect broken SDL palette support in emscripten shim). It doesn't look correct though, because html provides BGRA buffer and all blitters assume RGBA. Modifying union Colour works, but this is hacky solution.

It works too on SDL2 branch, with significantly better performance, both on 8bpp (with correct colors) and 32bpp (same BGRA issue, but it is faster than 8bpp). Maybe this PR should wait on SDL2, as it works much better on it.

Network is handled by emscripten as WebSockets, both UDP and TCP. It requires proxy, like this one: https://gist.github.com/Milek7/b921ec9f8d875fe2d5a8b06bfd533834
sockaddr sizes and NetworkAddress comparing needs to be investigated, current workaround is to delete domain name after resolving, and comparing generated hostnames by strcmp. Using original code caused duplicated entries in gamelist as UDP queries received weren't matched to correct entry.

@glx22
Copy link
Contributor

glx22 commented Apr 14, 2019

It's hidden in the commit-checker log but

*** b/src/video/sdl_v.h:56: Use tabs for indentation: '    uint32 cur_ticks;'
*** b/src/video/sdl_v.h:57: Use tabs for indentation: '    uint32 last_cur_ticks;'
*** b/src/video/sdl_v.h:58: Use tabs for indentation: '    uint32 next_tick;'
*** b/src/video/sdl_v.h:59: Use tabs for indentation: '    std::thread draw_thread;'
*** b/src/video/sdl_v.h:60: Use tabs for indentation: '    std::unique_lock<std::recursive_mutex> draw_lock;'

src/network/core/address.cpp Outdated Show resolved Hide resolved
src/network/core/address.cpp Show resolved Hide resolved
src/network/core/address.cpp Outdated Show resolved Hide resolved
src/network/core/address.cpp Show resolved Hide resolved
src/network/core/address.cpp Show resolved Hide resolved
src/network/core/address.h Outdated Show resolved Hide resolved
src/video/sdl_v.cpp Outdated Show resolved Hide resolved
src/video/sdl_v.cpp Show resolved Hide resolved
@@ -14,8 +14,16 @@

#include "video_driver.hpp"

#ifdef __EMSCRIPTEN__
void em_loop(void *arg);
Copy link
Member

Choose a reason for hiding this comment

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

is there a particular reason not to make this a (conditional) class method? would save the duplicate friend definition

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is called by function pointer by emscripten. Is it safe to assume that calling convention would match?

@@ -42,6 +52,12 @@ class VideoDriver_SDL : public VideoDriver {
int PollEvent();
bool CreateMainSurface(uint w, uint h);
void SetupKeyboard();

uint32 cur_ticks;
Copy link
Member

Choose a reason for hiding this comment

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

I don't like that these are necessary at all...

src/3rdparty/squirrel/squirrel/sqvm.cpp Show resolved Hide resolved
@Milek7
Copy link
Contributor Author

Milek7 commented Sep 17, 2019

waiting for #7086 to merge both sdl1 and sdl2 emscripten changes at same time

@Milek7 Milek7 closed this Sep 17, 2019
@Milek7 Milek7 reopened this Sep 19, 2019
@Milek7
Copy link
Contributor Author

Milek7 commented Dec 7, 2020

Superseded by #8355
I might create separate PR for libtimidity later.

@Milek7 Milek7 closed this Dec 7, 2020
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.

None yet

3 participants