lepoco / wpfui

WPF UI provides the Fluent experience in your known and loved WPF framework. Intuitive design, themes, navigation and new immersive controls. All natively and effortlessly.
https://wpfui.lepo.co
MIT License
7.4k stars 717 forks source link

Allow window dragging when maximized (from title bar) #36

Closed GuillaumeMiet closed 2 years ago

GuillaumeMiet commented 2 years ago

This is a standard feature in windows app. Main window should be able to be moved when it's maximized as a standard Windows application. When the drag starts, the window should automaticaly return to its last "normal" size.

MajorXAML commented 2 years ago

WindowChrome has all the functionality of a standard window, except for snap layouts. Dragging does not work because of this https://github.com/lepoco/wpfui/blob/main/WPFUI/Styles/Controls/Window.xaml#L50

Increase it to 40-50 and it will work, but the Titlebar buttons will become inactive. To fix that, you need to add this for each button WindowChrome.IsHitTestVisibleInChrome="True"

Btw, snap layout, I do not know how it is implemented, but I think it is buggy. simple

Aelarion commented 2 years ago

@MajorXAML good callout on the caption height and WindowChrome however I think this is a bit more complicated. The style comment indicates the caption height being set to 1 is a workaround to some theming issues, (I think) related to using SingleBorderWindow WindowStyle: https://github.com/lepoco/wpfui/blob/2ff8a5c1af28db949b0bda8c985ce4d2374e0ed9/WPFUI/Styles/Controls/Window.xaml#L35

Additionally, we have to contend with the TitleBar control handling the WindowChrome functionality (instead of the Window). It looks like this was already identified awhile ago from the comments in TitleBar.RootGrid_MouseDown. I tried enabling the implementation there, and it does work but has a funky side-effect that the window position is offset based on the Window.RestoreBounds value (e.g. when the drag starts, the window is restored to its original position on the screen and offset from the mouse, not under the mouse). It also has the downside of restoring the ParentWindow when the TitleBar is simply clicked, which isn't expected behavior.

restorebounds

Since RestoreBounds can't directly be set, a quick and dirty fix might be simply setting the ParentWindow.Left and .Top values after restoring the window on the drag event. In practice though that would likely cause some jumpy window behavior, so definitely not a great solution either.

Edit: fixed some line links

Aelarion commented 2 years ago

Sorry for the comment spam but just wanted to add while fresh in my mind --

There may be some hacky Win32 voodoo we could try strictly when the mouse drags on the TitleBar control while it is maximized. I remember having to do some SendMessage / WM_NCHITTEST black magic back in my WinForms days ...eyes glaze over thinking about dragging and resizing a WinForm with no title bar...

pomianowski commented 2 years ago

I've already tried a bit of Win32 parkour with SnapLayout, and I don't really like these sloppy workarounds.

I think it's best to dig in here and try to get something out https://github.com/dotnet/wpf/tree/main/src/Microsoft.DotNet.Wpf/src

Aelarion commented 2 years ago

After doing a bit more digging it seems like we can set the ParentWindow.Left and ParentWindow.Top values prior to changing the WindowState to prevent that jumpy behavior. From what I can find this is actually the recommended way to do it (see https://stackoverflow.com/questions/11703833/dragmove-and-maximize)

Not sure if it makes more sense to subscribe to this.MouseMove or the RootGrid one, but just was playing around and came up with this:

private void RootGrid_MouseMove(object sender, MouseEventArgs e)
{
    if (e.LeftButton != MouseButtonState.Pressed || ParentWindow == null) return;

    if (IsMaximized)
    {
        // calculate a point that makes sense to drag from
        var point = PointToScreen(e.MouseDevice.GetPosition(this));
        ParentWindow.Left = point.X - (ParentWindow.RestoreBounds.Width * 0.5);
        ParentWindow.Top = point.Y; // needs some tuning, window is placed a little low on the cursor

        // style has to be quickly swapped to None to avoid restore animation delay
        var style = ParentWindow.WindowStyle;
        ParentWindow.WindowStyle = WindowStyle.None;
        ParentWindow.WindowState = WindowState.Normal;
        ParentWindow.WindowStyle = style;

        ParentWindow.DragMove();
    }
}

In this case, I was specifically avoiding calling MaximizeWindow() to restore, since I'm not sure we want to respect the MaximizeActionOverride for WindowChrome behavior.

This actually works pretty good for the maximize-->drag functionality. As you can see in the implementation, we have to change the WindowStyle to None for a brief moment -- without doing this, you end up with a delay for the restore animation to play which is really annoying (and actually ends up messing with the drag location under the mouse).

The only snag I have with this is seeing like a frame or two of jank -- I can't quite put my finger on what's causing it, but it seems it's either the rendering while the WindowStyle is set to None or something with resizing (or both). All in all though, it's pretty tolerable.

Edit: adding gif to show working POC, unfortunately this doesn't capture the frame of jank

max-drag

And here's a quick screen cap of the resize jank frame:

max-restore-jank

pomianowski commented 2 years ago

@Aelarion When dragged from the edge in the multi-monitor setup it throws the application to the side.

image

pomianowski commented 2 years ago

Also, double-click stopped working sometimes

Aelarion commented 2 years ago

@pomianowski hmm, I tried this across 2 side by side monitors, I'm not able to reproduce the issue. I also am not sure how double clicking would be effected, I can't reproduce that issue either. Unless there's somewhere that the MouseMove event is stepping on the MouseDown event

I think definitely an issue to address though is this line:

https://github.com/lepoco/wpfui/blob/a0795c3e2435c1564d8548814e7b2f9eb5734668/WPFUI/Controls/TitleBar.cs#L484

I didn't consider this before but in a multi-monitor setup, where the "top" screen coordinates might be negative, this would cause monitor jumping. Definitely can fix setting to ParentWindow.Top = screenPoint.Y instead of hardcoded 0.

Can you just try that workaround to see if it helps with the window jumping? It's the only thing I can think of, other than maybe the Left calculation getting screwed up by DPI scaling or something funky:

https://github.com/lepoco/wpfui/blob/a0795c3e2435c1564d8548814e7b2f9eb5734668/WPFUI/Controls/TitleBar.cs#L483

Aelarion commented 2 years ago

Just tried scaling my monitor DPI way up to 150%, yup that was the problem. That's my own fault for not RTFM'ing on PointToScreen and DPI-aware funkiness with the application.

I will work on a fix for this -- essentially would just need to account for the scaling and multiple the final Left and Top values by appropriate X and Y DPI scaling factors and should be good to go.

pomianowski commented 2 years ago

@Aelarion, I think this is due to the rather non-standard size and asymmetrical arrangement, DPI can also be a problem because it was already troublesome with SnapLayout image

Aelarion commented 2 years ago

Just troubleshooting with DPI scaling, I can get this working, the only issue I see is in .NET Framework 4.6 VisualTreeHelper.GetDpi is not available (starts at 4.6.2). As a workaround, we can grab DPI values from Reflection, but that has its own trade-offs.

Can you see if this works for you? The asymmetrical window arrangement will have to be a separate issue I tackle -- just want to get the basic math stuff right first.

private void RootGrid_MouseMove(object sender, MouseEventArgs e)
{
    if (e.LeftButton != MouseButtonState.Pressed || ParentWindow == null) return;

    if (IsMaximized)
    {
        var dpiXProperty = typeof(SystemParameters).GetProperty("DpiX", System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Static);
        var dpiYProperty = typeof(SystemParameters).GetProperty("Dpi", System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Static);
        var dpiX = (int)dpiXProperty.GetValue(null, null);
        var dpiY = (int)dpiYProperty.GetValue(null, null);
        //var dpi = VisualTreeHelper.GetDpi(ParentWindow); --> not available in .NET 4.6
        var screenPoint = PointToScreen(e.MouseDevice.GetPosition(this));
        screenPoint.X *= (96.0 / dpiX);
        screenPoint.Y *= (96.0 / dpiY);

        // TODO: refine the Left value to be more accurate
        // - This calculation is good enough using the center
        //   of the titlebar, however this isn't quite accurate for
        //   how the OS operates.
        // - It should be set as a % (e.g. screen X / maximized width),
        //   then offset from the left to line up more naturally.
        //ParentWindow.Left = dpi.DpiScaleX * (screenPoint.X - (ParentWindow.RestoreBounds.Width * 0.5));
        //ParentWindow.Top = dpi.DpiScaleY * screenPoint.Y;
        ParentWindow.Left = screenPoint.X - (ParentWindow.RestoreBounds.Width * 0.5);
        ParentWindow.Top = screenPoint.Y;

        // style has to be quickly swapped to avoid restore animation delay
        var style = ParentWindow.WindowStyle;
        ParentWindow.WindowStyle = WindowStyle.None;
        ParentWindow.WindowState = WindowState.Normal;
        ParentWindow.WindowStyle = style;

        ParentWindow.DragMove();
    }
}

EDIT: fixed typo for DPI Y property ("Dpi" not "DpiY")

pomianowski commented 2 years ago

It begins to seem to me that this class has too much responsibility, can you set up a separate class to manage DPI and screen stuff?

Aelarion commented 2 years ago

@pomianowski agreed, will do when I get some more time tonight. Can you just confirm when you have time to test that the above code works for the window jumping issue you were seeing?

pomianowski commented 2 years ago

Hey @Aelarion, the DPI tweaks you added greatly improve the window drag behavior. In any case, double-click still randomly does not work.

I don't always get the error, but if I have one Demo app open in the debugger and open a second instance of the application outside of Visual Studio, the second instance easily generates this bug. Animation

I think the app sometimes catches (click + drag + click) instead of (click + click), thus triggering the swipe and ignoring the double-click maximization. Or (click + click (hold) + drag), as a result, it maximizes and restores immediately. Probably would have to check in MouseMove event whether DoubleClick was there a moment ago

Aelarion commented 2 years ago

@pomianowski glad to hear it! On the double click issue that would definitely make sense (click + drag).

I don't want to overstep my bounds but I think maybe we should open a separate issue for the double click / drag clash, seems like maybe we have an opportunity to clean up some logic in the event handlers.

pomianowski commented 2 years ago

@Aelarion sure, please send your DPI correction in new PR and I will make a new issue related to double click.