[PATCH] Add some comments

Khaled Hosny (via Code Review) gerrit at gerrit.libreoffice.org
Sun Apr 21 01:19:52 PDT 2013


Hi,

I have submitted a patch for review:

    https://gerrit.libreoffice.org/3520

To pull it, you can do:

    git pull ssh://gerrit.libreoffice.org:29418/core refs/changes/20/3520/1

Add some comments

Change-Id: I2a0dbf5f69efa0f35170c77a1efc9936cf9ecb94
---
M vcl/generic/glyphs/gcach_layout.cxx
1 file changed, 10 insertions(+), 2 deletions(-)



diff --git a/vcl/generic/glyphs/gcach_layout.cxx b/vcl/generic/glyphs/gcach_layout.cxx
index 8fbe9d7..1848ab0 100644
--- a/vcl/generic/glyphs/gcach_layout.cxx
+++ b/vcl/generic/glyphs/gcach_layout.cxx
@@ -148,7 +148,9 @@
         int nRunLen = nEndRunPos - nMinRunPos;
 
         // find matching script
-        // TODO: use ICU's UScriptRun API
+        // TODO: use ICU's UScriptRun API to properly resolves "common" and
+        // "inherited" script codes, probably use it in GetNextRun() and return
+        // the script there
         UScriptCode eScriptCode = USCRIPT_INVALID_CODE;
         for (int i = nMinRunPos; i < nEndRunPos; ++i)
         {
@@ -195,7 +197,8 @@
                 if (nCharPos >= 0)
                 {
                     rArgs.NeedFallback(nCharPos, bRightToLeft);
-                    // XXX: do we need this in harfbuzz?
+                    // 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]))
@@ -234,6 +237,10 @@
 
             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 / 64);
 
@@ -249,6 +256,7 @@
 
     // sort glyphs in visual order
     // and then in logical order (e.g. diacritics after cluster start)
+    // XXX: why?
     rLayout.SortGlyphItems();
 
     // determine need for kashida justification

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

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



More information about the LibreOffice mailing list