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

Is ControlFlow supposed to be sticky? (eventloop-2.0) #891

Closed
aloucks opened this issue May 30, 2019 · 4 comments · Fixed by #916
Closed

Is ControlFlow supposed to be sticky? (eventloop-2.0) #891

aloucks opened this issue May 30, 2019 · 4 comments · Fixed by #916
Labels
B - bug Dang, that shouldn't have happened D - easy Likely easier than most tasks here DS - wayland DS - x11 H - good first issue Ideal for new contributors

Comments

@aloucks
Copy link
Contributor

aloucks commented May 30, 2019

Based on the doc comment, I would believe that it is intended to be sticky. However, the x11 and wayland implementations are resetting it per loop iteration. It doesn't look like Windows is doing this.

winit/src/event_loop.rs

Lines 65 to 69 in 0df4369

/// ## Persistency
/// Almost every change is persistent between multiple calls to the event loop closure within a
/// given run loop. The only exception to this is `Exit` which, once set, cannot be unset. Changes
/// are **not** persistent between multiple calls to `run_return` - issuing a new call will reset
/// the control flow to `Poll`.

ControlFlow::Poll => {
// non-blocking dispatch
self.inner_loop.dispatch(Some(::std::time::Duration::from_millis(0)), &mut ()).unwrap();
control_flow = ControlFlow::default();

ControlFlow::Wait => {
self.inner_loop.dispatch(None, &mut ()).unwrap();
control_flow = ControlFlow::default();

ControlFlow::WaitUntil(deadline) => {
let start = ::std::time::Instant::now();
// compute the blocking duration
let duration = deadline.duration_since(::std::cmp::max(deadline, start));
self.inner_loop.dispatch(Some(duration), &mut ()).unwrap();
control_flow = ControlFlow::default();

@Osspial
Copy link
Contributor

Osspial commented May 30, 2019

It is indeed intended to be sticky.

@Osspial Osspial added DS - wayland DS - x11 D - easy Likely easier than most tasks here B - bug Dang, that shouldn't have happened labels May 30, 2019
@goddessfreya goddessfreya added the H - good first issue Ideal for new contributors label May 30, 2019
@elinorbgr
Copy link
Contributor

Ah, ok. I assumed it was only supposed to be sticky during a run of the event loop, rather than forever.

@aloucks
Copy link
Contributor Author

aloucks commented Jun 3, 2019

I assumed it was only supposed to be sticky during a run of the event loop, rather than forever.

Yes, that's how I understand it; Control flow is sticky for the duration of run_return and reset in between calls to that. The current implementation resets the control flow on each tick of the loop.

This part is correct:

pub fn run_return<F>(&mut self, mut callback: F)
where F: FnMut(Event<T>, &RootELW<T>, &mut ControlFlow)
{
let mut control_flow = ControlFlow::default();

@elinorbgr
Copy link
Contributor

Well, the fix is simple enough, I just don't have the time to work on it for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
B - bug Dang, that shouldn't have happened D - easy Likely easier than most tasks here DS - wayland DS - x11 H - good first issue Ideal for new contributors
Development

Successfully merging a pull request may close this issue.

4 participants