[Webkit-unassigned] [Bug 25068] [Chromium] Complex text support for Chromium Linux

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=25068


eric at webkit.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #31811|review?                     |review-
               Flag|                            |




------- Comment #13 from eric at webkit.org  2009-06-29 14:17 PDT -------
(From update of attachment 31811)
God I hate C.

Style:
 322     bool NextRTLScriptRun()
 352     bool NextLTRScriptRun()

Why copy/paste here?

 571         if (fromX == -1 && from < walker.numCodePoints()) {
 572             // |from| is within this script run. So we index the clusters
log to
 573             // find which glyph this code-point contributed to and find
its x
 574             // position.
 575             int glyph = walker.logClusters()[from];
 576             fromX = base + walker.xPositions()[glyph];
 577             fromAdvance = walker.advances()[glyph];
 578         } else
 579             from -= walker.numCodePoints();
 580 
 581         if (toX == -1 && to < walker.numCodePoints()) {
 582             int glyph = walker.logClusters()[to];
 583             toX = base + walker.xPositions()[glyph];
 584             toAdvance = walker.advances()[glyph];
 585         } else
 586             to -= walker.numCodePoints();

Again:

 593     // The position in question might be just after the text.
 594     const int rightEdge = base;
 595     if (fromX == -1 && !from)
 596         fromX = leftEdge;
 597     else if (walker.rtl())
 598        fromX += truncateFixedPointToInteger(fromAdvance);
 599 
 600     if (toX == -1 && !to)
 601         toX = rightEdge;
 602     else if (!walker.rtl())
 603         toX += truncateFixedPointToInteger(toAdvance);

Style:
 51 static HB_Fixed SkiaScalarToHarfbuzzFixed(SkScalar value)
 57 static HB_Bool StringToGlyphs(HB_Font hbFont, const HB_UChar16* characters,
hb_uint32 length, HB_Glyph* glyphs, hb_uint32* glyphsSize, HB_Bool isRTL)
 79 static void GlyphsToAdvances(HB_Font hbFont, const HB_Glyph* glyphs,
hb_uint32 numGlyphs, HB_Fixed* advances, int flags)
 105 static HB_Bool CanRender(HB_Font hbFont, const HB_UChar16* characters,
hb_uint32 length)


Style:
197 HB_FontClass harfbuzzSkiaClass = {
 198   StringToGlyphs,
 199   GlyphsToAdvances,
 200   CanRender,
 201   GetOutlinePoint,
 202   GetGlyphMetrics,
 203   GetFontMetric,
 204 };

This would probably have been easier in smaller pieces?  This code is pretty
ugly.  Mostly because it's C... and hard enough to stomach in small chunks. 
Silly manual memory management.

I still don't really know what this all is.  But at some level it doesn't
matter.  This is your port, and I don't need to be a gate keeper.  Please post
a new patch with fixed style though.


-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.