Deprecating Display#getDPI

[ShahzaibIbrahim]

Having the scale factor being based on the screen DPI leads to unexpected result e.g. Image too big/small. Having a screen dpi independent factor leads to consistent results.

When calling getDPI method as Device even with Printer instance, it will show you deprecation warning but should be fine when using the Printer object while calling getDPI. Here's the example:

[github-actions]

Test Results

   546 files  ±0     546 suites  ±0   33m 56s ⏱️ + 1m 42s
 4 413 tests ±0   4 396 ✅ ±0   17 💤 ±0  0 ❌ ±0 
16 722 runs  ±0  16 594 ✅  - 1  128 💤 +1  0 ❌ ±0 

Results for commit cf94ac5. ± Comparison against base commit 13934ee.

♻️ This comment has been updated with latest results.

[fedejeanne]

In general the change looks harmless, but I'm missing some context here. Could you please add some @deprecated comment to the JavaDoc of Display::getDPI and explain why it is wrong to use it? Replacing the value with 100 seems a bit esoteric.

[HeikoKlare]

The current change does actually not fit to what it is supposed to be (as mentioned in the PR title): it should be about deprecating Device.getDPI(), not Display.getDPI(), and about overwriting that deprecation for Printer.getDPI() as for the Printer class the method is still useful while for Display it is not. It would also need to be applied to the implementations on every OS.

[ShahzaibIbrahim]

@HeikoKlare PR title is updated now. It indeed deprecate the Display#getDPI and we keep the Printer#getDPI as is.

@fedejeanne I have added javadoc stating why is it being deprecated. also I adapted the value from 100 to 96 as Default Dots Per Inch (DPI) similar to here: https://github.com/eclipse-gef/gef-classic/pull/750/files

[HeikoKlare]

PR title is updated now. It indeed deprecate the Display#getDPI and we keep the Printer#getDPI as is.

But that's not what we want to do: it does not really make sense to deprecate an overwritten method, as consumers may just operate on the abstraction (i.e., Device#getDPI()), which is not deprecated. So we need to deprecate the most abstract method (Device#getDPI()) und "un-deprecate" the specialization that we want to keep (Printer#getDPI()).

[fipro78]

@HeikoKlare @fedejeanne @ShahzaibIbrahim

Hi all,

since you are now planning to deprecate Display#getDPI() I wonder how I should deal with this in a custom SWT widget implementation.

In NatTable I added code to render the table according to the DPI. I used Display#getDPI() to calculate the factor to use for painting on the GC accordingly.

As I didn't find information about how to migrate anywhere, I digged into the code and searched for information in tickets. I changed the usage of Display#getDPI() to the following:

(Display.getDefault().getPrimaryMonitor().getZoom() * 96) / 100)

If I start the SWT application with -Dswt.autoScale.updateOnRuntime=false the rendering of NatTable looks correct. Checking the values I see:

Display#getDPI().x = 144
(Display.getDefault().getPrimaryMonitor().getZoom() * 96) / 100) = 144

If I set -Dswt.autoScale.updateOnRuntime=true the rendering of NatTable is not correct anymore, because it automatically upscales. Actually it looks like it upscales twice, once by SWT itself and once by NatTable. Checking the values I see:

Display#getDPI().x = 96
(Display.getDefault().getPrimaryMonitor().getZoom() * 96) / 100) = 144

In Display#getDPI() I can see that DPIUtil.scaleDown() is called. So I assume with -Dswt.autoScale.updateOnRuntime=true the value is downscaled, with -Dswt.autoScale.updateOnRuntime=false it is not. At least that is the case with 4.36.

Is there an additional change in the upcoming SWT implementation that takes care of this?
Are there public methods that would help to deal with that fact? DPIUtil is internal, therefore also all the system property keys that would need to be checked.

Of course I can manually check the value of the system property swt.autoScale.updateOnRuntime (hardcoded as the key is not available via constant) and if it is set to true I take a constant value of 96 and if false calculate the DPI based on the primary monitor as shown above.

Is this the intention when deprecating Display#getDPI()? Then it should be documented somehow. And probably there should be some public API that helps in dealing with that situation. If I missed that information how to deal with the new DPI situation in SWT, please point me to that information, so I am able to make the necessary adjustments, or at least recommendations for NatTable adopters, to handle it correctly. And probably also in some Nebula widgets where Display#getDPI() is used at several places as already mentioned in vi-eclipse/Eclipse-Platform#205

BTW, I checked the Plug-in Migration Guide in the Eclipse Help and do not find any information about this topic. And actually I think the help generation is broken somehow, because the Incompatibility section of almost all entries is the same, which doesn't seem correct to me.

[akoch-yatta]

Hi @fipro78,

thanks for reaching out.

Generally, it should not be necessary to do any scaling at all on client side. This is of course only the case if some assumptions that SWT has are the same as yours. I'll try to explain, what I understand from your comment

since you are now planning to deprecate Display#getDPI() I wonder how I should deal with this in a custom SWT widget implementation.

In NatTable I added code to render the table according to the DPI. I used Display#getDPI() to calculate the factor to use for painting on the GC accordingly.

As I didn't find information about how to migrate anywhere, I digged into the code and searched for information in tickets. I changed the usage of Display#getDPI() to the following:

(Display.getDefault().getPrimaryMonitor().getZoom() * 96) / 100)

If I start the SWT application with -Dswt.autoScale.updateOnRuntime=false the rendering of NatTable looks correct. Checking the values I see:

Display#getDPI().x = 144
(Display.getDefault().getPrimaryMonitor().getZoom() * 96) / 100) = 144

If I set -Dswt.autoScale.updateOnRuntime=true the rendering of NatTable is not correct anymore, because it automatically upscales. Actually it looks like it upscales twice, once by SWT itself and once by NatTable. Checking the values I see:

Display#getDPI().x = 96
(Display.getDefault().getPrimaryMonitor().getZoom() * 96) / 100) = 144

In Display#getDPI() I can see that DPIUtil.scaleDown() is called. So I assume with -Dswt.autoScale.updateOnRuntime=true the value is downscaled, with -Dswt.autoScale.updateOnRuntime=false it is not. At least that is the case with 4.36.

Yes and no. swt.autoScale.updateOnRuntime causes this different values, but if I understand your setup correctly, the root cause for the effect you see comes from swt.autoScale, which has different default values for -Dswt.autoScale.updateOnRuntime=true (-Dswt.autoScale=quarter) and -Dswt.autoScale.updateOnRuntime=false (-Dswt.autoScale=integer200). If you e.g. start your application with -Dswt.autoScale.updateOnRuntime=false -Dswt.autoScale=quarter you should see the same issues.

Before talking about a solution I would like to understand what you would like to achieve. Let's say, i am on a 150% monitor - do you always want to draw on a 1,5 scale independent of the value of swt.autoScale? So do you always want to link your scaling to the monitor zoom?

[fipro78]

If you e.g. start your application with -Dswt.autoScale.updateOnRuntime=false -Dswt.autoScale=quarter you should see the same issues.

Correct.

Let's say, i am on a 150% monitor - do you always want to draw on a 1,5 scale independent of the value of swt.autoScale? So do you always want to link your scaling to the monitor zoom?

I want to scale the NatTable according to the rest of the SWT application. Fonts, grid lines, images etc. In the past I had to inspect the DPI setting and do the calculation manually, because on the GC you are drawing pixel based, and 10 pixels on a 150% monitor should be 15 pixels. That was not done automatically in the past, so I had to do this manually. Without this modification, the line heights for example are to small.

As NatTable also supports zooming in and out of the table, I need to set the initial value according to the setting of the application, so it looks like the rest of the application on startup.

[akoch-yatta]

Let's say, i am on a 150% monitor - do you always want to draw on a 1,5 scale independent of the value of swt.autoScale? So do you always want to link your scaling to the monitor zoom?

I want to scale the NatTable according to the rest of the SWT application. Fonts, grid lines, images etc. In the past I had to inspect the DPI setting and do the calculation manually, because on the GC you are drawing pixel based, and 10 pixels on a 150% monitor should be 15 pixels. That was not done automatically in the past, so I had to do this manually. Without this modification, the line heights for example are to small.

Sounds like a yes to me. In SWT the semantics of zoom got a bit messy over the years, but its hard to clean up or streamline it fully now, because a lot of workarounds (like yours) were build because of this. For example swt.autoScale=200 means images will be scaled by 200 (independent of monitor zoom), but fonts are still relying on the monitor zoom and the value of Display#getDPI is hardly unusable in this scenario.

As NatTable also supports zooming in and out of the table, I need to set the initial value according to the setting of the application, so it looks like the rest of the application on startup.

I assume you are fully drawing on a GC, am I correct? This sounds like a similar scenario GEF is facing and I assume your current solution is mostly similar to the solutions in GEF. I think the solution for you is to incorporate the monitor zoom into your custom table zoom. Two questions:

• Do you render something, that is currently affected by your DPI calculations, but not by the custom table zoom?
• How do you do the font scaling in your custom zoom? Do you scaling the font height with your scale factor and create a font out of that?

[fipro78]

I assume you are fully drawing on a GC, am I correct?

Yes

Do you render something, that is currently affected by your DPI calculations, but not by the custom table zoom?

I hope not. Even the embedded controls like dropdowns are affected by the custom table zoom.

How do you do the font scaling in your custom zoom? Do you scaling the font height with your scale factor and create a font out of that?

Yes, see

https://github.com/eclipse-nattable/nattable/blob/2942c0f7db81e6588ff1d1d984302046d117618d/org.eclipse.nebula.widgets.nattable.core/src/org/eclipse/nebula/widgets/nattable/NatTable.java#L438-L444

and

https://github.com/eclipse-nattable/nattable/blob/2942c0f7db81e6588ff1d1d984302046d117618d/org.eclipse.nebula.widgets.nattable.core/src/org/eclipse/nebula/widgets/nattable/util/GUIHelper.java#L131-L141

[akoch-yatta]

Follow-up question: How/where is the custom zoom applied to the GC, e.g. respective to coordinates/sizes etc. Are your using GC#setTransform similar to SWTGrapics or scaling it up yourself similar to ScaledGraphics before calling the GC API?

[fipro78]

I am scaling myself before calling the GC API.

[akoch-yatta]

Okay, so, as I assumed, the situation looks quite similar to GEF. We are currently spending quite some time to evaluate what is the best solution for this scenario and started to implement some hidden features for that in SWT.

Short disclaimer first: what I will describe is under development, not API, will probably change until it is done and is currently windows only and could break GTK and MacOS. Compatibility with GTK and MacOS are probably quite easy to achieve, but we are focusing on having a proper solution for windows first.

One big problem with drawing (or partially drawing complex elements) on a GC is when you use an autoScale mode like quarter and draw on uneven monitor zooms like 125% or 175% -> This will most certainly result in rendering artifacts because the conversions from points to pixels and vice versa leads to rounding issues.
In our opinion the only solution is to always draw on an unscaled GC and do the scaling yourself. Mid/long-term plan is to (hopefully) add this as a feature in SWT itself, but for now it is on your side. As you already have a custom zoom implemented, that should be quite easy:

1. First step is to disable the autoscaling for the Canvas you draw on with your GC. You can do that with canvas.setData("AUTOSCALE_DISABLED", true).
2. Second step is to incorporate the native zoom of the canvas (== monitor zoom) into your custom zoom. You can have a look into our WIP state for GEF: Zoom adaptions. Important is the zoomChangedListener to react on zoom changes on the Canvas and canvas.getData("NATIVE_ZOOM") to get the current zoom of it. If you multiply this zoom with your own custom zoom the result show achieve, what you want.

If you have follow-up questions, I would propose to either open a discussion here in the repo and continue there or we switch over to a discussion/issue in the NatTable repo.

[fipro78]

@akoch-yatta
Thanks for the updates. I have played around with this a bit, and it seems to work quite well. There are some flaws, but these are mainly related to the usage of Display#getDPI() in static helper classes. I will need to change those also to better align with the changes in SWT. IIRC these places are for Images which I need to work on also. At least if I got the information from the N&N right.

I opened a discussion in the NatTable forum: eclipse-nattable/nattable#164

Would be great if you could share any updates there or point to other discussions with informations, so I can update NatTable and probably also some Nebula widgets accordingly (e.g. the RichTextPainter is basically the same).