Skip to content

Commit

Permalink
MAJOR BUG FIX - We were not loading font metrics when loading a font …
Browse files Browse the repository at this point in the history
…via the builder.

Instead we were silently using the font metrics of the fallback font. This would often produce acceptable results and thus was not discovered until now. The bad part of this fix is that all the text visual regression tests are now failing as they were affected by using the wrong font metrics and are therefore a pixel or so out.

IMPORTANT: This fix needs to be merged with the main branch regardless of whether this branch itself is merged in entirety.
  • Loading branch information
danfickle committed Jan 12, 2019
1 parent 34cb500 commit a145329
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ public PdfBoxFontResolver(SharedContext sharedContext, PDDocument doc, FSCacheEx
_pdfUaConform = pdfUaConform;

// All fonts are required to be embedded in PDF/A documents, so we don't add the built-in fonts, if conformance is required.
_fontFamilies = (_pdfAConformance == PdfAConformance.NONE) ? createInitialFontMap() : new HashMap<String, FontFamily<FontDescription>>();
_fontFamilies = (_pdfAConformance == PdfAConformance.NONE && !pdfUaConform) ? createInitialFontMap() : new HashMap<String, FontFamily<FontDescription>>();
}

@Override
Expand Down Expand Up @@ -721,13 +721,31 @@ private PdfBoxRawPDFontMetrics getFontMetricsFromCache(String family, int weight
private void putFontMetricsInCache(String family, int weight, IdentValue style, PdfBoxRawPDFontMetrics metrics) {
_metricsCache.put(createFontMetricsCacheKey(family, weight, style), metrics);
}

private boolean loadMetrics() {
try {
PDFontDescriptor descriptor = _font.getFontDescriptor();
_metrics = PdfBoxRawPDFontMetrics.fromPdfBox(_font, descriptor);
putFontMetricsInCache(_family, _weight, _style, _metrics);
return true;
} catch (IOException e) {
XRLog.exception(
"Couldn't load font. Please check that it is a valid truetype font.");
return false;
}
}

private boolean realizeFont() {
if (_font == null && _fontSupplier != null) {
XRLog.load(Level.INFO, "Loading font(" + _family + ") from PDFont supplier now.");

_font = _fontSupplier.supply();
_fontSupplier = null;

if (!isMetricsAvailable()) {
// If we already have metrics, they must have come from the cache.
return loadMetrics();
}
}

if (_font == null && _supplier != null) {
Expand All @@ -744,10 +762,7 @@ private boolean realizeFont() {
_font = PDType0Font.load(_doc, is, _isSubset);

if (!isMetricsAvailable()) {
// If we already have metrics, they must have come from the cache.
PDFontDescriptor descriptor = _font.getFontDescriptor();
_metrics = PdfBoxRawPDFontMetrics.fromPdfBox(_font, descriptor);
putFontMetricsInCache(_family, _weight, _style, _metrics);
return loadMetrics();
}
} catch (IOException e) {
XRLog.exception("Couldn't load font. Please check that it is a valid truetype font.");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ public FSFontMetrics getFSFontMetrics(FontContext context, FSFont font, String s
PdfBoxRawPDFontMetrics metrics = des.getFontMetrics();

if (metrics == null) {
XRLog.exception("Font metrics not available. Probably a bug.");
continue;
}

Expand Down

0 comments on commit a145329

Please sign in to comment.