fix(windows): fix aero-snap of borderless window

[amrbashir]

• Tested on all platforms changed
• Compilation warnings were addressed
• cargo fmt has been run on this branch
• cargo doc builds successfully
• Added an entry to CHANGELOG.md if knowledge of this change could be valuable to users
• Updated documentation to reflect any user-facing changes, including notes of platform-specific behavior
• Created or updated an example program if it would help users understand this functionality
• Updated feature matrix, if new features were added or implemented

Fixes #1548.

Note: This PR is merely a port of the cpp code in this repo

Note: The window isn't borderless when first created and needs to be maximized at least once or resized or anything that redraws the window before it can work properly. This however not a bug in my implementation, it is a bug in the subclass handler, you can reproduce it by adding println!("ANYTHING") inside WM_NCCREATE. It will never be called which means there is a gap before the subclass handler gets attached.

[maroider]

I haven't had the chance to test this, as I'm currently in Linux-land, implementing #753.

As discussed on Discord yesterday, some messages can be delivered to the window procedure during the call to CreateWindow, and WM_NCCREATE is presumably among them. I'm going to look into fixing this, but that's going to have to sit on the back-burner while I work on #753.

[amrbashir]

It is ok for this PR to wait until the subclass handler issue is fixed, In fact you can mark the PR as draft until then.

[amrbashir]

I found a workaround, we can resize the window with one pixel then revert it back after attaching the subclass.

After this line:

winit/src/platform_impl/windows/window.rs

Line 122 in e8cdf8b

event_loop::subclass_window(win.window.0, subclass_input);

we can do something like this

event_loop::subclass_window(win.window.0, subclass_input);

let win_flags = win.window_state.lock().window_flags();
if !win_flags.contains(WindowFlags::DECORATIONS) {
	let original = win.inner_size();
	win.set_inner_size(
		PhysicalSize::new(original.width + 1, original.height).into(),
	);
    win.set_inner_size(original.into());
}

let me know if this works for you and I will commit it.

[maroider]

I don't particularly like the hack you've proposed, but @msiglreith is the Windows maintainer in the end.

I'd prefer to move from using a subclass to using public_window_callback as a window procedure, but I'm also not entirely comfortable with that given that the original reason for using a subclass doesn't seem to be documented anywhere.

Unfortunately, I can't give you a precise estimate of when I'll get on with fixing this, but I hope to look into this during the next 2-4 weeks.

[amrbashir]

This workaround isn't enough unfortunately because the transparency seems to be broken too because of the subclass issue.

To confirm that transparency works well with a normal window procedure I cloned this repo https://github.com/melak47/BorderlessWindow/ and added the same transparency code that winit uses, and it works as expected along with the aero-snap and resizing

[amrbashir]

I will mark this PR as ready for review, since #1933 got merged and fixed the issues I had.

[maroider]

I believe I've found a workable solution to the "window spill-over "by using parts of this SO answer (thanks, @MrGibus, for linking me this). It doesn't quite get rid of all of it, but I think this is about as correct as other applications seem to get it.

One quirk of this solution is that the window shadow appears to still be around, and users can as such resize the window on the left, right, and bottom sides of the window. It is unclear to me what the best way to handle this would be, although my first reaction would be to get rid of the shadow somehow.

The slight vertical size error still persists, but I believe this is an error in some internal win32 function. The error goes away when I use a stub handler for WM_NCPAINT that always returns 0 and does nothing else.

I did try to see if I could link to an internal function I found (ThemeDefWindowProcW from UxTheme.dll) by stepping through what DefWindowProcW called, but I didn't really try further once linking failed (maybe I could hook the function somehow). In any case, using internal APIs like that is probably not what we want to do, so even if it did work immediately, I wouldn't really want to push that to master.

It might be possible to make borderless applications correct in this regard by using this WM_NCPAINT technique, but it's not entirely clear to me if that would work, and I haven't investigated it further.

There's unfortunately a problem where changing from "borderless" to "bordered" when the window is maximized makes Windows think the window suddenly isn't maximized any more.

There's also apparently a bug with the window_debug example where I have to press B twice in order to initially make the window borderless.

Bonus fact: If you use a WM_NCPAINT handler like the following, you'll get yourself some Windows Vista/7-style window decorations after toggling decorations off and on again.

winuser::WM_NCPAINT => {
    if userdata
        .window_state
        .lock()
        .window_flags()
        .contains(WindowFlags::DECORATIONS)
    {
        winuser::DefWindowProcW(window, msg, wparam, lparam)
    } else {
        0
    }
}

[JulianRuiseco]

Hey.

I ran into the following issue with snapping while trying to implement egui. Is this a manifestation of the issue this pull request solves, or is there something else at play?

cant_snap.mp4

Edit: Maroider confirmed that this is likely related in the gitter channel. Thank you.

[amrbashir]

I guess there is nothing but BUGS in this PR and I am tempted to close it tbh.

// bug: each call adds an extra 16px to width and around 40px to height
// expected: nothing changes as we are just sitting the window size to the same current size
window.set_inner_size(window.inner_size());
window.set_inner_size(window.inner_size());
window.set_inner_size(window.inner_size());

It doesn't happen on master btw and it happens in this PR even before @maroider spill-over fix.

[maroider]

I guess there is nothing but BUGS in this PR

I guess that's what we get for abusing Windows in this way 😅. However, I think we can turn this PR into something usable, it just might take some more months before we get there (largely due to how sporadically I work on things).

[amrbashir]

I guess there is nothing but BUGS in this PR and I am tempted to close it tbh.

// bug: each call adds an extra 16px to width and around 40px to height
// expected: nothing changes as we are just sitting the window size to the same current size
window.set_inner_size(window.inner_size());
window.set_inner_size(window.inner_size());
window.set_inner_size(window.inner_size());

It doesn't happen on master btw and it happens in this PR even before @maroider spill-over fix.

I think I found the solution to this bug (at least fixed on my machine, need someone else to confirm), adapted from this SO answer.

Edit: it doesn't work without the spillover fix (4618897) and it doesn't make sense anyway.

// src/plaform_impl/windows/util.rs
// in `set_inner_size_physical()`
let dpi = hwnd_dpi(window);
let titlebar_height = winuser::GetSystemMetricsForDpi(winuser::SM_CYSIZE, dpi)
	+ winuser::GetSystemMetricsForDpi(winuser::SM_CXPADDEDBORDER, dpi) * 2
	+ winuser::GetSystemMetricsForDpi(winuser::SM_CYBORDER, dpi);

let rect = adjust_window_rect(
	window,
    RECT {
    	top: 0,
    	left: 0,
    	bottom: (y - titlebar_height as u32) as LONG,
    	right: x as LONG,
	},
)
.expect("adjust_window_rect failed");

This ONLY works when decorations is disabled. If the decorations are enabled, The window will shrink.
So we need to pass is_decorated bool to util::set_inner_size_physical.

util::set_inner_size_physical is called in two places:

• Window::set_inner_size and we can easily pass the decorations state there
• BufferedEvent::dispatch_event in eventlopp/runner.rs and I am not sure if we should pass a dummy bool or make another version of util::set_inner_size_physical that takes is_decorated bool so we can leave this usage as is.

[amrbashir]

I'd like to make some progress in this PR if possible and last time I checked, there was two major issues with this PR:

• Undecorated windows now have shadows and resizing handles. While I think the resizing handles is Okay, The shadows are not because it will mess up with transparent windows. So, I am not sure what would be the way forward with this. Remove the spillover fix?
• set_inner_size() overflows the desired size. (Should be fixed in (4c0c760), but need someone else to confirm and review if it is the desired approach). Also the fix (4c0c760) might not work if we removed the spillover fix (4618897) by @maroider. Haven't tested this scenario Edit: it doesn't work if we remove the spillover fix.

Here is the example I use test this size overflow behavior:

use simple_logger::SimpleLogger;
use winit::{
    event::{Event, WindowEvent},
    event_loop::{ControlFlow, EventLoop},
    window::WindowBuilder,
};

fn main() {
    SimpleLogger::new().init().unwrap();
    let event_loop = EventLoop::new();

    let window = WindowBuilder::new()
        .with_title("A fantastic window!")
        .with_inner_size(winit::dpi::LogicalSize::new(128.0, 128.0))
        .with_decorations(false)
        .build(&event_loop)
        .unwrap();

    event_loop.run(move |event, _, control_flow| {
        *control_flow = ControlFlow::Wait;
        // println!("{:?}", event);

        match event {
            Event::WindowEvent {
                event: WindowEvent::CloseRequested,
                ..
            } => {
                println!("{:?}", window.inner_size());
                window.set_inner_size(window.inner_size());
            }
            Event::MainEventsCleared => {
                window.request_redraw();
            }
            _ => (),
        }
    });
}

[aevyrie]

• Undecorated windows now have shadows and resizing handles. While I think the resizing handles is Okay, The shadows are not because it will mess up with transparent windows. So, I am not sure what would be the way forward with this. Remove the spillover fix?

Forgive my ignorance. Is it possible for your work in this PR to be configurable? Fixing the issue addressed in this PR would effectively unblock using winit for custom decorated windows, and making this bugfix opt-in (or auto disabled) for users of transparent windows could be a way of preventing a regression.

[amrbashir]

Unfortunately this PR is not done yet and there is currently no way to manage whether the shadow is present or not. It is not the goal of this PR.

[amrbashir]

I made another attempt at fixing the size overflow in 00ee199. It is inspired by the hit test function in this microsoft guide. It works by removing the WS_CAPTION and WS_SIZEBOX from the window styles of the undecorated window before passing it to AdjustWindowRect* functions. It works correctly without the spillover fix from 4618897. When used with the spillover fix, the window starts to shrink instead of getting bigger.

For now, I commented out the spillover fix. I don't know if it is worth to keep it anyways. What do you think @maroider ?

[amrbashir]

superseded by #2419