Remove zoom parameter from CoordinateSystemMapper#mapMonitorBounds()

[ShahzaibIbrahim]

The CoordinateSystemMapper#mapMonitorBounds() method currently accepts a zoom value and that is wrong for SingleZoomCoordinateMapper since it should always use the global zoom (DPIUtil#getDeviceZoom()) instead of monitor zoom. This commit changes how zoom is passed through Rectangle with monitor for MultiZoomCoordinateMapper while that monitor will be ignored for SingleZoomCoordinateMapper.

How to Test

• Run the following snippet with no VM argument (monitor-specific scaling off) with two monitors connected (250% and 150%)

import org.eclipse.swt.*;
import org.eclipse.swt.graphics.*;
import org.eclipse.swt.widgets.*;

public class OpenNewShellExample {

    public static void main(String[] args) {
        Display display = new Display();
        Shell mainShell = new Shell(display);
        mainShell.setText("Main Shell");
        mainShell.setSize(300, 200);

        Button openButton = new Button(mainShell, SWT.PUSH);
        openButton.setText("Open New Shell");
        openButton.setBounds(80, 70, 140, 30);

        openButton.addListener(SWT.Selection, e -> {
            Rectangle mainShellRect = mainShell.getMonitor().getBounds();
            Shell newShell = new Shell(mainShell);
            newShell.setText("New Shell");
            newShell.setSize(mainShellRect.width, mainShellRect.height);

            Label infoLabel = new Label(newShell, SWT.NONE);
            infoLabel.setLocation(100, 100);
            infoLabel.setSize(300, 20);
            infoLabel.setText("New Shell Size: " + mainShellRect.width + "x" + mainShellRect.height);
            newShell.open();
        });

        mainShell.open();
        while (!mainShell.isDisposed()) {
            if (!display.readAndDispatch())
                display.sleep();
        }
        display.dispose();
    }
}

• Keep the shell on primary monitor and click "Open New Shell" button
• A new shell will open with the same size of primary monitor (Global Zoom)
• Close the new shell
• Move the first shell to secondary monitor
• Click "Open New Shell" button again.
• The new shell that open should be the same size as the original zoom since monitor-specific scaling is not turned on.

Results

Before
At 250%

at 150%

Notice that zoom is adapted of monitor even though monitor-specific scaling is turned off.

After
At 250% (Primary)

At 150% (Secondary)

Therefore the size remain same (which is correct behaviour)

[github-actions]

Test Results

  118 files  ±0    118 suites  ±0   16m 33s ⏱️ - 1m 6s
4 652 tests +1  4 633 ✅  - 1  18 💤 +1  1 ❌ +1 
  338 runs  +8    334 ✅ +8   4 💤 ±0  0 ❌ ±0 

For more details on these failures, see this check.

Results for commit 54fbe7b. ± Comparison against base commit 136b375.

This pull request skips 1 test.

org.eclipse.swt.tests.junit.Test_org_eclipse_swt_browser_Browser_IE ‑ test_setUrl_remote_with_post

♻️ This comment has been updated with latest results.

[HeikoKlare]

In general, the change looks good to me. Could we add some tests to CoordinateSystemMapperTests that document and ensure the expected behavior for the two mappers? The snippets seems to be a good starting point for that.

[ShahzaibIbrahim]

In general, the change looks good to me. Could we add some tests to CoordinateSystemMapperTests that document and ensure the expected behavior for the two mappers? The snippets seems to be a good starting point for that.

I have added the test. Need to be reviewed.

[HeikoKlare]

The change is sound. Not taking the actual monitor zoom into account when using a single zoom coordinate system is the behavior we used to have for years. I did not find any issues when testing an SDK with disabled monitor-specific scaling.

I have adapted the added test as it previously tested the behavior of mapMonitorBounds based on monitors that were already initialized via mapMonitorBounds, such that potential regresison may be hidden by test setup and assertion logic applying the same error analogously.