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

Windows: Fix panic when calling set_fullscreen(None) #502

Merged
merged 2 commits into from
May 8, 2018
Merged

Windows: Fix panic when calling set_fullscreen(None) #502

merged 2 commits into from
May 8, 2018

Conversation

jckmgns
Copy link

@jckmgns jckmgns commented May 7, 2018

Fixes #501

This circumvents a panic when one calls set_fullscreen(None) on a window that has never been in fullscreen mode prior to calling that method.

@jckmgns jckmgns changed the title Windows: Fix panic when Windows: Fix panic when callen set_fullscreen(None) May 7, 2018
@jckmgns jckmgns changed the title Windows: Fix panic when callen set_fullscreen(None) Windows: Fix panic when calling set_fullscreen(None) May 7, 2018
@jckmgns
Copy link
Author

jckmgns commented May 7, 2018

Build and tested with Windows 7 (64bit)

CHANGELOG.md Outdated
@@ -16,6 +16,7 @@
- On X11, drag and drop now works reliably in release mode.
- Added `WindowBuilderExt::with_resize_increments` and `WindowBuilderExt::with_base_size` to X11, allowing for more optional hints to be set.
- Rework of the wayland backend, migrating it to use [Smithay's Client Toolkit](https://github.com/Smithay/client-toolkit).
- Fix panic when trying to call `set_fullscreen(None)` on a window that has not been fullscreened prior.
Copy link
Member

Choose a reason for hiding this comment

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

You need to specify which platform this is for.

Copy link
Member

Choose a reason for hiding this comment

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

(...and don't forget to rebase.)

Copy link
Author

Choose a reason for hiding this comment

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

Woops, my bad.

Jack Magnus added 2 commits May 8, 2018 01:49
* Add condition to prevent panic

Trying to call set_fullscreen(None) on a window that has never been in
fullscreen mode caused a panic before this change.
The responsible method now simply checks if this precondition is met and
returns (does nothing) otherwise.

* Add entry to CHANGELOG
Forgot to add that the to_fullscreen(None) bugfix is Windows only in
CHANGELOG.
@francesca64
Copy link
Member

Alright, I still want @edwin0cheng to verify that this is good, but then we should be good to merge. Thanks for the contribution!

@francesca64 francesca64 added the C - waiting on maintainer A maintainer must review this code label May 8, 2018
@francesca64 francesca64 mentioned this pull request May 8, 2018
@edwin0cheng
Copy link
Contributor

edwin0cheng commented May 8, 2018

Look good to me, and i tested it, it works properly,
saved_window_info only be used when the window is already in fullscreen mode, so ignoring it when it is not set should be fine.

@francesca64
Copy link
Member

@edwin0cheng thanks!

@francesca64 francesca64 merged commit 3632610 into rust-windowing:master May 8, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C - waiting on maintainer A maintainer must review this code DS - windows
Development

Successfully merging this pull request may close these issues.

3 participants