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

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

Open
MarcelBoerner opened this issue Apr 21, 2022 · 10 comments
Open
Labels
Windows Happens on Windows OS

Comments

@MarcelBoerner
Copy link

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 sravanlakkimsetti added the Windows Happens on Windows OS label Apr 21, 2022
@sravanlakkimsetti
Copy link
Member

@niraj-modi

@MarcelBoerner
Copy link
Author

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
Copy link
Contributor

merks commented Apr 25, 2022

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 niraj-modi changed the title Display class should not have the zoom Display zoom value to be based on the Monitor where application is launched. Apr 26, 2022
@niraj-modi
Copy link
Member

#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.

@niraj-modi
Copy link
Member

niraj-modi commented Apr 26, 2022

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

Related SWT FAQ: https://www.eclipse.org/swt/faq.php#winexternalmanifestfile

IIRC, with 'PerMonitor' we might miss on the WM_DPICHANGED event, but does it give you a better UI experience when switching across monitors and other scenarios that you are trying ?

@MarcelBoerner
Copy link
Author

MarcelBoerner commented Apr 27, 2022

#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
Copy link
Member

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
Copy link
Member

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
Copy link
Member

niraj-modi commented May 11, 2022

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

  1. Improvements with single Monitor:
  1. Improvements with multiple Monitors setup:

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.

@niraj-modi niraj-modi changed the title Display zoom value to be based on the Monitor where application is launched. HiDPI improvements in SWT with single and multiple monitor setup. May 11, 2022
akoch-yatta added a commit to akoch-yatta/eclipse.platform.swt that referenced this issue Mar 28, 2024
akoch-yatta added a commit to akoch-yatta/eclipse.platform.swt that referenced this issue Mar 28, 2024
akoch-yatta added a commit to akoch-yatta/eclipse.platform.swt that referenced this issue Mar 28, 2024
akoch-yatta added a commit to akoch-yatta/eclipse.platform.swt that referenced this issue Mar 28, 2024
akoch-yatta added a commit to akoch-yatta/eclipse.platform.swt that referenced this issue Mar 29, 2024
This contribution adds the base logics to support the dynamical adaption on SWT applications on DPI related zoom changes in the win32 implementation. It consists of:
- adds "swt.autoScale.updateOnRuntime"-flag to enable updating the widgets on runtime on DPI zoom changes
- Component to register callbacks on DPI changes. Each callback should be responsible for the attributes of its class

Contributes to eclipse-platform#62
and eclipse-platform#127
akoch-yatta added a commit to akoch-yatta/eclipse.platform.swt that referenced this issue Mar 29, 2024
akoch-yatta added a commit to akoch-yatta/eclipse.platform.swt that referenced this issue Mar 29, 2024
akoch-yatta added a commit to akoch-yatta/eclipse.platform.swt that referenced this issue Mar 29, 2024
amartya4256 pushed a commit to amartya4256/eclipse.platform.swt that referenced this issue Apr 12, 2024
This contribution adds the base logics to support the dynamical adaption on SWT applications on DPI related zoom changes in the win32 implementation. It consists of:
- adds "swt.autoScale.updateOnRuntime"-flag to enable updating the widgets on runtime on DPI zoom changes
- Component to register callbacks on DPI changes. Each callback should be responsible for the attributes of its class

Contributes to eclipse-platform#62
and eclipse-platform#127
amartya4256 pushed a commit to amartya4256/eclipse.platform.swt that referenced this issue Apr 12, 2024
amartya4256 pushed a commit to amartya4256/eclipse.platform.swt that referenced this issue Apr 12, 2024
amartya4256 pushed a commit to amartya4256/eclipse.platform.swt that referenced this issue Apr 12, 2024
akoch-yatta added a commit to akoch-yatta/eclipse.platform.swt that referenced this issue Apr 12, 2024
This contribution adds the base logics to support the dynamical adaption on SWT applications on DPI related zoom changes in the win32 implementation. It consists of:
- adds "swt.autoScale.updateOnRuntime"-flag to enable updating the widgets on runtime on DPI zoom changes
- Component to register callbacks on DPI changes. Each callback should be responsible for the attributes of its class

Contributes to eclipse-platform#62
and eclipse-platform#127
akoch-yatta added a commit to akoch-yatta/eclipse.platform.swt that referenced this issue Apr 12, 2024
amartya4256 added a commit to amartya4256/eclipse.platform.swt that referenced this issue Nov 14, 2024
This commit refactors usage of ImageDataProvider and
ImageFilenameProvider to make the code more readable.

contributes to eclipse-platform#62 and eclipse-platform#127
amartya4256 added a commit to amartya4256/eclipse.platform.swt that referenced this issue Nov 14, 2024
This commit refactors usage of ImageDataProvider and
ImageFilenameProvider to make the code more readable.

contributes to eclipse-platform#62 and eclipse-platform#127
fedejeanne pushed a commit to amartya4256/eclipse.platform.swt that referenced this issue Nov 27, 2024
This contribution encapsulates the metadata of the image in an
innerclass ImageHandle which is used to create a hashmap of zoom level
to imageHandle object inside an image object, making it straight
forward to obtain any metadata information from an image for a zoom
level.

contributes to eclipse-platform#62 and eclipse-platform#127
fedejeanne pushed a commit to amartya4256/eclipse.platform.swt that referenced this issue Nov 27, 2024
This contribution encapsulates the metadata of the image in an
innerclass ImageHandle which is used to create a hashmap of zoom level
to imageHandle object inside an image object, making it straight
forward to obtain any metadata information from an image for a zoom
level.

contributes to eclipse-platform#62 and eclipse-platform#127
fedejeanne pushed a commit that referenced this issue Nov 27, 2024
This contribution encapsulates the metadata of the image in an
innerclass ImageHandle which is used to create a hashmap of zoom level
to imageHandle object inside an image object, making it straight
forward to obtain any metadata information from an image for a zoom
level.

contributes to #62 and #127
akoch-yatta added a commit to akoch-yatta/eclipse.platform.swt that referenced this issue Dec 2, 2024
This contributes extracts two different strategies to provide a consistent coordinate system in the win32 implemenentation for a single-zoom and a multi-zoom environment. The existing logic remains unchanged in this commit. It is only moved and consolidated in the new inner classes in Display

Contributes to eclipse-platform#62 and eclipse-platform#131
akoch-yatta added a commit to akoch-yatta/eclipse.platform.swt that referenced this issue Dec 2, 2024
This contributes extracts two different strategies to provide a consistent coordinate system in the win32 implemenentation for a single-zoom and a multi-zoom environment. The existing logic remains unchanged in this commit. It is only moved and consolidated in the new inner classes in Display

Contributes to eclipse-platform#62 and eclipse-platform#131
akoch-yatta added a commit to akoch-yatta/eclipse.platform.swt that referenced this issue Dec 2, 2024
This contributes extracts two different strategies to provide a consistent coordinate system in the win32 implemenentation for a single-zoom and a multi-zoom environment. The existing logic remains unchanged in this commit. It is only moved and consolidated in the new inner classes in Display

Contributes to eclipse-platform#62 and eclipse-platform#131
akoch-yatta added a commit to akoch-yatta/eclipse.platform.swt that referenced this issue Dec 5, 2024
This contributes extracts two different strategies to provide a consistent coordinate system in the win32 implemenentation for a single-zoom and a multi-zoom environment. The existing logic remains unchanged in this commit. It is only moved and consolidated in the new inner classes in Display

Contributes to eclipse-platform#62 and eclipse-platform#131
mai-tran-03 pushed a commit to mai-tran-03/eclipse.platform.ui that referenced this issue Dec 10, 2024
This commit adds a listener for the ZoomChanged event to the canvas of a LineNumberRulerColumn. If the listener is notified of this event this means, that state, that differs over different zoom values, must be recalculated. Therefore the indentation are reset, when the event occurs.

Contributes to eclipse-platform/eclipse.platform.swt#62 and eclipse-platform/eclipse.platform.swt#131
akoch-yatta added a commit to akoch-yatta/eclipse.platform.swt that referenced this issue Dec 10, 2024
This contributes extracts two different strategies to provide a consistent coordinate system in the win32 implemenentation for a single-zoom and a multi-zoom environment. The existing logic remains unchanged in this commit. It is only moved and consolidated in the new inner classes in Display

Contributes to eclipse-platform#62 and eclipse-platform#131
akoch-yatta added a commit to akoch-yatta/eclipse.platform.swt that referenced this issue Dec 10, 2024
This contributes extracts two different strategies to provide a consistent coordinate system in the win32 implemenentation for a single-zoom and a multi-zoom environment. The existing logic remains unchanged in this commit. It is only moved and consolidated in the new inner classes in Display

Contributes to eclipse-platform#62 and eclipse-platform#131
akoch-yatta added a commit to akoch-yatta/eclipse.platform.swt that referenced this issue Dec 10, 2024
This contributes extracts two different strategies to provide a consistent coordinate system in the win32 implemenentation for a single-zoom and a multi-zoom environment. The existing logic remains unchanged in this commit. It is only moved and consolidated in the new inner classes in Display

Contributes to eclipse-platform#62 and eclipse-platform#131
akoch-yatta added a commit to akoch-yatta/eclipse.platform.swt that referenced this issue Dec 10, 2024
This contributes extracts two different strategies to provide a consistent coordinate system in the win32 implemenentation for a single-zoom and a multi-zoom environment. The existing logic remains unchanged in this commit. It is only moved and consolidated in the new inner classes in Display

Contributes to eclipse-platform#62 and eclipse-platform#131
akoch-yatta added a commit to akoch-yatta/eclipse.platform.swt that referenced this issue Dec 10, 2024
This contribution extracts two different strategies to provide a consistent coordinate system in the win32 implemenentation for a single-zoom and a multi-zoom environment. The existing logic remains unchanged in this commit. It is only moved and consolidated in the new inner classes in Display.

Contributes to eclipse-platform#62 and eclipse-platform#131
Fixes eclipse-platform/eclipse.platform.ui#2595
HeikoKlare pushed a commit to akoch-yatta/eclipse.platform.swt that referenced this issue Dec 10, 2024
This contribution extracts two different strategies to provide a consistent coordinate system in the win32 implemenentation for a single-zoom and a multi-zoom environment. The existing logic remains unchanged in this commit. It is only moved and consolidated in the new inner classes in Display.

Contributes to eclipse-platform#62 and eclipse-platform#131
Fixes eclipse-platform/eclipse.platform.ui#2595
HeikoKlare pushed a commit that referenced this issue Dec 10, 2024
This contribution extracts two different strategies to provide a consistent coordinate system in the win32 implemenentation for a single-zoom and a multi-zoom environment. The existing logic remains unchanged in this commit. It is only moved and consolidated in the new inner classes in Display.

Contributes to #62 and #131
Fixes eclipse-platform/eclipse.platform.ui#2595
amartya4256 added a commit to amartya4256/eclipse.platform.swt that referenced this issue Dec 11, 2024
This commit enforces device zoom to be used for scaling down the DPI
values instead of the native zoom.

contributes to eclipse-platform#62 and eclipse-platform#127
amartya4256 added a commit to amartya4256/eclipse.platform.swt that referenced this issue Dec 16, 2024
This contribution encapsulates the metadata of the image in an
innerclass ImageHandle which is used to create a hashmap of zoom level
to imageHandle object inside an image object, making it straight
forward to obtain any metadata information from an image for a zoom
level.

contributes to eclipse-platform#62 and eclipse-platform#127
amartya4256 added a commit to amartya4256/eclipse.platform.swt that referenced this issue Dec 16, 2024
This commit refactors usage of ImageDataProvider and
ImageFilenameProvider to make the code more readable.

contributes to eclipse-platform#62 and eclipse-platform#127
amartya4256 added a commit to amartya4256/eclipse.platform.swt that referenced this issue Dec 16, 2024
This commit refactors usage of ImageDataProvider and
ImageFilenameProvider to make the code more readable.

contributes to eclipse-platform#62 and eclipse-platform#127
amartya4256 added a commit to amartya4256/eclipse.platform.swt that referenced this issue Dec 16, 2024
This contribution encapsulates the metadata of the image in an
innerclass ImageHandle which is used to create a hashmap of zoom level
to imageHandle object inside an image object, making it straight
forward to obtain any metadata information from an image for a zoom
level.

contributes to eclipse-platform#62 and eclipse-platform#127
amartya4256 added a commit to amartya4256/eclipse.platform.swt that referenced this issue Dec 16, 2024
This commit refactors usage of ImageDataProvider and
ImageFilenameProvider to make the code more readable.

contributes to eclipse-platform#62 and eclipse-platform#127
amartya4256 added a commit to amartya4256/eclipse.platform.swt that referenced this issue Dec 17, 2024
This commit adds a check for imageMetadata for handle in case, there
already exists an entry in the zoomLeveltoImageHandle map for a zoom
level and it must throw an exception in that case.

contributes to eclipse-platform#62 and eclipse-platform#127
fedejeanne pushed a commit to amartya4256/eclipse.platform.swt that referenced this issue Dec 18, 2024
This commit adds a check for imageMetadata for handle in case, there
already exists an entry in the zoomLeveltoImageHandle map for a zoom
level and it must throw an exception in that case.

contributes to eclipse-platform#62 and eclipse-platform#127
fedejeanne pushed a commit that referenced this issue Dec 18, 2024
This commit adds a check for imageMetadata for handle in case, there
already exists an entry in the zoomLeveltoImageHandle map for a zoom
level and it must throw an exception in that case.

contributes to #62 and #127
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Windows Happens on Windows OS
Projects
None yet
Development

No branches or pull requests

6 participants