Skip to content

Refactor: Move Cocoa-specific DPI methods to CocoaDPIUtil #2278

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

Merged
merged 1 commit into from
Jul 3, 2025

Conversation

ShahzaibIbrahim
Copy link
Contributor

The autoScale methods no longer have relevance in the Win32 implementation, so this commit extracts the macOS-specific DPI scaling logic into a dedicated CocoaDPIUtil class. This improves platform separation and maintains cleaner, OS-specific code boundaries.

Copy link
Contributor

github-actions bot commented Jul 1, 2025

Test Results

   539 files   -  6     539 suites   - 6   31m 49s ⏱️ + 1m 49s
 4 368 tests  - 37   4 352 ✅  - 35   15 💤  - 3  0 ❌ ±0  1 🔥 +1 
16 704 runs   - 37  16 564 ✅  - 35  139 💤  - 3  0 ❌ ±0  1 🔥 +1 

For more details on these errors, see this check.

Results for commit b440e89. ± Comparison against base commit 51fed8e.

This pull request removes 37 tests.
AllWin32Tests org.eclipse.swt.graphics.ImageWin32Tests ‑ testImageDataForDifferentFractionalZoomsShouldBeDifferent
AllWin32Tests org.eclipse.swt.graphics.ImageWin32Tests ‑ testImageShouldHaveDimesionAsPerZoomLevel
AllWin32Tests org.eclipse.swt.tests.win32.Test_org_eclipse_swt_dnd_DND ‑ testByteArrayTransfer
AllWin32Tests org.eclipse.swt.tests.win32.Test_org_eclipse_swt_dnd_DND ‑ testFileTransfer
AllWin32Tests org.eclipse.swt.tests.win32.Test_org_eclipse_swt_dnd_DND ‑ testHtmlTransfer
AllWin32Tests org.eclipse.swt.tests.win32.Test_org_eclipse_swt_dnd_DND ‑ testImageTransfer_fromCopiedImage
AllWin32Tests org.eclipse.swt.tests.win32.Test_org_eclipse_swt_dnd_DND ‑ testImageTransfer_fromImage
AllWin32Tests org.eclipse.swt.tests.win32.Test_org_eclipse_swt_dnd_DND ‑ testImageTransfer_fromImageData
AllWin32Tests org.eclipse.swt.tests.win32.Test_org_eclipse_swt_dnd_DND ‑ testImageTransfer_fromImageDataFromImage
AllWin32Tests org.eclipse.swt.tests.win32.Test_org_eclipse_swt_dnd_DND ‑ testRtfTransfer
…

♻️ This comment has been updated with latest results.

The `autoScale` methods no longer have relevance in the Win32
implementation, so this commit extracts the macOS-specific DPI scaling
logic into a dedicated `CocoaDPIUtil` class. This improves platform
separation and maintains cleaner, OS-specific code boundaries.
Copy link
Contributor

@fedejeanne fedejeanne left a comment

Choose a reason for hiding this comment

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

LGTM but since this is for Mac, I'd like a second opinion. @BeckerWdf ?

@BeckerWdf
Copy link
Contributor

LGTM but since this is for Mac, I'd like a second opinion. @BeckerWdf ?

What exactly should I comment on? Should I test the change? If yes: How can I test this?

@fedejeanne
Copy link
Contributor

LGTM but since this is for Mac, I'd like a second opinion. @BeckerWdf ?

What exactly should I comment on? Should I test the change? If yes: How can I test this?

This PR brings no functional changes with it, it is merely a refactoring. The test consists in running Eclipse performing any action (open a view, edit something, drag stuff around) that runs through the changed method and check that it behaves as it did before.

Since this is done in the context of monitors and zooms, you should try it with different zoom levels. The best way is to have 2 monitors with different zooms and test the same things moving the Workbench/Views between the monitors but if you only have 1 monitor, you could simply try by changing the zoom level between operations.

Usually, this kind of testing can be done in no more than 10 minutes :-)

Thank you!

@fedejeanne
Copy link
Contributor

Thank you for reviewing!

The failing check is unrelated (a test failure on a previous run: #2098)

@fedejeanne fedejeanne merged commit 3d93396 into eclipse-platform:master Jul 3, 2025
20 of 22 checks passed
@fedejeanne fedejeanne deleted the master-287-Cocoa branch July 3, 2025 09:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Refactor DPIUtil] Move Cocoa-specific DPI methods to CocoaDPIUtil
3 participants