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

Update to glutin =0.22.0-alpha5, remove wasm_stub #260

Merged
merged 22 commits into from
Jan 6, 2020
Merged

Update to glutin =0.22.0-alpha5, remove wasm_stub #260

merged 22 commits into from
Jan 6, 2020

Conversation

iceiix
Copy link
Owner

@iceiix iceiix commented Jan 5, 2020

Update to 0.22.0 alpha 5 of glutin, removing the temporary custom wasm_stub placeholder forks https://github.com/iceiix/glutin/tree/wasm_stub and https://github.com/iceiix/winit/tree/wasm_stub, for #171 and #34

@iceiix

This comment has been minimized.

@iceiix
Copy link
Owner Author

iceiix commented Jan 5, 2020

error[E0107]: wrong number of type arguments: expected 1, found 0
   --> src/main.rs:370:31
    |
370 |                        event: glutin::event::Event) {
    |                               ^^^^^^^^^^^^^^^^^^^^ expected 1 type argument

Event is now generic over <T>: https://docs.rs/glutin/0.22.0-alpha5/glutin/event/enum.Event.htmt, for UserEvent(T).

@iceiix

This comment has been minimized.

@iceiix
Copy link
Owner Author

iceiix commented Jan 5, 2020

Fixed some of the errors, but what happened to grab_cursor and hide_cursor?

error[E0599]: no method named `grab_cursor` found for type `&winit::window::Window` in the current scope
   --> src/main.rs:396:37
    |
396 |                     window.window().grab_cursor(true).unwrap();
    |                                     ^^^^^^^^^^^ method not found in `&winit::window::Window`

error[E0599]: no method named `hide_cursor` found for type `&winit::window::Window` in the current scope
   --> src/main.rs:397:37
    |
397 |                     window.window().hide_cursor(true);
    |                

No mention of grab_cursor in glutin, but it can be found in winit… as new methods to migrate to, not from:

CHANGELOG.md
328:- **Breaking:** `Window::set_cursor_state` and `CursorState` enum removed in favor of the more composable `Window::grab_cursor` and `Window::hide_cursor`. As a result, grabbing the cursor no longer automatically hides it; you must call both methods to retain the old behavior on Windows and macOS. `Cursor::NoneCursor` has been removed, as it's no longer useful.

src/platform_impl/linux/wayland/window.rs
400:    pub grab_cursor: Option<bool>,
465:                grab_cursor: window.cursor_grab_changed.lock().unwrap().take(),

src/platform_impl/linux/wayland/event_loop.rs
740:            if let Some(grab_cursor) = window.grab_cursor {
741:                let surface = if grab_cursor {

This was back in "Version 0.16.0 (2018-06-25)".

Changed again in rust-windowing/winit@0df4369#diff-069ac86c0936036493207ee6822daae2L48, set_cursor_grab()

@iceiix
Copy link
Owner Author

iceiix commented Jan 5, 2020

Now compiles with no errors, a few warnings, but a bigger problem is the changing of the event loop model, from poll_events to run. It was changed for good reason: web compatibility. winit explains:

//! Winit no longer uses a EventLoop::poll_events() -> impl Iterator<Event>-based event loop model, since that can't be implemented properly on web and mobile platforms and works poorly on most desktop platforms. However, this model can be re-implemented to an extent on desktops with [EventLoopExtDesktop::run_return]. See that method's documentation for more reasons about why it's discouraged, beyond mobile/web compatibility reasons.

https://docs.rs/glutin/0.22.0-alpha5/glutin/event_loop/struct.EventLoop.html#method.run

Hijacks the calling thread and initializes the winit event loop with the provided closure.

https://docs.rs/glutin/0.22.0-alpha5/glutin/event_loop/enum.ControlFlow.html

Currently, we have a game loop while !game.should_close {, at the end events_loop.run is called, but it never returns, so although the app receives events it doesn't update:

Screen Shot 2020-01-05 at 2 03 26 PM

will have to restructure these loops... let the winit event loop take control (move game logic into run and control flow = poll?)

@iceiix
Copy link
Owner Author

iceiix commented Jan 5, 2020

Discussion in alacritty/alacritty#2438 (comment) explains:

The idea is to use run and have Winit drive the application's event loop

There is run_return: https://docs.rs/glutin/0.22.0-alpha5/glutin/platform/desktop/trait.EventLoopExtDesktop.html#tymethod.run_return - but it has serious caveats:

Despite its apperance at first glance, this is not a perfect replacement for poll_events. For example, this function will not return on Windows or macOS while a window is getting resized, resulting in all application logic outside of the event_handler closure not running until the resize operation ends. Other OS operations may also result in such freezes. This behavior is caused by fundamental limitations in the underyling OS APIs, which cannot be hidden by Winit without severe stability reprecussions.

You are strongly encouraged to use run, unless the use of this is absolutely necessary.

Trying to use run_return, it sort of works but doesn't update the game logic when no events are occurring. It will take some more work, but ought to bite the bullet and switch to run().

@iceiix

This comment has been minimized.

@iceiix iceiix changed the title Update to glutin =0.22.0-alpha5 Update to glutin =0.22.0-alpha5, remove wasm_stub Jan 6, 2020
@iceiix
Copy link
Owner Author

iceiix commented Jan 6, 2020

Fixed last of the glutin warnings, now builds and runs on native.

What about the web? Need to select a backend:

error: Please select a feature to build for web: `web-sys`, `stdweb`
  --> winit-0.20.0-alpha6/src/platform_impl/web/mod.rs:20:1
   |
20 | compile_error!("Please select a feature to build for web: `web-sys`, `stdweb`");
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Which feature to select, web-sys or stdweb? The example https://github.com/grovesNL/glow/tree/master/examples/hello has both

https://www.reddit.com/r/rust/comments/e7trlf/question_whats_the_difference_between_stdweb_and/

@iceiix
Copy link
Owner Author

iceiix commented Jan 6, 2020

Was using web-sys, but stdweb is higher level, appears simpler. Testing building with (similar to the glow hello example — not yet using glow):

cargo web start --target wasm32-unknown-unknown

fails, from my understanding, glutin isn't for the web — need to change to glow instead:

   Compiling glutin v0.22.0-alpha5
error[E0432]: unresolved import `self::platform_impl`
 --> .cargo/registry/src/github.com-1ecc6299db9ec823/glutin-0.22.0-alpha5/src/platform_impl/mod.rs:1:15
  |
1 | pub use self::platform_impl::*;
  |               ^^^^^^^^^^^^^ could not find `platform_impl` in `self`
error[E0433]: failed to resolve: could not find `Context` in `platform_impl`
   --> .cargo/registry/src/github.com-1ecc6299db9ec823/glutin-0.22.0-alpha5/src/context.rs:182:24
    |
182 |         platform_impl::Context::new_headless(el, &pf_reqs, &gl_attr, size).map(
    |                        ^^^^^^^ could not find `Context` in `platform_impl`
error[E0433]: failed to resolve: could not find `Context` in `platform_impl`
   --> .cargo/registry/src/github.com-1ecc6299db9ec823/glutin-0.22.0-alpha5/src/windowed.rs:362:24
    |
362 |         platform_impl::Context::new_windowed(wb, el, &pf_reqs, &gl_attr).map(
    |                        ^^^^^^^ could not find `Context` in `platform_impl`
error[E0412]: cannot find type `Context` in module `platform_impl`
  --> .cargo/registry/src/github.com-1ecc6299db9ec823/glutin-0.22.0-alpha5/src/context.rs:35:40
   |
35 |     pub(crate) context: platform_impl::Context,
   |                                        ^^^^^^^ not found in `platform_impl`
   |
help: possible candidates are found in other modules, you can import them into scope
   |
1  | use core::task::Context;
   |
1  | use crate::context::Context;
   |
1  | use std::task::Context;
   |

error[E0107]: wrong number of type arguments: expected 1, found 0
   --> .cargo/registry/src/github.com-1ecc6299db9ec823/glutin-0.22.0-alpha5/src/context.rs:178:15
    |
178 |         size: dpi::PhysicalSize,
    |               ^^^^^^^^^^^^^^^^^ expected 1 type argument
error[E0107]: wrong number of type arguments: expected 1, found 0
   --> .cargo/registry/src/github.com-1ecc6299db9ec823/glutin-0.22.0-alpha5/src/windowed.rs:180:32
    |
180 |     pub fn resize(&self, size: dpi::PhysicalSize) {
    |                                ^^^^^^^^^^^^^^^^^ expected 1 type argument
error: aborting due to 6 previous errors

The old wasm-pack build workflow also fails, but with different errors:

error[E0433]: failed to resolve: use of undeclared type or module `wasm_bindgen`
  --> .cargo/registry/src/github.com-1ecc6299db9ec823/instant-0.1.2/src/wasm.rs:92:13
   |
92 |     let v = js! { return performance.now(); };
   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ use of undeclared type or module `wasm_bindgen`
   |
   = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)

error[E0433]: failed to resolve: use of undeclared type or module `WasmDescribe`
  --> .cargo/registry/src/github.com-1ecc6299db9ec823/instant-0.1.2/src/wasm.rs:92:13
   |
92 |     let v = js! { return performance.now(); };
   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ use of undeclared type or module `WasmDescribe`
   |
   = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)

error[E0425]: cannot find function `inform` in this scope
  --> .cargo/registry/src/github.com-1ecc6299db9ec823/instant-0.1.2/src/wasm.rs:92:13
   |
92 |     let v = js! { return performance.now(); };
   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not found in this scope
   |
   = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)

error[E0425]: cannot find value `FUNCTION` in this scope
  --> .cargo/registry/src/github.com-1ecc6299db9ec823/instant-0.1.2/src/wasm.rs:92:13
   |
92 |     let v = js! { return performance.now(); };
   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not found in this scope
   |
   = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)

error: aborting due to 4 previous errors

@iceiix
Copy link
Owner Author

iceiix commented Jan 6, 2020

The last two errors appear to be an incompatibility in winit 0.20.0 release (versus 0.20.0-alpha6):

error[E0107]: wrong number of type arguments: expected 1, found 0
   --> .cargo/registry/src/github.com-1ecc6299db9ec823/glutin-0.22.0-alpha5/src/context.rs:178:15
    |
178 |         size: dpi::PhysicalSize,
    |               ^^^^^^^^^^^^^^^^^ expected 1 type argument
error[E0107]: wrong number of type arguments: expected 1, found 0
   --> .cargo/registry/src/github.com-1ecc6299db9ec823/glutin-0.22.0-alpha5/src/windowed.rs:180:32
    |
180 |     pub fn resize(&self, size: dpi::PhysicalSize) {
    |                                ^^^^^^^^^^^^^^^^^ expected 1 type argument
error: aborting due to 6 previous errors

reproduces on native, as well. https://github.com/rust-windowing/winit/commits/master has recent changes to DPI: rust-windowing/winit@3aa3880 - glutin may have to update.

@iceiix iceiix merged commit a020ed6 into master Jan 6, 2020
@iceiix iceiix deleted the glutin22 branch January 6, 2020 01:56
iceiix added a commit that referenced this pull request Dec 26, 2020
A small step for #446 🕸️ Web support, use web-sys to interface to the web.
Previously, we would try to use glutin on the web, which is not supported;
now glutin is only used on native: fixes #171 could not find Context in platform_impl.

winit is still used on both, but the GL context is created with web-sys and glow
(on the web), and created with glutin and used with glow (on native). stdweb is
no longer used, being replaced by web-sys.

Substantial refactoring to allow reusing the code between web/native:

* settings: use VirtualKeyCode from winit, not reexported from glutin
* std_or_web: remove broken localstoragefs/stdweb, add File placeholder
* render: disable skin_thread on wasm since we don't have threads

* gl: use glow types in gl wrapper (integers in native, but Web*Key in web)
* gl: web-sys WebGlUniformLocation does not implement Copy trait, so glow::UniformLocation doesn't so gl::Uniform can't
* gl: refactor context initialization, pass glow::Context to gl::init for consistency between native/web
* gl: update to glow with panicking tex_image_2d_multisample web-sys wrapper

* glsl: use shader version in GLSL for WebGL 2 and OpenGL 3.2

* shaders: add explicit float/int type conversions, required for WebGL
* shaders: specify mediump precision, required for WebGL
* shaders: specify fragment shader output locations for WebGL

* main: refactor handle_window_event to take a winit window, not glutin context
* main: handle resize outside of handle_window_event since it updates the glutin window (the only event which does this)
* main: use winit events in handle_window_event not reexported glutin events
* main: refactor game loop handling into tick_all()
* main: create winit window for WebGL, and use winit_window from glutin
* main: restore console_error_panic_hook,  mistakingly removed in (#260)
* main: remove force setting env RUST_BACKTRACE=1, no longer can set env on web

* www: index.js: fix wasm import path
* www: npm update, npm audit fix
* www: update readme to link to status on #446 🕸️ Web support
iceiix added a commit that referenced this pull request Dec 26, 2020
A small step for #446 🕸️ Web support, use web-sys to interface to the web.
Previously, we would try to use glutin on the web, which is not supported;
now glutin is only used on native: fixes #171 could not find Context in platform_impl.

winit is still used on both, but the GL context is created with web-sys and glow
(on the web), and created with glutin and used with glow (on native). stdweb is
no longer used, being replaced by web-sys.

Substantial refactoring to allow reusing the code between web/native:

* settings: use VirtualKeyCode from winit, not reexported from glutin
* std_or_web: remove broken localstoragefs/stdweb, add File placeholder
* render: disable skin_thread on wasm since we don't have threads

* gl: use glow types in gl wrapper (integers in native, but Web*Key in web)
* gl: web-sys WebGlUniformLocation does not implement Copy trait, so glow::UniformLocation doesn't so gl::Uniform can't
* gl: refactor context initialization, pass glow::Context to gl::init for consistency between native/web
* gl: update to glow with panicking tex_image_2d_multisample web-sys wrapper

* glsl: use shader version in GLSL for WebGL 2 and OpenGL 3.2

* shaders: add explicit float/int type conversions, required for WebGL
* shaders: specify mediump precision, required for WebGL
* shaders: specify fragment shader output locations for WebGL

* main: refactor handle_window_event to take a winit window, not glutin context
* main: handle resize outside of handle_window_event since it updates the glutin window (the only event which does this)
* main: use winit events in handle_window_event not reexported glutin events
* main: refactor game loop handling into tick_all()
* main: create winit window for WebGL, and use winit_window from glutin
* main: restore console_error_panic_hook,  mistakingly removed in (#260)
* main: remove force setting env RUST_BACKTRACE=1, no longer can set env on web

* www: index.js: fix wasm import path
* www: npm update, npm audit fix
* www: update readme to link to status on #446 🕸️ Web support
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.

1 participant