From 20b4f7c83d8b9cdff9c979b37d6699bb89e28ee3 Mon Sep 17 00:00:00 2001 From: DemchaAV Date: Tue, 9 Jun 2026 14:09:04 +0100 Subject: [PATCH 1/2] refactor(engine): resolve text line metrics through the Font contract The shared FontLibraryTextMeasurementSystem special-cased instanceof PdfFont to read real ascent/descent/leading; every other backend font fell back to a degraded lineHeight-only metric. That coupled the shared measurement system to engine.render.pdf.PdfFont and meant a new backend (e.g. a PPTX or canonical PDF backend) could obtain first-class line metrics only by editing shared code. Lift vertical metrics onto the backend-neutral Font seam: add default methods Font.lineMetrics(TextStyle) (degraded fallback) and Font.measurementCacheKey(TextStyle) (null = opt out of the process-wide cache), plus a neutral FontLineMetrics record in engine.font (kept out of engine.measurement to avoid a package cycle). PdfFont overrides both via its existing verticalMetrics. The measurement system now dispatches polymorphically with no instanceof and no PdfFont import, and the process-wide cache is namespaced by backend font type so distinct backends that share a cache key cannot collide. Binary-compatible (default methods only; japicmp green) and behaviour-neutral: PDF and Word produce identical metrics (existing suite unchanged), plus new polymorphism and cross-backend-isolation tests. engine.measurement no longer depends on engine.render.pdf. --- CHANGELOG.md | 16 +++ .../com/demcha/compose/engine/font/Font.java | 34 +++++ .../compose/engine/font/FontLineMetrics.java | 38 ++++++ .../FontLibraryTextMeasurementSystem.java | 62 ++++++---- .../compose/engine/render/pdf/PdfFont.java | 17 +++ .../FontLibraryTextMeasurementSystemTest.java | 116 ++++++++++++++++++ 6 files changed, 258 insertions(+), 25 deletions(-) create mode 100644 src/main/java/com/demcha/compose/engine/font/FontLineMetrics.java diff --git a/CHANGELOG.md b/CHANGELOG.md index 76df4809d..d71e250a1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -133,6 +133,22 @@ Open cycle — bug-fix / housekeeping. Entries land here as they merge. `engine.measurement`, `engine.font`, `engine.render`) are **not** deprecated. No public API or behaviour change. +### Internal + +- **Text-measurement line metrics resolve through the `Font` contract instead of a + PDF-specific fast path.** `FontLibraryTextMeasurementSystem` previously + special-cased `instanceof PdfFont` to obtain real ascent/descent/leading — every + other backend font fell back to a degraded `lineHeight`-only metric — which + coupled the shared measurement system to `engine.render.pdf.PdfFont` and meant a + new backend could get first-class metrics only by editing shared code. Vertical + metrics and the process-wide cache key now live on the backend-neutral `Font` + seam (`Font.lineMetrics(...)` + `Font.measurementCacheKey(...)`, both `default` + methods; new `FontLineMetrics` record), so a backend supplies first-class metrics + by overriding the contract and the shared measurement system no longer imports + `PdfFont`. Binary-compatible (default methods only; japicmp green) and + behaviour-neutral — PDF and Word produce identical metrics, covered by the + existing suite plus new polymorphism tests. + ### Tests / tooling - **Benchmark regression gate and measurement probe (benchmarks module, not part diff --git a/src/main/java/com/demcha/compose/engine/font/Font.java b/src/main/java/com/demcha/compose/engine/font/Font.java index a64a214df..41f7e583f 100644 --- a/src/main/java/com/demcha/compose/engine/font/Font.java +++ b/src/main/java/com/demcha/compose/engine/font/Font.java @@ -44,6 +44,40 @@ default T fontType(TextDecoration textDecoration) { double getCapHeight(TextStyle style); + /** + * Resolves backend-neutral vertical metrics (ascent, descent, leading) for the + * supplied style. + * + *

The shared text-measurement system calls this polymorphically, so a + * backend supplies first-class line metrics by overriding this method rather + * than being special-cased in shared measurement code. The default derives a + * degraded metric from {@link #getLineHeight} with zero descent and leading; + * backends with real font metrics (ascent/descent/leading) should override.

+ * + * @param style the resolved text style + * @return vertical metrics in document units + */ + default FontLineMetrics lineMetrics(TextStyle style) { + return new FontLineMetrics(Math.max(0.0, getLineHeight(style)), 0.0, 0.0); + } + + /** + * Returns a process-stable key identifying this font's metrics for the supplied + * style, or {@code null} to opt out of the shared process-wide line-metrics + * cache (per-session caching still applies). + * + *

Backends whose {@link #lineMetrics} computation is expensive — e.g. it + * reads font-descriptor tables — should return a stable non-null key so + * identical styles resolve once per process across sessions and threads. Cheap + * or stub backends can leave the default and skip the global cache.

+ * + * @param style the resolved text style + * @return a stable cache key, or {@code null} to skip the global cache + */ + default String measurementCacheKey(TextStyle style) { + return null; + } + double scale(double size); /** diff --git a/src/main/java/com/demcha/compose/engine/font/FontLineMetrics.java b/src/main/java/com/demcha/compose/engine/font/FontLineMetrics.java new file mode 100644 index 000000000..240f03ae7 --- /dev/null +++ b/src/main/java/com/demcha/compose/engine/font/FontLineMetrics.java @@ -0,0 +1,38 @@ +package com.demcha.compose.engine.font; + +/** + * Backend-neutral vertical text metrics for one resolved text style: the + * baseline-relative {@code ascent} and {@code descent} plus inter-line + * {@code leading}, all in document units. + * + *

This is the font-layer counterpart of + * {@code engine.measurement.TextMeasurementSystem.LineMetrics}. It lives in + * {@code engine.font} so the {@link Font} contract can expose vertical metrics + * without {@code engine.font} depending on {@code engine.measurement} (which + * already depends on {@code engine.font}); the measurement system converts this + * record into its own {@code LineMetrics} for layout consumers.

+ * + * @param ascent distance from the baseline to the glyph top + * @param descent distance from the baseline to the glyph bottom (non-negative) + * @param leading extra line leading applied by the backend font metrics + */ +public record FontLineMetrics(double ascent, double descent, double leading) { + + /** + * Returns the total baseline-to-baseline line height. + * + * @return {@code ascent + descent + leading} + */ + public double lineHeight() { + return ascent + descent + leading; + } + + /** + * Returns the distance from the line bottom to the text baseline. + * + * @return the baseline offset, equal to {@code descent} + */ + public double baselineOffsetFromBottom() { + return descent; + } +} diff --git a/src/main/java/com/demcha/compose/engine/measurement/FontLibraryTextMeasurementSystem.java b/src/main/java/com/demcha/compose/engine/measurement/FontLibraryTextMeasurementSystem.java index 500e219d5..f6ddad31c 100644 --- a/src/main/java/com/demcha/compose/engine/measurement/FontLibraryTextMeasurementSystem.java +++ b/src/main/java/com/demcha/compose/engine/measurement/FontLibraryTextMeasurementSystem.java @@ -5,7 +5,7 @@ import com.demcha.compose.engine.components.content.text.TextStyle; import com.demcha.compose.engine.components.geometry.ContentSize; import com.demcha.compose.engine.font.Font; -import com.demcha.compose.engine.render.pdf.PdfFont; +import com.demcha.compose.engine.font.FontLineMetrics; import java.util.HashMap; import java.util.Map; @@ -16,18 +16,24 @@ /** * Default measurement system backed by a document font library and a concrete * font implementation class supplied by the backend runtime. + * + *

Line metrics resolve polymorphically through the {@link Font} contract + * ({@link Font#lineMetrics(TextStyle)} plus {@link Font#measurementCacheKey(TextStyle)}), + * so every backend font — not only the PDF font — gets first-class metrics and + * can opt into the process-wide cache without this shared class being modified + * or special-cased per backend.

*/ public final class FontLibraryTextMeasurementSystem implements TextMeasurementSystem { private static final int GLOBAL_LINE_METRICS_CACHE_LIMIT = 50_000; private static final int SESSION_TEXT_WIDTH_CACHE_LIMIT = 10_000; - private static final ConcurrentMap GLOBAL_PDF_LINE_METRICS_CACHE = new ConcurrentHashMap<>(); + private static final ConcurrentMap GLOBAL_LINE_METRICS_CACHE = new ConcurrentHashMap<>(); private final FontLibrary fonts; private final Class> fontClass; private final Map> fontCache = new HashMap<>(); private final Map lineMetricsCache = new HashMap<>(); private final Map> textWidthCache = new HashMap<>(); - private final Map globalPdfStyleKeyCache = new HashMap<>(); + private final Map globalStyleKeyCache = new HashMap<>(); public FontLibraryTextMeasurementSystem(FontLibrary fonts, Class> fontClass) { this.fonts = Objects.requireNonNull(fonts, "fonts"); @@ -69,20 +75,24 @@ public LineMetrics lineMetrics(TextStyle style) { private LineMetrics resolveLineMetrics(TextStyle style) { Font font = resolveFont(style); - if (font instanceof PdfFont pdfFont) { - GlobalPdfStyleKey cacheKey = globalPdfStyleKey(pdfFont, style); - LineMetrics cached = GLOBAL_PDF_LINE_METRICS_CACHE.get(cacheKey); - if (cached != null) { - return cached; - } - var metrics = pdfFont.verticalMetrics(style); - LineMetrics resolved = new LineMetrics(metrics.ascent(), metrics.descent(), metrics.leading()); - cacheGlobalLineMetrics(cacheKey, resolved); - return resolved; + String cacheKey = font.measurementCacheKey(style); + if (cacheKey == null) { + // Backend opted out of the process-wide cache; the per-session + // lineMetricsCache (via lineMetrics(...)) still memoizes per style. + return toLineMetrics(font.lineMetrics(style)); } + GlobalStyleKey key = globalStyleKey(style, cacheKey); + LineMetrics cached = GLOBAL_LINE_METRICS_CACHE.get(key); + if (cached != null) { + return cached; + } + LineMetrics resolved = toLineMetrics(font.lineMetrics(style)); + cacheGlobalLineMetrics(key, resolved); + return resolved; + } - double lineHeight = Math.max(0.0, font.getLineHeight(style)); - return new LineMetrics(lineHeight, 0.0, 0.0); + private static LineMetrics toLineMetrics(FontLineMetrics metrics) { + return new LineMetrics(metrics.ascent(), metrics.descent(), metrics.leading()); } private double resolveTextWidth(Font font, TextStyle style, String text) { @@ -95,11 +105,16 @@ private Font resolveFont(TextStyle style) { .orElseThrow(() -> new IllegalStateException("Font not found for style: " + key.fontName()))); } - private GlobalPdfStyleKey globalPdfStyleKey(PdfFont font, TextStyle style) { - return globalPdfStyleKeyCache.computeIfAbsent(style, key -> GlobalPdfStyleKey.from(font, key)); + private GlobalStyleKey globalStyleKey(TextStyle style, String cacheKey) { + // Namespace the process-wide cache by backend font type: distinct backends + // may return the same measurementCacheKey (e.g. both key on "Helvetica") + // for different metrics, so without fontClass they would collide in the + // shared static cache. + return globalStyleKeyCache.computeIfAbsent(style, + key -> new GlobalStyleKey(fontClass.getName(), cacheKey, key.size(), key.decoration())); } - private static void cacheGlobalLineMetrics(GlobalPdfStyleKey key, LineMetrics metrics) { + private static void cacheGlobalLineMetrics(GlobalStyleKey key, LineMetrics metrics) { // Safety cap on the process-wide line-metrics cache. Distinct styles are // few in real use (a handful of font/size/decoration combos); this only // guards a pathological style explosion. Stop inserting once full instead @@ -107,8 +122,8 @@ private static void cacheGlobalLineMetrics(GlobalPdfStyleKey key, LineMetrics me // concurrent rendering (a thundering-herd recompute), so keeping the // existing entries is strictly better. This runs on a cache miss only, // never on the per-measurement get() path. - if (GLOBAL_PDF_LINE_METRICS_CACHE.size() < GLOBAL_LINE_METRICS_CACHE_LIMIT) { - GLOBAL_PDF_LINE_METRICS_CACHE.putIfAbsent(key, metrics); + if (GLOBAL_LINE_METRICS_CACHE.size() < GLOBAL_LINE_METRICS_CACHE_LIMIT) { + GLOBAL_LINE_METRICS_CACHE.putIfAbsent(key, metrics); } } @@ -117,7 +132,7 @@ public void clearCaches() { fontCache.clear(); lineMetricsCache.clear(); textWidthCache.clear(); - globalPdfStyleKeyCache.clear(); + globalStyleKeyCache.clear(); } int sessionTextWidthCacheSize() { @@ -128,10 +143,7 @@ int sessionTextWidthCacheSize() { return total; } - private record GlobalPdfStyleKey(String fontKey, double size, TextDecoration decoration) { - private static GlobalPdfStyleKey from(PdfFont font, TextStyle style) { - return new GlobalPdfStyleKey(font.measurementCacheKey(style), style.size(), style.decoration()); - } + private record GlobalStyleKey(String fontType, String fontKey, double size, TextDecoration decoration) { } } diff --git a/src/main/java/com/demcha/compose/engine/render/pdf/PdfFont.java b/src/main/java/com/demcha/compose/engine/render/pdf/PdfFont.java index 09386ce4d..59eb455b3 100644 --- a/src/main/java/com/demcha/compose/engine/render/pdf/PdfFont.java +++ b/src/main/java/com/demcha/compose/engine/render/pdf/PdfFont.java @@ -3,6 +3,7 @@ import com.demcha.compose.engine.components.content.text.TextStyle; import com.demcha.compose.engine.components.geometry.ContentSize; import com.demcha.compose.engine.font.FontBase; +import com.demcha.compose.engine.font.FontLineMetrics; import lombok.experimental.Accessors; import lombok.extern.slf4j.Slf4j; import org.apache.fontbox.util.BoundingBox; @@ -69,12 +70,28 @@ public VerticalMetrics verticalMetrics(TextStyle style) { return metrics; } + /** + * Bridges the PDFBox-derived {@link VerticalMetrics} to the backend-neutral + * {@link FontLineMetrics} the shared text-measurement system consumes, so the + * measurement system resolves PDF line metrics polymorphically rather than via + * an {@code instanceof PdfFont} special case. + * + * @param style the resolved text style + * @return ascent, descent, and leading in document units + */ + @Override + public FontLineMetrics lineMetrics(TextStyle style) { + VerticalMetrics metrics = verticalMetrics(style); + return new FontLineMetrics(metrics.ascent(), metrics.descent(), metrics.leading()); + } + /** * Returns a stable font identity for text measurement caches. * * @param style style selecting the concrete font variant * @return backend font name used for width and metric calculations */ + @Override public String measurementCacheKey(TextStyle style) { return fontType(style.decoration()).getName(); } diff --git a/src/test/java/com/demcha/compose/engine/measurement/FontLibraryTextMeasurementSystemTest.java b/src/test/java/com/demcha/compose/engine/measurement/FontLibraryTextMeasurementSystemTest.java index f44263882..30c5d7313 100644 --- a/src/test/java/com/demcha/compose/engine/measurement/FontLibraryTextMeasurementSystemTest.java +++ b/src/test/java/com/demcha/compose/engine/measurement/FontLibraryTextMeasurementSystemTest.java @@ -1,8 +1,13 @@ package com.demcha.compose.engine.measurement; import com.demcha.compose.engine.components.content.text.TextStyle; +import com.demcha.compose.engine.components.geometry.ContentSize; +import com.demcha.compose.engine.font.FontBase; +import com.demcha.compose.engine.font.FontLineMetrics; import com.demcha.compose.engine.render.pdf.PdfFont; import com.demcha.compose.font.DefaultFonts; +import com.demcha.compose.font.FontLibrary; +import com.demcha.compose.font.FontName; import org.junit.jupiter.api.Test; import java.lang.reflect.Modifier; @@ -45,4 +50,115 @@ void clearCachesShouldDiscardSessionTextWidthCache() { assertThat(measurement.sessionTextWidthCacheSize()).isZero(); } + + @Test + void resolvesBackendLineMetricsPolymorphicallyWithoutPdfSpecialCase() { + // A backend font that is NOT a PdfFont but supplies first-class metrics by + // overriding Font#lineMetrics. The shared measurement system must honour + // them via the contract, with no instanceof PdfFont fast-path. + FontLibrary library = DefaultFonts.standardLibrary(); + library.addFont(FontName.HELVETICA, FirstClassMetricsFont.class, new FirstClassMetricsFont()); + FontLibraryTextMeasurementSystem measurement = + new FontLibraryTextMeasurementSystem(library, FirstClassMetricsFont.class); + TextStyle style = helveticaStyle(); + + TextMeasurementSystem.LineMetrics metrics = measurement.lineMetrics(style); + + assertThat(metrics.ascent()).isEqualTo(10.0); + assertThat(metrics.descent()) + .describedAs("a non-PDF backend must get its real descent, not the degraded descent=0 fallback") + .isEqualTo(3.0); + assertThat(metrics.leading()).isEqualTo(1.0); + assertThat(metrics.lineHeight()).isEqualTo(14.0); + } + + @Test + void defaultLineMetricsDeriveFromLineHeightWithZeroDescentAndLeading() { + // A backend font that does NOT override Font#lineMetrics falls back to the + // contract default: ascent = line height, descent = leading = 0. + FontLibrary library = DefaultFonts.standardLibrary(); + library.addFont(FontName.HELVETICA, DefaultMetricsFont.class, new DefaultMetricsFont(20.0)); + FontLibraryTextMeasurementSystem measurement = + new FontLibraryTextMeasurementSystem(library, DefaultMetricsFont.class); + TextStyle style = helveticaStyle(); + + TextMeasurementSystem.LineMetrics metrics = measurement.lineMetrics(style); + + assertThat(metrics.ascent()).isEqualTo(20.0); + assertThat(metrics.descent()).isZero(); + assertThat(metrics.leading()).isZero(); + assertThat(metrics.lineHeight()).isEqualTo(20.0); + } + + @Test + void globalMetricsCacheIsNamespacedByBackendFontType() { + // Two different backend font types that return the SAME measurementCacheKey + // must not collide in the process-wide cache — the multi-backend invariant. + FontLibrary library = DefaultFonts.standardLibrary(); + library.addFont(FontName.HELVETICA, BackendAFont.class, new BackendAFont()); + library.addFont(FontName.HELVETICA, BackendBFont.class, new BackendBFont()); + FontLibraryTextMeasurementSystem a = new FontLibraryTextMeasurementSystem(library, BackendAFont.class); + FontLibraryTextMeasurementSystem b = new FontLibraryTextMeasurementSystem(library, BackendBFont.class); + TextStyle style = helveticaStyle(); + + // Resolve A first so it populates the shared static cache under the colliding key. + assertThat(a.lineMetrics(style).ascent()).isEqualTo(10.0); + assertThat(b.lineMetrics(style).ascent()) + .describedAs("backend B must get its own metrics, not backend A's value cached under the shared key") + .isEqualTo(20.0); + } + + private static TextStyle helveticaStyle() { + return new TextStyle(FontName.HELVETICA, 12.0, + TextStyle.DEFAULT_STYLE.decoration(), TextStyle.DEFAULT_STYLE.color()); + } + + /** Minimal non-PDF backend font that relies on the {@link com.demcha.compose.engine.font.Font} metric defaults. */ + private static class DefaultMetricsFont extends FontBase { + private final double lineHeight; + + DefaultMetricsFont(double lineHeight) { + super(new Object(), new Object(), new Object(), new Object()); + this.lineHeight = lineHeight; + } + + @Override public double getTextWidth(TextStyle style, String text) { return text == null ? 0.0 : text.length(); } + @Override public double getTextWidthNoSanitize(TextStyle style, String text) { return getTextWidth(style, text); } + @Override public double getLineHeight(TextStyle style) { return lineHeight; } + @Override public double getTextHeight(TextStyle style) { return lineHeight; } + @Override public double getCapHeight(TextStyle style) { return 0.0; } + @Override public double scale(double size) { return size; } + @Override public TextStyle adjustFontSizeToFit(String text, TextStyle style, double availableWidth) { return style; } + @Override public ContentSize getTightBounds(String text, TextStyle style) { return new ContentSize(0.0, 0.0); } + } + + /** Non-PDF backend font that overrides the metric seam with first-class ascent/descent/leading. */ + private static final class FirstClassMetricsFont extends DefaultMetricsFont { + FirstClassMetricsFont() { + super(99.0); // deliberately distinct from the overridden metrics below + } + + @Override + public FontLineMetrics lineMetrics(TextStyle style) { + return new FontLineMetrics(10.0, 3.0, 1.0); + } + + @Override + public String measurementCacheKey(TextStyle style) { + return "first-class|" + style.size(); + } + } + + /** Two backends below deliberately share a measurementCacheKey to exercise cross-backend cache isolation. */ + private static final class BackendAFont extends DefaultMetricsFont { + BackendAFont() { super(0.0); } + @Override public FontLineMetrics lineMetrics(TextStyle style) { return new FontLineMetrics(10.0, 0.0, 0.0); } + @Override public String measurementCacheKey(TextStyle style) { return "shared-collision-key"; } + } + + private static final class BackendBFont extends DefaultMetricsFont { + BackendBFont() { super(0.0); } + @Override public FontLineMetrics lineMetrics(TextStyle style) { return new FontLineMetrics(20.0, 0.0, 0.0); } + @Override public String measurementCacheKey(TextStyle style) { return "shared-collision-key"; } + } } From 0cda576e1ad9917539ff225afae604488bacf7a3 Mon Sep 17 00:00:00 2001 From: DemchaAV Date: Tue, 9 Jun 2026 14:17:21 +0100 Subject: [PATCH 2/2] refactor(engine): trim FontLineMetrics to a bare metrics carrier Drop the unused lineHeight()/baselineOffsetFromBottom() convenience methods: the measurement system reads ascent()/descent()/leading() directly and converts to TextMeasurementSystem.LineMetrics (which owns the derived helpers), so they had no callers. Review-driven dead-code removal; no behaviour change. --- .../compose/engine/font/FontLineMetrics.java | 23 ++++--------------- 1 file changed, 5 insertions(+), 18 deletions(-) diff --git a/src/main/java/com/demcha/compose/engine/font/FontLineMetrics.java b/src/main/java/com/demcha/compose/engine/font/FontLineMetrics.java index 240f03ae7..8a1d94134 100644 --- a/src/main/java/com/demcha/compose/engine/font/FontLineMetrics.java +++ b/src/main/java/com/demcha/compose/engine/font/FontLineMetrics.java @@ -12,27 +12,14 @@ * already depends on {@code engine.font}); the measurement system converts this * record into its own {@code LineMetrics} for layout consumers.

* + *

Carries only the three primitive components; the measurement system reads + * them via {@code ascent()}/{@code descent()}/{@code leading()} and converts to + * {@code TextMeasurementSystem.LineMetrics} (which owns the derived + * {@code lineHeight()} / baseline helpers) for layout consumers.

+ * * @param ascent distance from the baseline to the glyph top * @param descent distance from the baseline to the glyph bottom (non-negative) * @param leading extra line leading applied by the backend font metrics */ public record FontLineMetrics(double ascent, double descent, double leading) { - - /** - * Returns the total baseline-to-baseline line height. - * - * @return {@code ascent + descent + leading} - */ - public double lineHeight() { - return ascent + descent + leading; - } - - /** - * Returns the distance from the line bottom to the text baseline. - * - * @return the baseline offset, equal to {@code descent} - */ - public double baselineOffsetFromBottom() { - return descent; - } }