Fix Resolution Bug

[Carterpersall]

Uses Get-CimInstance fix #76 without creating a massive speed decrease

Closes #76 :

• Added two methods of getting screen resolution
  • First one runs if elevated, and gets the screen information from HKLM:\SYSTEM\CurrentControlSet\Control\GraphicsDrivers\Configuration\
    • Contains refresh rate too, so I added that in
  • Second method uses implementation in Fix incorrect screen resolution when dpi is set #131 which is about 4x slower for me but seems to be the only way to get it working that I can find

Possible avenues for speedup:

• HKEY_CURRENT_USER\Control Panel\Desktop\WindowMetrics has a DPI value but not per-monitor DPI
  • It is used in Windows 8.1 and before as per-display scaling was added in Windows 10
    • Maybe add a Windows version check
• HKEY_USERS\Whatever-Your-User-GUID-is\Control Panel\Desktop\PerMonitorSettings stores where every monitor's scale is set relative to the recommended value
  • Might work if the script can get the recommended scale of monitors
• Get-CimInstance -ClassName Win32_VideoController gives resolution, but only for one monitor per video adapter
  • Could use [System.Windows.Forms.SystemInformation]::MonitorCount to check if there's only one monitor connected

[rashil2000]

Have you checked this on multiple monitors? It fails with this error:

You can fix it by dividing later:

"$($monitors[$i].Bounds.Size.Width * ($scale[$i] / 96))x$($monitors[$i].Bounds.Size.Height * ($scale[$i] / 96))"

[rashil2000]

Even after that it seems to give incorrect answer, it shows me 3200x1800 when I have two monitors at 2560x1440 (enabled - 100% scaling) and 1920x1080 (disabled - 100% scaling).

[Carterpersall]

Have you checked this on multiple monitors?

You can fix it by dividing later:

"$($monitors[$i].Bounds.Size.Width * ($scale[$i] / 96))x$($monitors[$i].Bounds.Size.Height * ($scale[$i] / 96))"

This is how I initially had it when I was testing with multiple monitors, I changed it to the broken version later when I had only one so I didn't see this error.

[Carterpersall]

It seems that the way I'm currently getting the DPI always returns 120 (125%) for some reason. Guess I'm back to square one again sigh

[Carterpersall]

It should be working now. Added two different implementations depending on if the script is elevated.
The first one uses the registry location that has every saved display configuration, compares the timestamp key on each one, and takes the most recent one (current one). The location requires admin to access though which is why it checks if it is elevated beforehand. The location also includes each monitor's refresh rate, so I added that into the returned string.
The second method uses the implementation in #131 which is about 4x slower for me (50ms vs 200ms on a non-bottlenecked PowerShell instance).

[rashil2000]

I'm not really a fan of this method. It is extremely unlikely that anyone would run winfetch as admin (most users we get bug reports from don't even have admin rights on their PC), and even if they do, running as admin for just one function does not make much sense. Not to mention that there's essentially no way for users to know that resolution would be reported faster if they run winfetch as admin.

In essence the PR brings me back to the problems I had with #131, i.e., increased script complexity and a serious performance regression for the average use case.

[Carterpersall]

It is extremely unlikely that anyone would run winfetch as admin (most users we get bug reports from don't even have admin rights on their PC)

I almost always use PowerShell as admin, but I might be an outlier there.

running as admin for just one function does not make much sense.

I agree, but if someone is in an elevated instance already for some other reason, I don't see any reason not to use a faster method exclusive to elevated instances.

Not to mention that there's essentially no way for users to know that resolution would be reported faster if they run winfetch as admin.

We could add a comment into the config file generator that says that a faster method is used in elevated instances

In essence the PR brings me back to the problems I had with #131, i.e., increased script complexity and a serious performance regression for the average use case.

I usually would agree, but the current implementation is broken for everyone who don't have their scaling at 100% (which likely is the majority of users). And the implementation here, while slower, actually works no matter the scaling you have set in your system.

[rashil2000]

We could add a comment into the config file generator that says that a faster method is used in elevated instances

Since resolution segment is enabled by default, people will not check the config file for it.

(which likely is the majority of users).

It's majority of users who use a non-default scaling, which would actually be a small number.

Nevertheless, if we are going to merge the DLLImport code, I'd much rather prefer we do it in #131, because @HO-COOH was the original author for it.

As for this PR, we can settle on a middle ground - check for admin and use your method if true, otherwise fallback to what was present originally.

[Carterpersall]

It's majority of users who use a non-default scaling, which would actually be a small number.

The bug occurs for everyone who has their scaling set to something other than 100%. And Windows almost always has the default setting higher than 100% (for me it's 200%). This is because Windows tries to set the default scaling to match 720p, and the majority of people using this script will be running 1080p or higher. Thus, for the majority of users, the current implementation does not work correctly.

Nevertheless, if we are going to merge the DLLImport code, I'd much rather prefer we do it in #131, because @HO-COOH was the original author for it.

As for this PR, we can settle on a middle ground - check for admin and use your method if true, otherwise fallback to what was present originally.

To prevent this pull request conflicting with the changes in #131, how about #131 gets merged first, then this pull request?

[rashil2000]

To prevent this pull request conflicting with the changes in #131, how about #131 gets merged first, then this pull request?

Resolving conflicts should not be that difficult - so I'm fine with either

[Carterpersall]

I'd prefer to keep this as is and see what needs to be done once #131 is merged