null
*/
- private FontRecord defaultFontRecord() {
- FontRecord record = getExistingFontRecord(JFaceResources.DEFAULT_FONT);
+ private FontRecord defaultFontRecord(FontStyle style) {
+ FontRecord record = getExistingFontRecord(JFaceResources.DEFAULT_FONT, style);
if (record == null && Display.getCurrent() == null) {
- // No display for the current thread, so there is none to scope the
- // lookup to and none to create a font on. Fall back to the default
- // font already realized on the main display, which outlives every
- // other display, rather than fail outright. Reached e.g. from
- // getFontRecord()'s non-UI-thread fallback.
+ // no display to scope the lookup to and none to create a font on: use the
+ // main display's record even if the requested style is not realized there
+ // yet, so the style gets realized on that display rather than failing
DisplayFontRecords mainDisplayRecords = displayToFontRecords.get(mainDisplay);
if (mainDisplayRecords != null) {
record = mainDisplayRecords.get(JFaceResources.DEFAULT_FONT);
@@ -663,24 +707,65 @@ record = createFont(JFaceResources.DEFAULT_FONT, defaultFont.getFontData());
}
/**
- * Looks up an already-realized font record for the given symbolic name on
- * the current display. Returns null if there is none, in which
- * case the caller is responsible for creating one on the current display.
+ * Looks up an already-realized font record for the given symbolic name and
+ * style. A record the current display has already realized the exact style
+ * on is used first, so repeated lookups from that display keep returning the
+ * same font. Otherwise the main display's record is used if it already has
+ * that style, so callers from any display, including a thread with no
+ * Display of its own, reuse it instead of allocating a duplicate. Failing
+ * both, the current display's own record is returned even though it lacks
+ * the requested style, so that the style gets realized on the display owning
+ * the record; null is returned only if the current display has
+ * no record for the name at all, in which case the caller is responsible for
+ * creating one on it.
+ * + * Handing out the main display's font to another display is safe because + * the main display is assumed to outlive every other display the registry + * is used from, so the font cannot be disposed while another display is + * still using it. + *
+ *+ * Requiring the exact style to be present is also what keeps the lazy + * creation of styled fonts single-threaded: a record is only ever handed to + * a foreign display once the style it asks for has been realized, so only + * the record's own display ever reaches the creating branch of + * {@link FontRecord#get(FontStyle)}. Note that for {@link FontStyle#NORMAL} + * every existing record matches, since that font is realized when the record + * is created. + *
*/ - private FontRecord getExistingFontRecord(String symbolicName) { + private FontRecord getExistingFontRecord(String symbolicName, FontStyle style) { + FontRecord recordOnCurrentDisplay = null; Display currentDisplay = Display.getCurrent(); - if (currentDisplay == null) { - return null; + if (currentDisplay != null) { + DisplayFontRecords currentDisplayRecords = displayToFontRecords.get(currentDisplay); + if (currentDisplayRecords != null) { + recordOnCurrentDisplay = currentDisplayRecords.get(symbolicName); + // a style this display already realized itself stays the one it gets, so + // repeated lookups keep returning the same font instance + if (recordOnCurrentDisplay != null && recordOnCurrentDisplay.has(style)) { + return recordOnCurrentDisplay; + } + } } - DisplayFontRecords currentDisplayRecords = displayToFontRecords.get(currentDisplay); - return currentDisplayRecords != null ? currentDisplayRecords.get(symbolicName) : null; + + DisplayFontRecords mainDisplayRecords = displayToFontRecords.get(mainDisplay); + if (mainDisplayRecords != null) { + FontRecord recordOnMainDisplay = mainDisplayRecords.get(symbolicName); + // Only return main display record if exact font style already exists + if (recordOnMainDisplay != null && recordOnMainDisplay.has(style)) { + return recordOnMainDisplay; + } + } + + return recordOnCurrentDisplay; } /** * Returns the default font data. Creates it if necessary. */ private FontData[] defaultFontData() { - return defaultFontRecord().baseData; + return defaultFontRecord(FontStyle.NORMAL).baseData; } /** @@ -717,8 +802,7 @@ public FontData[] getFontData(String symbolicName) { * @return the font */ public Font get(String symbolicName) { - - return getFontRecord(symbolicName).getBaseFont(); + return getFont(symbolicName, FontStyle.NORMAL); } /** @@ -737,8 +821,7 @@ public Font get(String symbolicName) { * @since 3.0 */ public Font getBold(String symbolicName) { - - return getFontRecord(symbolicName).getBoldFont(); + return getFont(symbolicName, FontStyle.BOLD); } /** @@ -757,20 +840,20 @@ public Font getBold(String symbolicName) { * @since 3.0 */ public Font getItalic(String symbolicName) { - - return getFontRecord(symbolicName).getItalicFont(); + return getFont(symbolicName, FontStyle.ITALIC); } /** - * Return the font record for the key. + * Return the font for the given key and style. * @param symbolicName The key for the record. - * @return FontRecord + * @param style the requested style + * @return the font */ - private FontRecord getFontRecord(String symbolicName) { + private Font getFont(String symbolicName, FontStyle style) { Assert.isNotNull(symbolicName); - FontRecord existingRecord = getExistingFontRecord(symbolicName); + FontRecord existingRecord = getExistingFontRecord(symbolicName, style); if (existingRecord != null) { - return existingRecord; + return existingRecord.get(style); } FontData[] existingFontData = stringToFontData.get(symbolicName); @@ -778,20 +861,20 @@ private FontRecord getFontRecord(String symbolicName) { FontRecord fontRecord; if (existingFontData == null) { - fontRecord = defaultFontRecord(); + fontRecord = defaultFontRecord(style); } else { fontRecord = createFont(symbolicName, existingFontData); } if (fontRecord == null) { - fontRecord = defaultFontRecord(); + fontRecord = defaultFontRecord(style); if (Display.getCurrent() == null) { // log error but don't throw an exception to preserve existing functionality String msg = "Unable to create font \"" + symbolicName + "\" in a non-UI thread. Using default font instead."; //$NON-NLS-1$ //$NON-NLS-2$ Policy.logException(new SWTException(msg)); } } - return fontRecord; + return fontRecord.get(style); } @Override diff --git a/tests/org.eclipse.jface.tests/src/org/eclipse/jface/tests/resources/FontRegistryTest.java b/tests/org.eclipse.jface.tests/src/org/eclipse/jface/tests/resources/FontRegistryTest.java index 70a0ceea1ad..bbb9ff0f257 100644 --- a/tests/org.eclipse.jface.tests/src/org/eclipse/jface/tests/resources/FontRegistryTest.java +++ b/tests/org.eclipse.jface.tests/src/org/eclipse/jface/tests/resources/FontRegistryTest.java @@ -97,6 +97,101 @@ public void multipleDisplayDispose_italicFont() { testMultipleDisplayDispose(() -> fontRegistry.getItalic(JFaceResources.DEFAULT_FONT)); } + @Test + public void multipleDisplay_reusesMainDisplayFont_whenStyleAlreadyCached() { + assumeTrue(OS.isWindows(), "multiple Display instance only allowed on Windows"); + + FontRegistry fontRegistry = new FontRegistry(); + fontRegistry.put("myfont", new FontData[] { new FontData("Arial", 12, SWT.NORMAL) }); + Font mainFont = fontRegistry.get("myfont"); + + Display secondDisplay = initializeDisplayInSeparateThread(); + try { + Font fontFromSecondDisplayThread = secondDisplay.syncCall(() -> fontRegistry.get("myfont")); + assertEquals(mainFont, fontFromSecondDisplayThread, + "a font already realized on the main display should be reused from any other display's thread"); + assertEquals(Display.getCurrent(), fontFromSecondDisplayThread.getDevice(), + "the reused font is still owned by the main display"); + } finally { + secondDisplay.syncExec(secondDisplay::dispose); + } + } + + @Test + public void multipleDisplay_createsOwnFont_whenStyleNotYetCachedOnMainDisplay() { + assumeTrue(OS.isWindows(), "multiple Display instance only allowed on Windows"); + + FontRegistry fontRegistry = new FontRegistry(); + fontRegistry.put("myfont", new FontData[] { new FontData("Arial", 12, SWT.NORMAL) }); + fontRegistry.get("myfont"); // only realizes the NORMAL style on the main display + + Display secondDisplay = initializeDisplayInSeparateThread(); + Font boldFontOnSecondDisplay; + try { + boldFontOnSecondDisplay = secondDisplay.syncCall(() -> fontRegistry.getBold("myfont")); + + assertEquals(secondDisplay, boldFontOnSecondDisplay.getDevice(), + "bold style is not yet cached on the main display, so it must be created on the requesting display"); + } finally { + secondDisplay.syncExec(secondDisplay::dispose); + } + assertTrue(boldFontOnSecondDisplay.isDisposed(), + "fonts created for the second display must be disposed together with it"); + } + + @Test + public void multipleDisplay_reusesMainDisplayFont_onceStyleBecomesCachedThere() { + assumeTrue(OS.isWindows(), "multiple Display instance only allowed on Windows"); + + FontRegistry fontRegistry = new FontRegistry(); + fontRegistry.put("myfont", new FontData[] { new FontData("Arial", 12, SWT.NORMAL) }); + fontRegistry.get("myfont"); // only realizes the NORMAL style on the main display + + Display firstSecondDisplay = initializeDisplayInSeparateThread(); + Font boldFontOnFirstSecondDisplay = firstSecondDisplay.syncCall(() -> fontRegistry.getBold("myfont")); + assertEquals(firstSecondDisplay, boldFontOnFirstSecondDisplay.getDevice(), + "bold style is not yet cached on the main display, so it must be created on the requesting display"); + firstSecondDisplay.syncExec(firstSecondDisplay::dispose); + + // the main display now also realizes the bold style + Font boldFontOnMainDisplay = fontRegistry.getBold("myfont"); + + Display secondSecondDisplay = initializeDisplayInSeparateThread(); + try { + Font boldFontOnSecondSecondDisplay = secondSecondDisplay.syncCall(() -> fontRegistry.getBold("myfont")); + assertEquals(boldFontOnMainDisplay, boldFontOnSecondSecondDisplay, + "once the main display has realized the requested style, later lookups from any display must reuse it"); + } finally { + secondSecondDisplay.syncExec(secondSecondDisplay::dispose); + } + } + + @Test + public void multipleDisplay_keepsOwnFont_whenMainDisplayRealizesItLater() { + assumeTrue(OS.isWindows(), "multiple Display instance only allowed on Windows"); + + FontRegistry fontRegistry = new FontRegistry(); + fontRegistry.put("myfont", new FontData[] { new FontData("Arial", 12, SWT.NORMAL) }); + + Display secondDisplay = initializeDisplayInSeparateThread(); + try { + // the second display realizes the font before the main display has one to reuse + Font fontOnSecondDisplay = secondDisplay.syncCall(() -> fontRegistry.get("myfont")); + assertEquals(secondDisplay, fontOnSecondDisplay.getDevice(), + "nothing to reuse yet, so the second display must realize a font of its own"); + + // the main display realizing the same font afterwards must not change what + // the second display gets, or it would silently switch instances mid-flight + fontRegistry.get("myfont"); + + Font fontOnSecondDisplayAgain = secondDisplay.syncCall(() -> fontRegistry.get("myfont")); + assertSame(fontOnSecondDisplay, fontOnSecondDisplayAgain, + "a display that realized a font itself must keep getting that same instance"); + } finally { + secondDisplay.syncExec(secondDisplay::dispose); + } + } + @Test public void put_invalidatesCachedFont_onAllDisplays() { assumeTrue(OS.isWindows(), "multiple Display instance only allowed on Windows"); @@ -160,6 +255,9 @@ private static void testMultipleDisplayDispose(Supplier fontSupplier) { assumeTrue(OS.isWindows(), "multiple Display instance only allowed on Windows"); Display secondDisplay = initializeDisplayInSeparateThread(); + // the second display is asked first on purpose: the requested font is not + // realized on the main display yet, so there is nothing to reuse and the + // second display has to realize (and own) a font of its own Font fontOnSecondDisplay = secondDisplay.syncCall(fontSupplier::get); Font fontOnThisDisplayBeforeSecondDisplayDispose = fontSupplier.get();