Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

High-DPI and dynamic DPI changing support for Windows #224

Closed
wants to merge 22 commits into from
Closed

High-DPI and dynamic DPI changing support for Windows #224

wants to merge 22 commits into from

Conversation

GabrielMajeri
Copy link
Contributor

@GabrielMajeri GabrielMajeri commented Jul 13, 2017

winit currently cannot be used to create cross-platform high-DPI aware applications (see issue #105). This commit adds support for creating high-DPI aware applications for Windows.

The 2 main changes of this pull request:

  • the addition of code to call the various Windows functions to make the process high-DPI aware.
  • a new event, DPIChanged, that informs the users of the winit library that they should handle the new DPI.

I've also added a new high-dpi.rs example to showcase what this pull request does.

Visual example

This is a normal window, on which I've written text using GDI.

nohidpi

This is the same window, but after a DPI change. Notice how the text is blurry - Windows does not know the app is aware of high DPI displays, and therefore scales the rendered bitmap.

fail-hi-dpi

After the code changes, the text looks fine, even at higher DPI.

support

However, high-DPI support means that the user code also has to adapt to higher DPI. The example uses the same font and layout at all DPIs - this is bad, it should be calculating a new layout and choosing a new font based on DPI, but that is not the concern of the winit library.

The reason why the text is unreadable below is because I didn't account for high-DPI when writing the rendering code.

bad support

DPI changed event

While on Windows 7 DPI can only be changed by logging out, therefore closing all programs, and logging in again, starting with Windows 8 DPI can be dynamically changed, or differ from monitor to monitor.

Therefore I added a new WindowEvent, the DPIChanged event, which is sent when the window receives the WM_DPICHANGED message.

Implementation

The code I added uses the already existing shared_library dependency. It tries to load, in order, the functions for setting DPI awareness for Windows 10, Windows 8, and Windows Vista.

If those DLLs are missing or lack the functions, or if the process is already high-DPI aware (through a manifest), no harm is done.

Further changes

It would be great to add a MonitorId::get_monitor_dpi() function, which returns the DPI of the monitor on Windows 8 or newer, and the same system DPI for all monitors on Windows 7 or older.

@LPGhatguy
Copy link
Contributor

I had a less comprehensive pull request in #185 -- I think we came to the idea that it might be better to signal high DPI awareness via a manifest instead of at runtime.

@retep998, did your manifest linking investigation lead to any conclusions?

@GabrielMajeri
Copy link
Contributor Author

GabrielMajeri commented Jul 13, 2017

Embedding a manifest isn't too hard. Currently in my apps I use the embed-resource crate, which allows embeding a manifest by using a simple .rc file.

The problems with this approach would be:

  • it wouldn't work if the winit library is built as a separate dynamic library, because the manifest must be in the .exe, not the .dll.
  • we don't know what the user wants in the manifest. There could be additional settings required, and this approach only allows one manifest in it, there is no way to combine manifests.

@retep998
Copy link
Contributor

it wouldn't work if the winit library is built as a separate dynamic library, because the manifest must be in the .exe, not the .dll.

When is winit ever built as a separate dynamic library? 99% of the time all rust crates are statically linked together.

we don't know what the user wants in the manifest. There could be additional settings required, and this approach only allows one manifest in it, there is no way to combine manifests.

If Rust would finally let us specify arbitrary linker arguments, then you could use /MANIFESTINPUT to specify manifest input which is merged together. That way you can just set the DPI awareness and leave the rest up to the user.

@Ralith
Copy link
Contributor

Ralith commented Jul 14, 2017

When is winit ever built as a separate dynamic library? 99% of the time all rust crates are statically linked together.

Rust may be used to build a dynamic library to be loaded by foreign code. First-class support for this use case is one of rust's big selling points.

If DPI awareness needs to be set in the manifest, and cargo won't yet let us just hardware it to "aware," it seems like the only behaviorally predictable solution is a documentation one.

@GabrielMajeri
Copy link
Contributor Author

GabrielMajeri commented Jul 14, 2017

If Rust would finally let us specify arbitrary linker arguments

I agree that this would be the best solution for building .exes, although if winit is built into a DLL we'd still need these runtime calls to ensure the process as a whole is high-DPI aware.

This idea of loading function pointers and then calling the functions to set DPI awareness is used by GLFW. Of course, GLFW is more often than not loaded as a DLL, but as @Ralith said, someone could build a cdylib crate that uses winit.

seems like the only behaviorally predictable solution is a documentation one.

That could work, but we can't guarantee users of the library will do that. And since high-DPI screens aren't that common, the users of the winit library might not realize their apps are not high-DPI aware.

If winit handles window creation and management then it only seems logical to work as-is on all hardware.

As I said before, high-DPI support also involves some work done by users of the library, but by providing a cross-platform API of getting DPI and being informed of DPI changes, maybe users will also read up on high-DPI support and include it in their apps.

@Ralith
Copy link
Contributor

Ralith commented Jul 14, 2017

This idea of loading function pointers and then calling the functions to set DPI awareness is used by GLFW

I don't know the APIs in question, but it's probably not appropriate for winit-in-a-cdylib to automatically set the entire process to high DPI, if for example rust code is being used as a plugin that needs to play well with other existing code that has its own opinions.

@tomaka
Copy link
Contributor

tomaka commented Jul 14, 2017

Even for winit-as-a-regular-library it's probably not a great thing to set a process-global option.

@GabrielMajeri
Copy link
Contributor Author

GabrielMajeri commented Jul 14, 2017

These option can only be changed once, anyway. If the user wants to override it, the user only has to call these functions before winit. If the user's app has a manifest, that will also take priority.

The method @retep998 recommends, using manifests, would mean the user wouldn't be able to make the app DPI unaware later.

@retep998
Copy link
Contributor

I don't think winit should automatically set the DPI awareness. Although I do think it should provide the ability to change the DPI awareness when the user explicitly asks for it.

@GabrielMajeri
Copy link
Contributor Author

I've refactored the code. High-DPI awareness is now opt-in. The user has to call set_process_high_dpi_aware to make the process high-DPI aware. Also added some documentation linking to the page for app manifests.

@@ -44,7 +44,7 @@ impl Window {
{
let mut w_attr = Some(w_attr.clone());
let mut pl_attr = Some(pl_attr.clone());

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ew trailing whitespace.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That was in the original window.rs. I've removed it now.

This is the right function to use according to the documentation.
@kryptan
Copy link
Contributor

kryptan commented Oct 2, 2017

There is already the Window::hidpi_factor() function which currently always returns 1.0 on Windows. This PR doesn't change it while it probably should. I think value returned from that function should have the same type as parameter in the DPIChanged message.

Documentation for the WM_DPICHANGED message says that "the values of the X-axis and the Y-axis are identical for Windows apps". Do we need to confuse users with different values for X and Y if they are always the same?

Also, if you report this value as DPI then it is necessary to say in the documentation what is the default, non-scaled value is.

@kryptan
Copy link
Contributor

kryptan commented Oct 4, 2017

I see that there is a deprecated SetProcessDPIAware function which can be used on Vista to set DPI awareness and is used in this PR if newer APIs are not available. But I don't see any way to actually get the DPI on Vista. WM_DPICHANGED is only sent on Windows 8.1. I checked all functions in the High DPI Reference and all functions for getting DPI are available only on 8.1 or newer.

@GabrielMajeri
Copy link
Contributor Author

GabrielMajeri commented Oct 4, 2017

I'm no longer working on supporting this pull request. If anyone wants to reuse some of the code, feel free to.

@kryptan Could you also post your observations on issue #105? We should at least keep the discussion alive there.

@kryptan kryptan mentioned this pull request Oct 11, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

6 participants