Skip to content

Commit 39295f9

Browse files
hyrahulhyrahul
committed
Migrate PDF font layout to per-document GlyphLayoutManager (#2444)
Replace OpenPDF's deprecated, process-global LayoutProcessor with the per-document GlyphLayoutManager, which holds no static state and is safe for concurrent multi-document rendering (issue #2444). - Remove LayoutProcessor usage from FontHandler and PDFPage.validateSymbolicFont. - PDFPageDevice creates a per-document GlyphLayoutManager behind an opt-in property (birt.pdf.complex.font.layout.enabled / PdfEmitter.ComplexFontLayoutEnabled), off by default, so existing output is unchanged. It is created lazily on first draw, since the report's user properties are not available when the device is constructed. - Fonts are loaded lazily on first draw, resolved per font via the font factory rather than scanning font directories; kerning and ligatures are applied when enabled by the kerning-and-ligatures font configuration. - PDFPage.drawText sets the manager on the writer for fonts loaded into it, and draws with the base font the manager created, since the manager only recognises its own instance. Base-14/Type1/symbolic fonts draw on the normal path. - Fix the run direction to left-to-right: BIRT applies bidi reordering and shaping before the emitter, so right-to-left text must be drawn as delivered. - Add PdfConcurrentRenderTest as a concurrency regression guard. - Add glm-manual-check.rptdesign for manual verification of glyph layout. - Remove FontHandlingPdfTest, a manual LayoutProcessor demo tied to the removed API. - Document the new property in the emitter README. Signed-off-by: Rahul Pal <hyrahulpal@gmail.com>
1 parent 475a729 commit 39295f9

8 files changed

Lines changed: 497 additions & 222 deletions

File tree

‎engine/org.eclipse.birt.report.engine.emitter.pdf.tests/test/org/eclipse/birt/report/engine/emitter/pdf/AllTests.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11

22
/*******************************************************************************
3-
* Copyright (c) 2004, 2005 Actuate Corporation.
3+
* Copyright (c) 2004, 2026 Actuate Corporation.
44
*
55
* This program and the accompanying materials are made available under the
66
* terms of the Eclipse Public License 2.0 which is available at
@@ -29,6 +29,7 @@ public static Test suite() {
2929

3030
/* in package: org.eclipse.birt.report.engine.emitter.pdf */
3131
suite.addTestSuite(org.eclipse.birt.report.engine.emitter.pdf.PdfRenderTest.class);
32+
suite.addTestSuite(org.eclipse.birt.report.engine.emitter.pdf.PdfConcurrentRenderTest.class);
3233

3334
// $JUnit-END$
3435
return suite;

‎engine/org.eclipse.birt.report.engine.emitter.pdf.tests/test/org/eclipse/birt/report/engine/emitter/pdf/PdfConcurrentRenderTest.java‎

Lines changed: 159 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,159 @@
1+
/*******************************************************************************
2+
* Copyright (c) 2026 Rahul Pal and others.
3+
*
4+
* This program and the accompanying materials are made available under the
5+
* terms of the Eclipse Public License 2.0 which is available at
6+
* https://www.eclipse.org/legal/epl-2.0/.
7+
*
8+
* SPDX-License-Identifier: EPL-2.0
9+
*
10+
* Contributors:
11+
* Rahul Pal - initial implementation (issue #2444)
12+
*******************************************************************************/
13+
package org.eclipse.birt.report.engine.emitter.pdf;
14+
15+
import java.io.File;
16+
import java.util.ArrayList;
17+
import java.util.Collections;
18+
import java.util.List;
19+
import java.util.concurrent.Callable;
20+
import java.util.concurrent.ExecutorService;
21+
import java.util.concurrent.Executors;
22+
import java.util.concurrent.Future;
23+
import java.util.concurrent.TimeUnit;
24+
25+
import org.eclipse.birt.report.engine.api.IRunAndRenderTask;
26+
import org.eclipse.birt.report.engine.api.PDFRenderOption;
27+
28+
/**
29+
* Concurrency test for the PDF emitter font path (issue #2444).
30+
*
31+
* <p>
32+
* Renders the same report design to PDF from several threads in parallel,
33+
* sharing the single {@code engine} created by {@link EngineCase} (the
34+
* realistic multi-threaded usage). The test asserts that no render throws and
35+
* that every output PDF is produced and non-empty.
36+
* </p>
37+
*
38+
* <p>
39+
* This guards concurrent rendering through the font path after the removal of
40+
* the process-global {@code LayoutProcessor} state. As with the other tests in
41+
* this module, it asserts that rendering completes without error rather than
42+
* verifying glyph-level output. Note that, by the nature of race conditions, a
43+
* passing run does not by itself prove the absence of a race - the primary
44+
* safety argument is the design (per-document state, no static state); this
45+
* test is a regression guard and demonstration.
46+
* </p>
47+
*/
48+
public class PdfConcurrentRenderTest extends EngineCase {
49+
50+
/** Reused existing design from this test module. */
51+
private static final String DESIGN = "test/org/eclipse/birt/report/engine/emitter/pdf/issue-2429.rptdesign";
52+
53+
/** Number of concurrent render tasks. */
54+
private static final int THREADS = 8;
55+
56+
/** Upper bound on total render time before the test gives up. */
57+
private static final int TIMEOUT_SECONDS = 120;
58+
59+
/**
60+
* Output directory, cleaned at the start of each run so the PDFs from the last
61+
* run remain available for manual inspection.
62+
*/
63+
private File outputDir;
64+
65+
@Override
66+
protected void setUp() throws Exception {
67+
super.setUp(); // creates the shared engine (and configures fonts)
68+
outputDir = new File(System.getProperty("java.io.tmpdir"), "birt-pdf-concurrent");
69+
deleteRecursively(outputDir);
70+
outputDir.mkdirs();
71+
}
72+
73+
/**
74+
* Renders the same design concurrently on a shared engine and verifies that
75+
* every render succeeds and produces a non-empty PDF.
76+
*
77+
* @throws Exception
78+
*/
79+
public void testConcurrentPdfRenders() throws Exception {
80+
ExecutorService pool = Executors.newFixedThreadPool(THREADS);
81+
List<Future<File>> futures = new ArrayList<>();
82+
List<String> failures = Collections.synchronizedList(new ArrayList<>());
83+
84+
try {
85+
for (int i = 0; i < THREADS; i++) {
86+
final int id = i;
87+
Callable<File> task = () -> {
88+
try {
89+
return renderOnce(id);
90+
} catch (Throwable t) {
91+
failures.add("thread " + id + ": " + t.getClass().getSimpleName() + ": " + t.getMessage());
92+
t.printStackTrace();
93+
return null;
94+
}
95+
};
96+
futures.add(pool.submit(task));
97+
}
98+
99+
pool.shutdown();
100+
boolean finished = pool.awaitTermination(TIMEOUT_SECONDS, TimeUnit.SECONDS);
101+
assertTrue("Concurrent renders did not finish within " + TIMEOUT_SECONDS + "s", finished);
102+
} finally {
103+
pool.shutdownNow();
104+
}
105+
106+
if (!failures.isEmpty()) {
107+
StringBuilder sb = new StringBuilder();
108+
sb.append(failures.size()).append(" of ").append(THREADS).append(" concurrent renders failed:");
109+
for (String failure : failures) {
110+
sb.append("\n ").append(failure);
111+
}
112+
fail(sb.toString());
113+
}
114+
115+
// Every task must have produced a real, non-empty PDF.
116+
for (Future<File> future : futures) {
117+
File pdf = future.get();
118+
assertNotNull("Render produced no output", pdf);
119+
assertTrue("Missing output: " + pdf, pdf.isFile());
120+
assertTrue("Empty output: " + pdf, pdf.length() > 0L);
121+
}
122+
}
123+
124+
/**
125+
* Renders the shared design to a unique per-thread PDF against the shared
126+
* engine. Uses {@link EngineCase#createRunAndRenderTask(String)}, which opens
127+
* the design by path directly (no shared design file), so it is safe to call
128+
* concurrently.
129+
*/
130+
private File renderOnce(int id) throws Exception {
131+
File out = new File(outputDir, "render-" + id + ".pdf");
132+
133+
IRunAndRenderTask task = createRunAndRenderTask(DESIGN);
134+
try {
135+
PDFRenderOption options = new PDFRenderOption();
136+
options.setOutputFormat("pdf");
137+
options.setOutputFileName(out.getAbsolutePath());
138+
task.setRenderOption(options);
139+
task.run();
140+
} finally {
141+
task.close();
142+
}
143+
return out;
144+
}
145+
146+
/** Deletes a file or directory tree, ignoring files that cannot be removed. */
147+
private static void deleteRecursively(File file) {
148+
if (file == null || !file.exists()) {
149+
return;
150+
}
151+
File[] children = file.listFiles();
152+
if (children != null) {
153+
for (File child : children) {
154+
deleteRecursively(child);
155+
}
156+
}
157+
file.delete();
158+
}
159+
}

‎engine/org.eclipse.birt.report.engine.emitter.pdf.tests/test/org/eclipse/birt/report/engine/emitter/pdf/glm-manual-check.rptdesign‎

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<report xmlns="http://www.eclipse.org/birt/2005/design" version="3.2.27" id="1">
3+
<property name="createdBy">Eclipse BIRT Designer Version 4.25.0.qualifier</property>
4+
<property name="language">English</property>
5+
<text-property name="title">BIRT Issue 2444 - manual glyph layout check</text-property>
6+
<property name="units">mm</property>
7+
<property name="bidiLayoutOrientation">ltr</property>
8+
<property name="imageDPI">96</property>
9+
<property name="locale">en_US</property>
10+
<property name="pdfVersion">1.7</property>
11+
<styles>
12+
<style name="report" id="2">
13+
<property name="fontFamily">"Noto Sans"</property>
14+
</style>
15+
</styles>
16+
<page-setup>
17+
<simple-master-page name="NewSimpleMasterPage" id="3">
18+
<property name="type">a4</property>
19+
</simple-master-page>
20+
</page-setup>
21+
<body>
22+
<label id="10">
23+
<property name="fontSize">10pt</property>
24+
<property name="marginBottom">8pt</property>
25+
<text-property name="text">Manual check for issue #2444. Render this report twice: once with -Dbirt.pdf.complex.font.layout.enabled=false and once with =true, then compare the two PDFs side by side. Kerning is visible in the letter pairs below (AV, VA, WA, AW, Ma, Va).</text-property>
26+
</label>
27+
28+
<label id="11">
29+
<property name="fontSize">12pt</property>
30+
<property name="fontWeight">bold</property>
31+
<property name="marginTop">8pt</property>
32+
<text-property name="text">Kerning (Latin, Noto Sans)</text-property>
33+
</label>
34+
<label id="12">
35+
<property name="fontSize">40pt</property>
36+
<property name="marginBottom">6pt</property>
37+
<text-property name="text">AV cables VA Maya Vase WM WA AW with Kerning</text-property>
38+
</label>
39+
<label id="13">
40+
<property name="fontSize">40pt</property>
41+
<property name="fontWeight">bold</property>
42+
<property name="marginBottom">6pt</property>
43+
<text-property name="text">AV cables VA Maya Vase WM WA AW with Kerning</text-property>
44+
</label>
45+
46+
<label id="14">
47+
<property name="fontSize">12pt</property>
48+
<property name="fontWeight">bold</property>
49+
<property name="marginTop">8pt</property>
50+
<text-property name="text">Ligatures (Latin, Noto Sans)</text-property>
51+
</label>
52+
<label id="15">
53+
<property name="fontSize">40pt</property>
54+
<property name="marginBottom">6pt</property>
55+
<text-property name="text">office waffle affix fjord flight</text-property>
56+
</label>
57+
58+
<label id="16">
59+
<property name="fontSize">12pt</property>
60+
<property name="fontWeight">bold</property>
61+
<property name="marginTop">8pt</property>
62+
<text-property name="text">Arabic (needs an Arabic-capable font to be registered)</text-property>
63+
</label>
64+
<label id="17">
65+
<property name="fontSize">28pt</property>
66+
<property name="marginBottom">6pt</property>
67+
<text-property name="text">مرحبا بالعالم العربية</text-property>
68+
</label>
69+
70+
<label id="18">
71+
<property name="fontSize">12pt</property>
72+
<property name="fontWeight">bold</property>
73+
<property name="marginTop">8pt</property>
74+
<text-property name="text">Chinese (needs a CJK-capable font to be registered)</text-property>
75+
</label>
76+
<label id="19">
77+
<property name="fontSize">28pt</property>
78+
<property name="marginBottom">6pt</property>
79+
<text-property name="text">中文测试 汉字 排版</text-property>
80+
</label>
81+
</body>
82+
</report>

‎engine/org.eclipse.birt.report.engine.emitter.pdf/src/org/eclipse/birt/report/engine/emitter/pdf/PDFPage.java‎

Lines changed: 12 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*******************************************************************************
2-
* Copyright (c) 2004, 2007, 2024, 2025 Actuate Corporation and others
2+
* Copyright (c) 2004, 2026 Actuate Corporation and others
33
*
44
* This program and the accompanying materials are made available under the
55
* terms of the Eclipse Public License 2.0 which is available at
@@ -45,16 +45,15 @@
4545
import org.openpdf.text.Image;
4646
import org.openpdf.text.Rectangle;
4747
import org.openpdf.text.pdf.BaseFont;
48-
import org.openpdf.text.pdf.LayoutProcessor;
4948
import org.openpdf.text.pdf.PdfAction;
5049
import org.openpdf.text.pdf.PdfAnnotation;
5150
import org.openpdf.text.pdf.PdfArray;
5251
import org.openpdf.text.pdf.PdfBorderDictionary;
5352
import org.openpdf.text.pdf.PdfContentByte;
5453
import org.openpdf.text.pdf.PdfDestination;
5554
import org.openpdf.text.pdf.PdfDictionary;
56-
import org.openpdf.text.pdf.PdfObject;
5755
import org.openpdf.text.pdf.PdfName;
56+
import org.openpdf.text.pdf.PdfObject;
5857
import org.openpdf.text.pdf.PdfRectangle;
5958
import org.openpdf.text.pdf.PdfString;
6059
import org.openpdf.text.pdf.PdfStructureElement;
@@ -614,7 +613,6 @@ private void drawText(String text, float textX, float textY, FontInfo fontInfo,
614613
}
615614

616615
BaseFont font = getBaseFont(fontInfo);
617-
validateSymbolicFont(font);
618616
font.setIncludeCidSet(this.pageDevice.isIncludeCidSet());
619617

620618
float fontSize = fontInfo.getFontSize();
@@ -636,6 +634,16 @@ private void drawText(String text, float textX, float textY, FontInfo fontInfo,
636634
"PDF/A: " + fontInfo.getFontName() + " not embeddable." + e.getMessage());
637635
}
638636
}
637+
// issue #2444: OpenPDF's showText routes through the glyph layout manager when
638+
// one is set on the writer. The manager only recognises the base font it
639+
// created itself, so the font is swapped for its own instance; fonts it cannot
640+
// load (base-14/Type1/symbolic) draw on the normal path with no manager set.
641+
if (pageDevice.useManagerForFont(font)) {
642+
writer.setGlyphLayoutManager(pageDevice.getGlyphLayoutManager());
643+
font = pageDevice.getManagerFont(font);
644+
} else {
645+
writer.setGlyphLayoutManager(null);
646+
}
639647
contentByte.setFontAndSize(font, fontSize);
640648
} catch (IllegalArgumentException iae) {
641649
logger.log(Level.WARNING, iae.getMessage());
@@ -890,20 +898,4 @@ public void endArtifact() {
890898
public boolean isInArtifact() {
891899
return artifactDepth > 0;
892900
}
893-
894-
/**
895-
* Validate the font property of "specific". This is a marker of symbolic font
896-
* and needs enabled handling of OpenPDF LayoutProcessor for kerning to display
897-
* the font correctly.
898-
*/
899-
private void validateSymbolicFont(BaseFont font) {
900-
synchronized (LayoutProcessor.class) {
901-
if (font.isFontSpecific()) {
902-
if (LayoutProcessor.isEnabled()) {
903-
LayoutProcessor.disable();
904-
}
905-
LayoutProcessor.setKerning();
906-
}
907-
}
908-
}
909901
}

0 commit comments

Comments
 (0)