Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,12 @@
class GCWin32Tests {

@Test
public void gcZoomLevelMustChangeOnShellZoomChange() {
public void gcZoomLevelMustChangeOnShellZoomChange() throws Exception {
checkGcZoomLevelOnCanvas(DPIUtil.getNativeDeviceZoom());
checkGcZoomLevelOnCanvas(DPIUtil.getNativeDeviceZoom()*2);
}

private void checkGcZoomLevelOnCanvas(int expectedZoom) {
private void checkGcZoomLevelOnCanvas(int expectedZoom) throws Exception {
Display display = Display.getDefault();
Shell shell = new Shell(display);
CompletableFuture<Integer> gcNativeZoom = new CompletableFuture<>();
Expand All @@ -47,7 +47,8 @@ private void checkGcZoomLevelOnCanvas(int expectedZoom) {

DPITestUtil.changeDPIZoom(shell, expectedZoom);
canvas.update();
assertEquals("GCData must have a zoom level equal to the actual zoom level of the widget/shell", expectedZoom, (int) gcNativeZoom.join());
int returnedZoom = (int) gcNativeZoom.get(10000, TimeUnit.SECONDS);
assertEquals("GCData must have a zoom level equal to the actual zoom level of the widget/shell", expectedZoom, returnedZoom);
shell.dispose();
}

Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
/*******************************************************************************
* Copyright (c) 2024 Yatta Solutions
*
* This program and the accompanying materials
* are made available under the terms of the Eclipse Public License 2.0
* which accompanies this distribution, and is available at
* https://www.eclipse.org/legal/epl-2.0/
*
* SPDX-License-Identifier: EPL-2.0
*
* Contributors:
* Yatta Solutions - initial API and implementation
*******************************************************************************/
package org.eclipse.swt.widgets;

import java.time.*;
import java.util.concurrent.atomic.*;

import org.eclipse.swt.internal.*;
import org.eclipse.swt.widgets.Control.*;

public final class DPITestUtil {

private DPITestUtil() {
}

private static final int TIMEOUT_MILLIS = 10000;

public static void changeDPIZoom (Shell shell, int nativeZoom) {
DPIUtil.setDeviceZoom(nativeZoom);
Event event = shell.createZoomChangedEvent(nativeZoom);
shell.sendZoomChangedEvent(event, shell);
DPIChangeExecution data = (DPIChangeExecution) event.data;
waitForDPIChange(shell, TIMEOUT_MILLIS, data.taskCount);
}

private static void waitForDPIChange(Shell shell, int timeout, AtomicInteger scalingCounter) {
waitForPassCondition(shell, timeout, scalingCounter);
}

private static void waitForPassCondition(Shell shell, int timeout, AtomicInteger scalingCounter) {
final Instant timeOut = Instant.now().plusMillis(timeout);
final Display display = shell == null ? Display.getDefault() : shell.getDisplay();

while (Instant.now().isBefore(timeOut) && scalingCounter.get() != 0) {
if (!display.isDisposed()) {
display.readAndDispatch();
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -1977,7 +1977,7 @@ public String toString() {
void handleDPIChange(Event event, float scalingFactor) {
super.handleDPIChange(event, scalingFactor);
for (Control child : getChildren()) {
child.notifyListeners(SWT.ZoomChanged, event);
child.sendZoomChangedEvent(event, getShell());
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@


import java.util.*;
import java.util.concurrent.atomic.*;
import java.util.stream.*;

import org.eclipse.swt.*;
Expand Down Expand Up @@ -78,6 +79,9 @@ public abstract class Control extends Widget implements Drawable {
Font font;
int drawCount, foreground, background, backgroundAlpha = 255;

/** Cache for currently processed DPI change event to be able to cancel it if a new one is triggered */
Event currentDpiChangeEvent;

/**
* Prevents uninitialized instances from being created outside the package.
*/
Expand Down Expand Up @@ -4760,7 +4764,10 @@ public boolean setParent (Composite parent) {
if (parent.nativeZoom != nativeZoom) {
int newZoom = parent.nativeZoom;
Event zoomChangedEvent = createZoomChangedEvent(newZoom);
notifyListeners(SWT.ZoomChanged, zoomChangedEvent);
if (currentDpiChangeEvent != null) {
currentDpiChangeEvent.doit = false;
}
sendZoomChangedEvent(zoomChangedEvent, getShell());
}
int flags = OS.SWP_NOSIZE | OS.SWP_NOMOVE | OS.SWP_NOACTIVATE;
OS.SetWindowPos (topHandle, OS.HWND_BOTTOM, 0, 0, 0, 0, flags);
Expand Down Expand Up @@ -4953,19 +4960,24 @@ LRESULT WM_DESTROY (long wParam, long lParam) {
return null;
}

void handleMonitorSpecificDpiChange(int newNativeZoom, Rectangle newBoundsInPixels) {
private void handleMonitorSpecificDpiChange(int newNativeZoom, Rectangle newBoundsInPixels) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WM_DPICHANGED does nothing if nativeZoom == newNativeZoom. Since all events are processed asynchronous (even handling the initial DPI change event for the shell) it can happen that a shell is moved back and forth and the second event is not processed because of the above condition, leading to the shell having a wrong scaling. As an example, imagine you move a shell from 100% to 125% monitor. WM_DPICHANGED forwards this event as 100 != 125 and the event is queued. Then the shell is immediately moved back from 125% to 100%. WM_DPICHANGED does not forward the event because the shell's native zoom is still 100 (the event to change it is only queued but not processed), just like the new zoom. So only the first event will be processed, leaving behind a 125% shell on a 100% monitor.

A solution would be to remove the condition such that every call to WM_DPICHANGED will result in an event to be processed. As an alternative, the initial event on shell level could be processed synchronously to immediately set the shell's native zoom.

Another effect of the above issue is that the shell can shrink or enlarge when rapidly moving around. That will also be fixed with the solution proposals.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, I removed this condition. I don't see any harm in doing it, actually I just verified it by injecting the same multiple events for the same zoom change.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since you now made the event processing at shell level synchronous, it would probably be possible to revert that change.

DPIUtil.setDeviceZoom (newNativeZoom);
Event zoomChangedEvent = createZoomChangedEvent(newNativeZoom);
if (currentDpiChangeEvent != null) {
currentDpiChangeEvent.doit = false;
}
currentDpiChangeEvent = zoomChangedEvent;
notifyListeners(SWT.ZoomChanged, zoomChangedEvent);
this.setBoundsInPixels(newBoundsInPixels.x, newBoundsInPixels.y, newBoundsInPixels.width, newBoundsInPixels.height);
}

private Event createZoomChangedEvent(int zoom) {
Event createZoomChangedEvent(int zoom) {
Event event = new Event();
event.type = SWT.ZoomChanged;
event.widget = this;
event.detail = zoom;
event.doit = true;
event.data = new DPIChangeExecution();
return event;
}

Expand All @@ -4974,12 +4986,10 @@ LRESULT WM_DPICHANGED (long wParam, long lParam) {
int newNativeZoom = DPIUtil.mapDPIToZoom (OS.HIWORD (wParam));
if (getDisplay().isRescalingAtRuntime()) {
Device.win32_destroyUnusedHandles(getDisplay());
if (newNativeZoom != nativeZoom) {
RECT rect = new RECT ();
COM.MoveMemory(rect, lParam, RECT.sizeof);
handleMonitorSpecificDpiChange(newNativeZoom, new Rectangle(rect.left, rect.top, rect.right - rect.left, rect.bottom-rect.top));
return LRESULT.ZERO;
}
RECT rect = new RECT ();
COM.MoveMemory(rect, lParam, RECT.sizeof);
handleMonitorSpecificDpiChange(newNativeZoom, new Rectangle(rect.left, rect.top, rect.right - rect.left, rect.bottom-rect.top));
return LRESULT.ZERO;
} else {
int newZoom = DPIUtil.getZoomForAutoscaleProperty (newNativeZoom);
int oldZoom = DPIUtil.getZoomForAutoscaleProperty (nativeZoom);
Expand Down Expand Up @@ -5879,6 +5889,62 @@ LRESULT wmScrollChild (long wParam, long lParam) {
return null;
}

static class DPIChangeExecution {
AtomicInteger taskCount = new AtomicInteger();
private boolean asyncExec = true;

private void process(Control control, Runnable operation) {
boolean currentAsyncExec = asyncExec;
if (control instanceof Composite comp) {
// do not execute the DPI change asynchronously, if there is no
// layout manager available otherwise size calculations could lead
// to wrong results, because no final layout will be triggered
asyncExec &= (comp.layout != null);
}
if (asyncExec) {
control.getDisplay().asyncExec(operation::run);
} else {
operation.run();
}
// resetting it prevents to break asynchronous execution when the synchronous
// DPI change handling is finished
asyncExec = currentAsyncExec;
}

private void increment() {
taskCount.incrementAndGet();
}

private boolean decrement() {
return taskCount.decrementAndGet() <= 0;
}
}

void sendZoomChangedEvent(Event event, Shell shell) {
this.currentDpiChangeEvent = event;
if (event.data instanceof DPIChangeExecution dpiExecData) {
dpiExecData.increment();
dpiExecData.process(this, () -> {
try {
if (!this.isDisposed() && event.doit) {
notifyListeners(SWT.ZoomChanged, event);
}
} finally {
if (shell.isDisposed()) {
return;
}
if (dpiExecData.decrement()) {
if (event == currentDpiChangeEvent) {
currentDpiChangeEvent = null;
}
if (event.doit) {
shell.WM_SIZE(0, 0);
}
}
}
});
}
}

@Override
void handleDPIChange(Event event, float scalingFactor) {
Expand Down
Loading