Expose get_physical_extents for the MonitorId #442

fschutt · 2018-04-01T09:34:25Z

At least on Windows, X11, Wayland, macOS and iOS, this new function returns the physical size of the screen in millimeter. On Android, I can't implement it because there is no way for me to the access to the JavaVM. This is waiting on rust-mobile/android-rs-glue#116. I need this for two reasons:

As a poor mans DPI value until proper DPI support is added - for rasterizing the font with freetype, I need the correct screen DPI - see: Font sizes don't respect the monitor DPI on Linux servo/webrender#2596
I am working on a GIS application where correct scaling is important - i.e. I have to know how large the physical monitor is so I can calculate the map scale accordingly.

I haven't tested that the function actually returns the value in millimeter, though, just that it compiles.

francesca64 · 2018-04-13T18:29:05Z

Sorry for the wait.

This needs a CHANGELOG entry.
Looks good on X11, and returned values seem correct (though I must admit I didn't check with a ruler!)
Wayland implementation looks solid, though I've yet to test it.
I tested this on Windows 10, and it always returns None. On the first call, it fails at "reset to whatever DPI awareness was set before", and on all subsequent calls, it fails at "couldn't set to the required DPI awareness, reset and quit". On that note, I'm not really sure what the value is in returning None when it can't be reset, since we'd still have usable dimensions, and the process is still in an unexpected state. Also, is there a reason VERTSIZE/HORZSIZE aren't used?
The logic looks good on macOS (I'll try to test it later), though since the size values are 0 for an invalid display, would it make sense to return None in that case?
Is it possible for me to test the Android implementation if I use the android-rs-glue PR?
I have no idea about iOS.

fschutt · 2018-04-15T18:17:28Z

@francesca64 I'm working on the windows thing.

Why the VERTSIZE isn't used: https://stackoverflow.com/questions/577736/how-to-obtain-the-correct-physical-size-of-the-monitor - VERTSIZE / HORZSIZE don't always work as you expect them to.
I returned "None" because in the case that the app is DPI-unaware, it is technically a failure to not reset it to what DPI awareness it had before. However, I could set it to PROCESS_SYSTEM_DPI_AWARE, meaning that the application asked at least once for the DPI (usually on startup). I'm not sure about this, have to debug this on Windows
For iOS, there is no DPI, you would have to guess it from the pixel dimensions (i.e. "700px1300px = iPhone 5 = XXX mmXXX mm"). At least to my knowledge, there's no other way of getting this. The current design returns a "scale" value, not sure how accurate it is.
Android: The current code will not work without fixing the imports. Other than that, it should compile and run (solution is based off of this stackoverflow post, but I wanted to leave it in in case the next guy comes along and wants to work on DPI awareness for Android. However, the code uses the jni crate, which is also used by the PR. So, the code will probably work, but you need to fix the imports. tomaka wanted to re-export jni before the PR is merged, see the discussion there.

I'll test X11 and Wayland on a VM, just to be sure that the implementation is correct. Working on a windows fix.

fschutt · 2018-04-15T18:59:05Z

Also relevant: https://ofekshilon.com/2011/11/13/reading-monitor-physical-dimensions-or-getting-the-edid-the-right-way/

Windows "smartly" enlarges the monitors size in order to correct for the distance of the eye to the monitor. Which is - in my opinion - a complete shitpile of design. It may work, it may not work. Reading the EDID data from the windows registry seems to be the only proper way to do it, as ridicoulous as this may sound.

Note: this function is named get_physical_extents(), it doesn't have anything to do with the actual DPI itself.

fschutt · 2018-04-15T23:03:28Z

It seems that this is GLFW's solution:

if (IsWindows8Point1OrGreater()) {
    widthMM  = GetDeviceCaps(dc, HORZSIZE);
    heightMM = GetDeviceCaps(dc, VERTSIZE);
} else {
    widthMM  = (int) (dm.dmPelsWidth * 25.4f / GetDeviceCaps(dc, LOGPIXELSX));
    heightMM = (int) (dm.dmPelsHeight * 25.4f / GetDeviceCaps(dc, LOGPIXELSY));
}

https://github.com/glfw/glfw/blob/8d98a6e37d122f45439263c96dd50ac2f16b7f5f/src/win32_monitor.c#L82-L91

francesca64 · 2018-05-07T16:33:29Z

I've verified that this works as expected on macOS. For Windows, I assume that's not finished yet.

fschutt · 2018-05-08T13:27:01Z

Yeah, I haven't had much time to work on this... I'm basically trying to port the solution presented here: https://ofekshilon.com/2014/06/19/reading-specific-monitor-dimensions/ - this way it'll work on all versions on Windows and with multiple monitors, too.

DO NOT MERGE THIS YET

torkleyy

Two minor nits ^^

torkleyy · 2018-05-23T08:48:27Z

src/platform/linux/mod.rs

+
+    #[inline]
+    pub fn get_physical_extents(&self) -> Option<(u64, u64)> {
+        match self {


Should match on *self

torkleyy · 2018-05-23T08:49:28Z

src/platform/windows/monitor.rs

+    // Find the display_device with the correct monitor name
+    let display_device = get_display_device_from_monitor_name(monitor_name)?;
+    let device_id = wchar_as_string(&display_device.DeviceID[..]);
+    println!("device id - {:?}, monitor name: {:?}", device_id, monitor_name);


Should be removed or replaced by logging.

Thanks for helping out, but this PR is WIP, so I don't think this is pertinent.

(Also, winit has no logging infrastructure whatsoever, so your proposed alternative would be something of an undertaking. If you want winit to have logging sooner, then I recommend opening an issue so everyone has a chance to argue about whether or not to use slog. It's something I've been meaning to do, but I've been busy.)

francesca64 · 2018-06-03T01:39:55Z

src/platform/ios/mod.rs

+                let app: id = msg_send![Class::get("UIApplication")?, sharedApplication];
+                let delegate: id = msg_send![app, delegate];
+                let state: *mut c_void = *(&*delegate).get_ivar("glutinState");
+                let state = state as *mut DelegateState;


Okay, I've had a chance to work on iOS now. I don't believe this needs to be wrapped in setjmp, and you can just use self.delegate_state instead of re-retrieving everything.

francesca64 · 2018-06-03T01:40:18Z

src/platform/ios/mod.rs

+                let scale = (*state).scale;
+                let width = (*state).size.0;
+                let height = (*state).size.1;
+                Some(((width as f32 * scale) as u64, (height as f32 * scale) as u64))


and note that converting a float to an integer doesn't automatically round. Is this what you wanted?

francesca64 · 2018-06-03T01:43:38Z

src/platform/linux/x11/monitor.rs

@@ -86,6 +89,7 @@ pub fn get_available_monitors(x: &Arc<XConnection>) -> Vec<MonitorId> {
                        dimensions: ((*crtc).width as u32, (*crtc).height as u32),
                        position: ((*crtc).x as i32, (*crtc).y as i32),
                        primary: true,
+                        extents_mm: (x_mm as u64, y_mm as u64),


Fun fact: these can sometimes be 0. #543

francesca64 · 2018-06-03T01:46:42Z

I'm not sure if you've lost patience with us yet, but proper multi-plat DPI support will be landing soon so you'll be in for a bit of a rebase.

By "proper", I mean "proper except for Android", since it doesn't look like I can get access to the DPI factor without rust-mobile/android-rs-glue#116... if I'm wrong, please let me know.

fschutt · 2018-06-03T15:00:31Z

Oh I haven't, it's just that the reason why I needed this in the first place, which is webrender, has "fixed itself" because I noticed that I mixed up pixels and points: servo/webrender#2596 (comment)

So for my use-case, I don't really need it anymore... I'll definitely wait until proper DPI is done, then maybe I can get some redundancy out.

francesca64 · 2018-06-03T15:04:04Z

Proper DPI is over here #548, so it will probably be out in a week or so (though I should know better than to say things like that).

goddessfreya · 2019-07-26T07:17:43Z

This PR hasn't had any activity for over a year! If this is still relevant, please rebase this PR against master and reopen it. Thanks!

fschutt added 10 commits April 1, 2018 06:36

Added get_physical_extents for X11, Wayland and Android

740d47c

Added get_physical_extents for Windows

3f10fbc

Added get_physical_extents for MacOS and iOS

4bafdc7

Fix windows compile errors

edc16bd

Fixed iOS and X11 compile errors

703141c

Fixed more windows / iOS compile errors

2fb3aa5

Made get_physical_extents return an Option instead of panic

8a9c3cd

Fixed Android compile errors

9a27dd4

Fixed Android imports

7a74fdb

Removed android version of get_physical_extents

f9b7059

fschutt mentioned this pull request Apr 2, 2018

Added DPI to GlyphOptions to indicate the DPI for glyph rasterization servo/webrender#2597

Closed

fschutt force-pushed the expose_dpi branch 2 times, most recently from 55299ce to 95e7c01 Compare May 9, 2018 18:53

Getting monitor size on Windows doesn't work with multiple monitors yet

c2e030a

DO NOT MERGE THIS YET

fschutt force-pushed the expose_dpi branch from 95e7c01 to c2e030a Compare May 9, 2018 18:55

torkleyy reviewed May 23, 2018

View reviewed changes

francesca64 reviewed Jun 3, 2018

View reviewed changes

Osspial mentioned this pull request Mar 29, 2019

HiDPI: Missing interface to access physical dpi value. #826

Open

Osspial changed the base branch from master to winit-legacy June 13, 2019 05:36

goddessfreya closed this Jul 26, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Expose get_physical_extents for the MonitorId #442

Expose get_physical_extents for the MonitorId #442

fschutt commented Apr 1, 2018

francesca64 commented Apr 13, 2018

fschutt commented Apr 15, 2018 •

edited

Loading

fschutt commented Apr 15, 2018

fschutt commented Apr 15, 2018 •

edited

Loading

francesca64 commented May 7, 2018

fschutt commented May 8, 2018

torkleyy left a comment

torkleyy May 23, 2018

torkleyy May 23, 2018

francesca64 May 23, 2018

francesca64 Jun 3, 2018

francesca64 Jun 3, 2018

francesca64 Jun 3, 2018

francesca64 commented Jun 3, 2018

fschutt commented Jun 3, 2018

francesca64 commented Jun 3, 2018

goddessfreya commented Jul 26, 2019

Expose get_physical_extents for the MonitorId #442

Expose get_physical_extents for the MonitorId #442

Conversation

fschutt commented Apr 1, 2018

francesca64 commented Apr 13, 2018

fschutt commented Apr 15, 2018 • edited Loading

fschutt commented Apr 15, 2018

fschutt commented Apr 15, 2018 • edited Loading

francesca64 commented May 7, 2018

fschutt commented May 8, 2018

torkleyy left a comment

Choose a reason for hiding this comment

torkleyy May 23, 2018

Choose a reason for hiding this comment

torkleyy May 23, 2018

Choose a reason for hiding this comment

francesca64 May 23, 2018

Choose a reason for hiding this comment

francesca64 Jun 3, 2018

Choose a reason for hiding this comment

francesca64 Jun 3, 2018

Choose a reason for hiding this comment

francesca64 Jun 3, 2018

Choose a reason for hiding this comment

francesca64 commented Jun 3, 2018

fschutt commented Jun 3, 2018

francesca64 commented Jun 3, 2018

goddessfreya commented Jul 26, 2019

fschutt commented Apr 15, 2018 •

edited

Loading

fschutt commented Apr 15, 2018 •

edited

Loading