[Webkit-unassigned] [Bug 50365] Glyphs in vertical text tests are rotated 90 degrees clockwise on Chromium Linux and Windows

[bugzilla-daemon at webkit.org]

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


Kent Tamura <tkent at chromium.org> changed:

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




--- Comment #1 from Kent Tamura <tkent at chromium.org>  2010-12-02 01:03:34 PST ---
(From update of attachment 75343)
View in context: https://bugs.webkit.org/attachment.cgi?id=75343&action=review

I'm not familiar with this area.  So I make some comments on patch style.

* Please add a ChangeLog entry. WebCore/ChangeLog.
* Please make a patch at the top level directory.  You made the patch at WebKit/WebCore, should make at WebKit/.

You can use WebKitTools/Scripts/webkit-patch to make an acceptable patch.

> platform/graphics/chromium/FontLinux.cpp:106
> +            pos2[i].set(x+SkFloatToScalar(adv[i].width()), y);
> +            pos3[i].set(x+SkFloatToScalar(adv[i].width()), y-SkFloatToScalar(adv[i].width())); 

Add spaces around operators such as + -

> platform/graphics/chromium/FontLinux.cpp:132
> +                canvas->drawTextOnPath(glyphs+i, 2, path, 0, paint);

Add spaces around +

> platform/graphics/chromium/FontLinux.cpp:136
> +        } else {
> +            canvas->drawPosText(glyphs, numGlyphs << 1, pos, paint);
> +        }

Remove { and }

> platform/graphics/skia/GlyphPageTreeNodeSkia.cpp:72
> +            for (int i=0; i < bufferLength; i++) {
> +                hb_buffer_add_glyph(buffer, glyphs[i], 0, i);
> +            }

Add spaces around =.
Remove { and }

> platform/graphics/skia/GlyphPageTreeNodeSkia.cpp:80
> +            HB_GSUB_Select_Script(hb_face->gsub, 
> +                        HB_MAKE_TAG('D', 'F', 'L', 'T'), &script_index);
> +            HB_GSUB_Select_Feature(hb_face->gsub,
> +                        HB_MAKE_TAG('v', 'e', 'r', 't'), script_index,
> +                         0xffff, &feature_index);

You don't need wrap these lines. WebKit style allow looooong lines.

> platform/graphics/skia/GlyphPageTreeNodeSkia.cpp:90
> +                for (int i=0; i < bufferLength; i++) {
> +                    glyphs[i] = (Glyph) buffer->out_string[i].gindex;
> +                }

Remove { and }.
We prefer C++-style cast.

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