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

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

Closed
GuillaumeMiet opened this issue Jan 19, 2022 · 16 comments · Fixed by #42
Closed

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

GuillaumeMiet opened this issue Jan 19, 2022 · 16 comments · Fixed by #42
Labels
bug Something isn't working controls Changes to the appearance or logic of custom controls. locked-due-to-inactivity

Comments

@GuillaumeMiet
Copy link

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.

@pomianowski pomianowski added bug Something isn't working controls Changes to the appearance or logic of custom controls. labels Jan 19, 2022
@MajorXAML
Copy link
Contributor

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
Copy link
Contributor

Aelarion commented Jan 24, 2022

@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:

<Setter Property="WindowStyle" Value="SingleBorderWindow" />

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
Copy link
Contributor

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
Copy link
Member

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
Copy link
Contributor

Aelarion commented Jan 24, 2022

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 added a commit that referenced this issue Jan 25, 2022
Implement window dragging when maximized (fix #36)
@pomianowski pomianowski reopened this Jan 25, 2022
@pomianowski
Copy link
Member

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

image

@pomianowski
Copy link
Member

pomianowski commented Jan 25, 2022

Also, double-click stopped working sometimes

@Aelarion
Copy link
Contributor

@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:

ParentWindow.Top = 0d;

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:

ParentWindow.Left = screenPoint.X - (ParentWindow.RestoreBounds.Width * 0.5);

@Aelarion
Copy link
Contributor

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
Copy link
Member

pomianowski commented Jan 25, 2022

@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
Copy link
Contributor

Aelarion commented Jan 25, 2022

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
Copy link
Member

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
Copy link
Contributor

@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
Copy link
Member

pomianowski commented Jan 25, 2022

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
Copy link
Contributor

@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
Copy link
Member

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working controls Changes to the appearance or logic of custom controls. locked-due-to-inactivity
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants