[PATCH] Get rid of SalLayout::GetFallbackFontData()

Hi,

I have submitted a patch for review:

    https://gerrit.libreoffice.org/4285

To pull it, you can do:

    git pull ssh://gerrit.libreoffice.org:29418/core refs/changes/85/4285/1

Get rid of SalLayout::GetFallbackFontData()

This is only used in conjunction with SalLayout::GetNextGlyphs() in
vcl/source/gdi/pdfwriter_impl.cxx to retrieve the fallback font, if any,
used to layout the given glyph, but it is a very convoluted way to do a
straight forward thing, and hard to adapt for new SalLayout
implementations.

So now I just pass a fallback fonts array, when requested, in
GetNextGlyphs() itself.

Change-Id: I24e7931f64867a4fb4e7b728c65faa6198e24aba
---
M vcl/aqua/source/gdi/atsui/salatslayout.cxx
M vcl/coretext/ctlayout.cxx
M vcl/inc/graphite_layout.hxx
M vcl/inc/graphite_serverfont.hxx
M vcl/inc/sallayout.hxx
M vcl/source/gdi/pdfwriter_impl.cxx
M vcl/source/gdi/sallayout.cxx
M vcl/source/glyphs/graphite_layout.cxx
M vcl/win/source/gdi/winlayout.cxx
9 files changed, 43 insertions(+), 77 deletions(-)



diff --git a/vcl/aqua/source/gdi/atsui/salatslayout.cxx b/vcl/aqua/source/gdi/atsui/salatslayout.cxx
index 1b9776e..fd080f6 100644
--- a/vcl/aqua/source/gdi/atsui/salatslayout.cxx
+++ b/vcl/aqua/source/gdi/atsui/salatslayout.cxx
@@ -56,8 +56,6 @@
     virtual void    GetCaretPositions( int nArraySize, sal_Int32* pCaretXArray ) const;
     virtual bool    GetBoundRect( SalGraphics&, Rectangle& ) const;
 
-    const PhysicalFontFace* GetFallbackFontData( sal_GlyphId ) const;
-
     virtual void    InitFont() const;
     virtual void    MoveGlyph( int nStart, long nNewXPos );
     virtual void    DropGlyph( int nStart );
@@ -505,7 +503,8 @@
  * @return : number of glyph details that were provided
 **/
 int ATSLayout::GetNextGlyphs( int nLen, sal_GlyphId* pGlyphIDs, Point& rPos, int& nStart,
-    sal_Int32* pGlyphAdvances, int* pCharIndexes ) const
+    sal_Int32* pGlyphAdvances, int* pCharIndexes,
+    const PhysicalFontFace** pFallbackFonts ) const
 {
     if( nStart < 0 )                // first glyph requested?
         nStart = 0;
@@ -582,6 +581,9 @@
             const int nLevel = mpFallbackInfo->AddFallback( nFallbackFontID );
                // update sal_GlyphId with fallback level
             nGlyphId |= (nLevel << GF_FONTSHIFT);
+
+            if( pFallbackFonts )
+                *(pFallbackFonts++) = mpFallbackInfo->GetFallbackFontData( nLevel );
         }
 
         // update resulting glyphid array
@@ -1183,20 +1185,6 @@
 void ATSLayout::MoveGlyph( int /*nStart*/, long /*nNewXPos*/ ) {}
 void ATSLayout::DropGlyph( int /*nStart*/ ) {}
 void ATSLayout::Simplify( bool /*bIsBase*/ ) {}
-
-// get the PhysicalFontFace for a glyph fallback font
-// for a glyphid that was returned by ATSLayout::GetNextGlyphs()
-const PhysicalFontFace* ATSLayout::GetFallbackFontData( sal_GlyphId nGlyphId ) const
-{
-    // check if any fallback fonts were needed
-    if( !mpFallbackInfo )
-        return NULL;
-    // check if the current glyph needs a fallback font
-    int nFallbackLevel = (nGlyphId & GF_FONTMASK) >> GF_FONTSHIFT;
-    if( !nFallbackLevel )
-        return NULL;
-    return mpFallbackInfo->GetFallbackFontData( nFallbackLevel );
-}
 
 // =======================================================================
 
diff --git a/vcl/coretext/ctlayout.cxx b/vcl/coretext/ctlayout.cxx
index a5915d9..89ecf09 100644
--- a/vcl/coretext/ctlayout.cxx
+++ b/vcl/coretext/ctlayout.cxx
@@ -35,7 +35,8 @@
     virtual void    DrawText( SalGraphics& ) const;
 
     virtual int     GetNextGlyphs( int nLen, sal_GlyphId* pGlyphs, Point& rPos, int&,
-                        sal_Int32* pGlyphAdvances, int* pCharIndexes ) const;
+                        sal_Int32* pGlyphAdvances, int* pCharIndexes,
+                        const PhysicalFontFace** pFallbackFonts ) const;
 
     virtual long    GetTextWidth() const;
     virtual long    FillDXArray( sal_Int32* pDXArray ) const;
@@ -43,8 +44,6 @@
     virtual void    GetCaretPositions( int nArraySize, sal_Int32* pCaretXArray ) const;
     virtual bool    GetGlyphOutlines( SalGraphics&, PolyPolyVector& ) const;
     virtual bool    GetBoundRect( SalGraphics&, Rectangle& ) const;
-
-    const PhysicalFontFace* GetFallbackFontData( sal_GlyphId ) const;
 
     virtual void    InitFont( void) const;
     virtual void    MoveGlyph( int nStart, long nNewXPos );
@@ -224,7 +223,8 @@
 // -----------------------------------------------------------------------
 
 int CTLayout::GetNextGlyphs( int nLen, sal_GlyphId* pGlyphIDs, Point& rPos, int& nStart,
-    sal_Int32* pGlyphAdvances, int* pCharIndexes ) const
+    sal_Int32* pGlyphAdvances, int* pCharIndexes,
+    const PhysicalFontFace** pFallbackFonts ) const
 {
     if( !mpCTLine )
         return 0;
@@ -293,6 +293,8 @@
             }
         }
 
+        const PhysicalFontFace* pFallbackFont = NULL;
+
         // get the details for each interesting glyph
         // TODO: handle nLen>1
         for(; (--nLen >= 0) && (nSubIndex < nGlyphsInRun); ++nSubIndex, ++nStart )
@@ -303,6 +305,8 @@
                 *(pGlyphAdvances++) = pCGGlyphAdvs[ nSubIndex ].width;
             if( pCharIndexes )
                 *(pCharIndexes++) = pCGGlyphStrIdx[ nSubIndex] + mnMinCharPos;
+            if( pFallbackFonts )
+                *(pFallbackFonts++) = pFallbackFont;
             if( !nCount++ ) {
                 const CGPoint& rCurPos = pCGGlyphPos[ nSubIndex ];
                 rPos = GetDrawPosition( Point( mfFontScale * rCurPos.x, mfFontScale * rCurPos.y) );
@@ -447,26 +451,6 @@
 void CTLayout::MoveGlyph( int /*nStart*/, long /*nNewXPos*/ ) {}
 void CTLayout::DropGlyph( int /*nStart*/ ) {}
 void CTLayout::Simplify( bool /*bIsBase*/ ) {}
-
-// get the PhysicalFontFace for a glyph fallback font
-// for a glyphid that was returned by CTLayout::GetNextGlyphs()
-const PhysicalFontFace* CTLayout::GetFallbackFontData( sal_GlyphId /*nGlyphId*/ ) const
-{
-#if 0
-    // check if any fallback fonts were needed
-    if( !mpFallbackInfo )
-        return NULL;
-    // check if the current glyph needs a fallback font
-    int nFallbackLevel = (nGlyphId & GF_FONTMASK) >> GF_FONTSHIFT;
-    if( !nFallbackLevel )
-        return NULL;
-    pFallbackFont = mpFallbackInfo->GetFallbackFontData( nFallbackLevel );
-#else
-    // let CoreText's font cascading handle glyph fallback
-    const PhysicalFontFace* pFallbackFont = NULL;
-#endif
-    return pFallbackFont;
-}
 
 // =======================================================================
 
diff --git a/vcl/inc/graphite_layout.hxx b/vcl/inc/graphite_layout.hxx
index bbf6f15..b810834 100644
--- a/vcl/inc/graphite_layout.hxx
+++ b/vcl/inc/graphite_layout.hxx
@@ -127,7 +127,8 @@
 
     // methods using glyph indexing
     virtual int   GetNextGlyphs(int nLen, sal_GlyphId* pGlyphIdxAry, ::Point & rPos, int&,
-            sal_Int32* pGlyphAdvAry = 0, int* pCharPosAry = 0 ) const;
+            sal_Int32* pGlyphAdvAry = NULL, int* pCharPosAry = NULL,
+            const PhysicalFontFace** pFallbackFonts = NULL ) const;
 
     // used by glyph+font+script fallback
     virtual void    MoveGlyph( int nStart, long nNewXPos );
diff --git a/vcl/inc/graphite_serverfont.hxx b/vcl/inc/graphite_serverfont.hxx
index a0a3890..cfd2cd6 100644
--- a/vcl/inc/graphite_serverfont.hxx
+++ b/vcl/inc/graphite_serverfont.hxx
@@ -73,11 +73,12 @@
 
         // used by display layers
         virtual int     GetNextGlyphs( int l, sal_GlyphId* gia, Point& p, int& s,
-                        sal_Int32* gaa = NULL, int* cpa = NULL ) const
+                        sal_Int32* gaa = NULL, int* cpa = NULL,
+                        const PhysicalFontFace** pFallbackFonts = NULL ) const
         {
             maImpl.DrawBase() = maDrawBase;
             maImpl.DrawOffset() = maDrawOffset;
-            return maImpl.GetNextGlyphs(l, gia, p, s, gaa, cpa);
+            return maImpl.GetNextGlyphs(l, gia, p, s, gaa, cpa, pFallbackFonts);
         }
 
         virtual void    MoveGlyph( int nStart, long nNewXPos ) { maImpl.MoveGlyph(nStart, 
nNewXPos); };
diff --git a/vcl/inc/sallayout.hxx b/vcl/inc/sallayout.hxx
index 6b25f5b..491db056 100644
--- a/vcl/inc/sallayout.hxx
+++ b/vcl/inc/sallayout.hxx
@@ -203,8 +203,6 @@
     int             GetUnitsPerPixel() const                { return mnUnitsPerPixel; }
     int             GetOrientation() const                  { return mnOrientation; }
 
-    virtual const PhysicalFontFace* GetFallbackFontData( sal_GlyphId ) const;
-
     // methods using string indexing
     virtual int     GetTextBreak( long nMaxWidth, long nCharExtra=0, int nFactor=1 ) const = 0;
     virtual long    FillDXArray( sal_Int32* pDXArray ) const = 0;
@@ -214,7 +212,8 @@
 
     // methods using glyph indexing
     virtual int     GetNextGlyphs( int nLen, sal_GlyphId* pGlyphIdAry, Point& rPos, int&,
-                        sal_Int32* pGlyphAdvAry = NULL, int* pCharPosAry = NULL ) const = 0;
+                        sal_Int32* pGlyphAdvAry = NULL, int* pCharPosAry = NULL,
+                        const PhysicalFontFace** pFallbackFonts = NULL ) const = 0;
     virtual bool    GetOutline( SalGraphics&, ::basegfx::B2DPolyPolygonVector& ) const;
     virtual bool    GetBoundRect( SalGraphics&, Rectangle& ) const;
 
@@ -267,7 +266,8 @@
     virtual long    FillDXArray( sal_Int32* pDXArray ) const;
     virtual void    GetCaretPositions( int nArraySize, sal_Int32* pCaretXArray ) const;
     virtual int     GetNextGlyphs( int nLen, sal_GlyphId* pGlyphIdxAry, Point& rPos,
-                                   int&, sal_Int32* pGlyphAdvAry, int* pCharPosAry ) const;
+                                   int&, sal_Int32* pGlyphAdvAry, int* pCharPosAry,
+                                   const PhysicalFontFace** pFallbackFonts ) const;
     virtual bool    GetOutline( SalGraphics&, ::basegfx::B2DPolyPolygonVector& ) const;
 
     // used only by OutputDevice::ImplLayout, TODO: make friend
@@ -278,8 +278,6 @@
     virtual bool    LayoutText( ImplLayoutArgs& );
     virtual void    AdjustLayout( ImplLayoutArgs& );
     virtual void    InitFont() const;
-
-    virtual const PhysicalFontFace* GetFallbackFontData( sal_GlyphId ) const;
 
     void SetInComplete(bool bInComplete = true);
 
@@ -365,7 +363,8 @@
 
     // used by display layers
     virtual int     GetNextGlyphs( int nLen, sal_GlyphId* pGlyphIdxAry, Point& rPos, int&,
-                        sal_Int32* pGlyphAdvAry = NULL, int* pCharPosAry = NULL ) const;
+                        sal_Int32* pGlyphAdvAry = NULL, int* pCharPosAry = NULL,
+                        const PhysicalFontFace** pFallbackFonts = NULL ) const;
 
 protected:
                     GenericSalLayout();
diff --git a/vcl/source/gdi/pdfwriter_impl.cxx b/vcl/source/gdi/pdfwriter_impl.cxx
index ed69f76..7eedb3a 100644
--- a/vcl/source/gdi/pdfwriter_impl.cxx
+++ b/vcl/source/gdi/pdfwriter_impl.cxx
@@ -7621,7 +7621,7 @@
     sal_Int32 pUnicodesPerGlyph[nMaxGlyphs];
     int pCharPosAry[nMaxGlyphs];
     sal_Int32 nAdvanceWidths[nMaxGlyphs];
-    const PhysicalFontFace* pFallbackFonts[nMaxGlyphs];
+    const PhysicalFontFace* pFallbackFonts[nMaxGlyphs] = { NULL };
     bool bVertical = m_aCurrentPDFState.m_aFont.IsVertical();
     int nGlyphs;
     int nIndex = 0;
@@ -7749,13 +7749,11 @@
     aGlyphs.reserve( nTmpMaxGlyphs );
     // first get all the glyphs and register them; coordinates still in Pixel
     Point aGNGlyphPos;
-    while( (nGlyphs = rLayout.GetNextGlyphs( nTmpMaxGlyphs, pGlyphs, aGNGlyphPos, nIndex, 
nAdvanceWidths, pCharPosAry )) != 0 )
+    while( (nGlyphs = rLayout.GetNextGlyphs( nTmpMaxGlyphs, pGlyphs, aGNGlyphPos, nIndex, 
nAdvanceWidths, pCharPosAry, pFallbackFonts )) != 0 )
     {
         aUnicodes.clear();
         for( int i = 0; i < nGlyphs; i++ )
         {
-            pFallbackFonts[i] = rLayout.GetFallbackFontData( pGlyphs[i] );
-
             // default case: 1 glyph is one unicode
             pUnicodesPerGlyph[i] = 1;
             if( (pGlyphs[i] & GF_ISCHAR) )
diff --git a/vcl/source/gdi/sallayout.cxx b/vcl/source/gdi/sallayout.cxx
index 4be0ce7..4df3123 100644
--- a/vcl/source/gdi/sallayout.cxx
+++ b/vcl/source/gdi/sallayout.cxx
@@ -866,13 +866,6 @@
     return bRet;
 }
 
-// -----------------------------------------------------------------------
-
-const PhysicalFontFace* SalLayout::GetFallbackFontData( sal_GlyphId /*nGlyphId*/ ) const
-{
-    return NULL;
-}
-
 // =======================================================================
 
 GenericSalLayout::GenericSalLayout()
@@ -1353,7 +1346,8 @@
 // -----------------------------------------------------------------------
 
 int GenericSalLayout::GetNextGlyphs( int nLen, sal_GlyphId* pGlyphs, Point& rPos,
-    int& nStart, sal_Int32* pGlyphAdvAry, int* pCharPosAry ) const
+    int& nStart, sal_Int32* pGlyphAdvAry, int* pCharPosAry,
+    const PhysicalFontFace** /*pFallbackFonts*/ ) const
 {
     GlyphVector::const_iterator pG = m_GlyphItems.begin();
     GlyphVector::const_iterator pGEnd = m_GlyphItems.end();
@@ -1972,14 +1966,6 @@
 
 // -----------------------------------------------------------------------
 
-const PhysicalFontFace* MultiSalLayout::GetFallbackFontData( sal_GlyphId nGlyphId ) const
-{
-    int nFallbackLevel = (nGlyphId & GF_FONTMASK) >> GF_FONTSHIFT;
-    return mpFallbackFonts[ nFallbackLevel ];
-}
-
-// -----------------------------------------------------------------------
-
 void MultiSalLayout::DrawText( SalGraphics& rGraphics ) const
 {
     for( int i = mnLevel; --i >= 0; )
@@ -2111,7 +2097,8 @@
 // -----------------------------------------------------------------------
 
 int MultiSalLayout::GetNextGlyphs( int nLen, sal_GlyphId* pGlyphIdxAry, Point& rPos,
-    int& nStart, sal_Int32* pGlyphAdvAry, int* pCharPosAry ) const
+    int& nStart, sal_Int32* pGlyphAdvAry, int* pCharPosAry,
+    const PhysicalFontFace** pFallbackFonts ) const
 {
     // for multi-level fallback only single glyphs should be used
     if( mnLevel > 1 && nLen > 1 )
@@ -2141,6 +2128,10 @@
                     pGlyphAdvAry[i] = w;
                 }
                 pGlyphIdxAry[ i ] |= nFontTag;
+                if( pFallbackFonts )
+                {
+                    pFallbackFonts[ i ] =  mpFallbackFonts[ nLevel ];
+                }
             }
             rPos += maDrawBase;
             rPos += maDrawOffset;
diff --git a/vcl/source/glyphs/graphite_layout.cxx b/vcl/source/glyphs/graphite_layout.cxx
index 3898ab1..034842a 100644
--- a/vcl/source/glyphs/graphite_layout.cxx
+++ b/vcl/source/glyphs/graphite_layout.cxx
@@ -1209,7 +1209,8 @@
 // The logic in this method must match that expected in MultiSalLayout which
 // is used when glyph fallback is in operation.
 int GraphiteLayout::GetNextGlyphs( int length, sal_GlyphId * glyph_out,
-        ::Point & aPosOut, int &glyph_slot, sal_Int32 * glyph_adv, int *char_index) const
+        ::Point & aPosOut, int &glyph_slot, sal_Int32 * glyph_adv, int *char_index,
+        const PhysicalFontFace** /*pFallbackFonts*/ ) const
 {
   // Sanity check on the slot index.
   if (glyph_slot >= signed(mvGlyphs.size()))
diff --git a/vcl/win/source/gdi/winlayout.cxx b/vcl/win/source/gdi/winlayout.cxx
index fac3d03..3b5c4c4 100644
--- a/vcl/win/source/gdi/winlayout.cxx
+++ b/vcl/win/source/gdi/winlayout.cxx
@@ -532,7 +532,8 @@
 // -----------------------------------------------------------------------
 
 int SimpleWinLayout::GetNextGlyphs( int nLen, sal_GlyphId* pGlyphs, Point& rPos, int& nStart,
-    long* pGlyphAdvances, int* pCharIndexes ) const
+    long* pGlyphAdvances, int* pCharIndexes,
+    const PhysicalFontFace** /*pFallbackFonts*/ ) const
 {
     // return zero if no more glyph found
     if( nStart >= mnGlyphCount )
@@ -1631,7 +1632,8 @@
 // -----------------------------------------------------------------------
 
 int UniscribeLayout::GetNextGlyphs( int nLen, sal_GlyphId* pGlyphs, Point& rPos,
-    int& nStartx8, sal_Int32* pGlyphAdvances, int* pCharPosAry ) const
+    int& nStartx8, sal_Int32* pGlyphAdvances, int* pCharPosAry,
+    const PhysicalFontFace** /*pFallbackFonts*/ ) const
 {
     // HACK to allow fake-glyph insertion (e.g. for kashidas)
     // TODO: use iterator idiom instead of GetNextGlyphs(...)
@@ -2858,11 +2860,12 @@
 }
 
 int GraphiteWinLayout::GetNextGlyphs( int length, sal_GlyphId* glyph_out,
-        ::Point & pos_out, int &glyph_slot, long * glyph_adv, int *char_index) const
+        ::Point & pos_out, int &glyph_slot, long * glyph_adv, int *char_index,
+        const PhysicalFontFace** pFallbackFonts ) const
 {
     maImpl.DrawBase() = WinLayout::maDrawBase;
     maImpl.DrawOffset() = WinLayout::maDrawOffset;
-    return maImpl.GetNextGlyphs(length, glyph_out, pos_out, glyph_slot, glyph_adv, char_index);
+    return maImpl.GetNextGlyphs(length, glyph_out, pos_out, glyph_slot, glyph_adv, char_index, 
pFallbackFonts);
 }
 
 void GraphiteWinLayout::MoveGlyph( int glyph_idx, long new_x_pos )

-- 
To view, visit https://gerrit.libreoffice.org/4285
To unsubscribe, visit https://gerrit.libreoffice.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I24e7931f64867a4fb4e7b728c65faa6198e24aba
Gerrit-PatchSet: 1
Gerrit-Project: core
Gerrit-Branch: master
Gerrit-Owner: Khaled Hosny <khaledhosny@eglug.org>