Page MenuHome

Win32: Adjust Window Placement Allowing for DPI and Scale
ClosedPublic

Authored by Harley Acheson (harley) on Mar 31 2021, 2:13 AM.

Details

Summary

It might be hard to explain just how unimportant this patch is. It is really a "nobody will notice", "no rush", "optional", "last-mile" kind of correction.

If you have a Blend file saved that includes windows on multiple monitors and those monitors differ in scale and/or dpi the placement of windows on some monitors can be out by a small amount.

The following capture illustrates the issue as it exists in master now. The right side is my middle (main) monitor with one window, and there is another window on the left monitor which is set to 150% scaling factor. I place the two windows as I want them (shown on the top), then "Save Startup File", close blender, then open again. The bottom of the capture shows how the left window has moved down a bit - an amount equal to the increased height of the titlebar.

After this patch is applied, both windows stick to their saved places exactly.

This is because when we want to place a window where we want we need to account for the space used by the window chrome. But it is only with Windows 10 that we can do this with accuracy in situations like this where the dpi and scale change.

Diff Detail

Repository
rB Blender

Event Timeline

Harley Acheson (harley) requested review of this revision.Mar 31 2021, 2:13 AM
Harley Acheson (harley) created this revision.

@Ray Molenkamp (LazyDodo) - I probably could have done without the moving of MONITOR_DPI_TYPE and just used 0 as argument to GetDpiForMonitor(), rather than MDT_EFFECTIVE_DPI.

Harley Acheson (harley) edited the summary of this revision. (Show Details)Mar 31 2021, 2:23 AM
Harley Acheson (harley) edited the summary of this revision. (Show Details)Mar 31 2021, 6:54 PM
Harley Acheson (harley) updated this revision to Diff 37232.EditedMay 19 2021, 3:37 AM

Updated to current state of master.

Also fixes an issue (that predates my changes) reported here (T88384) and also here (T86804). In a nutshell the requested values have to be constrained to the monitor before accounting for dpi, scale, and window chrome sizes. This allows the placement of windows to be pixel-perfect.

Harley Acheson (harley) planned changes to this revision.May 19 2021, 3:19 PM

Will break this up to have a specific and simple fix for two bug reports that are not related to dpi.

Back to having this patch be only about using AdjustWindowRectExForDpi (if available) and not incorporate other changes.

Changes to correct placement at edges now in new patch here: https://developer.blender.org/D11314

intern/ghost/intern/GHOST_WindowWin32.h
51–61

What's the status of bumping Windows support to 8.1 minimum?

Harley Acheson (harley) planned changes to this revision.May 21 2021, 7:11 PM

This patch will get simpler after this lands: https://developer.blender.org/D11331

Updated to incorporate recent changes to master, notably the update to Windows API level.

Also created a separate adjustWindowRectForClosestMonitor() function since this nicely contained now.

Note there are slight changes outside of this, mostly in the constructor initialization.

Updated to change a class member initialization. And to remove some accidental cruft in a comment.

Updated to current state of master.

intern/ghost/intern/GHOST_WindowWin32.cpp
85

Leave out for now (soon to change).

88

Leave out for now (soon to change).

intern/ghost/intern/GHOST_WindowWin32.h
46

I'm not sure if there's an official position, but I don't think using _Inout_ et al annotations is a common pattern in Blender. Might be preferable to remove this for style consistency, or add it everywhere else if it's useful enough.

intern/ghost/intern/GHOST_WindowWin32.h
46

We're not using sal so having (or adding) the annotations isn't going to help all that much.

Updated to incorporate review suggestions.

@Nicholas Rishel (nicholas_rishel) - Leave out for now (soon to change

Yes, no worries. Have always assumed that your D11508 will go in before this one.

Updated to current state of master.

A couple comments but otherwise I think this is ready to go.

intern/ghost/intern/GHOST_WindowWin32.cpp
268–275

Reuse of width and height here makes the logic a little hard to follow. I'd recommend pulling these into new variables, which also gives the opportunity for a more descriptive name (edit is just for context, you likely have a better idea on appropriate names).

296

I'm guessing this newline is to delineate what the comment above applies to? I'd greenlight regardless but would prefer without newline.

This revision is now accepted and ready to land.Jun 22 2021, 10:59 PM

@Nicholas Rishel (nicholas_rishel) - Reuse of width and height here makes the logic a little hard to follow. I'd recommend...

How about this instead? This just passes a pointer to the rect that is adjusted, rather than those values. Seems clearer to me, is bit shorter too.

@Nicholas Rishel (nicholas_rishel) - Reuse of width and height here makes the logic a little hard to follow. I'd recommend...

How about this instead? This just passes a pointer to the rect that is adjusted, rather than those values. Seems clearer to me, is bit shorter too.

LGTM

updating to current state of master.