Windows: Implement `resolution` #147

Carterpersall · 2023-03-20T18:36:55Z

Hello, I'm a contributor to https://github.com/lptstr/winfetch (you've talked to the maintainer in #112), and would like to start applying some of the experience I gained there to this project.

Uses a combination of my implementation in Fix Resolution Bug lptstr/winfetch#156 and this one
- SetProcessDpiAwareness tells the OS to give the program the "real" resolution of each monitor instead of the scaled resolution
- Uses EnumDisplayMonitors to get the information of each monitor
  - Uses a callback function to get the information on each monitor using GetMonitorInfoW and store the result in a vector

TODO:

~~[ ] Implement my method in Fix Resolution Bug lptstr/winfetch#156~~
- ~~Only works in admin instances of PowerShell, but I need to test if it's the same for Rust~~
  - ~~It is the same in Rust, and it seems that the crate WinReg does not anticipate a registry key requiring special permissions as it is not working properly~~
- Probably more effort than it's worth

CarterLi · 2023-03-21T03:08:15Z

Why EnumDisplayMonitors instead of EnumDisplayDevicesW + EnumDisplaySettingsW (ENUM_CURRENT_SETTINGS)?

EnumDisplayDevicesW is not affected by DpiAwareness, so no SetProcessDpiAwareness needed.
EnumDisplayDevicesW doesn't use callback, so it is easier to use
EnumDisplayDevicesW provides refresh rate

While EnumDisplayMonitors is still useful to fetch scaled resolutions. with SetProcessDpiAwareness(PROCESS_DPI_UNAWARE), if needed

Carterpersall · 2023-03-21T14:05:38Z

Why EnumDisplayMonitors instead of EnumDisplayDevicesW + EnumDisplaySettingsW (ENUM_CURRENT_SETTINGS)?

Because I didn't know it existed. The Win32 API is such a chaotic mess of useful functions that something always seems to slip by.
I'll see if I can throw together another implementation using those functions.

CarterLi · 2023-03-21T15:40:54Z

This is an example that fetchs both pixel resolutions, scaled resolutions and refresh rates.

https://github.com/LinusDierheimer/fastfetch/blob/b3da6b0e89c0decb9ea648e1d98a75fa6ac40225/src/detection/displayserver/displayserver_windows.c#L48

CarterLi · 2023-03-21T15:57:40Z

There are other interesting things. For example: fetching OS caption without querying WMI: https://github.com/LinusDierheimer/fastfetch/blob/b3da6b0e89c0decb9ea648e1d98a75fa6ac40225/src/detection/os/os_windows.cpp#L34

Carterpersall · 2023-03-21T17:58:38Z

This is an example that fetchs both pixel resolutions, scaled resolutions and refresh rates.

LinusDierheimer/fastfetch@b3da6b0/src/detection/displayserver/displayserver_windows.c#L48

I got an implementation working, thanks for the tip though

There are other interesting things. For example: fetching OS caption without querying WMI: LinusDierheimer/fastfetch@b3da6b0/src/detection/os/os_windows.cpp#L34

Yeah, there are a lot of things that most programs default to calling WMI for that can be found by just using the registry. That's one thing I intend on working on

While working on the new implementation, I discovered that EnumDisplayDevicesW also returns the system's display adapters, giving yet another way to retrieve the system's GPUs.

Carterpersall · 2023-04-13T19:02:18Z

Implementation Speeds:

Command used: hyperfine --warmup 20 --prepare "cmd" --min-runs 500 ".\macchina.exe --show resolution"
- Compiled in release mode

Plugged in:

EnumDisplayDevicesW
- 14.5 ms
EnumDisplayMonitors
- 14.1 ms
WMI
- 203 ms

On Battery life:

EnumDisplayDevicesW
- 15.2 ms
EnumDisplayMonitors
- 13.5 ms
WMI
- 227.2 ms

When heavily throttled (used power plans to downclock CPU to 1.06 GHz to simulate a slow computer)

EnumDisplayDevicesW
- 49.2 ms
EnumDisplayMonitors
- 47.3 ms
WMI
- 697.3 ms

CarterLi · 2023-04-14T04:31:45Z

Besides being slow, WMI Win32_VideoController reports one resolution per GPU adapter only. I see no benefits use WMI here.

Only gets one monitor per adapter

Carterpersall · 2023-04-14T16:30:10Z

Besides being slow, WMI Win32_VideoController reports one resolution per GPU adapter only. I see no benefits use WMI here.

Yeah, I probably should've checked for that before adding it. I've removed the WMI implementation in the most recent commit

uttarayan21 · 2023-04-24T06:29:52Z

@grtcdr I don't think I'm qualified to review this. I don't have enough experience with windows to comment on this.
However I can help with the testing of this.

grtcdr · 2023-04-24T12:32:42Z

Neither am I, that's okay. I'd appreciate it if someone else (@CarterLi, perhaps) gave a final review before merging.

CarterLi · 2023-04-24T17:49:30Z

Sorry, but I'm not familiar with rust.

This is what I got on my MBP

macchina should use the same format on different platforms, including on Windows. That is: pixelHxpixelV@refreshRatefps (as scaledHxscaledV). However the code seems to use another format
I still see no benefits adding EnumDisplayMonitors as a backup, as EnumDisplaySettingsW usually won't fail (it's supported in Windows 2000+), but adding backups generally complicates the code.
The code seems verbose. I don't think it's necessary to push everything into a vector

Carterpersall · 2023-04-25T16:22:57Z

1. macchina should use the same format on different platforms, including on Windows. That is: `pixelH`x`pixelV`@`refreshRate`fps (as `scaledH`x`scaledV`). However the code [seems to use another format](https://github.com/Macchina-CLI/libmacchina/pull/147/files#diff-3b7ba22f3645054b5b1c07651b4c378e01c0f8eda295e24e4bedf4e6e3a79382R232)

Yeah, I probably should've checked the current formatting when implementing it. I've changed the formatting of the output to match the rest of the program.

2. I still see no benefits adding `EnumDisplayMonitors` as a backup, as `EnumDisplaySettingsW` usually won't fail (it's supported in Windows 2000+), but adding backups generally complicates the code.

I can remove the second implementation if you wish, but I don't think it complicates the code much due to the clear separator between the implementations.

3. The code seems verbose. [I don't think it's necessary to push everything into a vector](https://github.com/Macchina-CLI/libmacchina/pull/147/files#diff-3b7ba22f3645054b5b1c07651b4c378e01c0f8eda295e24e4bedf4e6e3a79382R181)

A vector has to be used in the selected location due to the need for a while statement instead of an iterator. I was able to use an iterator instead of a vector for the resolution variable, but I found that the implementation caused a significant decrease in readability with no tangible speed improvement.

CarterLi · 2023-04-25T16:34:51Z

I can remove the second implementation if you wish, but I don't think it complicates the code much due to the clear separator between the implementations.

It's useless. That's all.

A vector has to be used in the selected location due to the need for a while statement instead of an iterator. I was able to use an iterator instead of a vector for the resolution variable, but I found that the implementation caused a significant decrease in readability with no tangible speed improvement.

It has nothing to do with vectors or iterators. You call EnumDisplayDevicesW. You get a DISPLAY_DEVICEW instance. You pass DISPLAY_DEVICEW::DeviceName into EnumDisplaySettingsW. You are done.

https://github.com/CarterLi/fastfetch/blob/e5f851dcbb94de35c34fb8c5e3dd8300bb56a1cc/src/detection/displayserver/displayserver_windows.c#L49

CarterLi · 2023-04-25T16:47:16Z

src/windows/mod.rs

+            return Ok(resolutions
+                .iter()
+                .map(
+                    |resolution| match (resolution.dmDisplayFrequency, resolution.dmLogPixels) {


Did you try updating the scale factor of your monitors? For me, dmLogPixels won't be updated before I restarting my laptap.

In addition, according to MSDN, EnumDisplaySettings sets only these 5 DEMMODE members.

Did you test multiple monitors?

Did you try updating the scale factor of your monitors? For me, dmLogPixels won't be updated before I restarting my laptap.

In addition, according to MSDN, EnumDisplaySettings sets only these 5 DEMMODE members.

I didn't before now, and I can confirm that the issue exists for me as well. Along with that, I found that the current implementation doesn't properly get the scale for a second monitor even after a restart. I see a few options moving forward:

Only show the scaled resolution of the primary monitor and have the bug where it doesn't update as a known issue

Don't show a scaled resolution

Do what Fastfetch does and merge the two implementations into a single one that uses EnumDisplaySettingsW to get the resolution and refresh rate and uses EnumDisplayMonitors without SetProcessDpiAwareness to get the scaled resolution. This would likely be about twice as slow, but would add a maximum of a few milliseconds.

I'm going to try implementing option 3 and see how much slower it is.

Did you test multiple monitors?

When a monitor is added or removed, the output correctly updates

There is actually another method by using HDC and GetDeviceCaps.

CarterLi/fastfetch@4d7cb25#diff-1f993416a36137ee7af25d94be4928701d89ca08d9ab322fcb3eb0b5ec6d80b6R39

I dont know if it is faster than EnumDisplayMonitors. The code is much simplier at least.

CarterLi · 2023-04-25T16:48:25Z

src/windows/mod.rs

+                    |resolution| match (resolution.dmDisplayFrequency, resolution.dmLogPixels) {
+                        (0, 0) => format!("{}x{}", resolution.dmPelsWidth, resolution.dmPelsHeight),
+                        (0, _) => format!(
+                            "{}x{} (as {}x{})",


If the resolution is not scaled. Don't print (as {}x{})

CarterLi · 2023-04-25T16:51:12Z

src/windows/mod.rs

+                    PCWSTR::null(),
+                    index as u32,
+                    &mut devices[index],
+                    EDD_GET_DEVICE_INTERFACE_NAME,


If you don't use DISPLAY_DEVICE::DeviceID, don't set EDD_GET_DEVICE_INTERFACE_NAME

Carterpersall · 2023-04-25T17:35:54Z

It has nothing to do with vectors or iterators. You call EnumDisplayDevicesW. You get a DISPLAY_DEVICEW instance. You pass DISPLAY_DEVICEW::DeviceName into EnumDisplaySettingsW. You are done.

CarterLi/fastfetch@e5f851d/src/detection/displayserver/displayserver_windows.c#L49

I see what you mean now, I have no idea why I didn't do it like that in the first place.

…gsW combo

Carterpersall · 2023-04-26T17:57:10Z

There is actually another method by using HDC and GetDeviceCaps.

CarterLi/fastfetch@4d7cb25#diff-1f993416a36137ee7af25d94be4928701d89ca08d9ab322fcb3eb0b5ec6d80b6R39

I dont know if it is faster than EnumDisplayMonitors. The code is much simplier at least.

I created an implementation of this and the EnumDisplayDevicesW + EnumDisplaySettingsW combo and it seems that the combo is consistently faster by about 0.5ms

If the resolution is not scaled. Don't print (as {}x{})

That also should be fixed now

CarterLi · 2023-04-27T04:50:51Z

I created an implementation of this

No. Although fastfetch has switched to QueryDisplayConfig, you don't have to. Just keep using EnumDisplayDevicesW and GetDeviceCaps.

DISPLAY_DEVICEW displayDevice;

//...

HDC hdc = CreateICW(displayDevice.DeviceName, NULL, NULL, NULL);
int width = GetDeviceCaps(hdc, HORZRES);
int height = GetDeviceCaps(hdc, VERTRES);
DeleteDC(hdc);

fastfetch has switched to QueryDisplayConfig because QueryDisplayConfig provides decimal refresh rates which is used by some monitors. In addition, QueryDisplayConfig provides monitor name, which can be used to match the information of fastfetch's brightness module (like the backlight readout in macchina).

If you don't need these features, you don't need QueryDisplayConfig

Carterpersall added 2 commits March 14, 2023 10:44

Implementation working

1e25da9

fmt and clippy

0cbd78b

Carterpersall marked this pull request as draft March 20, 2023 18:37

New Implementation

a4da83e

Carterpersall added 2 commits April 13, 2023 10:47

Merge remote-tracking branch 'upstream/main' into Windows-Resolution

852cbed

Add WMI implementation and other slight tweaks

92923f8

Carterpersall marked this pull request as ready for review April 13, 2023 21:13

Kill WMI

e0c9f2a

Only gets one monitor per adapter

grtcdr requested a review from uttarayan21 April 21, 2023 13:46

Carterpersall added 2 commits April 25, 2023 08:48

Change output formatting

fa09675

Add some checks and comments + fix formatting

d1ba5b2

CarterLi reviewed Apr 25, 2023

View reviewed changes

Carterpersall added 2 commits April 25, 2023 15:51

Merge two iterators and add case for no scaling

1172e4d

Replace Everything with QueryDisplayConfig implementation

3e85155

Carterpersall added 2 commits April 26, 2023 12:29

Replace Everything again with EnumDisplayDevicesW + EnumDisplaySettin…

39fe676

…gsW combo

Account for scenarios where there is no scale

28fead9

grtcdr force-pushed the main branch from adeb33c to 6ae1304 Compare October 6, 2023 20:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Windows: Implement `resolution` #147

Windows: Implement `resolution` #147

Carterpersall commented Mar 20, 2023 •

edited

CarterLi commented Mar 21, 2023

Carterpersall commented Mar 21, 2023

CarterLi commented Mar 21, 2023 •

edited

CarterLi commented Mar 21, 2023

Carterpersall commented Mar 21, 2023

Carterpersall commented Apr 13, 2023

CarterLi commented Apr 14, 2023 •

edited

Carterpersall commented Apr 14, 2023

uttarayan21 commented Apr 24, 2023

grtcdr commented Apr 24, 2023

CarterLi commented Apr 24, 2023

Carterpersall commented Apr 25, 2023

CarterLi commented Apr 25, 2023

CarterLi Apr 25, 2023 •

edited

CarterLi Apr 25, 2023

Carterpersall Apr 25, 2023 •

edited

CarterLi Apr 25, 2023

CarterLi Apr 25, 2023

CarterLi Apr 25, 2023

Carterpersall commented Apr 25, 2023

Carterpersall commented Apr 26, 2023

CarterLi commented Apr 27, 2023 •

edited

Windows: Implement resolution #147

Are you sure you want to change the base?

Windows: Implement resolution #147

Conversation

Carterpersall commented Mar 20, 2023 • edited

CarterLi commented Mar 21, 2023

Carterpersall commented Mar 21, 2023

CarterLi commented Mar 21, 2023 • edited

CarterLi commented Mar 21, 2023

Carterpersall commented Mar 21, 2023

Carterpersall commented Apr 13, 2023

CarterLi commented Apr 14, 2023 • edited

Carterpersall commented Apr 14, 2023

uttarayan21 commented Apr 24, 2023

grtcdr commented Apr 24, 2023

CarterLi commented Apr 24, 2023

Carterpersall commented Apr 25, 2023

CarterLi commented Apr 25, 2023

CarterLi Apr 25, 2023 • edited

Choose a reason for hiding this comment

CarterLi Apr 25, 2023

Choose a reason for hiding this comment

Carterpersall Apr 25, 2023 • edited

Choose a reason for hiding this comment

CarterLi Apr 25, 2023

Choose a reason for hiding this comment

CarterLi Apr 25, 2023

Choose a reason for hiding this comment

CarterLi Apr 25, 2023

Choose a reason for hiding this comment

Carterpersall commented Apr 25, 2023

Carterpersall commented Apr 26, 2023

CarterLi commented Apr 27, 2023 • edited

Windows: Implement `resolution` #147

Windows: Implement `resolution` #147

Carterpersall commented Mar 20, 2023 •

edited

CarterLi commented Mar 21, 2023 •

edited

CarterLi commented Apr 14, 2023 •

edited

CarterLi Apr 25, 2023 •

edited

Carterpersall Apr 25, 2023 •

edited

CarterLi commented Apr 27, 2023 •

edited