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

fix: avoid writing window size of 0 to the config #4619

Merged
merged 4 commits into from
May 14, 2021

Conversation

mahela97
Copy link
Contributor

@mahela97 mahela97 commented Apr 9, 2021

Fix according to @keturn suggestion. Sorry I had to made a separate PR because I accidentally deleted the previous branch.

@pollend
Copy link
Member

pollend commented Apr 10, 2021

just trying to understand why we need to restore where the window was placed on the screen. what does GLFW do by default if you don't specify where the window is placed? is that up to the windows manager to drive that or is it by default placed at some fixed value like (0,0).

do you still need these PRs:

@keturn
Copy link
Member

keturn commented Apr 10, 2021

This is a change to the shutdown method, so I think it replaces #4603 but not the one that changes the initialise method.

The code looks good to me. We'll need someone else to test it because I wasn't able to reproduce this under Ubuntu.

@keturn keturn added the Type: Bug Issues reporting and PRs fixing problems label Apr 10, 2021
@keturn keturn added this to the v4.4.0 milestone Apr 10, 2021
@mahela97
Copy link
Contributor Author

just trying to understand why we need to restore where the window was placed on the screen. what does GLFW do by default if you don't specify where the window is placed? is that up to the windows manager to drive that or is it by default placed at some fixed value like (0,0).

do you still need these PRs:

Yes if we don't restore where the window was placed, it will always start with default values in #4160.

No those PRs won't be ne necessary anymore. Shall I remove those?

@mahela97
Copy link
Contributor Author

This is a change to the shutdown method, so I think it replaces #4603 but not the one that changes the initialise method.

The code looks good to me. We'll need someone else to test it because I wasn't able to reproduce this under Ubuntu.

Okay :)

@mahela97 mahela97 changed the title final-fix-#4603 final fix - avoid writing window size of 0 to the config-#4603 Apr 16, 2021
Copy link
Member

@Cervator Cervator left a comment

Choose a reason for hiding this comment

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

Was able to test this on Windows 10, which it may be specific to due to the preview-via-task-bar feature allowing you to trigger the normal closing of a window that's technically minimized.

Could replicate before this PR and confirm it fixed after! Thank you @mahela97 👍

Going to let this branch update for final CI testing then anybody can squash-merge after

@jdrueckert jdrueckert changed the title final fix - avoid writing window size of 0 to the config-#4603 fix: avoid writing window size of 0 to the config May 14, 2021
@jdrueckert jdrueckert merged commit 9b94536 into MovingBlocks:develop May 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Issues reporting and PRs fixing problems
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants