nog icon indicating copy to clipboard operation
nog copied to clipboard

[WIP] Dpi scaling, resolution changes and multimonitor support

Open fredizzimo opened this issue 3 years ago • 5 comments

This pull request started by me trying to fix #92. I found out that DWMWA_EXTENDED_FRAME_BOUNDS gives the actual visible window size and position in pixels and the GetWindowRect function gives the size including the invisible part. So to position the window correctly, we need to offset the window by the difference between the two values. This seems to work for all different windows I have tried, including firefox and chrome, and both works without any special cases and additionally the problem in #207 is fixed.

But in order to do that, we need to enable per monitor DPI awareness, since GetWindowRect returns virtual pixels instead of real pixels, so otherwise the two values are not comparable. That means that we need to convert points to pixels in a few places, so I added the utility function points_to_pixels. I also made all size configuration variables use points in pixels, so that they are consistent across monitors and scalings, and are properly relative to the font sizes. For the fonts I changed the size to be negative, which means that you specify the actual font size instead of the bounding rect, which makes the fonts sizes consistent with other programs like notepad. These two changes means that you might have to change your configuration a little bit to make it look like before, but I believe that this way is better and more natural.

One side effect of enabling the per monitor awareness is also that I no longer get the (unreported?) bug where the font of windows titlebars become very small.

This dpi awareness naturally led me to implement support for changing the scaling on the fly. And since changing resolution is closely related I added support for that too. In all cases, when there are some changes, the display arrays are re-created, which avoids the need of too much special case handling. This design naturally led me to support display connects and disconnects as well. For disconnects, if a workspace belongs to a display that's no longer available, it's migrated to the new primary display. If a new display is added, then nothing happens to the workspaces.

There was one complication when implementing this, the taskbar does not react immediately to the new dpi or resolution, so I had to detect taskbar size and position changes, and react to that in the same way as well. And this had the nice side effect that the workspace now correctly updates when resizing or moving the taskbar.

Resolution changes are a little bit more complicated in relation to the taskbar, since many query functions on the taskbar temporarily returns errors, so those errors are now ignored. They don't really matter, since the taskbar position event is sent very quickly after, and at that point the taskbar is valid again, so everything refreshes correctly.

Note that the pull request is still WIP. I need to go through the code, and possibly clean up some things first, this is the first time I write Rust, and I learned a few things when doing this, so I need to clean up the earlier commits. But feel free to give suggestions, I'm still very new to Rust.

Additionally there's still at least one known bug, if you have moved the workspace to another montior, then it will flash between the screens when the monitor size changes. I know the reason, but I haven't fixed it yet, the focused_grid_id field of the old display is not cleared.

It also most likely does not handle unplugging all of the monitors, and I'm not actually sure what to do in that case. Perhaps create a virtual display? Or exit the workmode?

Finally #90 reports that the monitor ids are not stable, so I think the comparison has to use device ids instead. I have personally not run into that issue, but I believe it's there.

fix #90 fix #92 fix #207

fredizzimo avatar Aug 04 '21 21:08 fredizzimo

Accidently closed sorry. I will try to look over this PR in the coming days.

Just reading your description already makes me excited to get this merged!

Thank you for your work!

TimUntersberger avatar Aug 04 '21 22:08 TimUntersberger

this is amazing 🥳

I'm gonna test this out locally tomorrow, I'm also pretty excited

ramirezmike avatar Aug 05 '21 02:08 ramirezmike

Thanks for the positive feedback!

But just to let you know, I might not have time to fix the issues and return to this until the beginning of next week. Until then, the code should be stable enough to test and try to break and find more issues than the ones I mentioned in the descriptions. But the actual final fixes before this can be merged probably has to wait, unless you want to take over and fix them yourself.

fredizzimo avatar Aug 05 '21 11:08 fredizzimo

It also most likely does not handle unplugging all of the monitors, and I'm not actually sure what to do in that case. Perhaps create a virtual display? Or exit the workmode?

my vote is for exiting work mode

But just to let you know, I might not have time to fix the issues and return to this until the beginning of next week

No worries!

Edit:

Just so we don't forget:

  • [ ] remove rules for firefox/chromium
  • [ ] document above change in changelog
  • [ ] update default config
  • [ ] handle unplugging all monitors
  • [ ] if you have moved the workspace to another montior, then it will flash between the screens when the monitor size changes. I know the reason, but I haven't fixed it yet, the focused_grid_id field of the old display is not cleared.

TimUntersberger avatar Aug 05 '21 13:08 TimUntersberger

Has any progress been made on this?

NotSpaulding avatar Jul 06 '22 10:07 NotSpaulding