[Libreoffice-commits] core.git: 9 commits - vcl/generic vcl/inc

Khaled Hosny khaledhosny at eglug.org
Mon Apr 29 03:46:31 PDT 2013


 vcl/generic/glyphs/gcach_layout.cxx |  280 +++++++++++++++++++++++-------------
 vcl/inc/generic/glyphcache.hxx      |   11 +
 vcl/inc/sallayout.hxx               |    3 
 3 files changed, 198 insertions(+), 96 deletions(-)

New commits:
commit c559b13b80ab4c8ca4e6ca713bce388ea650e02c
Author: Khaled Hosny <khaledhosny at eglug.org>
Date:   Sun Apr 28 15:56:26 2013 +0200

    [harfbuzz] Re-enable text justification
    
    It turned out that ApplyDXArray() is need to apply advance width
    adjustments after justification, so we can't just bypass it.
    
    So I just copied GenericSalLayout::ApplyDXArray() and stripped it of
    ICUism so it does not break with HarfBuzz, but I had to make
    m_GlyphItems non-private, so I'm not sure this is the right approach.
    
    Change-Id: I66d647c3590fdf912c39d0cf23ac72bcc7ca72c9

diff --git a/vcl/generic/glyphs/gcach_layout.cxx b/vcl/generic/glyphs/gcach_layout.cxx
index f147ab2..abd1ab8 100644
--- a/vcl/generic/glyphs/gcach_layout.cxx
+++ b/vcl/generic/glyphs/gcach_layout.cxx
@@ -106,13 +106,85 @@ void ServerFontLayout::AdjustLayout( ImplLayoutArgs& rArgs )
     }
 }
 
+// apply adjustments to glyph advances, e.g. as a result of justification.
 void ServerFontLayout::ApplyDXArray(ImplLayoutArgs& rArgs)
 {
-    // No idea what issue ApplyDXArray() was supposed to fix, but whatever
-    // GenericSalLayout::ApplyDXArray() does it just breaks our perfectly
-    // positioned text.
-    if (!bUseHarfBuzz)
+    if (bUseHarfBuzz)
+    {
+        if (m_GlyphItems.empty())
+            return;
+
+        // determine cluster boundaries and x base offset
+        const int nCharCount = rArgs.mnEndCharPos - rArgs.mnMinCharPos;
+        int* pLogCluster = (int*)alloca(nCharCount * sizeof(int));
+        size_t i;
+        int n,p;
+        long nBasePointX = -1;
+        if (mnLayoutFlags & SAL_LAYOUT_FOR_FALLBACK)
+            nBasePointX = 0;
+        for (p = 0; p < nCharCount; ++p)
+            pLogCluster[p] = -1;
+
+        for (i = 0; i < m_GlyphItems.size(); ++i)
+        {
+            n = m_GlyphItems[i].mnCharPos - rArgs.mnMinCharPos;
+            if ((n < 0) || (nCharCount <= n))
+                continue;
+            if (pLogCluster[n] < 0)
+                pLogCluster[n] = i;
+            if (nBasePointX < 0)
+                nBasePointX = m_GlyphItems[i].maLinearPos.X();
+        }
+        // retarget unresolved pLogCluster[n] to a glyph inside the cluster
+        // TODO: better do it while the deleted-glyph markers are still there
+        for (n = 0; n < nCharCount; ++n)
+            if ((p = pLogCluster[0]) >= 0)
+                break;
+        if (n >= nCharCount)
+            return;
+        for (n = 0; n < nCharCount; ++n)
+        {
+            if (pLogCluster[n] < 0)
+                pLogCluster[n] = p;
+            else
+                p = pLogCluster[n];
+        }
+
+        // calculate adjusted cluster widths
+        sal_Int32* pNewGlyphWidths = (sal_Int32*)alloca(m_GlyphItems.size() * sizeof(sal_Int32));
+        for (i = 0; i < m_GlyphItems.size(); ++i)
+            pNewGlyphWidths[i] = 0;
+
+        bool bRTL;
+        for (int nCharPos = p = -1; rArgs.GetNextPos(&nCharPos, &bRTL); )
+        {
+            n = nCharPos - rArgs.mnMinCharPos;
+            if ((n < 0) || (nCharCount <= n))  continue;
+
+            if (pLogCluster[n] >= 0)
+                p = pLogCluster[n];
+            if (p >= 0)
+            {
+                long nDelta = rArgs.mpDXArray[n];
+                if(n > 0)
+                    nDelta -= rArgs.mpDXArray[n - 1];
+                pNewGlyphWidths[p] += nDelta * mnUnitsPerPixel;
+            }
+        }
+
+        // move cluster positions using the adjusted widths
+        long nDelta = 0;
+        for (i = 0; i < m_GlyphItems.size(); ++i)
+        {
+            nDelta += pNewGlyphWidths[i] - m_GlyphItems[i].mnNewWidth;
+            m_GlyphItems[i].mnNewWidth = pNewGlyphWidths[i];
+            m_GlyphItems[i].maLinearPos.X() += nDelta;
+        }
+    }
+    else
+    {
         GenericSalLayout::ApplyDXArray(rArgs);
+    }
 }
 
 // =======================================================================
diff --git a/vcl/inc/sallayout.hxx b/vcl/inc/sallayout.hxx
index ad6cd94..5d99acb 100644
--- a/vcl/inc/sallayout.hxx
+++ b/vcl/inc/sallayout.hxx
@@ -367,8 +367,9 @@ protected:
 
     bool            GetCharWidths( sal_Int32* pCharWidths ) const;
 
-private:
     GlyphVector     m_GlyphItems;
+
+private:
     mutable Point   maBasePoint;
 
     // enforce proper copy semantic
commit ca0b580e2a21999d75bcd39ae5b6f81e9e42897b
Author: Khaled Hosny <khaledhosny at eglug.org>
Date:   Sun Apr 28 23:06:45 2013 +0200

    [harfbuzz] Fix shaping across text runs
    
    The 3rd parameter to hb_buffer_add_utf() should be the length of the
    whole text not the current run, so that HarfBuzz can take the context
    into account when shaping.
    
    Change-Id: I9e4e928d40078c3e3667cfdb6d8f24bf6e58263d

diff --git a/vcl/generic/glyphs/gcach_layout.cxx b/vcl/generic/glyphs/gcach_layout.cxx
index 496f2f2..f147ab2 100644
--- a/vcl/generic/glyphs/gcach_layout.cxx
+++ b/vcl/generic/glyphs/gcach_layout.cxx
@@ -350,8 +350,10 @@ bool HbLayoutEngine::layout(ServerFontLayout& rLayout, ImplLayoutArgs& rArgs)
             ((uint64_t) aFtFace->size->metrics.y_scale * (uint64_t) fUnitsPerEM) >> 16);
     hb_font_set_ppem(pHbFont, aFtFace->size->metrics.x_ppem, aFtFace->size->metrics.y_ppem);
 
+    int nTextLen = rArgs.mnEndCharPos - rArgs.mnMinCharPos;
+
     // allocate temporary arrays, note: round to even
-    int nGlyphCapacity = (3 * (rArgs.mnEndCharPos - rArgs.mnMinCharPos) | 15) + 1;
+    int nGlyphCapacity = (3 * nTextLen | 15) + 1;
 
     rLayout.Reserve(nGlyphCapacity);
 
@@ -393,7 +395,7 @@ bool HbLayoutEngine::layout(ServerFontLayout& rLayout, ImplLayoutArgs& rArgs)
         hb_buffer_set_direction(pHbBuffer, bRightToLeft ? HB_DIRECTION_RTL: HB_DIRECTION_LTR);
         hb_buffer_set_script(pHbBuffer, hb_icu_script_to_script(eScriptCode));
         hb_buffer_set_language(pHbBuffer, hb_language_from_string(sLanguage.getStr(), -1));
-        hb_buffer_add_utf16(pHbBuffer, rArgs.mpStr, nRunLen, nMinRunPos, nRunLen);
+        hb_buffer_add_utf16(pHbBuffer, rArgs.mpStr, nTextLen, nMinRunPos, nRunLen);
         hb_shape(pHbFont, pHbBuffer, NULL, 0);
 
         int nRunGlyphCount = hb_buffer_get_length(pHbBuffer);
@@ -412,12 +414,14 @@ bool HbLayoutEngine::layout(ServerFontLayout& rLayout, ImplLayoutArgs& rArgs)
                 if (nCharPos >= 0)
                 {
                     rArgs.NeedFallback(nCharPos, bRightToLeft);
+#if 0
                     // XXX: do we need this? HarfBuzz can take context into
                     // account when shaping
                     if  ((nCharPos > 0) && needPreviousCode(rArgs.mpStr[nCharPos-1]))
                         rArgs.NeedFallback(nCharPos-1, bRightToLeft);
                     else if  ((nCharPos + 1 < nEndRunPos) && needNextCode(rArgs.mpStr[nCharPos+1]))
                         rArgs.NeedFallback(nCharPos+1, bRightToLeft);
+#endif
                 }
 
                 if (SAL_LAYOUT_FOR_FALLBACK & rArgs.mnFlags)
commit 1f7d7d0f327aff54af8f9fa8b60e2460827dc4e1
Author: Khaled Hosny <khaledhosny at eglug.org>
Date:   Sun Apr 28 13:18:42 2013 +0200

    [harfbuzz] Fix text width calculation for real
    
    No more second guessing if text width, we know that information already,
    so pass it around instead of trying to re-create it.
    
    Change-Id: I19faacbc309d38753c3c9f7214dfa0bf59cc66b5

diff --git a/vcl/generic/glyphs/gcach_layout.cxx b/vcl/generic/glyphs/gcach_layout.cxx
index 856f8b4..496f2f2 100644
--- a/vcl/generic/glyphs/gcach_layout.cxx
+++ b/vcl/generic/glyphs/gcach_layout.cxx
@@ -76,33 +76,9 @@ long ServerFontLayout::GetTextWidth() const
 {
     long nWidth;
     if (bUseHarfBuzz)
-    {
-        GlyphVector aGlyphItems = GenericSalLayout::GetGlyphItems();
-
-        if( aGlyphItems.empty() )
-            return 0;
-
-        // initialize the extent
-        long nMinPos = 0;
-        long nMaxPos = 0;
-
-        for( GlyphVector::const_iterator pG = aGlyphItems.begin(), end = aGlyphItems.end(); pG != end ; ++pG )
-        {
-            // update the text extent with the glyph extent
-            long nXPos = pG->maLinearPos.X();
-            if( nMinPos > nXPos )
-                nMinPos = nXPos;
-            nXPos += pG->mnOrigWidth;
-            if( nMaxPos < nXPos )
-                nMaxPos = nXPos;
-        }
-
-        nWidth = nMaxPos - nMinPos;
-    }
+        nWidth = GetWidth();
     else
-    {
         nWidth = GenericSalLayout::GetTextWidth();
-    }
 
     return nWidth;
 }
@@ -448,9 +424,6 @@ bool HbLayoutEngine::layout(ServerFontLayout& rLayout, ImplLayoutArgs& rArgs)
                     continue;
             }
 
-            const GlyphMetric& rGM = rFont.GetGlyphMetric(nGlyphIndex);
-            int nGlyphWidth = rGM.GetCharWidth();
-
             long nGlyphFlags = 0;
             if (bRightToLeft)
                 nGlyphFlags |= GlyphItem::IS_RTL_GLYPH;
@@ -473,10 +446,7 @@ bool HbLayoutEngine::layout(ServerFontLayout& rLayout, ImplLayoutArgs& rArgs)
             int32_t nYAdvance = pHbPositions[i].y_advance >> 6;
 
             Point aNewPos = Point(aCurrPos.X() + nXOffset, -(aCurrPos.Y() + nYOffset));
-
-            GlyphItem aGI(nCharPos, nGlyphIndex, aNewPos, nGlyphFlags, nGlyphWidth);
-            aGI.mnNewWidth = nXAdvance;
-
+            const GlyphItem aGI(nCharPos, nGlyphIndex, aNewPos, nGlyphFlags, nXAdvance);
             rLayout.AppendGlyph(aGI);
 
             aCurrPos.X() += nXAdvance;
@@ -485,6 +455,7 @@ bool HbLayoutEngine::layout(ServerFontLayout& rLayout, ImplLayoutArgs& rArgs)
 
         hb_buffer_destroy(pHbBuffer);
     }
+    rLayout.SetWidth(aCurrPos.X());
 
     hb_font_destroy(pHbFont);
 
diff --git a/vcl/inc/generic/glyphcache.hxx b/vcl/inc/generic/glyphcache.hxx
index 2225971..cb4ffce 100644
--- a/vcl/inc/generic/glyphcache.hxx
+++ b/vcl/inc/generic/glyphcache.hxx
@@ -319,6 +319,7 @@ private:
     SAL_DLLPRIVATE  ServerFontLayout& operator=( const ServerFontLayout& );
 
     bool            bUseHarfBuzz;
+    long            mnTextWidth;
 
 public:
                     ServerFontLayout( ServerFont& );
@@ -328,6 +329,10 @@ public:
     virtual void    DrawText( SalGraphics& ) const;
     virtual long    GetTextWidth() const;
     ServerFont&     GetServerFont() const   { return mrServerFont; }
+
+    // used by layout engine
+    void            SetWidth( long nWidth ) { mnTextWidth = nWidth; }
+    long            GetWidth() const { return mnTextWidth; }
 };
 
 // =======================================================================
diff --git a/vcl/inc/sallayout.hxx b/vcl/inc/sallayout.hxx
index 6f8a49d..ad6cd94 100644
--- a/vcl/inc/sallayout.hxx
+++ b/vcl/inc/sallayout.hxx
@@ -367,8 +367,6 @@ protected:
 
     bool            GetCharWidths( sal_Int32* pCharWidths ) const;
 
-    GlyphVector     GetGlyphItems() const { return m_GlyphItems; }
-
 private:
     GlyphVector     m_GlyphItems;
     mutable Point   maBasePoint;
commit 2e302f18efe8f2e5f3c17d7734a8adcb670fbe42
Author: Khaled Hosny <khaledhosny at eglug.org>
Date:   Sat Apr 27 18:18:51 2013 +0200

    [harfbuzz] Follow variable naming conventions
    
    Not that I'm a fan of Hungarian notation, but for the sake of
    consistency. Fix some placement of opening braces along the way.
    
    Change-Id: Id6ea758fd438a4040e7451430a0f3a166efdec43

diff --git a/vcl/generic/glyphs/gcach_layout.cxx b/vcl/generic/glyphs/gcach_layout.cxx
index b4137bb..856f8b4 100644
--- a/vcl/generic/glyphs/gcach_layout.cxx
+++ b/vcl/generic/glyphs/gcach_layout.cxx
@@ -157,7 +157,7 @@ static bool needNextCode(sal_Unicode cChar)
 }
 
 #if ENABLE_HARFBUZZ
-static hb_blob_t *getFontTable(hb_face_t* /*face*/, hb_tag_t nTableTag, void* userData)
+static hb_blob_t *getFontTable(hb_face_t* /*face*/, hb_tag_t nTableTag, void* pUserData)
 {
     char pTagName[5];
     pTagName[0] = (char)(nTableTag >> 24);
@@ -166,9 +166,9 @@ static hb_blob_t *getFontTable(hb_face_t* /*face*/, hb_tag_t nTableTag, void* us
     pTagName[3] = (char)(nTableTag);
     pTagName[4] = 0;
 
-    ServerFont* rFont = (ServerFont*) userData;
+    ServerFont* pFont = (ServerFont*) pUserData;
     sal_uLong nLength;
-    const unsigned char* pBuffer = rFont->GetTable(pTagName, &nLength);
+    const unsigned char* pBuffer = pFont->GetTable(pTagName, &nLength);
 
     hb_blob_t* pBlob = NULL;
     if (pBuffer != NULL)
@@ -177,61 +177,61 @@ static hb_blob_t *getFontTable(hb_face_t* /*face*/, hb_tag_t nTableTag, void* us
     return pBlob;
 }
 
-static hb_bool_t getFontGlyph(hb_font_t* /*font*/, void* fontData,
+static hb_bool_t getFontGlyph(hb_font_t* /*font*/, void* pFontData,
         hb_codepoint_t ch, hb_codepoint_t vs,
         hb_codepoint_t* nGlyphIndex,
-        void* /*userData*/)
+        void* /*pUserData*/)
 {
-    ServerFont* rFont = (ServerFont*) fontData;
+    ServerFont* pFont = (ServerFont*) pFontData;
     *nGlyphIndex = 0;
 
     if (vs)
-        *nGlyphIndex = rFont->GetRawGlyphIndex(ch /*, vs*/); // XXX handle variation selectors
+        *nGlyphIndex = pFont->GetRawGlyphIndex(ch /*, vs*/); // XXX handle variation selectors
 
     if (*nGlyphIndex == 0)
-        *nGlyphIndex = rFont->GetRawGlyphIndex(ch);
+        *nGlyphIndex = pFont->GetRawGlyphIndex(ch);
 
     return *nGlyphIndex != 0;
 }
 
-static hb_position_t getGlyphAdvanceH(hb_font_t* /*font*/, void* fontData,
+static hb_position_t getGlyphAdvanceH(hb_font_t* /*font*/, void* pFontData,
         hb_codepoint_t nGlyphIndex,
-        void* /*userData*/)
+        void* /*pUserData*/)
 {
-    ServerFont* rFont = (ServerFont*) fontData;
-    const GlyphMetric& rGM = rFont->GetGlyphMetric(nGlyphIndex);
+    ServerFont* pFont = (ServerFont*) pFontData;
+    const GlyphMetric& rGM = pFont->GetGlyphMetric(nGlyphIndex);
     return rGM.GetCharWidth() << 6;
 }
 
-static hb_position_t getGlyphAdvanceV(hb_font_t* /*font*/, void* /*fontData*/,
+static hb_position_t getGlyphAdvanceV(hb_font_t* /*font*/, void* /*pFontData*/,
         hb_codepoint_t /*nGlyphIndex*/,
-        void* /*userData*/)
+        void* /*pUserData*/)
 {
     // XXX: vertical metrics
     return 0;
 }
 
-static hb_bool_t getGlyphOriginH(hb_font_t* /*font*/, void* /*fontData*/,
+static hb_bool_t getGlyphOriginH(hb_font_t* /*font*/, void* /*pFontData*/,
         hb_codepoint_t /*nGlyphIndex*/,
         hb_position_t* /*x*/, hb_position_t* /*y*/,
-        void* /*userData*/)
+        void* /*pUserData*/)
 {
     // the horizontal origin is always (0, 0)
     return true;
 }
 
-static hb_bool_t getGlyphOriginV(hb_font_t* /*font*/, void* /*fontData*/,
+static hb_bool_t getGlyphOriginV(hb_font_t* /*font*/, void* /*pFontData*/,
         hb_codepoint_t /*nGlyphIndex*/,
         hb_position_t* /*x*/, hb_position_t* /*y*/,
-        void* /*userData*/)
+        void* /*pUserData*/)
 {
     // XXX: vertical origin
     return true;
 }
 
-static hb_position_t getGlyphKerningH(hb_font_t* /*font*/, void* fontData,
+static hb_position_t getGlyphKerningH(hb_font_t* /*font*/, void* pFontData,
         hb_codepoint_t nGlyphIndex1, hb_codepoint_t nGlyphIndex2,
-        void* /*userData*/)
+        void* /*pUserData*/)
 {
     // This callback is for old style 'kern' table, GPOS kerning is handled by HarfBuzz directly
 
@@ -241,8 +241,8 @@ static hb_position_t getGlyphKerningH(hb_font_t* /*font*/, void* fontData,
     // other implementattions should be removed, don't seem to be used
     // anywhere.
 
-    ServerFont* rFont = (ServerFont*) fontData;
-    FT_Face aFace = rFont->GetFtFace();
+    ServerFont* pFont = (ServerFont*) pFontData;
+    FT_Face aFace = pFont->GetFtFace();
 
     FT_Error error;
     FT_Vector kerning;
@@ -257,48 +257,52 @@ static hb_position_t getGlyphKerningH(hb_font_t* /*font*/, void* fontData,
     return ret;
 }
 
-static hb_position_t getGlyphKerningV(hb_font_t* /*font*/, void* /*fontData*/,
+static hb_position_t getGlyphKerningV(hb_font_t* /*font*/, void* /*pFontData*/,
         hb_codepoint_t /*nGlyphIndex1*/, hb_codepoint_t /*nGlyphIndex2*/,
-        void* /*userData*/)
+        void* /*pUserData*/)
 {
     // XXX vertical kerning
     return 0;
 }
 
-static hb_bool_t getGlyphExtents(hb_font_t* /*font*/, void* fontData,
+static hb_bool_t getGlyphExtents(hb_font_t* /*font*/, void* pFontData,
         hb_codepoint_t nGlyphIndex,
-        hb_glyph_extents_t* extents,
-        void* /*userData*/)
+        hb_glyph_extents_t* pExtents,
+        void* /*pUserData*/)
 {
-    ServerFont* rFont = (ServerFont*) fontData;
-    FT_Face aFace = rFont->GetFtFace();
+    ServerFont* pFont = (ServerFont*) pFontData;
+    FT_Face aFace = pFont->GetFtFace();
     FT_Error error;
 
     error = FT_Load_Glyph(aFace, nGlyphIndex, FT_LOAD_DEFAULT);
-    if (!error) {
-        extents->x_bearing = aFace->glyph->metrics.horiBearingX;
-        extents->y_bearing = aFace->glyph->metrics.horiBearingY;
-        extents->width  =  aFace->glyph->metrics.width;
-        extents->height = -aFace->glyph->metrics.height;
+    if (!error)
+    {
+        pExtents->x_bearing = aFace->glyph->metrics.horiBearingX;
+        pExtents->y_bearing = aFace->glyph->metrics.horiBearingY;
+        pExtents->width  =  aFace->glyph->metrics.width;
+        pExtents->height = -aFace->glyph->metrics.height;
     }
 
     return !error;
 }
 
-static hb_bool_t getGlyphContourPoint(hb_font_t* /*font*/, void* fontData,
+static hb_bool_t getGlyphContourPoint(hb_font_t* /*font*/, void* pFontData,
         hb_codepoint_t nGlyphIndex, unsigned int nPointIndex,
         hb_position_t *x, hb_position_t *y,
-        void* /*userData*/)
+        void* /*pUserData*/)
 {
-    ServerFont* rFont = (ServerFont*) fontData;
-    FT_Face aFace = rFont->GetFtFace();
+    ServerFont* pFont = (ServerFont*) pFontData;
+    FT_Face aFace = pFont->GetFtFace();
     FT_Error error;
     bool ret = false;
 
     error = FT_Load_Glyph(aFace, nGlyphIndex, FT_LOAD_DEFAULT);
-    if (!error) {
-        if (aFace->glyph->format == FT_GLYPH_FORMAT_OUTLINE) {
-            if (nPointIndex < (unsigned int) aFace->glyph->outline.n_points) {
+    if (!error)
+    {
+        if (aFace->glyph->format == FT_GLYPH_FORMAT_OUTLINE)
+        {
+            if (nPointIndex < (unsigned int) aFace->glyph->outline.n_points)
+            {
                 *x = aFace->glyph->outline.points[nPointIndex].x;
                 *y = aFace->glyph->outline.points[nPointIndex].y;
                 ret = true;
@@ -330,7 +334,7 @@ class HbLayoutEngine : public ServerFontLayoutEngine
 {
 private:
     UScriptCode             meScriptCode;
-    hb_face_t*              maHbFace;
+    hb_face_t*              mpHbFace;
     int                     fUnitsPerEM;
 
 public:
@@ -342,20 +346,20 @@ public:
 
 HbLayoutEngine::HbLayoutEngine(ServerFont& rServerFont)
 :   meScriptCode(USCRIPT_INVALID_CODE),
-    maHbFace(NULL),
+    mpHbFace(NULL),
     fUnitsPerEM(0)
 {
     FT_Face aFtFace = rServerFont.GetFtFace();
     fUnitsPerEM = rServerFont.GetEmUnits();
 
-    maHbFace = hb_face_create_for_tables(getFontTable, &rServerFont, NULL);
-    hb_face_set_index(maHbFace, aFtFace->face_index);
-    hb_face_set_upem(maHbFace, fUnitsPerEM);
+    mpHbFace = hb_face_create_for_tables(getFontTable, &rServerFont, NULL);
+    hb_face_set_index(mpHbFace, aFtFace->face_index);
+    hb_face_set_upem(mpHbFace, fUnitsPerEM);
 }
 
 HbLayoutEngine::~HbLayoutEngine()
 {
-    hb_face_destroy(maHbFace);
+    hb_face_destroy(mpHbFace);
 }
 
 bool HbLayoutEngine::layout(ServerFontLayout& rLayout, ImplLayoutArgs& rArgs)
@@ -363,12 +367,12 @@ bool HbLayoutEngine::layout(ServerFontLayout& rLayout, ImplLayoutArgs& rArgs)
     ServerFont& rFont = rLayout.GetServerFont();
     FT_Face aFtFace = rFont.GetFtFace();
 
-    hb_font_t *aHbFont = hb_font_create(maHbFace);
-    hb_font_set_funcs(aHbFont, getFontFuncs(), &rFont, NULL);
-    hb_font_set_scale(aHbFont,
+    hb_font_t *pHbFont = hb_font_create(mpHbFace);
+    hb_font_set_funcs(pHbFont, getFontFuncs(), &rFont, NULL);
+    hb_font_set_scale(pHbFont,
             ((uint64_t) aFtFace->size->metrics.x_scale * (uint64_t) fUnitsPerEM) >> 16,
             ((uint64_t) aFtFace->size->metrics.y_scale * (uint64_t) fUnitsPerEM) >> 16);
-    hb_font_set_ppem(aHbFont, aFtFace->size->metrics.x_ppem, aFtFace->size->metrics.y_ppem);
+    hb_font_set_ppem(pHbFont, aFtFace->size->metrics.x_ppem, aFtFace->size->metrics.y_ppem);
 
     // allocate temporary arrays, note: round to even
     int nGlyphCapacity = (3 * (rArgs.mnEndCharPos - rArgs.mnMinCharPos) | 15) + 1;
@@ -409,21 +413,21 @@ bool HbLayoutEngine::layout(ServerFontLayout& rLayout, ImplLayoutArgs& rArgs)
         LanguageTag aLangTag(rArgs.meLanguage);
         OString sLanguage = OUStringToOString(aLangTag.getLanguage(), RTL_TEXTENCODING_UTF8);
 
-        hb_buffer_t *aHbBuffer = hb_buffer_create();
-        hb_buffer_set_direction(aHbBuffer, bRightToLeft ? HB_DIRECTION_RTL: HB_DIRECTION_LTR);
-        hb_buffer_set_script(aHbBuffer, hb_icu_script_to_script(eScriptCode));
-        hb_buffer_set_language(aHbBuffer, hb_language_from_string(sLanguage.getStr(), -1));
-        hb_buffer_add_utf16(aHbBuffer, rArgs.mpStr, nRunLen, nMinRunPos, nRunLen);
-        hb_shape(aHbFont, aHbBuffer, NULL, 0);
+        hb_buffer_t *pHbBuffer = hb_buffer_create();
+        hb_buffer_set_direction(pHbBuffer, bRightToLeft ? HB_DIRECTION_RTL: HB_DIRECTION_LTR);
+        hb_buffer_set_script(pHbBuffer, hb_icu_script_to_script(eScriptCode));
+        hb_buffer_set_language(pHbBuffer, hb_language_from_string(sLanguage.getStr(), -1));
+        hb_buffer_add_utf16(pHbBuffer, rArgs.mpStr, nRunLen, nMinRunPos, nRunLen);
+        hb_shape(pHbFont, pHbBuffer, NULL, 0);
 
-        int nRunGlyphCount = hb_buffer_get_length(aHbBuffer);
-        hb_glyph_info_t *aHbGlyphInfos = hb_buffer_get_glyph_infos(aHbBuffer, NULL);
-        hb_glyph_position_t *aHbPositions = hb_buffer_get_glyph_positions(aHbBuffer, NULL);
+        int nRunGlyphCount = hb_buffer_get_length(pHbBuffer);
+        hb_glyph_info_t *pHbGlyphInfos = hb_buffer_get_glyph_infos(pHbBuffer, NULL);
+        hb_glyph_position_t *pHbPositions = hb_buffer_get_glyph_positions(pHbBuffer, NULL);
 
         int32_t nLastCluster = -1;
         for (int i = 0; i < nRunGlyphCount; ++i) {
-            int32_t nGlyphIndex = aHbGlyphInfos[i].codepoint;
-            int32_t nCluster = aHbGlyphInfos[i].cluster;
+            int32_t nGlyphIndex = pHbGlyphInfos[i].codepoint;
+            int32_t nCluster = pHbGlyphInfos[i].cluster;
             int32_t nCharPos = nCluster;
 
             // if needed request glyph fallback by updating LayoutArgs
@@ -460,13 +464,13 @@ bool HbLayoutEngine::layout(ServerFontLayout& rLayout, ImplLayoutArgs& rArgs)
             if (bInCluster)
                 nGlyphFlags |= GlyphItem::IS_IN_CLUSTER;
 
-            if (hb_ot_layout_get_glyph_class(maHbFace, nGlyphIndex) == HB_OT_LAYOUT_GLYPH_CLASS_MARK)
+            if (hb_ot_layout_get_glyph_class(mpHbFace, nGlyphIndex) == HB_OT_LAYOUT_GLYPH_CLASS_MARK)
                 nGlyphFlags |= GlyphItem::IS_DIACRITIC;
 
-            int32_t nXOffset =  aHbPositions[i].x_offset >> 6;
-            int32_t nYOffset =  aHbPositions[i].y_offset >> 6;
-            int32_t nXAdvance = aHbPositions[i].x_advance >> 6;
-            int32_t nYAdvance = aHbPositions[i].y_advance >> 6;
+            int32_t nXOffset =  pHbPositions[i].x_offset >> 6;
+            int32_t nYOffset =  pHbPositions[i].y_offset >> 6;
+            int32_t nXAdvance = pHbPositions[i].x_advance >> 6;
+            int32_t nYAdvance = pHbPositions[i].y_advance >> 6;
 
             Point aNewPos = Point(aCurrPos.X() + nXOffset, -(aCurrPos.Y() + nYOffset));
 
@@ -479,10 +483,10 @@ bool HbLayoutEngine::layout(ServerFontLayout& rLayout, ImplLayoutArgs& rArgs)
             aCurrPos.Y() += nYAdvance;
         }
 
-        hb_buffer_destroy(aHbBuffer);
+        hb_buffer_destroy(pHbBuffer);
     }
 
-    hb_font_destroy(aHbFont);
+    hb_font_destroy(pHbFont);
 
     // sort glyphs in visual order
     // and then in logical order (e.g. diacritics after cluster start)
@@ -1039,7 +1043,7 @@ bool IcuLayoutEngine::layout(ServerFontLayout& rLayout, ImplLayoutArgs& rArgs)
                 sal_UCS4 aChar = rArgs.mpStr[ nCharPos ];
                 nGlyphIndex = rFont.FixupGlyphIndex( nGlyphIndex, aChar );
 
-                // #i99367# HACK: try to detect all diacritics
+                // i#99367# HACK: try to detect all diacritics
                 if( aChar>=0x0300 && aChar<0x2100 )
                     bDiacritic = IsDiacritic( aChar );
             }
@@ -1051,7 +1055,7 @@ bool IcuLayoutEngine::layout(ServerFontLayout& rLayout, ImplLayoutArgs& rArgs)
             int nNewWidth = nGlyphWidth;
             if( nGlyphWidth <= 0 )
                 bDiacritic |= true;
-            // #i99367# force all diacritics to zero width
+            // i#99367# force all diacritics to zero width
             // TODO: we need mnOrigWidth/mnLogicWidth/mnNewWidth
             else if( bDiacritic )
                 nGlyphWidth = nNewWidth = 0;
commit 8f748ce03b724ea67738054bc32325fdf950fc0c
Author: Khaled Hosny <khaledhosny at eglug.org>
Date:   Sat Apr 27 14:57:12 2013 +0200

    [harfbuzz] Check for SAL_USE_HARFBUZZ in one place
    
    Change-Id: I78efebb576dffa8d39e98283feb9aab2186b5a39

diff --git a/vcl/generic/glyphs/gcach_layout.cxx b/vcl/generic/glyphs/gcach_layout.cxx
index 22cb8f7..b4137bb 100644
--- a/vcl/generic/glyphs/gcach_layout.cxx
+++ b/vcl/generic/glyphs/gcach_layout.cxx
@@ -49,7 +49,12 @@
 
 ServerFontLayout::ServerFontLayout( ServerFont& rFont )
 :   mrServerFont( rFont )
-{}
+{
+    bUseHarfBuzz = false;
+#if ENABLE_HARFBUZZ
+    bUseHarfBuzz = (getenv("SAL_USE_HARFBUZZ") != NULL);
+#endif
+}
 
 void ServerFontLayout::DrawText( SalGraphics& rSalGraphics ) const
 {
@@ -60,7 +65,7 @@ void ServerFontLayout::DrawText( SalGraphics& rSalGraphics ) const
 
 bool ServerFontLayout::LayoutText( ImplLayoutArgs& rArgs )
 {
-    ServerFontLayoutEngine* pLE = mrServerFont.GetLayoutEngine();
+    ServerFontLayoutEngine* pLE = mrServerFont.GetLayoutEngine(bUseHarfBuzz);
     assert(pLE);
     bool bRet = pLE ? pLE->layout(*this, rArgs) : false;
     return bRet;
@@ -70,9 +75,8 @@ bool ServerFontLayout::LayoutText( ImplLayoutArgs& rArgs )
 long ServerFontLayout::GetTextWidth() const
 {
     long nWidth;
-#if ENABLE_HARFBUZZ
-    const char* pUseHarfBuzz = getenv("SAL_USE_HARFBUZZ");
-    if (pUseHarfBuzz) {
+    if (bUseHarfBuzz)
+    {
         GlyphVector aGlyphItems = GenericSalLayout::GetGlyphItems();
 
         if( aGlyphItems.empty() )
@@ -96,8 +100,10 @@ long ServerFontLayout::GetTextWidth() const
         nWidth = nMaxPos - nMinPos;
     }
     else
-#endif
-    nWidth = GenericSalLayout::GetTextWidth();
+    {
+        nWidth = GenericSalLayout::GetTextWidth();
+    }
+
     return nWidth;
 }
 
@@ -126,14 +132,11 @@ void ServerFontLayout::AdjustLayout( ImplLayoutArgs& rArgs )
 
 void ServerFontLayout::ApplyDXArray(ImplLayoutArgs& rArgs)
 {
-#if ENABLE_HARFBUZZ
     // No idea what issue ApplyDXArray() was supposed to fix, but whatever
     // GenericSalLayout::ApplyDXArray() does it just breaks our perfectly
     // positioned text.
-    const char* pUseHarfBuzz = getenv("SAL_USE_HARFBUZZ");
-    if (!pUseHarfBuzz)
-#endif
-    GenericSalLayout::ApplyDXArray(rArgs);
+    if (!bUseHarfBuzz)
+        GenericSalLayout::ApplyDXArray(rArgs);
 }
 
 // =======================================================================
@@ -1160,13 +1163,12 @@ bool IcuLayoutEngine::layout(ServerFontLayout& rLayout, ImplLayoutArgs& rArgs)
 
 // =======================================================================
 
-ServerFontLayoutEngine* ServerFont::GetLayoutEngine()
+ServerFontLayoutEngine* ServerFont::GetLayoutEngine(bool bUseHarfBuzz)
 {
     // find best layout engine for font, platform, script and language
     if (!mpLayoutEngine) {
 #if ENABLE_HARFBUZZ
-        const char* pUseHarfBuzz = getenv("SAL_USE_HARFBUZZ");
-        if (pUseHarfBuzz)
+        if (bUseHarfBuzz)
             mpLayoutEngine = new HbLayoutEngine(*this);
         else
 #endif
diff --git a/vcl/inc/generic/glyphcache.hxx b/vcl/inc/generic/glyphcache.hxx
index 322dd75..2225971 100644
--- a/vcl/inc/generic/glyphcache.hxx
+++ b/vcl/inc/generic/glyphcache.hxx
@@ -240,7 +240,7 @@ private:
     int                 ApplyGlyphTransform( int nGlyphFlags, FT_GlyphRec_*, bool ) const;
     bool                ApplyGSUB( const FontSelectPattern& );
 
-    ServerFontLayoutEngine* GetLayoutEngine();
+    ServerFontLayoutEngine* GetLayoutEngine( bool );
 
     typedef ::boost::unordered_map<int,GlyphData> GlyphList;
     mutable GlyphList           maGlyphList;
@@ -318,6 +318,8 @@ private:
     SAL_DLLPRIVATE  ServerFontLayout( const ServerFontLayout& );
     SAL_DLLPRIVATE  ServerFontLayout& operator=( const ServerFontLayout& );
 
+    bool            bUseHarfBuzz;
+
 public:
                     ServerFontLayout( ServerFont& );
     virtual bool    LayoutText( ImplLayoutArgs& );
commit 9cd7f7aded9ba171bf91a8223e6e8a868aedd792
Author: Khaled Hosny <khaledhosny at eglug.org>
Date:   Sat Apr 27 14:47:29 2013 +0200

    [harfbuzz] Fix text width calculation
    
    GenericSalLayout::GetTextWidth() uses GlyphItem's mnNewWidth when
    calculating text width, and though this seems logical (it is after all
    the actual with the glyph is contributing to the all over advance
    width), it results in shorter width calculation whenever glyph width
    adjustment is involved, no idea why!
    
    The #ifdef is there so that the ICU code path is not changed in anyway,
    but all of this should be merged into GenericSalLayout when ICU is
    gone.
    
    Change-Id: I7cbde1675b78e87c142513eb52a13ee5fdc60617

diff --git a/vcl/generic/glyphs/gcach_layout.cxx b/vcl/generic/glyphs/gcach_layout.cxx
index abd9adf..22cb8f7 100644
--- a/vcl/generic/glyphs/gcach_layout.cxx
+++ b/vcl/generic/glyphs/gcach_layout.cxx
@@ -67,6 +67,40 @@ bool ServerFontLayout::LayoutText( ImplLayoutArgs& rArgs )
 }
 
 // -----------------------------------------------------------------------
+long ServerFontLayout::GetTextWidth() const
+{
+    long nWidth;
+#if ENABLE_HARFBUZZ
+    const char* pUseHarfBuzz = getenv("SAL_USE_HARFBUZZ");
+    if (pUseHarfBuzz) {
+        GlyphVector aGlyphItems = GenericSalLayout::GetGlyphItems();
+
+        if( aGlyphItems.empty() )
+            return 0;
+
+        // initialize the extent
+        long nMinPos = 0;
+        long nMaxPos = 0;
+
+        for( GlyphVector::const_iterator pG = aGlyphItems.begin(), end = aGlyphItems.end(); pG != end ; ++pG )
+        {
+            // update the text extent with the glyph extent
+            long nXPos = pG->maLinearPos.X();
+            if( nMinPos > nXPos )
+                nMinPos = nXPos;
+            nXPos += pG->mnOrigWidth;
+            if( nMaxPos < nXPos )
+                nMaxPos = nXPos;
+        }
+
+        nWidth = nMaxPos - nMinPos;
+    }
+    else
+#endif
+    nWidth = GenericSalLayout::GetTextWidth();
+    return nWidth;
+}
+
 void ServerFontLayout::AdjustLayout( ImplLayoutArgs& rArgs )
 {
     GenericSalLayout::AdjustLayout( rArgs );
diff --git a/vcl/inc/generic/glyphcache.hxx b/vcl/inc/generic/glyphcache.hxx
index cc3ae49..322dd75 100644
--- a/vcl/inc/generic/glyphcache.hxx
+++ b/vcl/inc/generic/glyphcache.hxx
@@ -324,6 +324,7 @@ public:
     virtual void    AdjustLayout( ImplLayoutArgs& );
     virtual void    ApplyDXArray( ImplLayoutArgs& );
     virtual void    DrawText( SalGraphics& ) const;
+    virtual long    GetTextWidth() const;
     ServerFont&     GetServerFont() const   { return mrServerFont; }
 };
 
diff --git a/vcl/inc/sallayout.hxx b/vcl/inc/sallayout.hxx
index ad6cd94..6f8a49d 100644
--- a/vcl/inc/sallayout.hxx
+++ b/vcl/inc/sallayout.hxx
@@ -367,6 +367,8 @@ protected:
 
     bool            GetCharWidths( sal_Int32* pCharWidths ) const;
 
+    GlyphVector     GetGlyphItems() const { return m_GlyphItems; }
+
 private:
     GlyphVector     m_GlyphItems;
     mutable Point   maBasePoint;
commit fc893070c9255637950ac9844c1ad519b0115bd8
Author: Khaled Hosny <khaledhosny at eglug.org>
Date:   Sat Apr 27 12:47:02 2013 +0200

    [harfbuzz] Don't change the returned positions
    
    Use local variables instead of altering the returned glyph positions
    array, looks more cleaner this way.
    
    Change-Id: Ibbcced57777010bd045668a99d7beb0618abe226

diff --git a/vcl/generic/glyphs/gcach_layout.cxx b/vcl/generic/glyphs/gcach_layout.cxx
index 5cd1b07..abd9adf 100644
--- a/vcl/generic/glyphs/gcach_layout.cxx
+++ b/vcl/generic/glyphs/gcach_layout.cxx
@@ -426,20 +426,20 @@ bool HbLayoutEngine::layout(ServerFontLayout& rLayout, ImplLayoutArgs& rArgs)
             if (hb_ot_layout_get_glyph_class(maHbFace, nGlyphIndex) == HB_OT_LAYOUT_GLYPH_CLASS_MARK)
                 nGlyphFlags |= GlyphItem::IS_DIACRITIC;
 
-            aHbPositions[i].x_offset =  aHbPositions[i].x_offset >> 6;
-            aHbPositions[i].y_offset =  aHbPositions[i].y_offset >> 6;
-            aHbPositions[i].x_advance = aHbPositions[i].x_advance >> 6;
-            aHbPositions[i].y_advance = aHbPositions[i].y_advance >> 6;
+            int32_t nXOffset =  aHbPositions[i].x_offset >> 6;
+            int32_t nYOffset =  aHbPositions[i].y_offset >> 6;
+            int32_t nXAdvance = aHbPositions[i].x_advance >> 6;
+            int32_t nYAdvance = aHbPositions[i].y_advance >> 6;
 
-            Point aNewPos = Point(aCurrPos.X() + aHbPositions[i].x_offset, -(aCurrPos.Y() + aHbPositions[i].y_offset));
+            Point aNewPos = Point(aCurrPos.X() + nXOffset, -(aCurrPos.Y() + nYOffset));
 
             GlyphItem aGI(nCharPos, nGlyphIndex, aNewPos, nGlyphFlags, nGlyphWidth);
-            aGI.mnNewWidth = aHbPositions[i].x_advance;
+            aGI.mnNewWidth = nXAdvance;
 
             rLayout.AppendGlyph(aGI);
 
-            aCurrPos.X() += aHbPositions[i].x_advance;
-            aCurrPos.Y() += aHbPositions[i].y_advance;
+            aCurrPos.X() += nXAdvance;
+            aCurrPos.Y() += nYAdvance;
         }
 
         hb_buffer_destroy(aHbBuffer);
commit 827b63a7a46f56b46246950fcf9f4065136880ca
Author: Khaled Hosny <khaledhosny at eglug.org>
Date:   Sat Apr 27 12:02:10 2013 +0200

    [harfbuzz] Correctly apply glyph positions
    
    X and Y offsets should only affect the position of the current glyph,
    not any subsequent ones.
    
    Change-Id: I9dd1576cbdbb36b70f1898dc52702c02c4e851af

diff --git a/vcl/generic/glyphs/gcach_layout.cxx b/vcl/generic/glyphs/gcach_layout.cxx
index eef1fd0..5cd1b07 100644
--- a/vcl/generic/glyphs/gcach_layout.cxx
+++ b/vcl/generic/glyphs/gcach_layout.cxx
@@ -338,7 +338,7 @@ bool HbLayoutEngine::layout(ServerFontLayout& rLayout, ImplLayoutArgs& rArgs)
 
     rLayout.Reserve(nGlyphCapacity);
 
-    Point aNewPos(0, 0);
+    Point aCurrPos(0, 0);
     while (true)
     {
         int nMinRunPos, nEndRunPos;
@@ -431,15 +431,15 @@ bool HbLayoutEngine::layout(ServerFontLayout& rLayout, ImplLayoutArgs& rArgs)
             aHbPositions[i].x_advance = aHbPositions[i].x_advance >> 6;
             aHbPositions[i].y_advance = aHbPositions[i].y_advance >> 6;
 
-            aNewPos = Point(aNewPos.X() + aHbPositions[i].x_offset, aNewPos.Y() - aHbPositions[i].y_offset);
+            Point aNewPos = Point(aCurrPos.X() + aHbPositions[i].x_offset, -(aCurrPos.Y() + aHbPositions[i].y_offset));
 
             GlyphItem aGI(nCharPos, nGlyphIndex, aNewPos, nGlyphFlags, nGlyphWidth);
             aGI.mnNewWidth = aHbPositions[i].x_advance;
 
             rLayout.AppendGlyph(aGI);
 
-            aNewPos.X() += aHbPositions[i].x_advance;
-            aNewPos.Y() += aHbPositions[i].y_advance;
+            aCurrPos.X() += aHbPositions[i].x_advance;
+            aCurrPos.Y() += aHbPositions[i].y_advance;
         }
 
         hb_buffer_destroy(aHbBuffer);
commit d225318ead0154681982656bcd3e98011e022c34
Author: Khaled Hosny <khaledhosny at eglug.org>
Date:   Sat Apr 27 11:01:51 2013 +0200

    [harfbuzz] Correctly apply RTL width adjustments
    
    It turns out it is GenericSalLayout::ApplyDXArray() that is messing with
    glyph advance widths trying to recalculate them. It is very old code (it
    has been there since ICU were introduced back in 2002), but whatever
    issue it is fixing, HarfBuzz does not need it.
    
    Change-Id: I5c896d3f318e2f17d135f9eea599b917e04ed592

diff --git a/vcl/generic/glyphs/gcach_layout.cxx b/vcl/generic/glyphs/gcach_layout.cxx
index ca40e9d..eef1fd0 100644
--- a/vcl/generic/glyphs/gcach_layout.cxx
+++ b/vcl/generic/glyphs/gcach_layout.cxx
@@ -67,7 +67,6 @@ bool ServerFontLayout::LayoutText( ImplLayoutArgs& rArgs )
 }
 
 // -----------------------------------------------------------------------
-
 void ServerFontLayout::AdjustLayout( ImplLayoutArgs& rArgs )
 {
     GenericSalLayout::AdjustLayout( rArgs );
@@ -91,6 +90,18 @@ void ServerFontLayout::AdjustLayout( ImplLayoutArgs& rArgs )
     }
 }
 
+void ServerFontLayout::ApplyDXArray(ImplLayoutArgs& rArgs)
+{
+#if ENABLE_HARFBUZZ
+    // No idea what issue ApplyDXArray() was supposed to fix, but whatever
+    // GenericSalLayout::ApplyDXArray() does it just breaks our perfectly
+    // positioned text.
+    const char* pUseHarfBuzz = getenv("SAL_USE_HARFBUZZ");
+    if (!pUseHarfBuzz)
+#endif
+    GenericSalLayout::ApplyDXArray(rArgs);
+}
+
 // =======================================================================
 
 static bool lcl_CharIsJoiner(sal_Unicode cChar)
@@ -423,13 +434,7 @@ bool HbLayoutEngine::layout(ServerFontLayout& rLayout, ImplLayoutArgs& rArgs)
             aNewPos = Point(aNewPos.X() + aHbPositions[i].x_offset, aNewPos.Y() - aHbPositions[i].y_offset);
 
             GlyphItem aGI(nCharPos, nGlyphIndex, aNewPos, nGlyphFlags, nGlyphWidth);
-
-            // This is a hack to compensate for assumptions made elsewhere in
-            // the codebase, the right way is to use aHbPositions[i].x_advance
-            // instead of nGlyphWidth above, and leave mnNewWidth alone
-            // (whatever it is meant for)
-            if (i + 1 < nRunGlyphCount)
-                aGI.mnNewWidth = nGlyphWidth + (aHbPositions[i + 1].x_offset >> 6);
+            aGI.mnNewWidth = aHbPositions[i].x_advance;
 
             rLayout.AppendGlyph(aGI);
 
diff --git a/vcl/inc/generic/glyphcache.hxx b/vcl/inc/generic/glyphcache.hxx
index d9f8378..cc3ae49 100644
--- a/vcl/inc/generic/glyphcache.hxx
+++ b/vcl/inc/generic/glyphcache.hxx
@@ -322,6 +322,7 @@ public:
                     ServerFontLayout( ServerFont& );
     virtual bool    LayoutText( ImplLayoutArgs& );
     virtual void    AdjustLayout( ImplLayoutArgs& );
+    virtual void    ApplyDXArray( ImplLayoutArgs& );
     virtual void    DrawText( SalGraphics& ) const;
     ServerFont&     GetServerFont() const   { return mrServerFont; }
 };


More information about the Libreoffice-commits mailing list