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

Reduce flashing on window creation #1272

Merged
merged 1 commit into from
Oct 28, 2020
Merged

Reduce flashing on window creation #1272

merged 1 commit into from
Oct 28, 2020

Conversation

rhzk
Copy link
Collaborator

@rhzk rhzk commented Oct 2, 2020

Sets the correct window size directly after the window is built instead of adding it to the deferred queue.
Since this is done before ShowWindow is called it should not be any visible "flashing" occuring.
Updated Show() to handle maximized and minimized window, as this was also deferred and might have caused flashing.

@cmyr cmyr added the S-needs-review waits for review label Oct 7, 2020
Copy link
Contributor

@raphlinus raphlinus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, one small concern inline. I agree that the synchronous call to SetWindowPos in the window creation call is the best way to solve this.

@@ -1234,7 +1214,12 @@ impl WindowBuilder {
present_strategy: self.present_strategy,
};

let position = match self.position {
Some(pos) => pos,
None => Point::new(CW_USEDEFAULT as f64, CW_USEDEFAULT as f64),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am slightly uncomfortable with CW_USEDEFAULT being converted into f64. I know it's going to work, but feels like it's not quite the right type for the job.

Would it work for position to be (i32, i32) here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, makes more sense to cast self.position to i32

@luleyleo luleyleo added S-ready PR is ready to merge and removed S-needs-review waits for review labels Oct 17, 2020
@raphlinus
Copy link
Contributor

Any reason not to merge this now? There are no conflicts, and I've verified again that it works on my machine.

@jneem
Copy link
Collaborator

jneem commented Oct 28, 2020

I'm guessing it's because @rhzk doesn't have write access?

@jneem jneem merged commit 9e5f19a into linebender:master Oct 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-ready PR is ready to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants