Skip to content

Commit 90e112f

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. - Fonts are loaded lazily on first draw (the font configuration is not processed at construction time); TrueType/OpenType fonts are loaded with kerning and ligatures enabled. - PDFPage.drawText attaches the manager only for fonts loaded into it and detaches for base-14/Type1/symbolic fonts, since supportsFont throws for any font not loaded through the manager. - Add PdfConcurrentRenderTest as a concurrency regression guard. - 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 4fa4ce7 commit 90e112f

7 files changed

Lines changed: 408 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, 2005, 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: 155 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,155 @@
1+
/*******************************************************************************
2+
* Copyright (c) 2026 Rahul Pal
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.nio.file.Files;
17+
import java.util.ArrayList;
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+
import java.util.concurrent.atomic.AtomicInteger;
25+
26+
import org.eclipse.birt.report.engine.api.IRunAndRenderTask;
27+
import org.eclipse.birt.report.engine.api.PDFRenderOption;
28+
29+
/**
30+
* Concurrency test for the PDF emitter font path (issue #2444).
31+
*
32+
* <p>
33+
* Renders the same report design to PDF from several threads in parallel,
34+
* sharing the single {@code engine} created by {@link EngineCase} (the
35+
* realistic multi-threaded usage). The test asserts that no render throws and
36+
* that every output PDF is produced and non-empty.
37+
* </p>
38+
*
39+
* <p>
40+
* This guards concurrent rendering through the font path after the removal of
41+
* the process-global {@code LayoutProcessor} state. As with the other tests in
42+
* this module, it asserts that rendering completes without error rather than
43+
* verifying glyph-level output. Note that, by the nature of race conditions, a
44+
* passing run does not by itself prove the absence of a race - the primary
45+
* safety argument is the design (per-document state, no static state); this
46+
* test is a regression guard and demonstration.
47+
* </p>
48+
*/
49+
public class PdfConcurrentRenderTest extends EngineCase {
50+
51+
/** Reused existing design from this test module. */
52+
private static final String DESIGN = "test/org/eclipse/birt/report/engine/emitter/pdf/issue-2429.rptdesign";
53+
54+
/** Number of concurrent render tasks. */
55+
private static final int THREADS = 8;
56+
57+
/** Upper bound on total render time before the test gives up. */
58+
private static final int TIMEOUT_SECONDS = 120;
59+
60+
/** Temporary output directory, created per run and deleted in tearDown. */
61+
private File outputDir;
62+
63+
@Override
64+
protected void setUp() throws Exception {
65+
super.setUp(); // creates the shared engine (and configures fonts)
66+
outputDir = Files.createTempDirectory("birt-pdf-concurrent").toFile();
67+
}
68+
69+
@Override
70+
protected void tearDown() throws Exception {
71+
deleteRecursively(outputDir);
72+
super.tearDown();
73+
}
74+
75+
/**
76+
* @throws Exception
77+
*/
78+
public void testConcurrentPdfRenders() throws Exception {
79+
ExecutorService pool = Executors.newFixedThreadPool(THREADS);
80+
List<Future<File>> futures = new ArrayList<>();
81+
AtomicInteger failures = new AtomicInteger(0);
82+
83+
try {
84+
for (int i = 0; i < THREADS; i++) {
85+
final int id = i;
86+
Callable<File> task = () -> {
87+
try {
88+
return renderOnce(id);
89+
} catch (Throwable t) {
90+
failures.incrementAndGet();
91+
t.printStackTrace();
92+
throw t;
93+
}
94+
};
95+
futures.add(pool.submit(task));
96+
}
97+
98+
pool.shutdown();
99+
boolean finished = pool.awaitTermination(TIMEOUT_SECONDS, TimeUnit.SECONDS);
100+
assertTrue("Concurrent renders did not finish within " + TIMEOUT_SECONDS + "s", finished);
101+
102+
// Surface any worker exception (re-throws with stack trace).
103+
for (Future<File> f : futures) {
104+
f.get();
105+
}
106+
} finally {
107+
pool.shutdownNow();
108+
}
109+
110+
assertEquals("One or more concurrent renders failed", 0, failures.get());
111+
112+
// Every task must have produced a real, non-empty PDF.
113+
for (Future<File> f : futures) {
114+
File pdf = f.get();
115+
assertTrue("Missing output: " + pdf, pdf.isFile());
116+
assertTrue("Empty output: " + pdf, pdf.length() > 0L);
117+
}
118+
}
119+
120+
/**
121+
* Renders the shared design to a unique per-thread PDF against the shared
122+
* engine. Uses {@link EngineCase#createRunAndRenderTask(String)}, which opens
123+
* the design by path directly (no shared design file), so it is safe to call
124+
* concurrently.
125+
*/
126+
private File renderOnce(int id) throws Exception {
127+
File out = new File(outputDir, "render-" + id + ".pdf");
128+
129+
IRunAndRenderTask task = createRunAndRenderTask(DESIGN);
130+
try {
131+
PDFRenderOption options = new PDFRenderOption();
132+
options.setOutputFormat("pdf");
133+
options.setOutputFileName(out.getAbsolutePath());
134+
task.setRenderOption(options);
135+
task.run();
136+
} finally {
137+
task.close();
138+
}
139+
return out;
140+
}
141+
142+
/** Deletes a file or directory tree, ignoring files that cannot be removed. */
143+
private static void deleteRecursively(File file) {
144+
if (file == null || !file.exists()) {
145+
return;
146+
}
147+
File[] children = file.listFiles();
148+
if (children != null) {
149+
for (File child : children) {
150+
deleteRecursively(child);
151+
}
152+
}
153+
file.delete();
154+
}
155+
}

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

Lines changed: 16 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, 2007, 2024, 2025, 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,16 @@
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;
48+
import org.openpdf.text.pdf.GlyphLayoutManager;
4949
import org.openpdf.text.pdf.PdfAction;
5050
import org.openpdf.text.pdf.PdfAnnotation;
5151
import org.openpdf.text.pdf.PdfArray;
5252
import org.openpdf.text.pdf.PdfBorderDictionary;
5353
import org.openpdf.text.pdf.PdfContentByte;
5454
import org.openpdf.text.pdf.PdfDestination;
5555
import org.openpdf.text.pdf.PdfDictionary;
56-
import org.openpdf.text.pdf.PdfObject;
5756
import org.openpdf.text.pdf.PdfName;
57+
import org.openpdf.text.pdf.PdfObject;
5858
import org.openpdf.text.pdf.PdfRectangle;
5959
import org.openpdf.text.pdf.PdfString;
6060
import org.openpdf.text.pdf.PdfStructureElement;
@@ -614,7 +614,6 @@ private void drawText(String text, float textX, float textY, FontInfo fontInfo,
614614
}
615615

616616
BaseFont font = getBaseFont(fontInfo);
617-
validateSymbolicFont(font);
618617
font.setIncludeCidSet(this.pageDevice.isIncludeCidSet());
619618

620619
float fontSize = fontInfo.getFontSize();
@@ -642,6 +641,19 @@ private void drawText(String text, float textX, float textY, FontInfo fontInfo,
642641
// close to zero , increase by one MIN_FONT_SIZE step
643642
contentByte.setFontAndSize(font, MIN_FONT_SIZE * 2);
644643
}
644+
// issue #2444: OpenPDF's showText routes through the glyph layout manager
645+
// when one is attached. supportsFont(...) throws for any font not loaded
646+
// into the manager, so attach it only for fonts it has loaded
647+
// (TrueType/OpenType); detach for base-14/Type1/symbolic fonts so they draw
648+
// on the normal path.
649+
GlyphLayoutManager glm = pageDevice.getGlyphLayoutManager();
650+
if (glm != null) {
651+
if (pageDevice.useManagerForFont(font)) {
652+
pageDevice.getDocument().setGlyphLayoutManager(glm);
653+
} else {
654+
pageDevice.getDocument().setGlyphLayoutManager(null);
655+
}
656+
}
645657
if (characterSpacing != 0) {
646658
contentByte.setCharacterSpacing(characterSpacing);
647659
}
@@ -890,20 +902,4 @@ public void endArtifact() {
890902
public boolean isInArtifact() {
891903
return artifactDepth > 0;
892904
}
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-
}
909905
}

0 commit comments

Comments
 (0)