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

run_forever cannot bulk process events, unable to empty event queue #276

Closed
ColonelThirtyTwo opened this issue Sep 2, 2017 · 6 comments · Fixed by #913
Closed

run_forever cannot bulk process events, unable to empty event queue #276

ColonelThirtyTwo opened this issue Sep 2, 2017 · 6 comments · Fixed by #913
Labels
C - waiting on author Waiting for a response or another PR D - hard Likely harder than most tasks here P - high Vital to have S - api Design and usability
Milestone

Comments

@ColonelThirtyTwo
Copy link

In a program that uses poll_events in an infinite loop, poll_events will process events until the event queue is empty. If multiple events are enqueued while the program is rendering, the program can process each of them before beginning the relatively expensive task of rendering the next frame.

With run_forever though, this sort of "bulk event processing" cannot happen. The callback does not know if more events are in the queue, so it must unconditionally render a frame to the user. But while rendering a frame, more events can be enqueued. If rendering takes too long, the input queue becomes filled with events, and the run_forever callback cannot process them fast enough as it has to process older events first.

This severely affects interactivity; the program appears to the user that the program is constantly "catching up" with what the user did seconds ago (this is particularly noticeable when moving the mouse, which causes many events).


There are a couple workarounds to this issue. The obvious one is to use poll_events instead. However, if the program only needs to re-render in response to user input (common in editors), rendering all of the time wastes CPU and power.

A more complicated workaround is to have the event loop forward all of its events to the program's own queue (such as std::sync::mpsc), which can implement a "check if empty function". This introduces complexity in the program, though, and it would be better if winit had a simpler interface.


The API described in #231 would solve this issue. The program can call get_event with a short timeout until no events are left, then render the frame and call get_event with an infinite timeout.

@tomaka
Copy link
Contributor

tomaka commented Sep 2, 2017

Another work-around is to use the EventsLoopProxy.

@mitchmindtree
Copy link
Contributor

Another work-around is to use the EventsLoopProxy.

This is what I normally do, however #266 currently makes this a bit tedious on X11.

@ColonelThirtyTwo
Copy link
Author

I'm not sure how EventLoopProxy is supposed to help. I'm guessing you can spawn a thread that wakes up the main thread using EventLoopProxy if no events have been recieved in a short time, but that seems like a hack.

jturner314 added a commit to jturner314/gfx that referenced this issue Feb 17, 2018
This commit uses `.poll_events()` to allow processing multiple events
per rendered frame. (Before, only one event was processed per frame.)
This is necessary because it's easy for the user to generate events much
faster than frames can be rendered (e.g. by moving the mouse).

Additionally, with `.run_forever()`, rendering blocked on receiving
events. (An event had to occur before a frame was rendered.) In order to
see an animation such as the `particle` example, the user had to
generate events fast enough to cause frames to be rendered (e.g. by
moving the mouse over the window).

See also rust-windowing/winit#276 and
rust-windowing/winit#231
jturner314 added a commit to jturner314/gfx that referenced this issue Feb 17, 2018
This commit uses `.poll_events()` to allow processing multiple events
per rendered frame. (Before, only one event was processed per frame.)
This is necessary because it's easy for the user to generate events much
faster than frames can be rendered (e.g. by moving the mouse).

Additionally, with `.run_forever()`, rendering blocked on receiving
events. (An event had to occur before a frame was rendered.) In order to
see an animation such as the `particle` example, the user had to
generate events fast enough to cause frames to be rendered (e.g. by
moving the mouse over the window).

See also rust-windowing/winit#276 and
rust-windowing/winit#231
bors bot added a commit to gfx-rs/gfx that referenced this issue Feb 17, 2018
1822: Switch from .run_forever() to .poll_events() in gfx_app r=kvark a=jturner314

This commit uses `.poll_events()` to allow processing multiple events per rendered frame. (Before, only one event was processed per frame.) This is necessary because it's easy for the user to generate events much faster than frames can be rendered (e.g. by moving the mouse).

Additionally, with `.run_forever()`, rendering blocked on receiving events. (An event had to occur before a frame was rendered.) In order to see an animation such as the `particle` example, the user had to generate events fast enough to cause frames to be rendered (e.g. by moving the mouse over the window).

See also rust-windowing/winit#276 and rust-windowing/winit#231

PR checklist:
- [ ] `make` succeeds (on *nix)
  I don't see a Makefile. `make` says `make: *** No targets specified and no makefile found.  Stop.`
- [ ] `make reftests` succeeds
  I don't see a Makefile. I did run `cargo test`, though, and that succeeded (although it ran only 3 tests).
- [X] tested examples with the following backends:
  When I run `glxinfo`, I see `OpenGL core profile version string: 3.3 (Core Profile) Mesa 17.3.3`. I tested all of the examples. They all worked correctly, except `terrain_tessellated` (which failed to build even before this commit) and `triangle` (which still has the `.run_forever()` issue since it doesn't use `gfx_app`).
@francesca64 francesca64 added S - api Design and usability C - waiting on author Waiting for a response or another PR D - hard Likely harder than most tasks here P - high Vital to have labels May 6, 2018
@francesca64 francesca64 added this to the EventsLoop 2.0 milestone May 6, 2018
@Osspial
Copy link
Contributor

Osspial commented Apr 24, 2019

Closing this, since the EL2.0 rework is in the process of getting onto master and it fixes the design issues brought up here.

@Osspial Osspial closed this as completed Apr 24, 2019
@ColonelThirtyTwo
Copy link
Author

@Osspial Wait, so you're saying the redesign that's not yet released rewrites this to fix the issue? If it's not released yet, then this isn't fixed and should not be closed. I'm glad it's at least getting worked on though.

@Osspial
Copy link
Contributor

Osspial commented Apr 24, 2019

Sorry - I've been triaging stuff today, and may have gotten a bit over-zealous with closing stuff!

You're right that this shouldn't have been closed. The better solution would be to refer to this in the EL2.0 tracking issue. I'll go ahead and add that in there.

EDIT: actually, the eventloop-2.0 milestone exists for this reason.

@Osspial Osspial reopened this Apr 24, 2019
madsmtm added a commit to madsmtm/winit that referenced this issue Jun 11, 2022
* refactor(windows): `begin_resize_drag` now similar to gtk's (rust-windowing#200)

* refactor(windows): `begin_resize_drag` now similart to gtk's

* fix

* feat(linux): skipping taskbar will now also skip pager (rust-windowing#198)

* refactor(linux): clean dummy device_id (rust-windowing#195)

* refactor(linux): clean dummy device_id

* fmt

* feat(linux): allow resizing undecorated window using touch (rust-windowing#199)

* refactor(windows): only skip taskbar if needed when `set_visible` is called (rust-windowing#196)

* fix: increase borderless resizing inset (rust-windowing#202)

* fix: increase borderless resizing inset

* update some comments

* Replace winapi with windows crate bindings shared with WRY (rust-windowing#206)

* fix(deps): update rust crate libayatana-appindicator to 0.1.6 (rust-windowing#190)

Co-authored-by: Renovate Bot <bot@renovateapp.com>

* Add Windows crate and webview2-com-sys bindings

* Initial port to webview2-com-sys

* Finish conversion and remove winapi

* Fix renamed lint warning

* Fix all match arms referencing const variables

* Put back the assert instead of expect

* Point to the published version of webview2-com-sys

* Cleanup slightly weird BOOL handling

* Replace mem::zeroed with Default::default

* Add a summary in .changes

* Remove extra projects not in config.json

* Fix clippy warnings

* Update to 32-bit compatible webview2-com-sys

* Better fix for merge conflict

* Fix clippy errors on Windows

* Use path prefix to prevent variable shadowing

* Fix Windows clippy warnings with nightly toolchain

* Fix Linux nightly/stable clippy warnings

* Fix macOS nightly/stable clippy warnings

* Put back public *mut libc::c_void for consistency

* Re-run cargo fmt

* Move call_default_window_proc to util mod

* Remove unnecessary util::to_wstring calls

* Don't repeat LRESULT expression in match arms

* Replace bitwise operations with util functions

* Cleanup more bit mask & shift with util fns

* Prefer from conversions instead of as cast

* Implement get_xbutton_wparam

* Use *mut libc::c_void for return types

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Renovate Bot <bot@renovateapp.com>

* fix(keyboard): add mapping for space key on Windows (rust-windowing#209)

* fix(keyboard): add mapping for space key on Windows

* change file

* feat: impl Clone for EventLoopWindowTarget (rust-windowing#211)

* chore: add `on_issue_closed.yml` (rust-windowing#214)

* Update tray dependency version (rust-windowing#217)

* Delete on_issue_closed.yml (rust-windowing#221)

* refactor(linux): event loop (rust-windowing#233)

* Use crossbeam::channel

* Fix crossbeam channel import

* Add check on poll event

* Fix deadlock when unregistering shortcut on Linux (rust-windowing#230)

* Add fullscreen monitor selection support on Linux (rust-windowing#235)

* Add fullscreen monitor support on Linux

* Add change file

* Remove todo on videomode

* Fix clippy

* Update to 2021 edition (rust-windowing#236)

* Update to 2021 edition

* Fix clippy

* Add run_return on Linux (rust-windowing#237)

* Add run_return on Linux

* Add main context

* Add run_return trait on Linux (rust-windowing#238)

* Fix: rust-windowing#239 Update webview2-com and windows crates (rust-windowing#240)

* Replace webivew2-com-sys with prebuilt windows

* Use windows utility instead of direct GetLastError

* Bump windows version and add changelog

* Run cargo fmt

* Restore inverted matches macro

* Scope constants in match arms

* Fix inverted null check

* Update src/platform_impl/windows/util.rs

Co-authored-by: Amr Bashir <48618675+amrbashir@users.noreply.github.com>

* Use env_logger instead of simple_logger (rust-windowing#241)

* Use env_logger instead of simple_logger

* Make clippy happy

* Cherry pick commits to next (rust-windowing#244)

* feat(macos): Add `unhide_application` method, closes rust-windowing#182 (rust-windowing#231)

* feat(macos): Add `unhide_application` method

* Update src/platform/macos.rs

Co-authored-by: Amr Bashir <48618675+amrbashir@users.noreply.github.com>

* Reanme to `show_application()`

* Remove broken doc link

Co-authored-by: Amr Bashir <48618675+amrbashir@users.noreply.github.com>

* feat: Allow more strings to parse to keycode (rust-windowing#229)

* feat: support accelerator key strings `,` `-` `.` `Space` `Tab` and `F13`-`F24` (rust-windowing#228)

* feat(macOS): support more accelerator key strings

* Move function keys together

* Add `,` `-` `.` `Space` `F20-F24` for Windows

* Remove support for accelerators not found in `winapi`

* Add `,` `-` `.` `Space` `F13-F24` for Linux

* Update .changes

* Add the rest for Windows

* Add the rest for Linux

* Add the rest on macOS

* Update accelerator-strings.md

* Fix git comments

Co-authored-by: Kasper <kasperkh.kh@gmail.com>
Co-authored-by: Amr Bashir <48618675+amrbashir@users.noreply.github.com>

* Add redraw events on Linux (rust-windowing#245)

* Add redraw events on Linux

* Update doc of RequestRedraw Event

* Add change file

* Fix missing menu bar on borderless window (rust-windowing#247)

Credit goes to irh's work on winit commit f2de847

* refactor: improve `set_skip_taskbar` impl on Windows (rust-windowing#250)

* fix: emit errors on parsing an invalid accelerator for string, closes rust-windowing#135 (rust-windowing#252)

* chore: update comment

* fix(linux): fix focus events not firing properly (rust-windowing#253)

* fix(linux): fix focus events not firing properly

* add changelog

* chore: update focus events error message

* chore: fmt

* fix: revert windows-rs 0.28 version bump

* fix(linux): fix native menu items (rust-windowing#256)

* chore: remove examples commited by accident

* Update `ReceivedImeText` (rust-windowing#251)

* Allow receiving text without Ime on Windows

* Avoid panic todo

* Receive text without ime on mac

* Fix CursorMoved event on Linux

* Add ReceivedImeText on Linux

This only add Simple IME from GTK for now. We should add the actual IME
from system in the future.

* Fix redraw event that causes inifinite loop (rust-windowing#260)

* Fix redraw event that causes inifinite loop

* Refactor event loop

* Remove unused function

* Add doc comment on linux's run_return

* Ignore doc test on run_return

* Add non blocking iteration on Linux (rust-windowing#261)

* Docs: SystemTrayExtWindows::remove() is gone (rust-windowing#262)

Fix docs following rust-windowing#153

* Fix busy loop on Linux (rust-windowing#265)

* Update windows crate to 0.29.0 (rust-windowing#266)

* Update to windows 0.29.0

* Add change description

* Remove clippy check (rust-windowing#267)

* refactor(windows): align util function with win32 names

* chore: update PR template

* fix(linux): fire resized & moved events on min/maximize, closes rust-windowing#219 (rust-windowing#254)

* feat(linux): implement `raw_window_handle()` (rust-windowing#269)

* chore(deps): update to raw-window-handle 0.4

* add linux raw-window-handle support

* update macos/ios/android

* fix ios

* Fix core-video-sys dependency (rust-windowing#274)

* The `cocoa` crate links to AppKit, which made the symbol `CGDisplayCreateUUIDFromDisplayID` from ApplicationServices/ColorSync (which AppKit uses internally) available to us on macOS 10.8 to 10.13. (rust-windowing#275)

However, this does not work on macOS 10.7 (where AppKit does not link to ColorSync internally). Instead of relying on this, we should just link to ApplicationServices directly.

* Fix some invalid msg_send! usage (rust-windowing#276)

* Revert "Fix some invalid msg_send! usage (rust-windowing#276)" (rust-windowing#277)

This reverts commit a3a2e0cfc49ddfa8cdf65cf9870fb8e3d45b4bc0.

* Revert "The `cocoa` crate links to AppKit, which made the symbol `CGDisplayCreateUUIDFromDisplayID` from ApplicationServices/ColorSync (which AppKit uses internally) available to us on macOS 10.8 to 10.13. (rust-windowing#275)" (rust-windowing#279)

This reverts commit 6f9c468f26ddb60e29be2139397bfaf3b30eab1e.

* The `cocoa` crate links to AppKit, which made the symbol `CGDisplayCreateUUIDFromDisplayID` from ApplicationServices/ColorSync (which AppKit uses internally) available to us on macOS 10.8 to 10.13. (rust-windowing#280)

However, this does not work on macOS 10.7 (where AppKit does not link to ColorSync internally). Instead of relying on this, we should just link to ApplicationServices directly.

Co-authored-by: madsmtm <mads@marquart.dk>

* Fix some invalid msg_send! usage (rust-windowing#278)

Co-authored-by: madsmtm <mads@marquart.dk>

* Add exit code to ControlFlow::Exit (rust-windowing#281)

* Add exit code to ControlFlow::Exit

* Cargo fmt

* Add change files

Co-authored-by:  multisn8 <contact@multisamplednight.com>

* Add new_any_thread to Unix event loop (rust-windowing#282)

* Update windows crate to 0.30.0 (rust-windowing#283)

* Update windows crate to 0.30.0

* Simplify new-type usage

* Fix boxing in GWL_USERDATA

* Make sure everyone is using Get/SetWindowLongPtrW

* build the system_tray module when "ayatana" feature is enabled (rust-windowing#285)

Without those cfg feature checks, the "ayatana" feature does
actually not enable anything.

* Fix click events missing whe tray has menu (rust-windowing#291)

* Fix click events missing whe tray has menu

* Add change file

* Fix crash when tray has no menu (rust-windowing#294)

* chore: update pull request commit exmple

* fix(windows): send correct position for system tray events, closes rust-windowing#295 (rust-windowing#300)

* fix(windows): revert maximized state handling to winit impl, closes rust-windowing#193 (rust-windowing#299)

* fix(windows): revet maximized state handling to winit impl, closes rust-windowing#193

* add chanefile [skip ci]

* fix: `MenuItem::Quit` on Windows (rust-windowing#303)

* fix: `MenuItem::Close` on Windows

* use `PostQuitMessage` instead

Co-authored-by: amrbashir <amr.bashir2015@gmail.com>

* feat: v1 audit by Radically Open Security (rust-windowing#304)

* Update to gtk 0.15 (rust-windowing#288)

* Update to gtk 0.15

* Fix picky none on set_geometry_hint

* Fix CursorMoved position

Co-authored-by: Amr Bashir <48618675+amrbashir@users.noreply.github.com>
Co-authored-by: Bill Avery <wravery@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Renovate Bot <bot@renovateapp.com>
Co-authored-by: Lucas Fernandes Nogueira <lucasfernandesnog@gmail.com>
Co-authored-by: Kasper <kasperkh.kh@gmail.com>
Co-authored-by: amrbashir <amr.bashir2015@gmail.com>
Co-authored-by: Jay Oster <jay@kodewerx.org>
Co-authored-by: madsmtm <mads@marquart.dk>
Co-authored-by: multisn8 <contact@multisamplednight.com>
Co-authored-by: Aurélien Jacobs <aurel@gnuage.org>
Co-authored-by: Lucas Fernandes Nogueira <lucas@tauri.studio>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C - waiting on author Waiting for a response or another PR D - hard Likely harder than most tasks here P - high Vital to have S - api Design and usability
Development

Successfully merging a pull request may close this issue.

5 participants