libmacchina
libmacchina copied to clipboard
Windows: Implement `resolution`
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 https://github.com/lptstr/winfetch/pull/156/ and this one
SetProcessDpiAwarenesstells the OS to give the program the "real" resolution of each monitor instead of the scaled resolution- Uses
EnumDisplayMonitorsto get the information of each monitor- Uses a callback function to get the information on each monitor using
GetMonitorInfoWand store the result in a vector
- Uses a callback function to get the information on each monitor using
TODO:
- ~[ ] Implement my method in https://github.com/lptstr/winfetch/pull/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
- ~Only works in admin instances of PowerShell, but I need to test if it's the same for Rust~
Why EnumDisplayMonitors instead of EnumDisplayDevicesW + EnumDisplaySettingsW (ENUM_CURRENT_SETTINGS)?
EnumDisplayDevicesWis not affected byDpiAwareness, so noSetProcessDpiAwarenessneeded.EnumDisplayDevicesWdoesn't use callback, so it is easier to useEnumDisplayDevicesWprovides refresh rate
While EnumDisplayMonitors is still useful to fetch scaled resolutions. with SetProcessDpiAwareness(PROCESS_DPI_UNAWARE), if needed
Why
EnumDisplayMonitorsinstead ofEnumDisplayDevicesW+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.
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
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
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.
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
Besides being slow, WMI Win32_VideoController reports one resolution per GPU adapter only. I see no benefits use WMI here.
Besides being slow, WMI
Win32_VideoControllerreports 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
@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.
Neither am I, that's okay. I'd appreciate it if someone else (@CarterLi, perhaps) gave a final review before merging.
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 (asscaledHxscaledV). However the code seems to use another format - I still see no benefits adding
EnumDisplayMonitorsas a backup, asEnumDisplaySettingsWusually 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
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.
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
It has nothing to do with vectors or iterators. You call
EnumDisplayDevicesW. You get aDISPLAY_DEVICEWinstance. You passDISPLAY_DEVICEW::DeviceNameintoEnumDisplaySettingsW. 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.
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
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