[ 
https://issues.apache.org/jira/browse/PDFBOX-4106?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16471291#comment-16471291
 ] 

John Hewson edited comment on PDFBOX-4106 at 5/11/18 12:00 AM:
---------------------------------------------------------------

(!) So we have a problem. While looking at building a proper ToUnicodeMap for 
PDFBOX-4189 I've encountered some significant issues related to this feature. 
Given that this was shipped in 2.0.9, we're likely going to face some hard 
choices.

Most of the handling of vertical text in this patch is fine. It generally does 
a good job of handling the intricacies of both PDF and OpenType and juggling 
the sometimes competing concerns.

The issue is that the new APIs added to FontBox's TrueTypeFont are incompatible 
with PDFBox's multi-threading. Unlike most of PDFBox, fonts in FontBox must be 
thread safe - they are cached and shared globally between all PDDocument 
instances. For this reason a FontBoxFont must not contain any document-specific 
state.

The following fields added to TrueTypeFont contain document-specific state:

{{List<String> enabledGsubFeatures}}

The following methods added to TrueTypeFont write document-specific state:

{{enableVerticalSubstitutions()}}
 {{enableGsubFeature(String)}}
 {{disableGsubFeature(String)}}

The following methods added to TrueTypeFont read document-specific state:

{{getUnicodeCmapLookup()}}
 {{getUnicodeCmapLookup(boolean)}}

None of the additions are thread safe, anyone who calls these methods will 
clobber the corresponding state for all other threads. This problem can even 
occur if the user is manipulating more than one document within a single 
thread. *There's no way around this and no way to fix these methods* - 
document-specific state can't live in shared global fonts :(

(flag) Tough choices: Given that these are all just auxiliary methods limited 
to just FontBox's TrueTypeFont class, we _could_ remove them. These are very 
obscure methods which have only been around for a few weeks. It's unlikely that 
_any_ users would be affected by their removal. Obviously the missing 
functionality will be implemented in the appropriate location, so vertical text 
will still work in 2.0 and the existing PDType0Font.loadVertical API - which is 
the only thing that matters - will remain unchanged! :)

Yes, I'm suggesting a *very obscure* breaking API change to 2.0 but as it 
stands we have just shipped a breaking functionality change to 2.0 and broken 
existing functionality in an irreparable manner, and that seems worse. Thoughts?


was (Author: jahewson):
(!) So we have a problem. While looking at building a proper ToUnicodeMap for 
PDFBOX-4189 I've encountered some significant issues related to this feature. 
Given that this was shipped in 2.0.9, we're likely going to face some hard 
choices.

Most of the handling of vertical text in this patch is fine. It generally does 
a good job of handling the intricacies of both PDF and OpenType and juggling 
the sometimes competing concerns.

The issue is that the new APIs added to FontBox's TrueTypeFont are incompatible 
with PDFBox's multi-threading. Unlike most of PDFBox, fonts in FontBox must be 
thread safe - they are cached and shared globally between all PDDocument 
instances. For this reason a FontBoxFont must not contain any document-specific 
state.

The following fields added to TrueTypeFont contain document-specific state:

{{List<String> enabledGsubFeatures}}

The following methods added to TrueTypeFont write document-specific state:

{{enableVerticalSubstitutions()}}
 {{enableGsubFeature(String)}}
 {{disableGsubFeature(String)}}

The following methods added to TrueTypeFont read document-specific state:

{{getUnicodeCmapLookup()}}
 {{getUnicodeCmapLookup(boolean)}}

None of the additions are thread safe, anyone who calls these methods will 
clobber the corresponding state for all other threads. This problem can even 
occur if the user is manipulating more than one document with even just one 
thread. *There's no way around this and no way to fix these methods* - 
document-specific state can't live in shared global fonts :(

(flag) Tough choices: Given that these are all just auxiliary methods limited 
to just FontBox's TrueTypeFont class, we _could_ remove them. These are very 
obscure methods which have only been around for a few weeks. It's unlikely that 
_any_ users would be affected by their removal. Obviously the missing 
functionality will be implemented in the appropriate location, so vertical text 
will still work in 2.0 and the existing PDTyoe0Font.loadVertical API - which is 
the only thing that matters - will remain unchanged! :)

Yes, I'm suggesting a *very obscure* breaking API change to 2.0 but as it 
stands we have just shipped a breaking functionality change to 2.0 and broken 
existing functionality in an irreparable manner, and that seems worse. Thoughts?

> Vertical text creation
> ----------------------
>
>                 Key: PDFBOX-4106
>                 URL: https://issues.apache.org/jira/browse/PDFBOX-4106
>             Project: PDFBox
>          Issue Type: Bug
>          Components: FontBox, Parsing, Writing
>            Reporter: Aaron Madlon-Kay
>            Assignee: Tilman Hausherr
>            Priority: Major
>              Labels: embed, gsub, parsing, vertical
>             Fix For: 2.0.9, 3.0.0 PDFBox
>
>         Attachments: 
> 0001-Add-OpenTypeScript-class-to-get-OT-script-tags-for-c.patch, 
> 0002-Optimize-Unicode-script-storage-and-lookup.patch, 
> 0003-Parse-GSUB-table.patch, 
> 0004-Abstract-cmap-lookup-into-an-interface.patch, 
> 0005-Implement-GSUB-substitution-on-TrueTypeFont.patch, 
> 0006-Use-vhea-vmtx-to-fix-vertical-displacements-in-PCIDF.patch, 
> 0007-Add-factory-methods-for-loading-TTF-as-vertical-font.patch, 
> 0008-Implement-vertical-metrics-support-when-embedding-subsetting.patch, 
> FIX-0001-PDFBOX-4106-Remove-early-outs-leading-to-spurious-wa.patch, 
> FIX-0002-PDFBOX-4106-Document-GlyphSubstitutionTable-public-m.patch, 
> FIX-0003-PDFBOX-4106-Correct-deltaGlyphID-data-size.patch, 
> FIX-0004-PDFBOX-4106-Remove-unnecessary-vertical-displacement.patch, 
> FIX-0005-PDFBOX-4106-Remove-duplicate-DW2-creation.patch, 
> FIX-0006-PDFBOX-4106-Fix-non-embedded-vertical-font-rendering.patch, 
> FIX-0007-PDFBOX-4106-Fix-incorrect-parsing-of-W2-first-format.patch, 
> FIX-0008-PDFBOX-4106-Rename-misleading-field.patch, 
> FIX-0009-PDFBOX-4106-Allow-retrieving-vmtx-topSideBearing.patch, 
> FIX-0010-PDFBOX-4106-Correct-vmtx-embedding-for-proportional-.patch, 
> sample_code.txt, vertical.pdf
>
>
> I needed to output vertical Japanese text, but was stymied by several 
> limitations:
> * No API to load a TTF as Identity-V encoding
> * No support for 'vert' glyph substitution
> * No support for vertical metrics ('vhea' and 'vmtx' tables are parsed but 
> not used at all)
> I have attached a series of patches that implement the above features. 
> Highlights:
> * The GSUB glyph substitution table is parsed (limitation: type 1 lookups 
> only; this is sufficient for many features including 'vert'/'vrt2' vertical 
> glyph substitution)
> * Cmap lookup makes use of GSUB when features are enabled on a TTF
> * 'vhea' and 'vmtx' metrics are applied to PDCIDFont when appropriate, and 
> are embedded/subsetted correctly through the DW2/W2 CIDFont dictionary
> * An API has been added for loading a TTF as a vertical font, setting 
> Identity-V encoding and enabling 'vert'/'vrt2' substitution
> Each patch could approximately be split out into a separate ticket, if 
> desired.
> Also attached is some sample code that exercises these patches and 
> illustrates the effect of vertical glyph positioning. The sample output PDF 
> is also attached.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org
For additional commands, e-mail: dev-h...@pdfbox.apache.org

Reply via email to