Skip to content

Refactor win32 window state code#730

Merged
Osspial merged 11 commits intorust-windowing:masterfrom
Osspial:window_state_overhaul
Feb 4, 2019
Merged

Refactor win32 window state code#730
Osspial merged 11 commits intorust-windowing:masterfrom
Osspial:window_state_overhaul

Conversation

@Osspial
Copy link
Copy Markdown
Contributor

@Osspial Osspial commented Nov 29, 2018

  • Tested on all platforms changed
  • Added an entry to CHANGELOG.md if knowledge of this change could be valuable to users

This PR refactors the Windows state-setting code to make it significantly easier to read and modify going forward. It also fixes #723 and a few other miscellaneous untracked issues I found when doing the refactor.

TODO:

@august64 august64 added the DS - win32 Affects the Win32/Windows backend label Dec 2, 2018
@august64 august64 mentioned this pull request Jan 1, 2019
@Osspial Osspial force-pushed the window_state_overhaul branch from b04622c to 5851c56 Compare January 1, 2019 19:57
@Osspial Osspial force-pushed the window_state_overhaul branch from 5851c56 to 4cba678 Compare January 1, 2019 20:26
@Osspial
Copy link
Copy Markdown
Contributor Author

Osspial commented Feb 4, 2019

There hasn't been much movement on getting this reviewed, but this fixes quite a few issues and improves the code quality in the Windows backend so I'm gonna go ahead merge this.

@Osspial Osspial merged commit 7be1d16 into rust-windowing:master Feb 4, 2019
@JasonHise
Copy link
Copy Markdown

Was about to submit an issue but it looks like this is still open and is probably the same thing? Our particular use case is that the window is shrinking when toggling back and forth between windowed and borderless fullscreen on windows, my shot in the dark guess would be that there is an inconsistency between whether the window still/already has a border when the client rect gets saved and restored, respectively.

@aloucks
Copy link
Copy Markdown
Contributor

aloucks commented Aug 22, 2019

I experienced something similar where the size/position wasn't restoring correctly, but eventually sorted it out by setting everything manually instead of using the set_fullscreen API. This also allows you to fullscreen across two monitors (but assumes they are the same size). The logic boils down to:

enum WindowMode {
    Fullscreen {
        last_position: LogicalPosition,
        last_size: LogicalSize,
    },
    Windowed,
}

pub fn toggle_window_mode(&mut self) {
    match self.window_mode {
        WindowMode::Windowed => {
            let last_position = self.window.outer_position().unwrap();
            let last_size = self.window.inner_size();
            let monitor = self.window.current_monitor();

            let dpi_factor = monitor.hidpi_factor();
            let x = monitor.position().x as _;

            // If the window is wider than the current monitor, stretch it across all monitors
            let mut inner_physical_size = monitor.size();
            if last_size.to_physical(dpi_factor).width > inner_physical_size.width {
                let monitor_count = self.window.available_monitors().count();
                inner_physical_size.width *= monitor_count as f64;
            }

            self.window.set_visible(false);
            self.window.set_decorations(false);
            self.window.set_outer_position(LogicalPosition::from((x, 0)));
            self.window.set_inner_size(inner_physical_size.to_logical(dpi_factor));
            self.window.set_visible(true);

            self.window_mode = WindowMode::Fullscreen {
                last_position,
                last_size,
            };
        }
        WindowMode::Fullscreen {
            last_position,
            last_size,
        } => {
            self.window.set_visible(false);
            self.window.set_decorations(true);
            self.window.set_inner_size(last_size);
            self.window.set_outer_position(last_position);
            self.window.set_visible(true);

            self.window_mode = WindowMode::Windowed;
        }
    }
}

@Osspial
Copy link
Copy Markdown
Contributor Author

Osspial commented Aug 26, 2019

@JasonHise I think you're right about that. There's probably some further simplification of our underlying model that we could do to fix that, but fixing that is fairly low on my personal priority list.

@Osspial
Copy link
Copy Markdown
Contributor Author

Osspial commented Sep 17, 2019

@JasonHise Actually, looking back at this, it looks like that's a separate bug entirely! It looks like there was a small oversight in some code that arrived later and we weren't handling the window rect restore correctly. It's a fairly simple fix though, so I've made a PR here: #1172.

Sorry about dismissing your issue earlier!

@JasonHise
Copy link
Copy Markdown

@Osspial awesome, thanks!

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

Labels

DS - win32 Affects the Win32/Windows backend

Development

Successfully merging this pull request may close these issues.

On Windows, hide_cursor hides cursor for all Winit windows

4 participants