chore: Extract window_wrapper and the update loop to their own modules #1939
Conversation
fredizzimo
force-pushed
the
fsundvik/split-window-wrapper
branch
from
a3ae28d
to
a077e4b
Compare
| window.outer_position().ok(), | ||
| ); | ||
|
|
||
| std::process::exit(RUNNING_TRACKER.exit_code()); |
There was a problem hiding this comment.
Not sure if it's best to handle the exit directly here. Possibly it could be worth bubbling it up instead, returning ControlFlow::ExitWithCode instead.
| std::process::exit(RUNNING_TRACKER.exit_code()); | |
| return ControlFlow::ExitWithCode(RUNNING_TRACKER.exit_code()); |
There was a problem hiding this comment.
Probably yes, but this code is copied from the original code, and I'm not going to change any logic during this refactoring.
src/window/update_loop.rs
Outdated
| let expected_frame_length_seconds = 0.0 / refresh_rate; | ||
| let frame_duration = Duration::from_secs_f32(expected_frame_length_seconds); | ||
|
|
||
| if frame_start - self.previous_frame_start > frame_duration { |
There was a problem hiding this comment.
Is the division of 0 intended? Because then frame_duration is empty, causing the if clause to be always entered. Possibly you meant 1.0 instead.
There was a problem hiding this comment.
Yes, it's supposed to be 1.0. I don't know how 0.0 has ended up here, since I think I copied the code from mod.rs, and it's supposed to be exactly the same. I must somehow have typed something by mistake.
I will fix that.
| if let FocusedState::UnfocusedNotDrawn = self.focused { | ||
| self.focused = FocusedState::Unfocused; | ||
| } | ||
| self.previous_frame_start = frame_start; |
There was a problem hiding this comment.
If this is entered as soon as the frame is over, then I suppose it's (from the relative perspective of the next frame) more or less when the previous frame ended/the current frame started. Which would also make more sense for the deadline logic, if it was to look at the start of the last frame in order to get the deadline of the current frame, it'd need to add the expected frame duration twice (but it only does it once).
It's a clash between semantics and naming, I believe.
There was a problem hiding this comment.
The naming is copied from the original code. current_frame_start would perhaps make more sense, but then there's the variable frame_start, which does not make much sense at all, since the loop can be run multiple times until a frame is finally drawn. But I guess that previous_frame_start was chosen to differentiate from that.
But generally, the calculations here are flawed, and that's one of the reasons the smooth rendering branch improves things.
fredizzimo
force-pushed
the
fsundvik/split-window-wrapper
branch
from
a077e4b
to
b17fa15
Compare
|
Thank you! |
What kind of change does this PR introduce?
This splits the
window/mod.rsintoupdate_loopandwindow_wrapper. The opengl window creation has also been split into two functions, the actual window creation and the context creation.This will make it easier to review the render loop changes in #1870, and to allow some platforms to run in a thread while other platforms don't.
Did this PR introduce a breaking change?