-
Notifications
You must be signed in to change notification settings - Fork 909
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 waiting for messages on Windows #3950
Improve waiting for messages on Windows #3950
Conversation
Current version in crates.io:TestWith0.30.5.mp4With my changes:TestWithPatch.mp4 |
124513a
to
c858ac7
Compare
Rebased on top of master branch. |
I may be unavailable during a week but if you have questions/comments, I would happily address them in next Saturday. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Initial review
// It is a timer used on timed waits. | ||
// It is created lazily in case if we have `ControlFlow::WaitUntil`. | ||
// Keep it as a field to avoid recreating it on every `ControlFlow::WaitUntil`. | ||
high_precision_timer: Option<HANDLE>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we use OwnedHandle
for this instead to simplify cleanup?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Honestly, it looks much messier as result because types used in std
and in windows-sys
crate are not compatible.
Btw, why use windows-sys
crate instead of windows
crate? windows
crate does all error checking itself and has more ergonomic interface overall.
self.wait_and_dispatch_message(None); | ||
|
||
if let Some(code) = self.exit_code() { | ||
break code; | ||
} | ||
|
||
self.dispatch_peeked_messages(); | ||
|
||
if let Some(code) = self.exit_code() { | ||
break code; | ||
} | ||
|
||
self.wait_for_messages(None); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please clarify why we've removed the exit_code()
check between waiting for messages and dispatching peeked messages.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As I understand, it was here to handle cases when exit codes were set in callbacks to messages that called in wait_and_dispatch_message
. I don't do any dispatch in new self.wait_for_messages
so there is no new exit code that can appear before dispatch_peeked_messages
. Also, before the loop, exit code is reset at
winit/src/platform_impl/windows/event_loop.rs
Line 231 in c858ac7
self.window_target.clear_exit(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems notable that the ordering of dispatch_peeked_
followed by wait_for_messages
(instead of waiting then dispatching) here isn't consistent with the pump_app_events
implementation, which seems like it could affect the order of dispatching some initial events for an app.
I believe wait_for_messages
dispatches the AboutToWait
event (runner.prepare_wait()
) to notify the app that it's about to block and wait for input so I wonder if it might still make sense to check for a possible exit code from the app after wait_for_messages()
fn create_high_precision_timer() -> Option<HANDLE> { | ||
unsafe { | ||
// A timeout of None means wait indefinitely (so we don't need to call SetTimer) | ||
let timer_id = timeout.map(|timeout| SetTimer(0, 0, dur2timeout(timeout), None)); | ||
let get_status = GetMessageW(msg, 0, 0, 0); | ||
if let Some(timer_id) = timer_id { | ||
KillTimer(0, timer_id); | ||
} | ||
// A return value of 0 implies `WM_QUIT` | ||
if get_status == 0 { | ||
PumpStatus::Exit(0) | ||
let handle: HANDLE = CreateWaitableTimerExW( | ||
ptr::null(), | ||
ptr::null(), | ||
CREATE_WAITABLE_TIMER_HIGH_RESOLUTION, | ||
TIMER_ALL_ACCESS, | ||
); | ||
// CREATE_WAITABLE_TIMER_HIGH_RESOLUTION is supported only after | ||
// Win10 1803 but it is already default option for rustc | ||
// (std uses it to implement `std::thread::sleep`). | ||
if handle == 0 { | ||
None | ||
} else { | ||
PumpStatus::Continue | ||
Some(handle) | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would rather make these freestanding functions than having them inlined into wait_for_messages
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, would fix tomorrow. I just kept the structure that was used before.
{ | ||
let control_flow_timeout = match runner.control_flow() { | ||
ControlFlow::Wait => None, | ||
ControlFlow::Poll => Some(Duration::ZERO), | ||
ControlFlow::WaitUntil(wait_deadline) => { | ||
let start = Instant::now(); | ||
Some(wait_deadline.saturating_duration_since(start)) | ||
}, | ||
}; | ||
timeout = min_timeout(timeout, control_flow_timeout); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
{ | |
let control_flow_timeout = match runner.control_flow() { | |
ControlFlow::Wait => None, | |
ControlFlow::Poll => Some(Duration::ZERO), | |
ControlFlow::WaitUntil(wait_deadline) => { | |
let start = Instant::now(); | |
Some(wait_deadline.saturating_duration_since(start)) | |
}, | |
}; | |
timeout = min_timeout(timeout, control_flow_timeout); | |
} | |
let timeout = { | |
let control_flow_timeout = match runner.control_flow() { | |
ControlFlow::Wait => None, | |
ControlFlow::Poll => Some(Duration::ZERO), | |
ControlFlow::WaitUntil(wait_deadline) => { | |
let start = Instant::now(); | |
Some(wait_deadline.saturating_duration_since(start)) | |
}, | |
}; | |
min_timeout(timeout, control_flow_timeout) | |
}; |
Slightly cleaner this way
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should I declare new binding let timeout
or just update existing variable timeout
? I would update code tomorrow.
// API) and there's no API to construct or initialize a `MSG`. This | ||
// is the simplest way avoid uninitialized memory in Rust | ||
let mut msg = unsafe { mem::zeroed() }; | ||
let msg_status = wait_for_msg(&mut msg, timeout); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did we stop using wait_for_msg
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because I rewrote code to use MsgWaitForMultipleObjectsEx
instead of GetMessageW
. MsgWaitForMultipleObjectsEx
only wait until a new message is available but doesn't take it and doesn't remove it from queue, while GetMessageW
does wait and take it.
In new version, wait_for_messages
only waits until message is available, then dispatch_peeked_messages
takes them from queue and dispatches, until the queue is empty.
So, as I understand, old version of code needed to manage both waiting and dispatching messages so it was split it into subfunctions. New version of code only does waiting while all dispatching done by dispatch_peeked_messages
so there is no need to create a subfunction which just repeats the name of outer function wait_for_messages
.
Also, after I move inner functions out of wait_for_messages
, as you requested in #3950 (comment) , wait_for_messages
would be tiny and splitting it further would be pointless.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe there are some messages that MsgWaitForMultipleObjectsEx
does not receive, which is why we used the timer system and GetMessageW
here.
cc @rib you were the one to set up this system
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't really recall there being a particular issue with MsgWaitForMultipleObjectsEx
and at the time I suppose it just seemed simpler to use GetMessage
while we were only waiting for messages and not waiting on any object handles.
The main concern I had when last iterating on this code was to do with the indirection of a dedicated waiter thread and there were some funky details to do with redraw handling I don't really recall.
This change to use hrtimers seems good to me, and can see that it makes sense to use MsgWaitForMultipleObjectsEx
to integrate the timer handle.
It seems to make sense that the waiting is now decoupled, since MsgWaitForMultipleObjectsEx
doesn't take any data when it has something to wake up for - unlike GetMessageW
which would wait and take.
The timer resolution issue was fixed in Feb 2021 : cfbe846 It was later removed in July 2023: 4208402#diff-bb05c5e1236bdaa7f268bcbbbb8cc9f378a48eb3ed480884f0c6471a9cc933edL467 Just sayin... |
@filnet Well, As for the issue being fixed, well, I see that it is still open. And as can be seen in attached video, currently WaitUntil just don't work correctly (e.g. getting 30 FPS when I ask for 60). |
66f208e
to
6f60c87
Compare
6f60c87
to
33ec57a
Compare
I just noticed that I used word "precision" instead of "resolution" everywhere. Fixed it now. |
} | ||
} | ||
|
||
/// Dispatch all queued messages via `PeekMessageW` | ||
fn dispatch_peeked_messages(&mut self) { | ||
let runner = &self.window_target.runner_shared; | ||
|
||
// Before we potentially exit, make sure to consistently emit an event for the wake up | ||
runner.wakeup(); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe this should be coupled with the implementation of wait_for_messages
.
Conceptually the intention was to have a sequence like:
- notify the app that it's about to block/wait
- use OS API to block
- notify the app of a wake up
... then dispatch/handle all events that triggered the wake up
So long as dispatch_peeked_messages
only gets called once immediately after wait_for_messages
returns then notifying the wake up here is OK, but it feels to me like it would be more correct to notify about the wakeup immediately after MsgWaitForMultipleObjectsEx
returns and avoid any risk of dispatching multiple wake ups if someone makes a change later that ends up calling dispatch_peeked_messages
multiple times per wait.
I just wanted to mention the previous fix for completeness sake... If there is a better solution then we should use it. EDIT: found the issue. |
@filnet Sorry if my previous message come out as a too forceful, just I wanted to make downsides of changing timer frequency clear. If you want to read what the problems with changing frequency timer, you may read this article: https://randomascii.wordpress.com/2013/07/08/windows-timer-resolution-megawatts-wasted/ |
Previous version used [`SetTimer`] with `GetMessageW` for waiting. The downside of UI timers like ones created by `SetTimer`, is that they may be late by up to 15-16 ms. To fix this behaviour, I added use of high resolution timers created by [`CreateWaitableTimerExW`] with the flag `CREATE_WAITABLE_TIMER_HIGH_RESOLUTION`. In my previous experience, waiting on such timers have precision of roundly 0.5 ms which is the best available on Windows at the moment. I use [`MsgWaitForMultipleObjectsEx`] to wait simultaneously for both timer and newly arriving events. Unfortunately, high resolution timers are available only since Windows 10 1803. However: 1. Win 10 is already getting to the end of support, like all previous versions, so it is OK to rely on APIs introduced in it; 2. I use `dwMilliseconds` parameter of `MsgWaitForMultipleObjectsEx` as a fallback. It should perform not worse compared to waiting for events from `SetTimer`. I also refactored code to remove event dispatching from function responsible for waiting for events. This provides more clear separations of concern and avoids unnecessary duplication of dispatching logic. After [review] from @rib, I also moved the waiting itself from `wait_for_messages` method to separate function, so it is clearly seen that `wait_for_messages` do 3 things: notify app that we about to wait, wait, notify that we have new events. I have tested behaviour using a egui app with Vulkan rendering with `VK_PRESENT_MODE_IMMEDIATE_KHR`, and older version consistently have twice less FPS than requested (e.g. 30 FPS when limit is 60 and 60 FPS when limit is 120) while newer version works more correctly (almost always 60 FPS when limit is 60, and only 5-10 frames missing when FPS is set to 120 or more). Fixes rust-windowing#1610 [`CreateWaitableTimerExW`]: https://learn.microsoft.com/en-us/windows/win32/api/synchapi/nf-synchapi-createwaitabletimerexw [`MsgWaitForMultipleObjectsEx`]: https://learn.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-msgwaitformultipleobjectsex [`SetTimer`]: https://learn.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-settimer [review]: rust-windowing#3950 (comment)
33ec57a
to
821b50a
Compare
Previous version used [`SetTimer`] with `GetMessageW` for waiting. The downside of UI timers like ones created by `SetTimer`, is that they may be late by up to 15-16 ms. To fix this behaviour, I added use of high resolution timers created by [`CreateWaitableTimerExW`] with the flag `CREATE_WAITABLE_TIMER_HIGH_RESOLUTION`. In my previous experience, waiting on such timers have precision of roundly 0.5 ms which is the best available on Windows at the moment. I use [`MsgWaitForMultipleObjectsEx`] to wait simultaneously for both timer and newly arriving events. Unfortunately, high resolution timers are available only since Windows 10 1803. However: 1. Win 10 is already getting to the end of support, like all previous versions, so it is OK to rely on APIs introduced in it; 2. I use `dwMilliseconds` parameter of `MsgWaitForMultipleObjectsEx` as a fallback. It should perform not worse compared to waiting for events from `SetTimer`. I also refactored code to remove event dispatching from function responsible for waiting for events. This provides more clear separations of concern and avoids unnecessary duplication of dispatching logic. After [review] from @rib, I also moved the waiting itself from `wait_for_messages` method to separate function, so it is clearly seen that `wait_for_messages` do 3 things: notify app that we about to wait, wait, notify that we have new events. I have tested behaviour using a egui app with Vulkan rendering with `VK_PRESENT_MODE_IMMEDIATE_KHR`, and older version consistently have twice less FPS than requested (e.g. 30 FPS when limit is 60 and 60 FPS when limit is 120) while newer version works more correctly (almost always 60 FPS when limit is 60, and only 5-10 frames missing when FPS is set to 120 or more). Fixes rust-windowing#1610 [`CreateWaitableTimerExW`]: https://learn.microsoft.com/en-us/windows/win32/api/synchapi/nf-synchapi-createwaitabletimerexw [`MsgWaitForMultipleObjectsEx`]: https://learn.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-msgwaitformultipleobjectsex [`SetTimer`]: https://learn.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-settimer [review]: rust-windowing#3950 (comment)
821b50a
to
cb2f4de
Compare
I rebased on top of master branch and also made changes requested by at #3950 (comment) |
|
||
if self.exit_code().is_none() { | ||
self.wait_for_messages(timeout); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have some concern here. Right now, it looks like this sequence happens here:
- Call to
runner.wakeup()
. - Call to
wait_for_messages
- Inside
wait_for_messages
immeadiately callsrunner.prepare_wait()
. - Waits for events.
- Call
runner.wakeup()
wait_for_messages
finishes- dispatch events
- Call
runner.prepare_wait()
My concern that there is no check if there are any new events between first wakeup
and first prepare_wait
. Maybe we should do sequence of wakeup
. dispatch, wait_for_messages
, dispatch, prepare_wait
?
@rib I tag you because you previously asked to change calls to wakeup
and prepare_wait
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, sorry that I don't have much time to follow the details here.
If we say that prepare_wait
and wakeup
are conceptually supposed to surround the "OS wait point" then it looks like we currently have two competing definitions of what that OS wait point should be...
It looks pump_app_events
is intending to define the relinquish of control to the application loop (via PumpStatus::Continue
) as the point where the Winit event loop will wait.
This arguably makes sense because pump_app_events
is supposed to be non-blocking and any application using pump_app_events
in their own loop should do something to throttle that loop before repeated called to pump_app_events
(so applications using pump_app_events
are arguably responsible for the wait, and they would also be expected to call pump_app_events
after they wake up).
Sorry if I confused matters by overlooking these details when first commenting about changes here.
It looks like this needs to pick one definition.
Either:
- Define the wait point by the act of relinquishing control to the app via
::Continue
and have some argument forwait_for_messages()
that tells that function to skip overprepare_wait
andwakeup
when used from here. - Define the wait point as the call to
wait_for_messages_impl
and instead remove the call to.wakeup()
at the start ofpump_app_events
and remove the call toprepare_wait
just before we return with a::Continue
status.
I guess option 1 would be most consistent with the current semantics.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think, maybe we should fix it in another PR? This PR doesn't change behaviour at this point and it is focused on fixing waiting precision.
CI failure seems to be irrelevant because it is about another platform (Redox). |
Previous version used [`SetTimer`] with `GetMessageW` for waiting. The downside of UI timers like ones created by `SetTimer`, is that they may be late by up to 15-16 ms. To fix this behaviour, I added use of high resolution timers created by [`CreateWaitableTimerExW`] with the flag `CREATE_WAITABLE_TIMER_HIGH_RESOLUTION`. In my previous experience, waiting on such timers have precision of roundly 0.5 ms which is the best available on Windows at the moment. I use [`MsgWaitForMultipleObjectsEx`] to wait simultaneously for both timer and newly arriving events. Unfortunately, high resolution timers are available only since Windows 10 1803. However: 1. Win 10 is already getting to the end of support, like all previous versions, so it is OK to rely on APIs introduced in it; 2. I use `dwMilliseconds` parameter of `MsgWaitForMultipleObjectsEx` as a fallback. It should perform not worse compared to waiting for events from `SetTimer`. I also refactored code to remove event dispatching from function responsible for waiting for events. This provides more clear separations of concern and avoids unnecessary duplication of dispatching logic. After [review] from @rib, I also moved the waiting itself from `wait_for_messages` method to separate function, so it is clearly seen that `wait_for_messages` do 3 things: notify app that we about to wait, wait, notify that we have new events. I have tested behaviour using a egui app with Vulkan rendering with `VK_PRESENT_MODE_IMMEDIATE_KHR`, and older version consistently have twice less FPS than requested (e.g. 30 FPS when limit is 60 and 60 FPS when limit is 120) while newer version works more correctly (almost always 60 FPS when limit is 60, and only 5-10 frames missing when FPS is set to 120 or more). Fixes rust-windowing#1610 [`CreateWaitableTimerExW`]: https://learn.microsoft.com/en-us/windows/win32/api/synchapi/nf-synchapi-createwaitabletimerexw [`MsgWaitForMultipleObjectsEx`]: https://learn.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-msgwaitformultipleobjectsex [`SetTimer`]: https://learn.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-settimer [review]: rust-windowing#3950 (comment)
cb2f4de
to
d15d1fe
Compare
@notgull Just reminding that I still wait for a code review. |
I will review this today or tomorrow. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM aside from some minor nits
/// Implementation detail of [EventLoop::wait_for_messages]. | ||
/// Does actual system-level waiting and doesn't process any messages itself, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/// Implementation detail of [EventLoop::wait_for_messages]. | |
/// Does actual system-level waiting and doesn't process any messages itself, | |
/// Implementation detail of [EventLoop::wait_for_messages]. | |
/// | |
/// Does actual system-level waiting and doesn't process any messages itself, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
/// This function should not return error if parameters valid | ||
/// but there is no guarantee about that at MSDN docs | ||
/// so we return result of GetLastError if fail. | ||
/// ## Safety | ||
/// timer must be a valid timer handle created by create_high_resolution_timer. | ||
/// timeout divided by 100 nanoseconds must be more than 0 and less than i64::MAX. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/// This function should not return error if parameters valid | |
/// but there is no guarantee about that at MSDN docs | |
/// so we return result of GetLastError if fail. | |
/// ## Safety | |
/// timer must be a valid timer handle created by create_high_resolution_timer. | |
/// timeout divided by 100 nanoseconds must be more than 0 and less than i64::MAX. | |
/// This function should not return error if parameters valid | |
/// but there is no guarantee about that at MSDN docs | |
/// so we return result of GetLastError if fail. | |
/// | |
/// ## Safety | |
/// | |
/// timer must be a valid timer handle created by create_high_resolution_timer. | |
/// timeout divided by 100 nanoseconds must be more than 0 and less than i64::MAX. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
Previous version used [`SetTimer`] with `GetMessageW` for waiting. The downside of UI timers like ones created by `SetTimer`, is that they may be late by up to 15-16 ms. To fix this behaviour, I added use of high resolution timers created by [`CreateWaitableTimerExW`] with the flag `CREATE_WAITABLE_TIMER_HIGH_RESOLUTION`. In my previous experience, waiting on such timers have precision of roundly 0.5 ms which is the best available on Windows at the moment. I use [`MsgWaitForMultipleObjectsEx`] to wait simultaneously for both timer and newly arriving events. Unfortunately, high resolution timers are available only since Windows 10 1803. However: 1. Win 10 is already getting to the end of support, like all previous versions, so it is OK to rely on APIs introduced in it; 2. I use `dwMilliseconds` parameter of `MsgWaitForMultipleObjectsEx` as a fallback. It should perform not worse compared to waiting for events from `SetTimer`. I also refactored code to remove event dispatching from function responsible for waiting for events. This provides more clear separations of concern and avoids unnecessary duplication of dispatching logic. After [review] from @rib, I also moved the waiting itself from `wait_for_messages` method to separate function, so it is clearly seen that `wait_for_messages` do 3 things: notify app that we about to wait, wait, notify that we have new events. I have tested behaviour using a egui app with Vulkan rendering with `VK_PRESENT_MODE_IMMEDIATE_KHR`, and older version consistently have twice less FPS than requested (e.g. 30 FPS when limit is 60 and 60 FPS when limit is 120) while newer version works more correctly (almost always 60 FPS when limit is 60, and only 5-10 frames missing when FPS is set to 120 or more). Fixes rust-windowing#1610 [`CreateWaitableTimerExW`]: https://learn.microsoft.com/en-us/windows/win32/api/synchapi/nf-synchapi-createwaitabletimerexw [`MsgWaitForMultipleObjectsEx`]: https://learn.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-msgwaitformultipleobjectsex [`SetTimer`]: https://learn.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-settimer [review]: rust-windowing#3950 (comment)
d15d1fe
to
8301d4d
Compare
@notgull I rebased on top of master branch and applied proposed changes. |
Previous version used
SetTimer
withGetMessageW
for waiting.The downside of UI timers like ones created by
SetTimer
,is that they may be late by up to 15-16 ms.
To fix this behaviour, I added use of high resolution timers created by
CreateWaitableTimerExW
with the flagCREATE_WAITABLE_TIMER_HIGH_RESOLUTION
.In my previous experience, waiting on such timers have precision of roundly 0.5 ms which is the best available on Windows at the moment.
I use
MsgWaitForMultipleObjectsEx
to wait simultaneously for both timer and newly arriving events.Unfortunately, high resolution timers are available only since Windows 10 1803. However:
dwMilliseconds
parameter ofMsgWaitForMultipleObjectsEx
as a fallback. It should perform not worse compared to waiting for events fromSetTimer
.I also refactored code to remove event dispatching from function responsible for waiting for events. This provides more clear separations of concern and avoids unnecessary duplication of dispatching logic.
After review from @rib, I also moved the waiting itself from
wait_for_messages
method to separate function, so it is clearly seen thatwait_for_messages
do 3 things: notify app that we about to wait, wait, notify that we have new events.I have tested behaviour using a egui app with Vulkan rendering with
VK_PRESENT_MODE_IMMEDIATE_KHR
, and older version consistently have twice less FPS than requested (e.g. 30 FPS when limit is 60 and 60 FPS when limit is 120) while newer version works more correctly (almost always 60 FPS when limit is 60, and only 5-10 frames missing when FPS is set to 120 or more).Fixes #1610
changelog
module if knowledge of this change could be valuable to users