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

Handle hidpi scaling. #105

Closed
nical opened this issue Dec 24, 2016 · 87 comments · Fixed by #548
Closed

Handle hidpi scaling. #105

nical opened this issue Dec 24, 2016 · 87 comments · Fixed by #548
Labels
C - in progress Implementation is proceeding smoothly C - needs discussion Direction must be ironed out D - hard Likely harder than most tasks here P - high Vital to have S - api Design and usability S - enhancement Wouldn't this be the coolest?
Milestone

Comments

@nical
Copy link

nical commented Dec 24, 2016

The wording here is intentionally vague because I don't know very well what the possibilities are and what makes sens for winit. It is safe to say that it would be good for winit to expose some way to indicate whether the application is "dpi-aware" and avoid having the window upscaled by the window manager for apps that don't want this behavior.

On Fedora 25 with wayland (now enabled by default), winit windows are all upscaled on hidpi screens.

cc @vberger

@tomaka tomaka added S - api Design and usability S - enhancement Wouldn't this be the coolest? labels Dec 24, 2016
@elinorbgr
Copy link
Contributor

I believe this needs some discussion, as DPI is not handled the same way on all platforms.

The wayland way is:

  • Each monitor has a scale factor, defaulting to 1.0, but it can be higher for HiDPI screens (typically 2.0)
  • Each window known on which monitor(s) it is displayed, and what their scale factor are
  • Each window draws in its buffer with the scale factor of its choice and notify the server about it (if not set, defaults to 1.0)
  • If the scale factors of the screen(s) and the one chosen by the application do not match, the server scales the contents of the window appropriately

@tomaka
Copy link
Contributor

tomaka commented Dec 26, 2016

From what I gathered, on Win32 each thread has a HiDPI-awareness level:

  • A window on a thread that is not HiDPI-aware (the default) is assumed to use logical pixels and will be scaled by the operating system.
  • A window on a thread that is HiDPI-aware is supposed to query the HiDPI level of the monitor and is assumed to be scaled based on this information. For example if you create a window of size 1280x1024 on a monitor that reports a resolution of 1280x1024 (EDIT: you can also directly query the number of physical pixels) and a HiDPI level 2.0, the window will occupy half of the screen (and if the thread was not HiDPI-aware, the window would occupy the whole screen).
  • A window on a thread that is per-monitor-HiDPI-aware is the same as HiDPI-aware, except that it handles changing the current monitor itself. If you are only HiDPI-aware, Windows will automatically scale the window if you move it from one monitor to another that doesn't have the same HiDPI level. But if you are per-monitor-aware, it's the application that is responsible for this.

Notably, the third option is only available on Windows 8.1+.

I couldn't find whether moving a window from a thread to another, or changing the HiDPI awareness level during the execution, modifies the existing windows. The HiDPI awareness is supposed to be set once at initialization. The older way to deal with that was per application, and you could only set it once (all further calls would fail). Maybe this is also the case for threads.

jrmuizel pushed a commit to jrmuizel/winit that referenced this issue Mar 29, 2017
Merge from upstream to get OSMesa changes

<!-- Reviewable:start -->
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/glutin/105)
<!-- Reviewable:end -->
@kryptan
Copy link
Contributor

kryptan commented Oct 4, 2017

We need to support 3 things to support high DPI:

  1. Notify OS that we are high DPI aware.
  2. Get scaling factor of the window. There is Window::hidpi_factor() function for this.
  3. Receive notifications from the OS about change of the scale factor.

On Windows situation is as follows:

Notifying OS that we are high DPI aware

  • On Windows 10 Anniversary Update or later we can set DPI awareness individually for each created window:

    1. Call SetThreadDpiAwarenessContext before creating the window.
    2. Create window.
    3. Restore DPI awareness of the thread by calling SetThreadDpiAwarenessContext again.

Quote from some unofficial documentation:

SetThreadDpiAwarenessContext() function was introduced in windows 10 aniversary update. This allows a thread to change its DPI awareness level at will, i.e. for some time duration a thread can set itself as DPI unaware, while at other times it cadn set itself as per monitor aware, or system aware. This allows the thread to create windows which have different DPI awareness levels. Using this we can build an application in which scaling of some UI elements is handled by windows, while scaling for others is handled by the application itself.

More info from the MSDN.

  • Windows 8.1 or later. DPI awareness is set for the entire process and can be set only once with the SetProcessDpiAwareness. If DPI awareness is set in the app manifest that value will take precedence and SetProcessDpiAwareness will fail.

  • On Windows Vista deprecated SetProcessDPIAware can be used.

Getting scaling factor of the window

  • Windows 10 - use GetDpiForWindow. It seems it was introduced in the Anniversary Update.

  • Windows 8.1 or later - use GetDpiForMonitor.

  • Windows Vista. Interestingly, there is a way to set DPI awareness on Windows Vista (SetProcessDPIAware) but I don't see a way to get the DPI value. 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. EDIT: For older Windows you can get the DPI by calling GetDeviceCaps with LOGPIXELSX and LOGPIXELSY.

Receiving notifications from the OS about change of the scale factor

WM_DPICHANGED message is the way to go. It is sent by Windows 8.1 or later to DPI aware windows. It provides two values - X and Y DPI. MSDN says "the values of the X-axis and the Y-axis are identical for Windows apps". It is not clear why it explicitly mentions "Windows apps" as if there were some non Windows apps which could receive this message. I think for simplicity we can assume that they are always identical. I'm not sure which actions are required upon receiving this message. In MSDN they are calling SetWindowPos to change window size after receiving this message, I'm not sure why.

@retep998
Copy link
Contributor

retep998 commented Oct 4, 2017

@kryptan For older Windows you can get the DPI by calling GetDeviceCaps with LOGPIXELSX and LOGPIXELSY.

@kryptan
Copy link
Contributor

kryptan commented Oct 4, 2017

Even on Windows 10 Anniversary Update it is still necessary to call SetProcessDpiAwareness to prevent Windows from scaling your app. If you only use SetThreadDpiAwarenessContext you will get both real values of DPI and scaling from the OS which leads to double scaling.

PR should be ready soon.

@kryptan
Copy link
Contributor

kryptan commented Oct 4, 2017

@kryptan For older Windows you can get the DPI by calling GetDeviceCaps with LOGPIXELSX and LOGPIXELSY.

It works but requires user to log out after changing scaling.

@retep998
Copy link
Contributor

retep998 commented Oct 5, 2017

Yes, because older Windows only supports the notion of system DPI, not per monitor DPI, and changes to the system DPI require relogging. Regardless, that is the correct API to use for those older systems.

@kryptan
Copy link
Contributor

kryptan commented Oct 7, 2017

Currently Window::get_inner_size_points() invokes platform-specific code to get window size in virtual pixels while Window::get_inner_size_pixels() just multiplies this value by hidpi_factor. This means that platform backend needs to divide window size in real pixels by hidpi_factor and round that to integer while winit will then multiply it back. This logic should be switched, i.e winit should query platform backends for size in real pixels, otherwise we will get rounding errors. This requires changes to all backends.

For my Windows patch I will probably just follow the existing logic despite the rounding errors that it causes.

@elinorbgr
Copy link
Contributor

Aside from implementation details I'd like to raise the questions of what winit should assume from its users:

  • Are winit-based apps expected to always be hidpi-aware?
  • Should we insert a mechanism for winit users to activate dpi-awareness for their program?

Thing is, as in wayland the user can bascially draw at any dpi they want, we need to be sure to advertize the proper used dpi factor to the server, other wise the sizes won't match.

@kryptan
Copy link
Contributor

kryptan commented Oct 9, 2017

Should we insert a mechanism for winit users to activate dpi-awareness for their program?

Should it be per-program or per-window?

My current implementation for Windows is here. Windows has application global DPI-awareness. Latest Windows 10 has mechanism for per-window DPI-awareness but I hasn't been able to make it work. Setting awareness for window different from the global one causes weird issues, so my current implementation has one global function - become_hidpi_aware(). Nevertheless, other systems probably allow per-window DPI-awareness, so this should also need to be supported.

@kryptan
Copy link
Contributor

kryptan commented Oct 11, 2017

To move forward I think we need to agree on the cross-platform interface and then implement it in backends. This is kind of an RFC for the proposed API changes to winit.

Detailed design

  1. Add become_hidpi_aware() global function. It will be implemented at least for Windows but may be a no-op on other platforms.

  2. Add MonitorId::get_hidpi_factor() -> f32 method. This method is necessary to calclulate sizes of the windows that you will create.

  3. Add WindowAttributes::hidpi_aware: bool field. This is a hint to the OS whether this new window should be scaled for high-DPI displays by the OS or by the applications itself. Note that at least on Windows we may not have control whether created window will be scaled by the OS or not so this is just a hint.

  4. All methods in the Window struct should operate on physical pixels for high-DPI aware windows and on virtual pixels for high-DPI unaware windows. Namely get_position, set_position, get_inner_size, get_outer_size, set_inner_size, set_cursor_position. Otherwise we'll probably need to duplicate all of them like it is done for get_inner_size, and I don't see much need for this. Users can call Window::hidpi_factor() and calculate size in virtual pixels themselves if necessary.

  5. Remove Window::get_inner_size_points and Window::get_inner_size_pixels (replaced by Window::get_inner_size, see above).

  6. Add WindowEvent::HiDPIFactorChanged(f32). This event will be executed when Window::hidpi_factor() changes. All backends must ensure that value passed to this event equals to the value returned from the Window::hidpi_factor().

All backends must ensure that for each window, either one of the following invariants holds:

  1. Window::hidpi_factor() always returns 1 and all methods in Window are using virtual pixels (i.e. it is high-DPI unaware window, its content is scaled by the OS).
  2. Window::hidpi_factor() returns real value and all methods in Window are using physical pixels (i.e. it is high-DPI aware window, its content must be scaled by the application).

Window::hidpi_factor() can be thought of as the scaling that is expected from the application.

Alternatives

Don't create become_hidpi_aware() global function, instead make application high-DPI aware when a window with WindowAttributes::hidpi_aware == true is created. The issue with this approach is that you want to know hidpi_factor before you create first window to correctly specify its size in pixels, but if we don't make process high-DPI aware on Windows then MonitorId::get_hidpi_factor() will always return 1 instead of real value.

Unresolved questions

Should we return bool from become_hidpi_aware() to indicate whether process has succesfully become high-DPI aware or already was? If we return bool should we always return true on platforms where it is a no-op?

What should be the default value for WindowAttributes::hidpi_aware?


Thoughts?

@jwilm
Copy link
Contributor

jwilm commented Oct 11, 2017

Is all the become_hidpi_aware functionality really necessary? For an application that doesn't care about it, wouldn't they just assume it's 1.0? I also wonder that, if we do need such a method, shouldn't it be a window builder method rather than a global?

The rest of this sounds good to me--both the MonitorId improvements and the additional event to the WindowEvent enum.

@kryptan
Copy link
Contributor

kryptan commented Oct 11, 2017

Is all the become_hidpi_aware functionality really necessary? For an application that doesn't care about it, wouldn't they just assume it's 1.0? I also wonder that, if we do need such a method, shouldn't it be a window builder method rather than a global?

If you don't care about it you just don't call it and assume that hidpi factor is 1.0. Whether it can be a builder method I think I addressed in the Alternatives section. By the time you create your window you already need to know hidpi factor to specify window size but to get it from MonitorId::get_hidpi_factor you need to make the process high-DPI aware first.

@tomaka
Copy link
Contributor

tomaka commented Oct 11, 2017

I may be wrong, but for me HiDPI awareness is just a thing that exists to preserve backward compatibility.
I don't think it's necessary to have a concept of not being HiDPI-aware.

@kryptan
Copy link
Contributor

kryptan commented Oct 11, 2017

@tomaka I agree with this but on Windows we can't just make process hidpi aware without consent of the programmer. Winit may be used as part of a larger application, e.g. in dll, where there are windows created by other, non-winit, code. Enabling high-dpi awareness will mess this up. Therefore, I think become_hidpi_aware() should be there. On the other hand, WindowAttributes::hidpi_aware may not be necessary and we'll just attempt to make each window hidpi aware.

@elinorbgr
Copy link
Contributor

I have no concerns regarding wayland implementation, as wayland allows very fine control over dpi-awareness, we should be able to implement this API without much trouble.

However, I must say, I find it weird to have both a global become_dpi_aware() and a WindowAttributes::hidpi_aware. These just seem redundant.

@tomaka
Copy link
Contributor

tomaka commented Oct 11, 2017

Winit may be used as part of a larger application, e.g. in dll, where there are windows created by other, non-winit, code.

But this reasoning would also mean that these DLLs are forbidden to enable HiDPI-awareness as well, as they would mess up the rest of the application.

What I mean, is that more generally it's not possible to isolate a code that creates a window in its own little module or its own little library. Any code that creates a window (whether it is from winit or not) should specify what it does when it comes to HiDPI-awareness. Therefore I'd be in favour of making HiDPI-awareness mandatory for winit.

@kryptan
Copy link
Contributor

kryptan commented Oct 11, 2017

But this reasoning would also mean that these DLLs are forbidden to enable HiDPI-awareness as well, as they would mess up the rest of the application.

I have the following use case in mind. There is an existing C++ Windows application which has a GUI and which can use DLL plugins. These DLL plugins may need to display their own GUI to show user some settings. This is not just a theory, see e.g. #159. If the original application is not HiDPI aware then plugin written in Rust should not attempt to change that. This issue was previously raised here.

@kryptan
Copy link
Contributor

kryptan commented Oct 11, 2017

As an alternative we can provide an option to opt-out of the DPI-awareness in which case winit will not attempt to change any process-global settings.

@kryptan
Copy link
Contributor

kryptan commented Oct 12, 2017

I think I found a reasonable solution. The use case that I mentioned above is very niche while probably 99% of winit users on Windows are writing standalone executables where changing process-global settings is not an issue.

I propose the following: don't introduce become_hidpi_aware() global function and don't add WindowAttributes::hidpi_aware option, instead always enable high-DPI awareness when necessary (when monitor DPI is requested or window is created). For the niche use case when you are writing a DLL loaded by another process we can add a cargo feature which will prevent enabling DPI awareness on Windows. The only thing this feature would do is turning our internal function which enables DPI awareness into a no-op.

tomaka pushed a commit that referenced this issue Oct 17, 2017
* Implement public API for high-DPI #105

* Recover get_inner_size_points and get_inner_size_pixels and change their implementation assuming get_inner_size() returns size in pixels

* Update changelog for high-DPI changes
@elinorbgr
Copy link
Contributor

Tbh I'm not quite sure. The risk of storing the DPI factor in it would be a user keeping an old Coordinates value around, and then trying to use it while the actual dpi factor changed in between, and gets invalid results due to that.

Keeping it out would make clear that the DPI factor is some context value that may change and needs to be tracked, I guess.

On the other hand, integrating the DPI factor in the struct is certainly more convenient if we assume that there are supposed to always be short-lived anyway.

@francesca64
Copy link
Member

That risk sounds like it's largely mitigated by the existence of HiDPIFactorChanged.

@elinorbgr
Copy link
Contributor

Yeah, but in that case I think we need to make it very clear that the Coordinates objects are created for a given dpi, and that a DPI change would invalidate them (and add an API to update them to a new DPI, I guess).

@francesca64
Copy link
Member

I think I'm still in favor of including it, since when possible users should favor responding to events over directly querying things, and I think they're more likely to follow that recommendation if the DPI is included.

(I'm assuming you agree with that recommendation. It's partially motivated by our inability to guarantee that queries will always have a low cost.)

@Ralith
Copy link
Contributor

Ralith commented May 15, 2018

It's partially motivated by our inability to guarantee that queries will always have a low cost.

Wouldn't that be a strong argument to not build an API that forces the backends to make such queries on every event with a coordinate in it? I'd think the queries could be made cheap just by caching the latest value for each window, though.

There's also the downside that anyone who is going to be passing around or storing lots of logical coordinates (say, for a GUI impl) will need to come up with their own type and do a lot of converting.

@francesca64
Copy link
Member

@Ralith true, the performance issue was a weak argument on my part. Why would including the DPI factor in the struct force application developers to make their own type and do lots of converting, though?

@Ralith
Copy link
Contributor

Ralith commented May 15, 2018

Because if the DPI factor is embedded, the struct is larger than necessary for performing computations purely in logical space, which is undesirable. People could just ignore the cost, but it's an incentive against using it all the same.

@francesca64
Copy link
Member

I'm not sure I share that sentiment, but since embedding the DPI factor would be less transparent/explicit anyway, I think I've changed my mind about it. @vberger's struct looks good as-is.

@retep998
Copy link
Contributor

How do you handle the coordinate object having an embedded dpi factor when the window is overlapping multiple displays with different dpi factors on each?

@anderejd
Copy link

The idea I was thinking of regarding named types instead of tuples was something more like this:

struct LogicalCoordinates { x: f64, y: f64}

struct PhysicalCoordinates { x: f64, y: f64}

With the primary purpose of not mixing them up, if we need both in the API.

I'm unsure if the DPI factor should be bundled with the coordinates or not.

@francesca64
Copy link
Member

@anderejd sounds good to me. It's simple, composable, clearly communicates the intentions, and provides us with more hope for getting this done before this issue hits 100 comments.

@elinorbgr
Copy link
Contributor

For the record: my suggestion was voluntarily going farther than yours @anderejd , I was just thinking that in APIs where there is no obvious "better choice" between them (which may be many), offering an abstraction over both could be useful.

Nevertheless, this can easily be done with your suggestion as well, by adding appropriates .to_physical(dpi) and .to_logical(dpi) methods to these types.

So, from my point of view both are pretty equivalent, I don't have any strong preference between any of the two approaches.

@francesca64
Copy link
Member

So, which functions should take/return logical pixels, besides with_dimensions (and presumably with_max_dimensions and with_min_dimensions)?

@anderejd
Copy link

That seems reasonable based on what has been written earlier in this issue, @vberger ?

@elinorbgr
Copy link
Contributor

I agree that with_dimensions, with_max_dimensions and with_min_dimensions can hardly be made meaningful while taking something else than logical pixels.

Now, taking the rest of winit API, there are the places where coordinates or dimensions are used:

  • Input handling (pointer and touch events): I believe we need to be consistent in this whole group. Now, I think the two possibilities can be heard. On the one hand, providing logical pixels here allows most app to have and input handling logic be completely transparent to DPI, assuming for most apps, DPI will only affect the actual drawing, but not the size or location of the UI elements it'll use. Now, this is obviously not the case for all apps. The question is, which default do we want to consider?
  • Monitor characteristics:, notably the dimensions of the monitor. Well, I hope we all agree this doesn't make sens to report anything other than physical pixels?
  • Window resizing:, advertising the new size of the window when it changes. On the one hand, it would make sense to unify it with the builder API, so use logical pixels. On the other hand, an app needs to know the physical pixel size of the surface it'll draw on. And finally, we probably want the dimensions of the window to be reported in the same units as the location of the inputs relative to it.

So. From this I mostly see two possibilities for a roughly general and coherent API:

  • Use physical pixels everywhere except where we really can't (with_dimensions, with_max_dimensions and with_min_dimensions), and document that they can be converted to logical pixels to make the input handling logic DPI-invariant.
  • Use logical pixels everywhere except where we really can't (monitor dimensions), and document that they should be converted to physical pixels to find out the dimensions of the drawing canvas (and to draw in general).

I would tend to favor the second case, because it matches the fact that DPI awareness really only matters for drawing, most of the time. But both options seem viable to me, so it's not an hill I'll die on.

@francesca64
Copy link
Member

Option 2 makes more sense to me. Making drawing special probably makes more sense to the average user than making window creation special.

Either way, besides API documentation, I think we really ought to have an example program for this. I'm not sure where it should live (since without a drawing surface there's nothing to demonstrate) but I'm confident it would improve the chances of people actually supporting HiDPI.

@Osspial
Copy link
Contributor

Osspial commented May 23, 2018

If we end up going with option 2, I don't think we should have winit tell the OS it's DPI-aware by default. That way, the worst-case-scenario with the program not handling DPI scaling is that the OS scales the entire window up, and everything looks blocky or a bit blurry. However, if we're telling the OS that we're handling DPI scaling, but the program isn't, and is also using pixel-based coordinates for drawing, we end up with apps that looks like this:

Improper DPI Scaling

Which is broken to the point of being unusable.

@francesca64
Copy link
Member

@Osspial AFAIK "DPI awareness" the way you refer to it only exists on Windows.

@elinorbgr
Copy link
Contributor

I believe it was stated a few times that winit would explicitly not support non-dpi aware apps...

Furthermore, using the logical/physical pixels wrapping structs should make it rather difficult to accidentally be non-dpi aware, wouldn't it?

@francesca64
Copy link
Member

Alright, PR is up: #548

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C - in progress Implementation is proceeding smoothly C - needs discussion Direction must be ironed out D - hard Likely harder than most tasks here P - high Vital to have S - api Design and usability S - enhancement Wouldn't this be the coolest?
Development

Successfully merging a pull request may close this issue.