Skip to content

Conversation

arunjose696
Copy link
Contributor

Previously, system fonts were not cached during their creation in ScalingSWTFontRegistry. This change ensures that system fonts, are now cached in fontsKeyMap

Copy link
Contributor

github-actions bot commented Jun 16, 2025

Test Results

   545 files  + 6     545 suites  +6   27m 42s ⏱️ - 3m 34s
 4 401 tests +38   4 383 ✅ +36   18 💤 +3  0 ❌  - 1 
16 728 runs  +38  16 588 ✅ +36  140 💤 +3  0 ❌  - 1 

Results for commit 0b25f46. ± Comparison against base commit d36f616.

♻️ This comment has been updated with latest results.

Copy link
Contributor

@HeikoKlare HeikoKlare left a comment

Choose a reason for hiding this comment

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

Overall, the change looks fine to me. The FontData copy can now be improved with the just added copy constructor.

Still I would like to have @akoch-yatta have another at look at this change as it affects quite central, crucial code.

@arunjose696 arunjose696 force-pushed the arunjose696/cacheFontData branch from 10cbd46 to 83723da Compare June 17, 2025 07:09
Previously, system fonts were not cached during their creation in
ScalingSWTFontRegistry. This change ensures that system fonts, are now cached in fontsKeyMap
@HeikoKlare HeikoKlare force-pushed the arunjose696/cacheFontData branch from 83723da to 0b25f46 Compare June 17, 2025 10:48
@HeikoKlare
Copy link
Contributor

Taking another look, I understood again that this is really only about adding the system font to the cache based on font data.
I tested around with the change, also at different zooms. Since it affects the system font, issues with it should be easily perceivable.

Thus I merge this to proceed with the work based on top of it. Still would be nice if you can double-check once you are back, @akoch-yatta.

@HeikoKlare HeikoKlare merged commit 8c7b933 into eclipse-platform:master Jun 17, 2025
17 checks passed
@HeikoKlare HeikoKlare deleted the arunjose696/cacheFontData branch June 17, 2025 11:09
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.

2 participants