Deprecate DefaultSWTFontRegistry and Completely switch to ScalingSWTFontRegistry #2212

arunjose696 · 2025-06-06T12:30:34Z

Summary
This PR introduces the following changes:

1)Always use ScalingSWTFontRegistry
2)Rename DefaultSWTFontRegistry -> LegacySWTFontRegistry as this is not default anymore
3)Add a system property to use LegacySWTFontRegistry without monitor specific scaling active
4)Deprecate the LegacySWTFontRegistry(DefaultSWTFontRegistry)
5)additionally the change also makes sure all fonts are cached when they are created, this is necessary to make sure the systemfonts would also be cached when they are created (this change is to fix Test_org_eclipse_swt_graphics_GC.test_setFontLorg_eclipse_swt_graphics_Font which was broken with ScalingSwtFontRegistry)

Reproduction Steps
To use the legacy font registry instead of the default scaling registry, launch the application with the following system properties:

-Dswt.autoScale.updateOnRuntime=false
-Dswt.autoScale=false
-Dswt.fontRegistry=legacy

With these flags, the LegacySWTFontRegistry will be used.

If Dswt.fontRegistry is not set or Monitorscaling is enabled(current default behaviour) , SWT will default to using ScalingSWTFontRegistry.

github-actions · 2025-06-06T12:40:00Z

Test Results

545 files ±0 545 suites ±0 31m 41s ⏱️ + 1m 9s
4 403 tests +1 4 385 ✅ +1 18 💤 ±0 0 ❌ ±0
16 733 runs +1 16 593 ✅ +1 140 💤 ±0 0 ❌ ±0

Results for commit 432847b. ± Comparison against base commit de1f3ef.

This pull request removes 6 and adds 7 tests. Note that renamed tests count towards both.

org.eclipse.swt.internal.DefaultSWTFontRegistryTests ‑ fontsAlwaysDependOnPrimaryZoom
org.eclipse.swt.internal.DefaultSWTFontRegistryTests ‑ fontsAreCached
org.eclipse.swt.internal.DefaultSWTFontRegistryTests ‑ systemFontsAlwaysDependOnPrimaryZoom
org.eclipse.swt.internal.DefaultSWTFontRegistryTests ‑ systemFontsAreCached
org.eclipse.swt.widgets.ControlWin32Tests ‑ testDoNotScaleFontCorrectlyInNoAutoScaleSzenario
org.eclipse.swt.widgets.ControlWin32Tests ‑ testScaleFontCorrectlyInAutoScaleSzenario

org.eclipse.swt.internal.LegacySWTFontRegistryTests ‑ fontsAlwaysDependOnPrimaryZoom
org.eclipse.swt.internal.LegacySWTFontRegistryTests ‑ fontsAreCached
org.eclipse.swt.internal.LegacySWTFontRegistryTests ‑ systemFontsAlwaysDependOnPrimaryZoom
org.eclipse.swt.internal.LegacySWTFontRegistryTests ‑ systemFontsAreCached
org.eclipse.swt.widgets.ControlWin32Tests ‑ testDoNotScaleFontInNoAutoScaleScenarioWithLegacyFontRegistry
org.eclipse.swt.widgets.ControlWin32Tests ‑ testScaleFontCorrectlyInAutoScaleScenario
org.eclipse.swt.widgets.ControlWin32Tests ‑ testScaleFontCorrectlyInNoAutoScaleScenario

♻️ This comment has been updated with latest results.

arunjose696 · 2025-06-12T08:30:37Z

bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/internal/ScalingSWTFontRegistry.java

 		private Font createAndCacheFont(int zoom) {
 			Font newFont = createFont(zoom);
-			customFontHandlesKeyMap.put(Font.win32_getHandle(newFont), this);
+			FontData clonedFontData = new FontData(newFont.getFontData()[0].toString());


cloning FontData should be replaced with new constructors if #2128 gets merged

This part is actually a fix for an issue independent of the refactoring. I would extract that one out + a separate test the checks that

Raised a new PR for this #2237 , Not removing the changes in this PR as the CI runs would fail for this

bundles/org.eclipse.swt/Eclipse SWT Tests/win32/org/eclipse/swt/widgets/ControlWin32Tests.java

bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/internal/LegacySWTFontRegistry.java

bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/internal/SWTFontProvider.java

akoch-yatta · 2025-06-13T08:16:35Z

bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/internal/ScalingSWTFontRegistry.java

 		private Font createAndCacheFont(int zoom) {
 			Font newFont = createFont(zoom);
-			customFontHandlesKeyMap.put(Font.win32_getHandle(newFont), this);
+			FontData clonedFontData = new FontData(newFont.getFontData()[0].toString());


This part is actually a fix for an issue independent of the refactoring. I would extract that one out + a separate test the checks that

fedejeanne

Just some minor comments

bundles/org.eclipse.swt/Eclipse SWT Tests/win32/org/eclipse/swt/widgets/ControlWin32Tests.java

bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/internal/SWTFontProvider.java

bundles/org.eclipse.swt/Eclipse SWT Tests/win32/org/eclipse/swt/widgets/ControlWin32Tests.java

fedejeanne

@arunjose696 please add the necessary @SuppressWarnings("removal") so that the checks do not fail

@SuppressWarnings("removal") missing.

1)Always use ScalingSWTFontRegistry 2)Rename DefaultSWTFontRegistry -> LegacySWTFontRegistry as this is not default anymore 3)Add a system property to use LegacySWTFontRegistry without monitor specific scaling active 4)Deprecate the LegacySWTFontRegistry(DefaultSWTFontRegistry)

arunjose696 · 2025-06-23T15:33:05Z

@arunjose696 please add the necessary @SuppressWarnings("removal") so that the checks do not fail

Have added SuppressWarnings, to usages of deprecated class(LegacySWTFontRegistry), still see the warnings, @fedejeanne is it required to add annotations elsewhere?

fedejeanne · 2025-06-24T06:29:16Z

Yes, you missed one spot: bundles/org.eclipse.swt/Eclipse SWT Tests/win32/org/eclipse/swt/internal/LegacySWTFontRegistryTests.java

You can find the new warnings when clicking in the failing check:

Then look for the new issues (they are at the top of the list):

fedejeanne

I stand corrected, the failures are unrelated and you already added the necessary @SuppressWarnings

akurtakov · 2025-06-25T08:59:09Z

This has caused warnings in I-build e.g. https://download.eclipse.org/eclipse/downloads/drops4/I20250625-0320/compilelogs/plugins/org.eclipse.swt.win32.win32.aarch64_3.131.0.v20250624-0912/test.html

arunjose696 · 2025-06-25T12:28:15Z

This has caused warnings in I-build e.g. https://download.eclipse.org/eclipse/downloads/drops4/I20250625-0320/compilelogs/plugins/org.eclipse.swt.win32.win32.aarch64_3.131.0.v20250624-0912/test.html

This seems to be a false positive, have added necessary @SuppressWarnings for this function, dont see any warnings appearing in IDE as well.

fedejeanne · 2025-06-26T08:45:14Z

It just occurs to me: could it be that the check is complaining about the deprecation without removal?

The IDE is smart enough to know that suppressing the "removal" notices also suppresses the "deprecation" notices, but I don't know about the check.

arunjose696 force-pushed the arunjose696/312 branch from e0cf8ee to 99f0ab8 Compare June 6, 2025 12:31

arunjose696 linked an issue Jun 6, 2025 that may be closed by this pull request

Completely switch to ScalingSWTFontRegistry vi-eclipse/Eclipse-Platform#312

Closed

arunjose696 force-pushed the arunjose696/312 branch 2 times, most recently from 3241eaf to aeccc6c Compare June 6, 2025 12:54

arunjose696 marked this pull request as draft June 6, 2025 12:56

arunjose696 force-pushed the arunjose696/312 branch 10 times, most recently from c3ea0da to 1553d4d Compare June 12, 2025 08:28

arunjose696 commented Jun 12, 2025

View reviewed changes

arunjose696 marked this pull request as ready for review June 12, 2025 08:39

arunjose696 force-pushed the arunjose696/312 branch from 1553d4d to e84b5b4 Compare June 12, 2025 08:53

akoch-yatta reviewed Jun 13, 2025

View reviewed changes

arunjose696 force-pushed the arunjose696/312 branch 3 times, most recently from 7e3ee0c to 99b63f0 Compare June 17, 2025 11:59

fedejeanne reviewed Jun 23, 2025

View reviewed changes

arunjose696 force-pushed the arunjose696/312 branch from 99b63f0 to df39b43 Compare June 23, 2025 11:24

fedejeanne previously approved these changes Jun 23, 2025

View reviewed changes

fedejeanne reviewed Jun 23, 2025

View reviewed changes

arunjose696 force-pushed the arunjose696/312 branch from df39b43 to 432847b Compare June 23, 2025 14:25

fedejeanne approved these changes Jun 24, 2025

View reviewed changes

fedejeanne merged commit a413b4b into eclipse-platform:master Jun 24, 2025
12 of 17 checks passed

fedejeanne deleted the arunjose696/312 branch June 24, 2025 09:12

arunjose696 mentioned this pull request Aug 6, 2025

Font size of Line number ruler is different when using swt.autoScale=int200 #2385

Closed

HeikoKlare mentioned this pull request Aug 11, 2025

[Win32] Wrong line height in StyledText when deviceZoom > nativeZoom #2405

Closed

Deprecate DefaultSWTFontRegistry and Completely switch to ScalingSWTFontRegistry #2212

Deprecate DefaultSWTFontRegistry and Completely switch to ScalingSWTFontRegistry #2212

Uh oh!

Conversation

arunjose696 commented Jun 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Jun 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results

Uh oh!

arunjose696 Jun 12, 2025

Choose a reason for hiding this comment

Uh oh!

akoch-yatta Jun 13, 2025

Choose a reason for hiding this comment

Uh oh!

arunjose696 Jun 16, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

akoch-yatta Jun 13, 2025

Choose a reason for hiding this comment

Uh oh!

fedejeanne left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fedejeanne left a comment

Choose a reason for hiding this comment

Uh oh!

arunjose696 commented Jun 23, 2025

Uh oh!

fedejeanne commented Jun 24, 2025

Uh oh!

fedejeanne left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

akurtakov commented Jun 25, 2025

Uh oh!

arunjose696 commented Jun 25, 2025

Uh oh!

fedejeanne commented Jun 26, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

arunjose696 commented Jun 6, 2025 •

edited

Loading

github-actions bot commented Jun 6, 2025 •

edited

Loading