Community
Participate
Working Groups
Please take a look at <https://msdn.microsoft.com/en-us/library/windows/desktop/dd464646(v=vs.85).aspx> Since Windows 10 each monitor can have its own scaling factor. While you are at implementing HiDPI support, better do it right from current state of the art. Currently, Monitor lacks a getDpi() function.
I agree we need monitor-specific scaling support, but I think this would be a better API in the Monitor class: int getZoom() The returned value would be the zoom level that is used in ImageDataProvider#getImageData(int) callbacks on that monitor. As long as SWT or the underlying platform doesn't support per-monitor zoom levels, this API would return the internal DPIUtil#deviceZoom for all monitors. We already have a "logical" DPI value at Display#getDPI() that never returned actual dots per inch, and that has been kept stable when the HiDPI Image constructors were added. I wouldn't use the term "DPI" in new APIs.
Created attachment 272273 [details] (partial) fix - added a field "zoom" to the Device and used that to implement Transform, GC and FontMetrics - Since the device zoom cannot be a static field anymore, the DPIUtil was changed drastically (not sure if API-compatibility is necessary for internal classes, though) - the scaling for a Printer should always be 100 (which means getDeviceZoom() should probably be abstract in Device and only implemented in its subclasses, but yet again I was going for API-compatibility)
(In reply to S. Schulz from comment #2) > Created attachment 272273 [details] > (partial) fix > > - added a field "zoom" to the Device and used that to implement Transform, > GC and FontMetrics > - Since the device zoom cannot be a static field anymore, the DPIUtil was > changed drastically (not sure if API-compatibility is necessary for internal > classes, though) > - the scaling for a Printer should always be 100 (which means > getDeviceZoom() should probably be abstract in Device and only implemented > in its subclasses, but yet again I was going for API-compatibility) Thanks for proposing this patch. Since Monitor doesn't extend from Device this patch won't solve the use-case of per monitor specific zoom use-case. Also, please share a real use-case of Monitor#getZoom() API, so it will help us in coming with a better designed API.
(In reply to Niraj Modi from comment #3) > (In reply to S. Schulz from comment #2) > > Created attachment 272273 [details] > > (partial) fix > > > > - added a field "zoom" to the Device and used that to implement Transform, > > GC and FontMetrics > > - Since the device zoom cannot be a static field anymore, the DPIUtil was > > changed drastically (not sure if API-compatibility is necessary for internal > > classes, though) > > - the scaling for a Printer should always be 100 (which means > > getDeviceZoom() should probably be abstract in Device and only implemented > > in its subclasses, but yet again I was going for API-compatibility) > > Thanks for proposing this patch. Since Monitor doesn't extend from Device > this patch won't solve the use-case of per monitor specific zoom use-case. > > Also, please share a real use-case of Monitor#getZoom() API, so it will help > us in coming with a better designed API. That's true, it's only for multiple Devices, not Monitors. However I'm not sure how to implement that on a per-monitor-basis. Everything that uses the Device zoom (Image, GC & Transform at last) only ever use the device, the class Monitor is hardly ever used (hence why I thought the class Display was representing a single monitor). I assume you guys have a better understanding of the API, but IMHO the zoom should not be used to create any resources at all. Only when the controls / images are displayed should the zoom factor in. But that's an enormous change... I'm willing to work on this issue, but I don't know how to proceed. Any advice?
(In reply to S. Schulz from comment #4) > (In reply to Niraj Modi from comment #3) > > (In reply to S. Schulz from comment #2) > > > Created attachment 272273 [details] > > > (partial) fix > > > > > > - added a field "zoom" to the Device and used that to implement Transform, > > > GC and FontMetrics > > > - Since the device zoom cannot be a static field anymore, the DPIUtil was > > > changed drastically (not sure if API-compatibility is necessary for internal > > > classes, though) > > > - the scaling for a Printer should always be 100 (which means > > > getDeviceZoom() should probably be abstract in Device and only implemented > > > in its subclasses, but yet again I was going for API-compatibility) > > > > Thanks for proposing this patch. Since Monitor doesn't extend from Device > > this patch won't solve the use-case of per monitor specific zoom use-case. > > > > Also, please share a real use-case of Monitor#getZoom() API, so it will help > > us in coming with a better designed API. > > That's true, it's only for multiple Devices, not Monitors. However I'm not > sure how to implement that on a per-monitor-basis. Everything that uses the > Device zoom (Image, GC & Transform at last) only ever use the device, the > class Monitor is hardly ever used (hence why I thought the class Display was > representing a single monitor). > > I assume you guys have a better understanding of the API, but IMHO the zoom > should not be used to create any resources at all. Only when the controls / > images are displayed should the zoom factor in. But that's an enormous > change... Ok, we were more interested on the exact use-case(s) that you are targeting. To have a better understanding of the requirement, you can also share SWT Snippets or JUnit test cases that you see is breaking and shall work once we have your patch changes in. > I'm willing to work on this issue, but I don't know how to proceed. Any > advice? Thanks for you interest, currently we use Gerrit for code sharing/review. Once we have clear understanding on the use-case front we can short it out.
New Gerrit change created: https://git.eclipse.org/r/120240
gdk_monitor_get_scale_factor in GTK 3.22 should satisfy this need.
Eric, would you please handle the GTK side?
Thanks Alex! I've a WIP progress patch for GTK with the new APIs, but still requires testing with the HiDPI monitor. Will post the patch shortly.
New Gerrit change created: https://git.eclipse.org/r/120695
(In reply to Eclipse Genie from comment #10) > New Gerrit change created: https://git.eclipse.org/r/120695 The gtk patch uses gdk_monitor_get_scale_factor for GTK >= 3.22 and gdk_screen_get_monitor_scale_factor for older versions, but both the APIs return scale factor 1 even for a HiDPI monitor with 200% scaling. In OS.java, the GDK_SCALE is set to 1. If GDK_SCALE is set to 2 in OS.java, then the GTK APIs return the scale factor as 2. But, removing the code doesn't have any effect. Looks like GDK_SCALE is getting the correct value automatically. I think the launcher code also sets GDK_SCALE=1, but removing that too doesn't have effect on the GTK APIs. Alex/Eric any ideas on how to get the correct scale factor?
New Gerrit change created: https://git.eclipse.org/r/120847
(In reply to Eclipse Genie from comment #6) > New Gerrit change created: https://git.eclipse.org/r/120240 This patch has the Windows and Mac implementation of the Monitor.getZoom() API. To enable dynamic DPI on Windows 10 and above, dpiAwareness setting has to be enabled using java.exe.manifest as mentioned here: https://bugs.eclipse.org/bugs/show_bug.cgi?id=533135#c5
Snippet to test the new API - https://bugs.eclipse.org/bugs/attachment.cgi?id=273491
Gerrit change https://git.eclipse.org/r/120240 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?id=13e0a92eb06604985b73e89050f2814cadcbf876
(In reply to Lakshmi Shanmugam from comment #11) > (In reply to Eclipse Genie from comment #10) > > New Gerrit change created: https://git.eclipse.org/r/120695 > > The gtk patch uses gdk_monitor_get_scale_factor for GTK >= 3.22 and > gdk_screen_get_monitor_scale_factor for older versions, but both the APIs > return scale factor 1 even for a HiDPI monitor with 200% scaling. > > In OS.java, the GDK_SCALE is set to 1. If GDK_SCALE is set to 2 in OS.java, > then the GTK APIs return the scale factor as 2. But, removing the code > doesn't have any effect. Looks like GDK_SCALE is getting the correct value > automatically. > > I think the launcher code also sets GDK_SCALE=1, but removing that too > doesn't have effect on the GTK APIs. > > Alex/Eric any ideas on how to get the correct scale factor? Hi Alex, any ideas? What I'm trying to find out is, does gdk_monitor_get_scale_factor() return the correct scale factor? I tested the patch with Fedora 27 and it doesn't give scale factor 2 on HiDPI monitor.
(In reply to Lakshmi Shanmugam from comment #16) > (In reply to Lakshmi Shanmugam from comment #11) > > (In reply to Eclipse Genie from comment #10) > > > New Gerrit change created: https://git.eclipse.org/r/120695 > > > > The gtk patch uses gdk_monitor_get_scale_factor for GTK >= 3.22 and > > gdk_screen_get_monitor_scale_factor for older versions, but both the APIs > > return scale factor 1 even for a HiDPI monitor with 200% scaling. > > > > In OS.java, the GDK_SCALE is set to 1. If GDK_SCALE is set to 2 in OS.java, > > then the GTK APIs return the scale factor as 2. But, removing the code > > doesn't have any effect. Looks like GDK_SCALE is getting the correct value > > automatically. > > > > I think the launcher code also sets GDK_SCALE=1, but removing that too > > doesn't have effect on the GTK APIs. > > > > Alex/Eric any ideas on how to get the correct scale factor? > > Hi Alex, any ideas? > What I'm trying to find out is, does gdk_monitor_get_scale_factor() return > the correct scale factor? I tested the patch with Fedora 27 and it doesn't > give scale factor 2 on HiDPI monitor. Look at the latest patch. After changing Display scale to 200% in gnome settings I can see getZoom properly giving me 200%. Seems to be working fine to me.
Just for the record in the default case it gives me 100.
(In reply to Alexander Kurtakov from comment #17) > Look at the latest patch. After changing Display scale to 200% in gnome > settings I can see getZoom properly giving me 200%. Seems to be working fine > to me. Thanks for looking into this, Alex. This is strange, I tried the latest patch but getZoom() still gives me 100. I tried Fedora 27 with both setups: single standard resolution monitor and standard + hidpi monitor. I changed the scale factor from the command line using: gsettings set org.gnome.desktop.interface scaling-factor 2 What is your setup? how do you set the scale factor?
(In reply to Lakshmi Shanmugam from comment #19) > (In reply to Alexander Kurtakov from comment #17) > > Look at the latest patch. After changing Display scale to 200% in gnome > > settings I can see getZoom properly giving me 200%. Seems to be working fine > > to me. > Thanks for looking into this, Alex. > > This is strange, I tried the latest patch but getZoom() still gives me 100. > I tried Fedora 27 with both setups: single standard resolution monitor and > standard + hidpi monitor. I changed the scale factor from the command line > using: > gsettings set org.gnome.desktop.interface scaling-factor 2 > > What is your setup? how do you set the scale factor? Fedora 28 1. setting app 2. Devices 3. Display 4. Change Scale to 200%
Sravan, can you please test the patch on your linux box?
(In reply to Lakshmi Shanmugam from comment #21) > Sravan, can you please test the patch on your linux box? I tested on Sravan's VM with Fedora 27 and I get scale factor as 2. May be the difference is because the snippet is running on X11 on his setup. On my system it doesn't seem to run on X11 (OS.isX11() returns false) even if I set GDK_BACKEND=x11.
(In reply to Lakshmi Shanmugam from comment #22) > (In reply to Lakshmi Shanmugam from comment #21) > > Sravan, can you please test the patch on your linux box? > I tested on Sravan's VM with Fedora 27 and I get scale factor as 2. > May be the difference is because the snippet is running on X11 on his setup. > On my system it doesn't seem to run on X11 (OS.isX11() returns false) even > if I set GDK_BACKEND=x11. I run on wayland and still see correct value.
(In reply to Alexander Kurtakov from comment #23) > (In reply to Lakshmi Shanmugam from comment #22) > > (In reply to Lakshmi Shanmugam from comment #21) > > > Sravan, can you please test the patch on your linux box? > > I tested on Sravan's VM with Fedora 27 and I get scale factor as 2. > > May be the difference is because the snippet is running on X11 on his setup. > > On my system it doesn't seem to run on X11 (OS.isX11() returns false) even > > if I set GDK_BACKEND=x11. > > I run on wayland and still see correct value. Ok, I'm not sure what is wrong in my setup. Eric/Leo, can one of you please try out the new API? I just want to make sure the code works on another system before I commit the changes and that the problem is specific to my setup.
I tried the snippet attached to this bug with Alex's latest patch (v14) on Gtk/Wayland, works well. Environment: - 4 monitor setup - Fedora 27, Gtk3.22 Wayland - 1 monitor has 200% HiDpi The snippet reports 200-100-100-100 correctly and when I move the snippet onto the screen with hiDpi, it makes it self bigger. When I move it back onto a regular screen, it makes itself smaller again. Kinda cool. This is similar to what gnome does with it's native apps as well. Made a little video (a bit blurry unfortunately, but you get the idea): https://www.youtube.com/watch?v=Xixj1UoLslg&feature=youtu.be
Gerrit change https://git.eclipse.org/r/120695 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?id=ecb072664278dc6cfba0ce56ea06e669e0675959
Gerrit change https://git.eclipse.org/r/120847 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?id=d3da5c5ceec7fff7568841d9eb69a77433b7dac6
New Gerrit change created: https://git.eclipse.org/r/121709
Gerrit change https://git.eclipse.org/r/121709 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?id=cdc67cbff3fb701108f19e6dbec01306872e31dc
(In reply to Eclipse Genie from comment #29) > Gerrit change https://git.eclipse.org/r/121709 was merged to [master]. > Commit: > http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/ > ?id=cdc67cbff3fb701108f19e6dbec01306872e31dc Thanks Nikita for the patch, it refactors the code related to new API and primarily simplifies the Windows implementation.
Resolving.
Verified in I20180507-2205 on Win10.
