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

Main/secondary display issue #420

Closed
TheSheepfolder opened this issue Nov 10, 2018 · 15 comments
Closed

Main/secondary display issue #420

TheSheepfolder opened this issue Nov 10, 2018 · 15 comments

Comments

@TheSheepfolder
Copy link

When I press spacebar to open QuickLook when File Explorer is on my secondary display, the window opens up really large, and it's too big to even see or click on the title bar to resize. It fills the whole screen and you have to press spacebar to close it, because the X button is off the screen. Pressing spacebar to open a file on the main display works perfectly normal and correct size.
I have a main display with 2736 x 1824 resolution, and the secondary display with 1280 x 1024 (the problem is on the secondary display).

Steps to reproduce the behavior:

  1. Open File Explorer and move the window to the secondary display.
  2. Select a file to preview, and press spacebar to open QuickLook for the file
  3. The QuickLook windows opens but it is really big, fills the whole screen, and the title bar is off the top of the screen (can't click there).

Expected behaviour:
The QuickLook windows opens but it is really big, fills the whole screen, and the title bar is off the top of the screen (can't click there).

Desktop:

  • OS Version: Windows 10 1809
  • QuickLook Version: 3.6.4 from Microsoft Store

Screenshots
main display screenshot
secondary display screenshot

@xupefei
Copy link
Member

xupefei commented Nov 10, 2018

I have just merged RP related to the DPI a few days before.
@Verrickt, could you give some comments?

@Verrickt
Copy link
Contributor

Weird. System DPI scaling is disabled,which is desired. But WPF's DPI scaling is not working, which is unexpected.
@TheSheepfolder Does your Windows 10 version>=1607 and .net framework version >=4.6.2 ?

Get window version by open CMD, type winver
Get .net framework version by open Powershell and type [System.Runtime.InteropServices.RuntimeInformation]::get_FrameworkDescription()

Besides, what's your monitor's DPI scaling setting? Does preview other type of files produce the same behavior?

Also, does open preview on your main display and move it to the secondary display changes the window size (if DPI setting are different)?

@TheSheepfolder
Copy link
Author

Windows version is 1809, .NET Framework 4.7.2.
Main monitor is 2736 x 1824 at 200% scaling, secondary monitor 1280 x 1024 at 100%.
I've found three things:

  1. The window size stays the correct size if I drag it from the main to secondary display (and I can't grab the title bar if I open it on secondary display, so can't check dragging that way).
  2. The problem I originally described only happens with text-based files (.txt, .sln, .vbs, .cs, .md, .json). Other files like images, archives, PDFs are normal.
  3. However, I've also now noticed that even if it's not a text-based file, the window doesn't open in the centre of the secondary display, it's a little to the top-left, and sometimes spills back onto the main display. This centering problem doesn't happen on the main display.

@xupefei
Copy link
Member

xupefei commented Nov 10, 2018

It seems that the font size has been scaled down (which is desired) but the window size did not. Let me check ...

@xupefei
Copy link
Member

xupefei commented Nov 10, 2018

https://docs.microsoft.com/en-us/windows/desktop/api/wingdi/nf-wingdi-getdevicecaps:

On a multiple monitor system, if hdc is the desktop, GetDeviceCaps returns the capabilities of the primary monitor.

I think here is the reason. Can you try the follows:

  1. Set the small monitor as primary
  2. Open a preview on the large monitor
  3. Is the window very small?

If so, then I think I should replace GetDeviceCaps with something else, e.g., VisualTreeHelper.GetDpi.

@Verrickt
Copy link
Contributor

I can reproduce the issue.
And I have been experimenting for a while and found a workaround, but need more investigation to find root cause.
Phenomenon is that the actual window size is multiplied by primary screen DPI scaling factor(in this case, 200%).

Here's the workaround:

Ignore DPI of WindowHelper.GetCurrentWindowRect

   var screen = Screen.FromPoint(Cursor.Position).WorkingArea;
   return new Rect(
                new Point(screen.X , screen.Y ),
                new Size(screen.Width , screen.Height ));

call ResizeAndCenter of ViewerWindow.BeginShow(IViewer matchedPlugin, string path, Action<string, ExceptionDispatchInfo> exceptionHandler) after Window.Show()

if (Visibility != Visibility.Visible)
{
       Show();
       ResizeAndCenter(newSize, _canOldPluginResize, ContextObject.CanResize);
}

Call ResizeAndCenter restored window size to normal, at the cost of window flicker

@xupefei
How did you move ViewerWindow to screen with current cursor focus?
I think maybe this is related.
I don't think we're affected of GetDeviceCaps, since we're now per monitor DPI aware. Just set size to 800*600 and WPF will handle the DPI scaling.
The real problem is that set size (800,600) with 200% primary display will create (1600,1200) at secondary display
capture

@Verrickt
Copy link
Contributor

Verrickt commented Nov 10, 2018

@xupefei

I think here is the reason. Can you try the follows:

  1. Set the small monitor as primary
  2. Open a preview on the large monitor
  3. Is the window very small?

I signed out between step 1 and 2 so GDI is aware of change of primary monitor.
However the issue persists
My Primary display is 1333*768 at 100% and Secondary 1920*1200 at 200%
Open QL at primary display works fine, while open at secondary display the window size is huge.

Primary
1333_768_100_primary

Secondary
1920_1200_200_secondary

If so, then I think I should replace GetDeviceCaps with something else, e.g., VisualTreeHelper.GetDpi.

VisualTreeHelper.GetDpi has its shortcomings too.
It requires a Visual and if

  • Visual is not visible
    VisualTreeHelper.GetDpi returns DPI of primary display
  • 'Visual' is visible
    VisualTreeHelper.GetDpi returns DPI of display the visual currently on.

I mean since QL is now declared as Per monitor DPI aware, we should not care DPI any more if we're using native WPF controls. Similar to UWP, we just ignore DPI when designing and thinking, and system will make sure it looked great on any DPI scaling factors.

I'll investigate what will happen if we remove all explicit DPI queries.

@xupefei
Copy link
Member

xupefei commented Nov 10, 2018

Update: Ouch I am wrong. The MoveWindow API is only called when switching to another file. When opening a new window, https://github.com/QL-Win/QuickLook/blob/master/QuickLook/ViewerWindow.Actions.cs#L97 to Line 100 are used.

I don't think we can get rid of DPI things when moving the window because I use Win32 API to resize the window and move it to the desired position: https://github.com/QL-Win/QuickLook.Common/blob/44448c9f28dc826db4c15e3e1b0b352b743b0885/Helpers/WindowHelper.cs#L56.
The reason is that if you just set Left=xxx; Top=xxx; Width=xxx; Height=xxx; one by one, WPF will change each property one by one, resulting in four times of window flickering.

I wonder when the numbers go wrong. Can you check:

  1. Let the primary display uses 200% DPI, and an explorer window a 100% DPI display)
  2. Is the return value of WindowHelper.GetCurrentWindowRect() scaled?

Maybe we can move the window to the target display before calling ResizeAndCenter. But we need to check at first.

@xupefei
Copy link
Member

xupefei commented Nov 10, 2018

Well, I guess:

  1. When creating a new ViewWindow, WPF assumes that it is on the primary display.
  2. When setting Left, Top, Width, and Height, WPF multiple each by the DPI of the primary display, since it is assumed as such.
  3. When calling Show(), The Left, Top, Width, and Height haven't been changed back. Now the window is at the centre of the second display, with Left, Top, Width, and Height multiplied by 2.

@Verrickt
Copy link
Contributor

Verrickt commented Nov 10, 2018

I wonder when the numbers go wrong. Can you check:

  1. Let the primary display uses 200% DPI, and an explorer window a 100% DPI display)
  2. Is the return value of WindowHelper.GetCurrentWindowRect() scaled?

The rect is scaled down. (683,364)*200%=(1366,768)

screen
{960,144.5,683,364}
    Bottom: 508.5
    BottomLeft: {960,508.5}
    BottomRight: {1643,508.5}
    Height: 364
    IsEmpty: false
    Left: 960
    Location: {960,144.5}
    Right: 1643
    Size: {683,364}
    Top: 144.5
    TopLeft: {960,144.5}
    TopRight: {1643,144.5}
    Width: 683
    X: 960
    Y: 144.5

The reason is that
WindowHelper.GetCurrentWindowRect() called DpiHelper.GetCurrentScaleFactor(), which returns main DPI - 200%.

var screen = Screen.FromPoint(Cursor.Position).WorkingArea;
var scale = DpiHelper.GetCurrentScaleFactor();
return new Rect(
                new Point(screen.X / scale.Horizontal, screen.Y / scale.Vertical),
                new Size(screen.Width / scale.Horizontal, screen.Height / scale.Vertical));

As I mentioned in workaround, I think we should remove the DPI query here.

Well, I guess:

1.When creating a new ViewWindow, WPF assumes that it is on the primary display.
2. When setting Left, Top, Width, and Height, WPF multiple it by the DPI of the primary display, since it is assumed as such.
3. When calling Show(), The Left, Top, Width, and Height haven't been changed back. Now the window is at the centre of the second display, with Left, Top, Width, and Height multiplied by 2.

If this is the case, maybe we can

  1. Hide the window
  2. Call ResizeAndCenter() to normalize the size
  3. Call Window.Show()

Update: Not working☹

@Verrickt
Copy link
Contributor

Found the culprit!
Window.Show() will change Window.Height and Window.Width
Before Window.Show()
Height:634,Width:802
After Window.Show()
Height:1230,Width:1604

@xupefei
Copy link
Member

xupefei commented Nov 10, 2018

If you change

Left = newLeft;
Top = newTop;
if (double.IsNaN(Left) || double.IsNaN(Top)) // first time showing
    WindowStartupLocation = WindowStartupLocation.CenterScreen;

to

Left = 970;
Top = 150;
//if (double.IsNaN(Left) || double.IsNaN(Top)) // first time showing
//    WindowStartupLocation = WindowStartupLocation.CenterScreen;

Will the window scale?

@xupefei
Copy link
Member

xupefei commented Nov 10, 2018

Found this: https://stackoverflow.com/questions/13230151/how-to-force-wpf-startup-window-to-specific-screen:

Based on my tests, per-monitor DPI awareness only kicks in after window is already created and placed on some monitor. Before that, apparently Left/Top properties of the window scale with DPI of the primary monitor.

Seems like yet another lovely bug of Windows 10. Hostely, I am already tired to play with all kinds of freaking bugs and workarounds for all day. There is a reason why Windows Development Environment sucks.

To the problem, if what this guy said is true, we can then get rid of Top/Left/Width/Height properties by just sending a MoveWindow Dispatcher command with a correct DispatcherPriority. The code should be invoked after SourceInitialized and before Render.

@Verrickt
Copy link
Contributor

Verrickt commented Nov 11, 2018

I've report the issue to WPF team at M$ and is expecting no response🙂. And I agree with you, Windows Desktop Development definitely sucks. It's 8102 , even Electron, a Web based GUI framework, handles GUI stuff better than the native Windows GUI framework

Winform/WPF have not been updated in years and UWP is nowhere function equivalent of them
Want triggers on ControlTemplate? Sorry, you can't. But you can use VisualStateManager. It's only several hundred lines of XAML. Oh, is that too much ? No problem, use VS blend and you get a automatically generated hundreds lines of XAML. Now you just need to edit some of the hundreds lines of generated XAML to achieve what could been done in 3 lines of XAML with Trigger. And now you never need to worry about issue caused by the order of triggers. How elegant!

Alright, enough complaint. I'll try your suggestion when I have time

@xupefei
Copy link
Member

xupefei commented Oct 11, 2020

This issue should be fixed in https://ci.appveyor.com/project/xupefei/quicklook/builds/35688242/artifacts. Could you have a try?

Jethro-Alter added a commit to Jethro-Alter/QuickLook that referenced this issue Dec 21, 2020
* Update Translations.config

* Update Translations.config

* Update Translations.config

* Create Privacy.md

* Rename Privacy.md to PRIVACY.md

* Update PRIVACY.md

* Update PRIVACY.md

* Fix QL-Win#579: search box changed in Windows 10 1909

* Fix QL-Win#579: also deal with non-English UIs

* don't bring existing window to the front

* Revert "Fix QL-Win#644: still use focusable window on Windows 7 and 8"

This reverts commit 452574e.

Revert "Fix QL-Win#644 step 1: correctly bring window back to top when clicked"

This reverts commit af608dc.

* Fix QL-Win#401: use the native image provider to render static GIFs

* Update Translations.config

Hungarian language added.

* Fix QL-Win#669: convert image to sRGB only when the original ColorSpace is RGB, sRGB, or scRGB

* Upgrade Magick.NET packages

* Fix QL-Win#669 again: stupid syntax mistake :(

* Bulk update NuGet packages

* Update Translations.config

Typo correction on line 61 and 69. Fixing slight translation mistakes and some words also replaced with more commonly used words.

* Support File Path longer than 260 (tweak needed: https://www.tenforums.com/tutorials/51704-enable-disable-win32-long-paths-windows-10-a.html)

* Add EXR to ImageViewer

* Add MXF to VideoViewer

* Decrease update checking frequency to monthly

* Fix QL-Win#715: add 1s timeout before any previewing request

* Fix QL-Win#711: adjust UI before loading any PDF page

* Fix QL-Win#729: add a config flag to hide the tray icon

* Fix QL-Win#731: use app folder for saving data in the portable mode

* Fix QL-Win#733: detect Markdown encoding

* Fix QL-Win#734: add AVIF format

* Update .appveyor.yml

* added german translation for newly added string

* fix a typo

* Fix QL-Win#744: Components are not correctly disposed upon exit

* Fix QL-Win#477: Add JFIF files

* Update bug_report.md

* typo

* Fix QL-Win#420, QL-Win#452, QL-Win#757: windows positioning for monitors with different DPIs

* Update Common to fix plugin resize

* Fix QL-Win#759: fix thumbnail orientation for some camera models

* Fix QL-Win#760, support CR3

* removed dup of ".asf" and added ".mka" for testing

Co-authored-by: Piteriuz <piteriuz@outlook.com>
Co-authored-by: Pengfei Xu <xupaddy@gmail.com>
Co-authored-by: Bobcruise <bobcruise07@gmail.com>
Co-authored-by: davinhanif <39960696+davinhanif@users.noreply.github.com>
Co-authored-by: Alexander Raab <alexander.raab1@gmx.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants