Merge pull request #151 from DemchaAV/refactor/font-vertical-metrics-contract

refactor(engine): backend-neutral text line metrics via the Font contract

Author: DemchaAV

CHANGELOG.md

@@ -162,6 +162,22 @@ Open cycle — bug-fix / housekeeping. Entries land here as they merge.
   public entry point and carries no binary-compatibility promise, so the move is
   excluded from the japicmp gate rather than treated as a breaking removal.
 
+### 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<T>`
+  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

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.
+     *
+     * <p>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.</p>
+     *
+     * @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).
+     *
+     * <p>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.</p>
+     *
+     * @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);
 
     /**

src/main/java/com/demcha/compose/engine/font/FontLineMetrics.java

@@ -0,0 +1,25 @@
+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.
+ *
+ * <p>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.</p>
+ *
+ * <p>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.</p>
+ *
+ * @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) {
+}

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.
+ *
+ * <p>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.</p>
  */
 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<GlobalPdfStyleKey, LineMetrics> GLOBAL_PDF_LINE_METRICS_CACHE = new ConcurrentHashMap<>();
+    private static final ConcurrentMap<GlobalStyleKey, LineMetrics> GLOBAL_LINE_METRICS_CACHE = new ConcurrentHashMap<>();
 
     private final FontLibrary fonts;
     private final Class<? extends Font<?>> fontClass;
     private final Map<TextStyle, Font<?>> fontCache = new HashMap<>();
     private final Map<TextStyle, LineMetrics> lineMetricsCache = new HashMap<>();
     private final Map<TextStyle, Map<String, Double>> textWidthCache = new HashMap<>();
-    private final Map<TextStyle, GlobalPdfStyleKey> globalPdfStyleKeyCache = new HashMap<>();
+    private final Map<TextStyle, GlobalStyleKey> globalStyleKeyCache = new HashMap<>();
 
     public FontLibraryTextMeasurementSystem(FontLibrary fonts, Class<? extends Font<?>> 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,20 +105,25 @@ 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
         // of clear()-ing: the old full flush wiped every hot entry under
         // 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) {
     }
 
 }

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();
     }

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<Object> {
+        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"; }
+    }
 }