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

citra-qt : Fix a bug in our fullscreen implementation #3159

Merged
merged 1 commit into from Dec 7, 2017

Conversation

Projects
None yet
5 participants
@FearlessTobi
Copy link
Contributor

FearlessTobi commented Nov 27, 2017

Fixes the bug described here. (Could also fix #3040 but I don´t have a Ubuntu VM set up to test it.)
Therefore, the contents of ToggleFullscreen() have been refactored into ShowFullscreen() and HideFullscreen() (feel free to suggest better names for them) to make it easier to fix the bug.
Also tracking of the window geometry before it was fullscreened has been added.

Thanks @CDAGaming for helping me with clang format.


This change is Reviewable

@wwylele

This comment has been minimized.

Copy link
Member

wwylele commented Nov 27, 2017

Yes, this fixes #3040


void GMainWindow::ShowFullscreen() {
if (ui.action_Single_Window_Mode->isChecked()) {
UISettings::values.geometry = saveGeometry();

This comment has been minimized.

@jroweboy

jroweboy Nov 29, 2017

Member

I didn't think fullscreen mode affects the window geometry. Is it really required to save the geometry?

Instead shouldn't you be storing the window state and then just if (state == normal) showNormal() else if state == maximized showMaximized() or something like that

This comment has been minimized.

@FearlessTobi

FearlessTobi Nov 30, 2017

Contributor

I actually tested this when I tried to figure out a solution. But I had two concerns, so I went with saving the Geometry:

  • When I tested just saving the state, when escaping fullscreen everthing showed up, except the Windows Program bar and the Taskbar. Citra essentially was still in fullscreen.
  • I couldn´t find a way to properly store the state of the render window when not in single window mode.

And another minor advantage of saving the geometry is, that not just state of the window but also it´s screen position before it was fullscreen is stored and will get restored.
Also i can´t really find any cases where would be better to just store the state. Imo the code even looks cleaner without another if-statement or switch.

@FearlessTobi FearlessTobi force-pushed the FearlessTobi:really-fix-fullscreen branch from eb4872e to 6ef0ac4 Dec 4, 2017

@FearlessTobi FearlessTobi force-pushed the FearlessTobi:really-fix-fullscreen branch from 6ef0ac4 to 8942bfd Dec 4, 2017

@FearlessTobi

This comment has been minimized.

Copy link
Contributor

FearlessTobi commented Dec 4, 2017

Rebased and squashed commits.

@bunnei

bunnei approved these changes Dec 7, 2017

@bunnei bunnei merged commit 040006f into citra-emu:master Dec 7, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@FearlessTobi FearlessTobi deleted the FearlessTobi:really-fix-fullscreen branch Dec 7, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment