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

Improve Emscripten support #857

Closed
wants to merge 24 commits into from
Closed

Conversation

cactorium
Copy link

Mostly backporting winit functions into emscripten's platform module. It compiles and works against a modified version of badboy's glium example: https://github.com/cactorium/spring-crabs/tree/emscripten

@tomaka
Copy link
Contributor

tomaka commented Feb 13, 2017

Err.. I don't understand what you're trying to achieve, but don't duplicate winit. Just continue importing it even on emscripten, but just don't call any of its functions.

@cactorium
Copy link
Author

That breaks because winit doesn't support emscripten:

   Compiling winit v0.5.10
error[E0432]: unresolved import `self::platform::*`
 --> /home/kelvin/.cargo/registry/src/gh.neting.cc-1ecc6299db9ec823/winit-0.5.10/src/platform/mod.rs:1:9
  |
1 | pub use self::platform::*;
  |         ^^^^^^^^^^^^^^^^^^ Could not find `platform` in `platform`

error[E0432]: unresolved import `this_platform_is_not_supported`
  --> /home/kelvin/.cargo/registry/src/gh.neting.cc-1ecc6299db9ec823/winit-0.5.10/src/platform/mod.rs:22:5
   |
22 | use this_platform_is_not_supported;
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ no `this_platform_is_not_supported` in the root

# etc...

Like it's impossible to get it to compile for emscripten, unless there's some flag or trick that I don't know about?

@tomaka
Copy link
Contributor

tomaka commented Feb 13, 2017

Winit should get a "null" backend which always returns an error when you try to use it.

@cactorium
Copy link
Author

cactorium commented Feb 13, 2017

winit's got this thing in src/platform/mod.rs that's causing the error whenever you're targetting asmjs-unknown-emscripten:

#[cfg(all(not(target_os = "ios"), not(target_os = "windows"), not(target_os = "linux"),
  not(target_os = "macos"), not(target_os = "android"), not(target_os = "dragonfly"),
  not(target_os = "freebsd"), not(target_os = "openbsd")))]
use this_platform_is_not_supported;

A fresh pull of the master branch of winit fails to compile with target asmjs-unknown-emscripten:

[kelvin@batmanbatman repos]$ cd winit/
[kelvin@batmanbatman winit]$ git pull
Already up-to-date.
[kelvin@batmanbatman winit]$ cargo build --target asmjs-unknown-emscripten
warning: unused manifest key: package.categories
    Updating registry `https://github.com/rust-lang/crates.io-index`
   Compiling libc v0.2.20
   Compiling lazy_static v0.2.2
   Compiling shared_library v0.1.5
   Compiling winit v0.5.10 (file:///home/kelvin/repos/winit)
error[E0432]: unresolved import `self::platform::*`
 --> src/platform/mod.rs:1:9
  |
1 | pub use self::platform::*;
  |         ^^^^^^^^^^^^^^^^^^ Could not find `platform` in `platform`

error[E0432]: unresolved import `this_platform_is_not_supported`
  --> src/platform/mod.rs:22:5
   |
22 | use this_platform_is_not_supported;
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ no `this_platform_is_not_supported` in the root

error[E0433]: failed to resolve. Could not find `Window2` in `platform`
   --> src/window.rs:114:22
    |
114 |         let w = try!(platform::Window2::new(events_loop.events_loop.clone(), &self.window, &self.platform_specific));
    |                      ^^^^^^^^^^^^^^^^^^^^^^ Could not find `Window2` in `platform`

error[E0412]: type name `platform::MonitorId` is undefined or not in scope
   --> src/window.rs:306:24
    |
306 |     data: VecDequeIter<platform::MonitorId>,
    |                        ^^^^^^^^^^^^^^^^^^^ undefined or not in scope
    |
...

@tomaka
Copy link
Contributor

tomaka commented Feb 13, 2017

Yes, what I mean is that winit should be modified to add a "null" implementation that is used instead of this "this_platform_is_not_supported" thing.

@cactorium
Copy link
Author

Ohhh okay, sorry!

@cactorium
Copy link
Author

So I think I got the default platform thing working, but I can't test because of the winit API transition thing; glutin doesn't compile with either emscripten or linux against the tip of winit, with or without my changes... Any recommendations on what to do next?

@tomaka
Copy link
Contributor

tomaka commented Feb 13, 2017

Oh, I forgot about that.
You could do your changes against the dev-0.5 branch of winit.
This branch still uses the old API.

winit_builder: winit::WindowBuilder)
-> Result<Window, CreationError> {

let winit_window = winit_builder.build().unwrap();
Copy link
Contributor

Choose a reason for hiding this comment

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

The idea is that you wouldn't use winit at all. Just don't call it, and remove the winit_window field from the struct.

@@ -190,7 +161,7 @@ impl Window {

#[inline]
pub fn create_window_proxy(&self) -> WindowProxy {
WindowProxy
self.winit_window.create_window_proxy()
Copy link
Contributor

Choose a reason for hiding this comment

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

It's fine to panic! here. Nobody uses the window proxy, and it's going to be removed soon.

@cactorium
Copy link
Author

Fixed both I think

@cactorium
Copy link
Author

So I've got some work done on adding in the hooks for mouse/keyboard events as well, would those be good to add to this PR, or would opening a new PR after this one's merged be better?

@tomaka
Copy link
Contributor

tomaka commented Feb 14, 2017

@cactorium As you wish.
Basically I wanted to publish winit 0.5.11 (which I forgot to do yesterday), then ask you to tweak the Cargo.toml of glutin in this PR to point to winit 0.5.11, then merge.

@cactorium
Copy link
Author

So I guess I'll see how far I get before winit 0.5.11 gets published and submit whatever work I get done by then? Most of the event stuff should be working pretty solidly right now; the biggest hole left is converting to keys to VirtualKeyCodes

@cactorium
Copy link
Author

All's ready except for the version change

@gui1117
Copy link

gui1117 commented Feb 15, 2017

I just want to mention that I started implementing emscripten plateform in winit there

I included keyboard but mouse because there was an issue with mousemove when the cursor is grabbed

@tomaka
Copy link
Contributor

tomaka commented Feb 15, 2017

I'm hesitating between putting the emscripten stuff in glutin or in winit.

The initial reason why I wanted to stick to glutin is that you can only use OpenGL with emscripten anyway, and that the concept of a "window" is a bit blurry when it comes to web pages.
That being said Android has the same problem where you're manipulating a window but the whole screen.

But that's a pretty weak argument, so I'm not sure.

@cactorium
Copy link
Author

cactorium commented Feb 16, 2017 via email

@cactorium
Copy link
Author

My bad, I missed the version bump on winit. Cargo.toml's updated

thiolliere added 4 commits March 19, 2017 18:12
grab mode use pointerlock https://www.w3.org/TR/pointerlock/
this behavior is not the same as other backend.

I wanted this because as set_cursor_position is not available on
emscripten backend. the only way to have a FPS camera is to use
pointerlock. So you can use grab to have FPS camera on emscripten.

also show_mouse is implement in javascript as in a pull request that
hasn't been merged emscripten-core/emscripten#4616
cactorium and others added 8 commits March 20, 2017 21:12
two fixes:
* when mouse is grabbed send relative mousemove event
* when mouse grabbed is refused, ask again.
Implement touch + fix mouse grab + catch more errors
@gui1117
Copy link

gui1117 commented May 3, 2017

I got an issue when I use serde before building window.
I don't know what to do with it : https://github.com/thiolliere/spring-crabs/

@cactorium
Copy link
Author

What errors are you getting? It looks like it runs fine on my computer

@gui1117
Copy link

gui1117 commented May 3, 2017

it works on rustc 1.18.0-beta.1 (4dce67253 2017-04-25) but on rustc 1.17.0 (56124baa9 2017-04-24) it fails when running on the web:

Successfully compiled asm.js code (total compilation time 2664ms; storage initialization failed (consider filing a bug); 4 functions compiled slowly: $j:12:8 (507ms), fj:16:8 (324ms), Sj:13:8 (808ms), dj:6:8 (2602ms)) spring-crabs.js
emscripten_webgl_create_context failed: explicitSwapControl is not supported, please rebuild with -s OFFSCREENCANVAS_SUPPORT=1 to enable targeting the experimental OffscreenCanvas specification! spring-crabs.js:1:467981

thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: BackendCreationError(OsError("Error while calling emscripten_webgl_create_context: Internal error in the library (success detected as failure)"))', /checkout/src/libcore/result.rs:859 index.htm:141:13

note: Run with `RUST_BACKTRACE=1` for a backtrace. index.htm:141:13

@gui1117
Copy link

gui1117 commented May 3, 2017

I rewrite the example. Now it fails also on beta also.

Also I don't know if this is the right place for discussing on this issue. I think it is related to emscripten backend itself.

@gui1117
Copy link

gui1117 commented May 8, 2017

I have an issue related to hidpi_factor.

On some smartphone (I suspect with HDPI of 2) my game was only showing the bottom-left quarter of the expected view. I solved it by forcing hdpi_factor method to always return 1.

Also I found that our implementation of get_inner_size is not coherent with winit implementation.

I don't have any hardware with HDPI different from one to test.
I can propose to write get_inner_size_ as winit do and forcing hdpi_factor to one but that doesn't sound very good...

@cactorium
Copy link
Author

cactorium commented May 10, 2017

Would something like this fix it? You probably have more experience with dealing with emscripten than me at this point, but it looks like using emscripten_request_fullscreen_strategy with scalingMode set to one of the EMSCRIPTEN_FULLSCREEN_CANVAS_SCALE_* options might be an easier (but maybe not better? I'm not sure) workaround

     #[inline]
     pub fn get_inner_size_pixels(&self) -> Option<(u32, u32)> {
-        self.get_inner_size()
+        self.get_inner_size().map(|(w, h)| {
+            let hdpi = self.hdpi_factor();
+            ((hdpi*(w as f32)) as u32, (hdpi*(h as f32)) as u32)
+        })
     }

Also what versions of stuff are you running? I can't build against the latest of everything because of the new EventLoop system (btw @tomaka would it make sense to make another PR to get this stuff working with the 0.8.0 version of glutin/newer version of winit?)

@LukasKalbertodt
Copy link

Any updates on this?

@cactorium
Copy link
Author

Not as far as I know, but the code here's dated now, since winit switched the EventLoop thing at 0.6. I'm working on redoing the work here to the latest glutin, but there hasn't been any word from the maintainers about it. My fork's master branch might be usable right now, in case anyone needs emscripten working right now

@aep
Copy link

aep commented Jul 4, 2017

@cactorium actually your fork doesnt work out of the box, or am i missing something?

aep@derp: ~/kram/glutin cargo build --target=wasm32-unknown-emscripten --release
   Compiling lazy_static v0.2.8
   Compiling khronos_api v1.0.1
   Compiling bitflags v0.9.1
   Compiling libc v0.2.24
   Compiling log v0.3.8
   Compiling xml-rs v0.6.0
   Compiling shared_library v0.1.6
   Compiling winit v0.6.4
error[E0432]: unresolved import `self::platform::*`
 --> /home/aep/.cargo/registry/src/gh.neting.cc-1ecc6299db9ec823/winit-0.6.4/src/platform/mod.rs:1:9
  |
1 | pub use self::platform::*;
  |         ^^^^^^^^^^^^^^^^^^ Could not find `platform` in `self`

error[E0432]: unresolved import `this_platform_is_not_supported`
  --> /home/aep/.cargo/registry/src/gh.neting.cc-1ecc6299db9ec823/winit-0.6.4/src/platform/mod.rs:22:5
   |
22 | use this_platform_is_not_supported;
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ no `this_platform_is_not_supported` in the root

error[E0433]: failed to resolve. Could not find `Window2` in `platform`
   --> /home/aep/.cargo/registry/src/gh.neting.cc-1ecc6299db9ec823/winit-0.6.4/src/window.rs:113:22
    |
113 |         let w = try!(platform::Window2::new(events_loop.events_loop.clone(), &self.window, &self.platform_specific));
    |                      ^^^^^^^^^^^^^^^^^^^^^^ Could not find `Window2` in `platform`

[...]

@cactorium
Copy link
Author

@aep Hmm, it looks like one of the pull requests I merged broke some stuff, and I can't be bothered with trying to fix it if it'll basically be stuck at the version 0.7 anyways. It looks like "emscripten_events" still builds correctly, so I guess you could use that one for now

@francesca64
Copy link
Member

I'm guessing this PR has been subsumed by subsequent additions to winit?

@goddessfreya
Copy link
Contributor

Closing for inactivity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

7 participants