Migrate PDF font layout to per-document GlyphLayoutManager (#2444)

[hyrahul]

What this does

Migrates the PDF emitter's complex-font layout from OpenPDF's deprecated,
process-global LayoutProcessor to the per-Document GlyphLayoutManager
(OpenPDF's designated replacement, from LibrePDF/OpenPDF#1519). The manager holds
no static state and is designed for concurrent multi-document rendering, which is
the goal of #2444.

The global LayoutProcessor state is the root of the thread-safety problem in
#2444 — the same class of race that surfaced in production as #2385 (multi-threaded
PDF export failing with Chinese characters). By moving to per-document state, that
class of race is removed. (Note: races are non-deterministic, so this is addressed
by the design change rather than proven by a test; see Testing below.)

Approach

• FontHandler and PDFPage.validateSymbolicFont — removed all
  LayoutProcessor usage.
• PDFPageDevice — creates a per-Document GlyphLayoutManager, gated behind
  an opt-in property (birt.pdf.complex.font.layout.enabled system property or
  PdfEmitter.ComplexFontLayoutEnabled user property), off by default so
  existing output is unchanged.
• Lazy font loading — the font configuration is not processed when the device
  is constructed (the registered font paths are empty at that point), so fonts are
  loaded into the manager lazily on the first draw. TrueType/OpenType fonts are
  loaded with kerning and ligatures enabled via FontOptions.
• PDFPage.drawText — attaches the manager only for fonts it has loaded, and
  detaches (setGlyphLayoutManager(null)) for base-14/Type1/symbolic fonts. This
  is required because GlyphLayoutManager.supportsFont(...) throws for any font not
  loaded through it, so un-loadable fonts must not reach it — they draw on the
  normal path instead.

Why off by default / no regression

With the property off, no manager is created or attached, and rendering is
byte-for-byte unchanged. The removed LayoutProcessor path was only ever reachable
when LayoutProcessor.supportsFont(...) was true, i.e. for fonts loaded into it;
default rendering already flowed through showTextBasic.

Testing

• Added PdfConcurrentRenderTest (in emitter.pdf.tests) — renders the same
  design from multiple threads on a shared engine and asserts none fail. It is a
  regression guard / demonstration of concurrent rendering, not a proof that
  the race is gone (by nature, races can't be deterministically triggered). The
  real safety argument is the per-document design.
• Verified manually that rendering succeeds with the feature on and is
  unchanged with it off.
• I have not been able to automatically verify glyph-level correctness
  (kerning/ligatures/non-Latin shaping) — automated glyph-position checks are
  impractical, consistent with the existing tests in this module. I'd very much
  value review against the non-Latin cases this feature originally targeted
  (Arabic Content with Arabic Characters is incorrect for output format PDF since 4.17 #2133, Chinese BIRT 4.22 Export PDF fails in multi threaded environment when report uses Chinese characters #2385), since you have the reference for what correct
  output looks like.

Removed test

Removed FontHandlingPdfTest — a manual demo (main() with hardcoded C:/temp/
paths, no assertions; its own Javadoc says results must be validated manually) built
entirely on the LayoutProcessor API this PR removes. Its purpose is superseded by
the GlyphLayoutManager path.

OpenPDF version

No version change needed — both bundles already declare
com.github.librepdf.openpdf;bundle-version="[3.0.0,4.0.0)" in their manifests.

Open questions / for review

1. FopGlyphProcessor interaction — GlyphLayoutManager and the existing
   setGlyphSubstitutionEnabled(...) (FopGlyphProcessor, Enhance pdf glyph substitution handling (#2133) #2141) are mutually
   exclusive. This PR does not yet coordinate them; guidance welcome (@speckyspooky).
2. Symbolic fonts — validateSymbolicFont's handling is now implicit (symbolic
   fonts take the detached/normal path). Please confirm this preserves the intended
   behaviour for Webdings-type fonts.
3. Opening as draft to confirm the direction before polishing.

[merks]

It would be better if you maintained a single commit by rebasing on master (as necessary), amending your commit and force pushing the changes.

[merks]

Many small comments. In general copyright dates should be a range and type names should be imported not fully qualified in the body.

[hvbtup]

It would be better if you maintained a single commit by rebasing on master (as necessary), amending your commit and force pushing the changes.

For Git noobs like me:

Once you --amend to an already-pushed commit, you need to use the --force option with git push. I was always scared of using it, but this seems to be the usual way.

[hvbtup]

Some questions here and there.
I think we need a test case that allows manually checking the PDF results.
In particular, some of the examples from the old issues with Arabic, Chinese etc, and a demo report with a large font size to make the kerning actually visible.
Like this text: "AV cables VA Maya Vase WM WA AW with Kerning".

[hvbtup]

I startet a CI run. Using the build artifacts, it will be easier for other users to test.

[hyrahul]

Pushed an update (amended, single commit):

• Lazy per-font loading via FontFactory.getFontImp().getFontPath(...) instead of scanning font directories
• Kerning/ligatures now driven by the existing kerning-and-ligatures config rather than always on
• FONT_LOAD_SIZE constant replacing the 12f
• Added glm-manual-check.rptdesign for the manual check
• Test cleanup moved to setUp so the PDFs survive for inspection; failures now reported as a summary
• Style points: imports, no explicit default initialisers, NLS marker, conditional expression, copyright ranges, Since 4.24

Verification details in the comment above.

[merks]

It generally looks good to me. Just some very nitty suggestion.

If @hvbtup considers this good to go then I'm fine with that. I can't really judge the low level implementation details personally.

[merks]

Looks fine to me.

[hvbtup]

I couldn't spot an error in the code, but I tested it and it simply does not work.

What I did for testing:

1. Downloaded and unpacked the build artifact (birt-report-designer-win32-x86_64.zip)
2. Downloaded the report glm-manual-check.rptdesign
3. Opened the report in the designer
4. No Kerning or ligatures visibile (so far, this is expected)
5. Closed the designer
6. Modified the file birt.ini and added -Dbirt.pdf.complex.font.layout.enabled=true
7. Repeated the steps 3-5, still no kerning, still, this is expected
8. Modified the file D:\eclipse\birt-pr-2463\eclipse\plugins\org.eclipse.birt.report.engine.fonts_4.25.0.v202606101527\fontsConfig.xml and changed the line 68 to <kerning-and-ligatures enabled="true" />
9. Repeated steps 3-5, still no kerning.

Since the kerning is enabled in fontConfig.xml and in the system property now, this should work AFAIK, but it does not.
The visible result is no different with kerning enabled, the gap between the letters V and A is still the same (too far).

I also created a report of my own, which shows the system property and user property; with the same result. Please find it attached here.

kerning1.zip

[hyrahul]

Test Case Verification Update

@hvbtup Thanks for sharing the rptdesign template. It was really helpful in reproducing and verifying one of the test-case scenarios.

I’ve made some minor fixes based on the observations and verified the following scenarios before and after the fix:

System Property	User Property	Result
Off	No user property	enabled=false
Off	user prop=true	enabled=true(was broken)
On	No user property	enabled=true
On	user prop=true	enabled=true

With Arial (which has kern pairs, unlike the bundled Noto Sans), the content stream changes from a plain show-text to a positioned array, and the adjustments match the font's kern table:

kerning off: <0024 0039 ...>Tj
kerning on: [<0024>74.22<0039>91.8<...>]TJ

0024=A, 0039=V — 74.22 matches Arial's AV kern of -74; W/A shows 37.11, matching -37. Your own report renders the same way via the user property alone: [<0024>74.22<0039>...]TJ for "AV module VA".

The attached results show the behavior before and after the fix.

Could you please give your test case another shot with the latest changes?
It would be great to confirm the fix from your scenario as well.

Thanks!

[hvbtup]

OK, I can now see that it seems to works with my test report.

What did you change in the code to make it work now?

I can also see that kerning works for the English text in the glm-manual-check.rptdesign.

I can not verify if the rendering of the Arabic and Chinese texts is correct in that report's PDF output.
I don't know if its wrong or right, I only see that the Arabic text is rendered differently.

To me it seems that there might be an issue with the reading direction LTR / RTL?

I think we need someone who can read Arabic to verify if it's correct.

[hyrahul]

What did you change in the code to make it work now? -> Three fixes:

1. setupGlyphLayoutManager() ran in the constructor, before userProperties was assigned, so the user property was always null there. The manager is now created lazily on first draw.

2. Document.setGlyphLayoutManager(...) sets a field on that instance, but PdfContentByte.showText reads it from writer.getPdfDocument(), a different object. Now set via PdfWriter.setGlyphLayoutManager(...).

3. loadFont(...) creates its own BaseFont and only recognises that instance, so supportsFont returned false for BIRT's font. The device now keeps the manager's base font and draws with it.

I can also see that kerning works for the English text in the glm-manual-check.rptdesign.

I can not verify if the rendering of the Arabic and Chinese texts is correct in that report's PDF output. I don't know if its wrong or right, I only see that the Arabic text is rendered differently.

To me it seems that there might be an issue with the reading direction LTR / RTL? -> On the Arabic: you were right, the reading direction was wrong. BIRT applies bidi reordering and shaping before the emitter, so the text arrives in visual order using Arabic presentation forms (U+FExx). The manager was running its own bidi over that and reversing it a second time. Fixed by setting FontOptions.setRunDirectionLtr(), so the manager draws the text as delivered.

I think we need someone who can read Arabic to verify if it's correct. -> The output now matches the source string and starts from the right margin, but I can't judge the finer points, so a second pair of eyes on that would still be welcome.

[hyrahul]

Hi @merks and @hvbtup
All review comments are addressed and verified. CI is green. Ready to merge from my side whenever you've had a chance to look.

[merks]

If @hvbtup approves then I'm fine with the changes.

[hvbtup]

@wimjongman @merks I think for the next release this change should be emphasized; people who use BIRT for non-latin writing system should definitely test. Also, it should be pointed out that, for kerning and ligatures to work, two settings are necessary:
First, either a PDF emitter specific setting in the rptdesign file or a System property.
Second, <kerning-and-ligatures enabled="true" /> in fontsConfig.xml.

[merks]

@wimjongman

I don't know where such documentation should exist that people would find it. We have the root READ.md buch not much else.

I have no clue how the website is maintained.

https://github.com/eclipse-birt/birt-website/commits/main/

There is a massive ignored backlog of security issues

https://github.com/eclipse-birt/birt-website/security/dependabot