Skip to content

Commit 2e3220a

Browse files
committed
Reuse the main display's font across displays when already available
Now that each display keeps its own font cache, a symbolic name requested from several displays ends up with a separate native font handle per display, even for names that never vary between displays, such as the JFace default font. Prefer a font already realized on the registry's main display whenever it has the exact style requested; only when it does not does the requesting display realize and cache one of its own. This keeps resource usage close to what a single shared cache would give, while remaining correct per display. Callers without a display of their own, which so far could only fall back to the default font, reuse any already realized font the same way. Handing a font owned by the main display to another display is safe because the main display outlives every other display the registry is used from, so the font cannot be disposed while another display still uses it. The flip side is that the font returned for a symbolic name is no longer necessarily owned by the caller's display. A display that had to realize a font itself keeps getting that one, so repeated lookups do not silently switch to a different instance once the main display catches up. That costs nothing, since such a copy only exists where it had to be created anyway. Reusing only a record that already carries the requested style also keeps the lazy creation of styled fonts confined to the display owning the record, so it needs no synchronization of its own. Since all of this depends on which style is being asked for, the style is now known while a record is looked up, rather than being applied to the record only afterwards. Tests cover reuse when the style is already available on the main display, falling back to a display's own font when it is not, and reuse starting once the main display realizes the style later. Assisted-by: Claude Opus 5 <noreply@anthropic.com>
1 parent 64e522b commit 2e3220a

2 files changed

Lines changed: 206 additions & 35 deletions

File tree

bundles/org.eclipse.jface/src/org/eclipse/jface/resource/FontRegistry.java

Lines changed: 108 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,10 @@
6464
@NoExtend
6565
public class FontRegistry extends ResourceRegistry {
6666

67+
private enum FontStyle {
68+
NORMAL, BOLD, ITALIC
69+
}
70+
6771
/**
6872
* FontRecord is a private helper class that holds onto a font
6973
* and can be used to generate its bold and italic version.
@@ -109,16 +113,29 @@ void dispose() {
109113
* Return the base Font.
110114
* @return Font
111115
*/
112-
public Font getBaseFont() {
116+
private Font getBaseFont() {
113117
return baseFont;
114118
}
115119

120+
/**
121+
* Return the font for the given style, creating it lazily if necessary.
122+
* @param style the requested style
123+
* @return the font
124+
*/
125+
public Font get(FontStyle style) {
126+
return switch (style) {
127+
case NORMAL -> getBaseFont();
128+
case BOLD -> getBoldFont();
129+
case ITALIC -> getItalicFont();
130+
};
131+
}
132+
116133
/**
117134
* Return the bold Font. Create a bold version
118135
* of the base font to get it.
119136
* @return Font
120137
*/
121-
public Font getBoldFont() {
138+
private Font getBoldFont() {
122139
if (boldFont != null) {
123140
return boldFont;
124141
}
@@ -132,6 +149,20 @@ public Font getBoldFont() {
132149
return boldFont;
133150
}
134151

152+
/**
153+
* Returns whether the given style has already been realized for this
154+
* record.
155+
* @param style the style to check
156+
* @return whether the given style is already available
157+
*/
158+
public boolean has(FontStyle style) {
159+
return switch (style) {
160+
case NORMAL -> baseFont != null;
161+
case BOLD -> boldFont != null;
162+
case ITALIC -> italicFont != null;
163+
};
164+
}
165+
135166
/**
136167
* Get a version of the base font data with the specified
137168
* style.
@@ -158,7 +189,7 @@ private FontData[] getModifiedFontData(int style) {
158189
* base font to get it.
159190
* @return Font
160191
*/
161-
public Font getItalicFont() {
192+
private Font getItalicFont() {
162193
if (italicFont != null) {
163194
return italicFont;
164195
}
@@ -236,7 +267,7 @@ void invalidate(String symbolicName) {
236267
return;
237268
}
238269
FontRecord defaultRecord = records.get(JFaceResources.DEFAULT_FONT);
239-
Font defaultFont = defaultRecord != null ? defaultRecord.getBaseFont() : null;
270+
Font defaultFont = defaultRecord != null ? defaultRecord.get(FontStyle.NORMAL) : null;
240271
replacedRecord.getAllocatedFonts().stream().filter(font -> font != defaultFont).forEach(staleFonts::add);
241272
}
242273

@@ -605,7 +636,16 @@ Font calculateDefaultFont() {
605636
* @return Font
606637
*/
607638
public Font defaultFont() {
608-
return defaultFontRecord().getBaseFont();
639+
return defaultFont(FontStyle.NORMAL);
640+
}
641+
642+
/**
643+
* Return the default font in the given style, creating it if necessary.
644+
* @param style the requested style
645+
* @return the font
646+
*/
647+
private Font defaultFont(FontStyle style) {
648+
return defaultFontRecord(style).get(style);
609649
}
610650

611651
/**
@@ -628,14 +668,12 @@ public FontDescriptor getDescriptor(String symbolicName) {
628668
/**
629669
* Returns the default font record.
630670
*/
631-
private FontRecord defaultFontRecord() {
632-
FontRecord record = getExistingFontRecord(JFaceResources.DEFAULT_FONT);
671+
private FontRecord defaultFontRecord(FontStyle style) {
672+
FontRecord record = getExistingFontRecord(JFaceResources.DEFAULT_FONT, style);
633673
if (record == null && Display.getCurrent() == null) {
634-
// No display for the current thread, so there is none to scope the
635-
// lookup to and none to create a font on. Fall back to the default
636-
// font already realized on the main display, which outlives every
637-
// other display, rather than fail outright. Reached e.g. from
638-
// getFontRecord()'s non-UI-thread fallback.
674+
// no display to scope the lookup to and none to create a font on: use the
675+
// main display's record even if the requested style is not realized there
676+
// yet, so the style gets realized on that display rather than failing
639677
DisplayFontRecords mainDisplayRecords = displayToFontRecords.get(mainDisplay);
640678
if (mainDisplayRecords != null) {
641679
record = mainDisplayRecords.get(JFaceResources.DEFAULT_FONT);
@@ -659,24 +697,61 @@ record = createFont(JFaceResources.DEFAULT_FONT, defaultFont.getFontData());
659697
}
660698

661699
/**
662-
* Looks up an already-realized font record for the given symbolic name on
663-
* the current display. Returns <code>null</code> if there is none, in which
664-
* case the caller is responsible for creating one on the current display.
700+
* Looks up an already-realized font record for the given symbolic name and
701+
* style: the main display's record is preferred whenever it already has the
702+
* exact style requested (so callers from any display, including a thread
703+
* with no Display of its own, reuse it instead of allocating a duplicate),
704+
* otherwise the current display's own record (if any) is used. Returns
705+
* <code>null</code> if neither has a matching record, in which case the
706+
* caller is responsible for creating one on the current display.
707+
* <p>
708+
* Handing out the main display's font to another display is safe because
709+
* the main display is assumed to outlive every other display the registry
710+
* is used from, so the font cannot be disposed while another display is
711+
* still using it.
712+
* </p>
713+
* <p>
714+
* Requiring the exact style to be present is also what keeps the lazy
715+
* creation of styled fonts single-threaded: a record is only ever handed to
716+
* a foreign thread once the style it asks for has been realized, so only
717+
* the record's own display ever reaches the creating branch of
718+
* {@link FontRecord#get(FontStyle)}. Note that this means the main
719+
* display's record always wins for {@link FontStyle#NORMAL}, whose font is
720+
* realized when the record is created.
721+
* </p>
665722
*/
666-
private FontRecord getExistingFontRecord(String symbolicName) {
723+
private FontRecord getExistingFontRecord(String symbolicName, FontStyle style) {
724+
FontRecord recordOnCurrentDisplay = null;
667725
Display currentDisplay = Display.getCurrent();
668-
if (currentDisplay == null) {
669-
return null;
726+
if (currentDisplay != null) {
727+
DisplayFontRecords currentDisplayRecords = displayToFontRecords.get(currentDisplay);
728+
if (currentDisplayRecords != null) {
729+
recordOnCurrentDisplay = currentDisplayRecords.get(symbolicName);
730+
// a style this display already realized itself stays the one it gets, so
731+
// repeated lookups keep returning the same font instance
732+
if (recordOnCurrentDisplay != null && recordOnCurrentDisplay.has(style)) {
733+
return recordOnCurrentDisplay;
734+
}
735+
}
670736
}
671-
DisplayFontRecords currentDisplayRecords = displayToFontRecords.get(currentDisplay);
672-
return currentDisplayRecords != null ? currentDisplayRecords.get(symbolicName) : null;
737+
738+
DisplayFontRecords mainDisplayRecords = displayToFontRecords.get(mainDisplay);
739+
if (mainDisplayRecords != null) {
740+
FontRecord recordOnMainDisplay = mainDisplayRecords.get(symbolicName);
741+
// Only return main display record if exact font style already exists
742+
if (recordOnMainDisplay != null && recordOnMainDisplay.has(style)) {
743+
return recordOnMainDisplay;
744+
}
745+
}
746+
747+
return recordOnCurrentDisplay;
673748
}
674749

675750
/**
676751
* Returns the default font data. Creates it if necessary.
677752
*/
678753
private FontData[] defaultFontData() {
679-
return defaultFontRecord().baseData;
754+
return defaultFontRecord(FontStyle.NORMAL).baseData;
680755
}
681756

682757
/**
@@ -713,8 +788,7 @@ public FontData[] getFontData(String symbolicName) {
713788
* @return the font
714789
*/
715790
public Font get(String symbolicName) {
716-
717-
return getFontRecord(symbolicName).getBaseFont();
791+
return getFont(symbolicName, FontStyle.NORMAL);
718792
}
719793

720794
/**
@@ -733,8 +807,7 @@ public Font get(String symbolicName) {
733807
* @since 3.0
734808
*/
735809
public Font getBold(String symbolicName) {
736-
737-
return getFontRecord(symbolicName).getBoldFont();
810+
return getFont(symbolicName, FontStyle.BOLD);
738811
}
739812

740813
/**
@@ -753,41 +826,41 @@ public Font getBold(String symbolicName) {
753826
* @since 3.0
754827
*/
755828
public Font getItalic(String symbolicName) {
756-
757-
return getFontRecord(symbolicName).getItalicFont();
829+
return getFont(symbolicName, FontStyle.ITALIC);
758830
}
759831

760832
/**
761-
* Return the font record for the key.
833+
* Return the font for the given key and style.
762834
* @param symbolicName The key for the record.
763-
* @return FontRecord
835+
* @param style the requested style
836+
* @return the font
764837
*/
765-
private FontRecord getFontRecord(String symbolicName) {
838+
private Font getFont(String symbolicName, FontStyle style) {
766839
Assert.isNotNull(symbolicName);
767-
FontRecord existingRecord = getExistingFontRecord(symbolicName);
840+
FontRecord existingRecord = getExistingFontRecord(symbolicName, style);
768841
if (existingRecord != null) {
769-
return existingRecord;
842+
return existingRecord.get(style);
770843
}
771844

772845
FontData[] existingFontData = stringToFontData.get(symbolicName);
773846

774847
FontRecord fontRecord;
775848

776849
if (existingFontData == null) {
777-
fontRecord = defaultFontRecord();
850+
fontRecord = defaultFontRecord(style);
778851
} else {
779852
fontRecord = createFont(symbolicName, existingFontData);
780853
}
781854

782855
if (fontRecord == null) {
783-
fontRecord = defaultFontRecord();
856+
fontRecord = defaultFontRecord(style);
784857
if (Display.getCurrent() == null) { // log error but don't throw an exception to preserve existing functionality
785858
String msg = "Unable to create font \"" + symbolicName + "\" in a non-UI thread. Using default font instead."; //$NON-NLS-1$ //$NON-NLS-2$
786859
Policy.logException(new SWTException(msg));
787860
}
788861
}
789862

790-
return fontRecord;
863+
return fontRecord.get(style);
791864
}
792865

793866
@Override

tests/org.eclipse.jface.tests/src/org/eclipse/jface/tests/resources/FontRegistryTest.java

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,101 @@ public void multipleDisplayDispose_italicFont() {
9797
testMultipleDisplayDispose(() -> fontRegistry.getItalic(JFaceResources.DEFAULT_FONT));
9898
}
9999

100+
@Test
101+
public void multipleDisplay_reusesMainDisplayFont_whenStyleAlreadyCached() {
102+
assumeTrue(OS.isWindows(), "multiple Display instance only allowed on Windows");
103+
104+
FontRegistry fontRegistry = new FontRegistry();
105+
fontRegistry.put("myfont", new FontData[] { new FontData("Arial", 12, SWT.NORMAL) });
106+
Font mainFont = fontRegistry.get("myfont");
107+
108+
Display secondDisplay = initializeDisplayInSeparateThread();
109+
try {
110+
Font fontFromSecondDisplayThread = secondDisplay.syncCall(() -> fontRegistry.get("myfont"));
111+
assertEquals(mainFont, fontFromSecondDisplayThread,
112+
"a font already realized on the main display should be reused from any other display's thread");
113+
assertEquals(Display.getCurrent(), fontFromSecondDisplayThread.getDevice(),
114+
"the reused font is still owned by the main display");
115+
} finally {
116+
secondDisplay.syncExec(secondDisplay::dispose);
117+
}
118+
}
119+
120+
@Test
121+
public void multipleDisplay_createsOwnFont_whenStyleNotYetCachedOnMainDisplay() {
122+
assumeTrue(OS.isWindows(), "multiple Display instance only allowed on Windows");
123+
124+
FontRegistry fontRegistry = new FontRegistry();
125+
fontRegistry.put("myfont", new FontData[] { new FontData("Arial", 12, SWT.NORMAL) });
126+
fontRegistry.get("myfont"); // only realizes the NORMAL style on the main display
127+
128+
Display secondDisplay = initializeDisplayInSeparateThread();
129+
Font boldFontOnSecondDisplay;
130+
try {
131+
boldFontOnSecondDisplay = secondDisplay.syncCall(() -> fontRegistry.getBold("myfont"));
132+
133+
assertEquals(secondDisplay, boldFontOnSecondDisplay.getDevice(),
134+
"bold style is not yet cached on the main display, so it must be created on the requesting display");
135+
} finally {
136+
secondDisplay.syncExec(secondDisplay::dispose);
137+
}
138+
assertTrue(boldFontOnSecondDisplay.isDisposed(),
139+
"fonts created for the second display must be disposed together with it");
140+
}
141+
142+
@Test
143+
public void multipleDisplay_reusesMainDisplayFont_onceStyleBecomesCachedThere() {
144+
assumeTrue(OS.isWindows(), "multiple Display instance only allowed on Windows");
145+
146+
FontRegistry fontRegistry = new FontRegistry();
147+
fontRegistry.put("myfont", new FontData[] { new FontData("Arial", 12, SWT.NORMAL) });
148+
fontRegistry.get("myfont"); // only realizes the NORMAL style on the main display
149+
150+
Display firstSecondDisplay = initializeDisplayInSeparateThread();
151+
Font boldFontOnFirstSecondDisplay = firstSecondDisplay.syncCall(() -> fontRegistry.getBold("myfont"));
152+
assertEquals(firstSecondDisplay, boldFontOnFirstSecondDisplay.getDevice(),
153+
"bold style is not yet cached on the main display, so it must be created on the requesting display");
154+
firstSecondDisplay.syncExec(firstSecondDisplay::dispose);
155+
156+
// the main display now also realizes the bold style
157+
Font boldFontOnMainDisplay = fontRegistry.getBold("myfont");
158+
159+
Display secondSecondDisplay = initializeDisplayInSeparateThread();
160+
try {
161+
Font boldFontOnSecondSecondDisplay = secondSecondDisplay.syncCall(() -> fontRegistry.getBold("myfont"));
162+
assertEquals(boldFontOnMainDisplay, boldFontOnSecondSecondDisplay,
163+
"once the main display has realized the requested style, later lookups from any display must reuse it");
164+
} finally {
165+
secondSecondDisplay.syncExec(secondSecondDisplay::dispose);
166+
}
167+
}
168+
169+
@Test
170+
public void multipleDisplay_keepsOwnFont_whenMainDisplayRealizesItLater() {
171+
assumeTrue(OS.isWindows(), "multiple Display instance only allowed on Windows");
172+
173+
FontRegistry fontRegistry = new FontRegistry();
174+
fontRegistry.put("myfont", new FontData[] { new FontData("Arial", 12, SWT.NORMAL) });
175+
176+
Display secondDisplay = initializeDisplayInSeparateThread();
177+
try {
178+
// the second display realizes the font before the main display has one to reuse
179+
Font fontOnSecondDisplay = secondDisplay.syncCall(() -> fontRegistry.get("myfont"));
180+
assertEquals(secondDisplay, fontOnSecondDisplay.getDevice(),
181+
"nothing to reuse yet, so the second display must realize a font of its own");
182+
183+
// the main display realizing the same font afterwards must not change what
184+
// the second display gets, or it would silently switch instances mid-flight
185+
fontRegistry.get("myfont");
186+
187+
Font fontOnSecondDisplayAgain = secondDisplay.syncCall(() -> fontRegistry.get("myfont"));
188+
assertSame(fontOnSecondDisplay, fontOnSecondDisplayAgain,
189+
"a display that realized a font itself must keep getting that same instance");
190+
} finally {
191+
secondDisplay.syncExec(secondDisplay::dispose);
192+
}
193+
}
194+
100195
@Test
101196
public void put_invalidatesCachedFont_onAllDisplays() {
102197
assumeTrue(OS.isWindows(), "multiple Display instance only allowed on Windows");
@@ -160,6 +255,9 @@ private static void testMultipleDisplayDispose(Supplier<Font> fontSupplier) {
160255
assumeTrue(OS.isWindows(), "multiple Display instance only allowed on Windows");
161256

162257
Display secondDisplay = initializeDisplayInSeparateThread();
258+
// the second display is asked first on purpose: the requested font is not
259+
// realized on the main display yet, so there is nothing to reuse and the
260+
// second display has to realize (and own) a font of its own
163261
Font fontOnSecondDisplay = secondDisplay.syncCall(fontSupplier::get);
164262

165263
Font fontOnThisDisplayBeforeSecondDisplayDispose = fontSupplier.get();

0 commit comments

Comments
 (0)