eclipse-platform / eclipse.platform.swt

Eclipse SWT
https://www.eclipse.org/swt/
Eclipse Public License 2.0
118 stars 136 forks source link

HiDPI improvements in SWT with single and multiple monitor setup. #62

Open MarcelBoerner opened 2 years ago

MarcelBoerner commented 2 years ago

When using the swts Display class it has different methods which take care about the zoom. The Display class should not have the zooms, only the devices should have the zoom. When you call the getMonitor method of the display class, the zoom to the devices is set to the ones of the primary monitor on windows versions above 8.1: image As you see this can not happen since the else case can not be reached on windows 8.1. And even if the dpi is used you do not get the results which are helping. When you have dpi unaware, the dpi is always 96 pixels, if you have dpi aware application mode than you get the one from the primary monitor and if you have the per monitor (v2) you get the correct dpi. But the last mode can not be used since the dpi awareness only works on the main window and its direct child. The other controls like tabs and so on are not taking part in the per monitor awareness.

sravanlakkimsetti commented 2 years ago

@niraj-modi

MarcelBoerner commented 2 years ago

As Addition i checked the fixes done in the past like: Per Monitor HIDPI Fix for event notification

I tried it with the per monitorv2 feature "on" in the javaw.exe.manifest: <dpiAwareness xmlns="http://schemas.microsoft.com/SMI/2016/WindowsSettings">PerMonitorV2</dpiAwareness>

The application is in permonitorv2 mode: image

So I expected that a zoom changed event is fired when moving my application from one monitor to the other. My Primary monitor is at 3840x2160 @150% scaling and the second 3840x2160 @175% scaling. The application was started at the secondary monitor:

When moving the application from secondary monitor to primary monitor i get no event: image

When moving the application from primary monitor to secondary monitor, i get an event: image

As you can see this is a problem since the oldSWTZoom is gotten from the static field of the dpi Util, which is the "150%" rounded down to 100%. The first step i suggest is to get WM_DPICHANGED event handled correctly, so that you recognize the zoom change. And i would suggest that a control knows his monitor, and the monitor knows his zoom. The zoomchanged event should not only work when changing the zoom within a monitor, but also when moving from one monitor to the other sending the old value and the new value.

merks commented 2 years ago

Just an FYI, note that the only place this event is currently handled is here:

https://github.com/eclipse-platform/eclipse.platform.ui/blob/d93628bf06e59a63852a7d51bbd486ff16adb760/bundles/org.eclipse.ui.workbench/Eclipse%20UI/org/eclipse/ui/internal/WorkbenchWindow.java#L939-L953

Clearly we will not want to suggest to the user to restart Eclipse when they move windows between monitors. This also hints at the extent to which the overall application is not able to deal with dynamic resolution changes...

niraj-modi commented 2 years ago

https://github.com/eclipse-platform/eclipse.platform.swt/issues/62#issuecomment-1108133857

Reason for this behavior: Since SWT by-design is taking starting reference zoom value of Primary monitor only while initialization:

So, let's use this bug to discuss and investigate a solution here, that's why I updated the bug title.

niraj-modi commented 2 years ago

On a side note, can you experiment with 'PerMonitor' settings here:

PerMonitor
MarcelBoerner commented 2 years ago

#62 (comment)

Reason for this behavior: Since SWT by-design is taking starting reference zoom value of Primary monitor only while initialization:

* so when you moved from Secondary to Primary it's didn't notice any change.

* where-as moving from Primary to Secondary was noticed and event was thrown.

So, let's use this bug to discuss and investigate a solution here, that's why I updated the bug title.

I first would like to understand why this design is chosen. Let's think about the cases, microsoft described at which situations the event is fired:

  1. Multiple-monitor setups where each display has a different scale factor and the application is moved from one display to another (such as a 4K and a 1080p display)
  2. Docking and undocking a high DPI laptop with a low-DPI external display (or vice versa)
  3. Connecting via Remote Desktop from a high DPI laptop/tablet to a low-DPI device (or vice versa)
  4. Making a display-scale-factor settings change while applications are running

The current solution in swt Controls class supports only the cases 3 and 4.(2 is only partly supported depending if you have you laptop monitor opened, then external monitor is used as secondary device by default, when the laptop is closed it is used as primary)

What is the goal we'd like to reach? We want the application to adapt to the scaling to the monitor on which the application currently is placed. -> the control class seems to be the correct starting point to support this. In each of the 4 cases above the WM_DPICHANGED event is fired. So SWT could potentially react to each situation.

So let's look at the control classes code:

LRESULT WM_DPICHANGED (long wParam, long lParam) {
    // Map DPI to Zoom and compare
    int nativeZoom = DPIUtil.mapDPIToZoom (OS.HIWORD (wParam));
    int newSWTZoom = DPIUtil.getZoomForAutoscaleProperty (nativeZoom);
    int oldSWTZoom = DPIUtil.getDeviceZoom();

    // Throw the DPI change event if zoom value changes
    if (newSWTZoom != oldSWTZoom) {
        Event event = new Event();
        event.type = SWT.ZoomChanged;
        event.widget = this;
        event.detail = newSWTZoom;
        event.doit = true;
        notifyListeners(SWT.ZoomChanged, event);
        return LRESULT.ZERO;
    }
    return LRESULT.ONE;
}

Why is the oldSWTZoom relevant? It is not fired with the event. So what is the behavior achieved by this code?

For case 1: When moving from primary to secondary your application gets an event, which say, zoomchanged, but when moving it back it does not tell the same. => This is not expected, since our goal is to adapt to the scaling of the monitor on which the application is currently placed. For case 2: if you dock an external monitor with a different scaling, move your application to the secondary external monitor, the event is fired, and we can adapt to the scaling. Now the user disconnects the external monitor. -> Your application automatically is moved to the primary monitor -> no event is fired, so it does not adapt to the scaling of the primary monitor => This is not expected. Since you have the wrong scaling then. For case 3: I am not sure what happens here and what would be the correct behavior. When my application runs on a low dpi monitor and i connect with a high DPI Monitor, what should happen to the application then? For case 4: I start my application on a monitor with scaling @100% then i change the scaling to @150%. -> The zoom change event is not fired. I change the scaling from @100% to @175% -> the event is fired. My application can adapt. Now i change it back from 175% to 100% -> no zoom change event is fired. -> my application can not adapt => This is not expected, since it has some kind of inconsistent behavior on which i can not react.

What would be the expectation? When a zoom changes, my application is noticed about it and i can react on the change. But especially such cases like case 4 where changing in one direction fires an zoomchanged event and changing it back does not fire an event, could always lead to unexpected behavior in the application due to not adapting to the zoom.

So my question: Would it not be correct and consistent to always fire the zoom changed event whenever WM_DPICHANGED event occures by os? If not please be so kind to explain the situation or usecase when i do want the inconsistent behavior?

SyntevoAlex commented 2 years ago

To my understanding, the biggest blocker in current design is that in Windows, DPI is assigned per top-level window (Shell in terms of SWT) , whereas methods in DPIUtil do not use Shell as argument, so it uses a single DPI value for all monitors. With such design, we can only resort to supporting some scenarios at cost of other scenarios.

A proper fix would be to teach DpiUtil to consider Shell in question. Unfortunately, that will require massive (but probably not too complex) changes.

If someone volunteers to do that, I could assist to a reasonable extent.

niraj-modi commented 2 years ago

In my test with Single monitor on changing the OS zoom, the SWT.ZoomChanged event is not consistent. I am investing a fix to make the SWT.ZoomChanged event to fire consistently.

niraj-modi commented 2 years ago

There are multiple areas which are required to be addressed here, listing them what I recall:

  1. Improvements with single Monitor:

    • [ ] #125
    • [ ] #127
  2. Improvements with multiple Monitors setup:

    • [ ] #128
    • [ ] #130
    • [ ] #131

I have put some starting pointer on few of the issues for someone to start with. IMHO, this list will evolve and achieving these will require dedicated efforts (development and testing both)

Thanks @SyntevoAlex for offering to help here and I will also pitch in as needed.

vogella commented 4 months ago

Here is another example with cut-off text in a shell using the notification API (which basically it display a shell with some content) and text scaling of 150%.

image

SWT Snippet (requires org.eclipse.jface.notifications as MANIFEST.MF dependency:



import org.eclipse.jface.notifications.AbstractNotificationPopup;
import org.eclipse.jface.widgets.WidgetFactory;
import org.eclipse.swt.SWT;
import org.eclipse.swt.layout.FillLayout;
import org.eclipse.swt.layout.GridData;
import org.eclipse.swt.widgets.Composite;
import org.eclipse.swt.widgets.Display;
import org.eclipse.swt.widgets.Label;
import org.eclipse.swt.widgets.Shell;

public class Snippet081NotificationPopup {

    private Display display = new Display();
    private Shell shell = WidgetFactory.shell(SWT.NONE).layout(new FillLayout(SWT.VERTICAL)).create(display);

    public static void main(String[] args) {
        new Snippet081NotificationPopup().start();
    }

    private void start() {

        shell.open();

        NotificationFactory.showNotification(shell,
                "This is text 1. This is text 2. This is text 3. This is text 4 This is text 5. This is text 6. This is text 7. This is text 8");
        while (!shell.isDisposed()) {
            if (!display.readAndDispatch())
                display.sleep();
        }
        display.dispose();
    }

    private class NotificationFactory {

        public static void showNotification(Shell shell, String message) {
            NotificationPopUp popup = new NotificationFactory.NotificationPopUp(shell.getDisplay(), shell, message);
            popup.open();
        }

        private static class NotificationPopUp extends AbstractNotificationPopup {

            private String fText;

            public NotificationPopUp(Display display, Shell shell, String text) {
                super(display);
                this.fText = text;
                setParentShell(shell);
                setDelayClose(20000);
            }

            @Override
            protected String getPopupShellTitle() {
                return "Warnung";
            }

            @Override
            protected void createContentArea(Composite parent) {
                Label label = new Label(parent, SWT.WRAP);
                label.setBackground(parent.getBackground());
                label.setText(fText);
                GridData gridData = new GridData(GridData.FILL_BOTH);
//          adjustLabelHeight(parent.getDisplay(), gridData);
                label.setLayoutData(gridData);
            }
//
//      private void adjustLabelHeight(Display display, GridData gridData) {
//          int dpi = display.getDPI().x; // Assume uniform DPI scaling for both x and y
//          // 125 % scaling
//          if (dpi >= 120 && dpi < 144) { // 150% scaling is typically around 144 DPI
//              gridData.heightHint = 80;
//          }
//          // Zwischen 150 % und 200 % scaling
//          if (dpi >= 144 && dpi < 192) { // 150% scaling is typically around 144 DPI
//              gridData.heightHint = 100;
//          }
//      }
        }
    }
}

////