Use same glyph width in PDF drawing and font subset

During PDF drawing we already know the glyph width, but when subsetting
we get it again from the font subset.

For fonts without CFF table this is redundant but harmless, but for
fonts with CFF table, if the CFF and hmtx table (the authoritative
source of glyph widths) disagree, we will be drawing the PDF string with
the wrong adjustment resulting in displaced glyphs. This is a font bug,
but avoiding redundancy and having one source of truth is an improvement
overall.

I kept the code that calculates advance widths for font subsets, just in
case it is is still used elsewhere.

Change-Id: I757cd0c2ebb6477b2f840d0005e84b5a131f7efb
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/135442
Tested-by: Jenkins
Tested-by: Caolán McNamara <caolanm@redhat.com>
Reviewed-by: Caolán McNamara <caolanm@redhat.com>

4 files changed, 53 insertions, 32 deletions

diff --git a/vcl/inc/pdf/pdfwriter_impl.hxx b/vcl/inc/pdf/pdfwriter_impl.hxx
index ae461895a55b..869ba7883458 100644
--- a/vcl/inc/pdf/pdfwriter_impl.hxx
+++ b/vcl/inc/pdf/pdfwriter_impl.hxx
@@ -287,15 +287,19 @@ class GlyphEmit
     // performance: actually this should probably a vector;
     std::vector<sal_Ucs>            m_CodeUnits;
     sal_uInt8                       m_nSubsetGlyphID;
+    sal_Int32                       m_nGlyphWidth;
 
 public:
-    GlyphEmit() : m_nSubsetGlyphID(0)
+    GlyphEmit() : m_nSubsetGlyphID(0), m_nGlyphWidth(0)
     {
     }
 
     void setGlyphId( sal_uInt8 i_nId ) { m_nSubsetGlyphID = i_nId; }
     sal_uInt8 getGlyphId() const { return m_nSubsetGlyphID; }
 
+    void setGlyphWidth( sal_Int32 nWidth ) { m_nGlyphWidth = nWidth; }
+    sal_Int32 getGlyphWidth() const { return m_nGlyphWidth; }
+
     void addCode( sal_Ucs i_cCode )
     {
         m_CodeUnits.push_back(i_cCode);
@@ -817,7 +821,7 @@ i12626
     void appendLiteralStringEncrypt( std::string_view rInString, const sal_Int32 nInObjectNumber, OStringBuffer& rOutBuffer );
 
     /* creates fonts and subsets that will be emitted later */
-    void registerGlyph(const GlyphItem* pGlyph, const vcl::font::PhysicalFontFace* pFont, const std::vector<sal_Ucs>& rCodeUnits, sal_uInt8& nMappedGlyph, sal_Int32& nMappedFontObject);
+    void registerGlyph(const GlyphItem* pGlyph, const vcl::font::PhysicalFontFace* pFont, const std::vector<sal_Ucs>& rCodeUnits, sal_Int32 nGlyphWidth, sal_uInt8& nMappedGlyph, sal_Int32& nMappedFontObject);
 
     /*  emits a text object according to the passed layout */
     /* TODO: remove rText as soon as SalLayout will change so that rText is not necessary anymore */
diff --git a/vcl/source/gdi/pdfwriter_impl.cxx b/vcl/source/gdi/pdfwriter_impl.cxx
index 23ca498ea73b..4ae1d9f5d242 100644
--- a/vcl/source/gdi/pdfwriter_impl.cxx
+++ b/vcl/source/gdi/pdfwriter_impl.cxx
@@ -2681,6 +2681,7 @@ bool PDFWriterImpl::emitFonts()
                 pEncoding[ nEnc ] = nEnc;
                 pEncToUnicodeIndex[ nEnc ] = static_cast<sal_Int32>(aCodeUnits.size());
                 pCodeUnitsPerGlyph[ nEnc ] = item.second.countCodes();
+                pWidths[ nEnc ] = item.second.getGlyphWidth();
                 for( sal_Int32 n = 0; n < pCodeUnitsPerGlyph[ nEnc ]; n++ )
                     aCodeUnits.push_back( item.second.getCode( n ) );
                 if( item.second.getCode(0) )
@@ -2693,7 +2694,7 @@ bool PDFWriterImpl::emitFonts()
                 }
             }
             FontSubsetInfo aSubsetInfo;
-            if( pGraphics->CreateFontSubset( aTmpName, subset.first, aGlyphIds, pEncoding, pWidths, nGlyphs, aSubsetInfo ) )
+            if( pGraphics->CreateFontSubset( aTmpName, subset.first, aGlyphIds, pEncoding, nullptr, nGlyphs, aSubsetInfo ) )
             {
                 // create font stream
                 osl::File aFontFile(aTmpName);
@@ -3901,9 +3902,17 @@ void PDFWriterImpl::createDefaultCheckBoxAppearance( PDFWidget& rBox, const PDFW
     const GlyphItem aItem(0, 0, pMap->GetGlyphIndex(cMark),
                           DevicePoint(), GlyphItemFlags::NONE, 0, 0, 0);
     const std::vector<sal_Ucs> aCodeUnits={ cMark };
+    sal_Int32 nGlyphWidth = 0;
+    SalGraphics *pGraphics = GetGraphics();
+    if (pGraphics)
+        nGlyphWidth = m_aFontCache.getGlyphWidth(pDevFont,
+                                                 aItem.glyphId(),
+                                                 aItem.IsVertical(),
+                                                 pGraphics);
+
     sal_uInt8 nMappedGlyph;
     sal_Int32 nMappedFontObject;
-    registerGlyph(&aItem, pDevFont, aCodeUnits, nMappedGlyph, nMappedFontObject);
+    registerGlyph(&aItem, pDevFont, aCodeUnits, nGlyphWidth, nMappedGlyph, nMappedFontObject);
 
     appendNonStrokingColor( replaceColor( rWidget.TextColor, rSettings.GetRadioCheckTextColor() ), aDA );
     aDA.append( ' ' );
@@ -5801,6 +5810,7 @@ sal_Int32 PDFWriterImpl::getSystemFont( const vcl::Font& i_rFont )
 void PDFWriterImpl::registerGlyph(const GlyphItem* pGlyph,
                                   const vcl::font::PhysicalFontFace* pFont,
                                   const std::vector<sal_Ucs>& rCodeUnits,
+                                  sal_Int32 nGlyphWidth,
                                   sal_uInt8& nMappedGlyph,
                                   sal_Int32& nMappedFontObject)
 {
@@ -5831,6 +5841,7 @@ void PDFWriterImpl::registerGlyph(const GlyphItem* pGlyph,
         // add new glyph to emitted font subset
         GlyphEmit& rNewGlyphEmit = rSubset.m_aSubsets.back().m_aMapping[ nFontGlyphId ];
         rNewGlyphEmit.setGlyphId( nNewId );
+        rNewGlyphEmit.setGlyphWidth( nGlyphWidth );
         for (const auto nCode : rCodeUnits)
             rNewGlyphEmit.addCode(nCode);
 
@@ -6291,10 +6302,6 @@ void PDFWriterImpl::drawLayout( SalLayout& rLayout, const OUString& rText, bool
 
         assert(!aCodeUnits.empty() || bUseActualText || pGlyph->IsInCluster());
 
-        sal_uInt8 nMappedGlyph;
-        sal_Int32 nMappedFontObject;
-        registerGlyph(pGlyph, pFont, aCodeUnits, nMappedGlyph, nMappedFontObject);
-
         sal_Int32 nGlyphWidth = 0;
         SalGraphics *pGraphics = GetGraphics();
         if (pGraphics)
@@ -6303,6 +6310,10 @@ void PDFWriterImpl::drawLayout( SalLayout& rLayout, const OUString& rText, bool
                                                      pGlyph->IsVertical(),
                                                      pGraphics);
 
+        sal_uInt8 nMappedGlyph;
+        sal_Int32 nMappedFontObject;
+        registerGlyph(pGlyph, pFont, aCodeUnits, nGlyphWidth, nMappedGlyph, nMappedFontObject);
+
         int nCharPos = -1;
         if (bUseActualText || pGlyph->IsInCluster())
             nCharPos = pGlyph->charPos();
diff --git a/vcl/source/gdi/salgdilayout.cxx b/vcl/source/gdi/salgdilayout.cxx
index da8c7e2a5ae8..003e7f4d4c02 100644
--- a/vcl/source/gdi/salgdilayout.cxx
+++ b/vcl/source/gdi/salgdilayout.cxx
@@ -1029,17 +1029,20 @@ bool SalGraphics::CreateTTFfontSubset(vcl::AbstractTrueTypeFont& rTTF, const OSt
         aTempEncs[0] = 0;
     }
 
-    std::unique_ptr<sal_uInt16[]> pMetrics
-        = GetTTSimpleGlyphMetrics(&rTTF, aShortIDs, nGlyphCount, bVertical);
-    if (!pMetrics)
-        return false;
+    if (pGlyphWidths)
+    {
+        std::unique_ptr<sal_uInt16[]> pMetrics
+            = GetTTSimpleGlyphMetrics(&rTTF, aShortIDs, nGlyphCount, bVertical);
+        if (!pMetrics)
+            return false;
 
-    sal_uInt16 nNotDefAdv = pMetrics[0];
-    pMetrics[0] = pMetrics[nNotDef];
-    pMetrics[nNotDef] = nNotDefAdv;
-    for (i = 0; i < nOrigGlyphCount; ++i)
-        pGlyphWidths[i] = pMetrics[i];
-    pMetrics.reset();
+        sal_uInt16 nNotDefAdv = pMetrics[0];
+        pMetrics[0] = pMetrics[nNotDef];
+        pMetrics[nNotDef] = nNotDefAdv;
+        for (i = 0; i < nOrigGlyphCount; ++i)
+            pGlyphWidths[i] = pMetrics[i];
+        pMetrics.reset();
+    }
 
     // write subset into destination file
     return (CreateTTFromTTGlyphs(&rTTF, rSysPath.getStr(), aShortIDs, aTempEncs, nGlyphCount)
diff --git a/vcl/unx/generic/fontmanager/fontmanager.cxx b/vcl/unx/generic/fontmanager/fontmanager.cxx
index e4ca98171059..46e8ae1624c7 100644
--- a/vcl/unx/generic/fontmanager/fontmanager.cxx
+++ b/vcl/unx/generic/fontmanager/fontmanager.cxx
@@ -1051,21 +1051,24 @@ bool PrintFontManager::createFontSubset(
     rInfo.m_aFontBBox   = tools::Rectangle( Point( xMin, yMin ), Size( xMax-xMin, yMax-yMin ) );
     rInfo.m_nCapHeight  = yMax; // Well ...
 
-    // fill in glyph advance widths
-    std::unique_ptr<sal_uInt16[]> pMetrics = GetTTSimpleGlyphMetrics( pTTFont,
-                                                              pGID,
-                                                              nGlyphs,
-                                                              false/*bVertical*/ );
-    if( pMetrics )
+    if (pWidths)
     {
-        for( int i = 0; i < nGlyphs; i++ )
-            pWidths[pOldIndex[i]] = pMetrics[i];
-        pMetrics.reset();
-    }
-    else
-    {
-        CloseTTFont( pTTFont );
-        return false;
+        // fill in glyph advance widths
+        std::unique_ptr<sal_uInt16[]> pMetrics = GetTTSimpleGlyphMetrics( pTTFont,
+                                                                  pGID,
+                                                                  nGlyphs,
+                                                                  false/*bVertical*/ );
+        if( pMetrics )
+        {
+            for( int i = 0; i < nGlyphs; i++ )
+                pWidths[pOldIndex[i]] = pMetrics[i];
+            pMetrics.reset();
+        }
+        else
+        {
+            CloseTTFont( pTTFont );
+            return false;
+        }
     }
 
     bool bSuccess = ( SFErrCodes::Ok == CreateTTFromTTGlyphs( pTTFont,