Bug 79216 - [BlackBerry] Upstream helper classes for skia
Summary: [BlackBerry] Upstream helper classes for skia
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other Other
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks: 73119
  Show dependency treegraph
 
Reported: 2012-02-22 02:43 PST by Robin Cao
Modified: 2012-02-24 05:03 PST (History)
5 users (show)

See Also:


Attachments
patch (14.01 KB, patch)
2012-02-22 02:49 PST, Robin Cao
no flags Details | Formatted Diff | Diff
Updated patch for landing (13.97 KB, patch)
2012-02-23 23:13 PST, Robin Cao
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Robin Cao 2012-02-22 02:43:19 PST
Add helper classes in platform/graphics/blackberry/skia
    ImageBufferData.h
    PlatformSupport.cpp
    PlatformSupport.h
Comment 1 Robin Cao 2012-02-22 02:49:55 PST
Created attachment 128166 [details]
patch
Comment 2 Antonio Gomes 2012-02-23 19:24:32 PST
Comment on attachment 128166 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=128166&action=review

> Source/WebCore/platform/graphics/blackberry/skia/PlatformSupport.cpp:100
> +    FcBool b;
> +    int i;

b and i does not make it easy to understand their use.

> Source/WebCore/platform/graphics/blackberry/skia/PlatformSupport.cpp:114
> +#if 0
> +    // FIXME: Fontconfig tells us to use aggressive hinting, but FreeType breaks with our Adobe fonts
> +    if (FcPatternGetInteger(match, FC_HINT_STYLE, 0, &i) == FcResultMatch)
> +         out->hintStyle = i;
> +#endif

up to drop this comment?

> Source/WebCore/platform/graphics/blackberry/skia/PlatformSupport.cpp:147
> +        if (U16_IS_SURROGATE(characters[i])
> +         && U16_IS_SURROGATE_LEAD(characters[i])
> +         && i != numCharacters - 1
> +         && U16_IS_TRAIL(characters[i + 1])) {
> +              FcCharSetAddChar(cset, U16_GET_SUPPLEMENTARY(characters[i], characters[i+1]));
> +          i++;
> +        } else
> +              FcCharSetAddChar(cset, characters[i]);

wrong code style (indentation)

> Source/WebCore/platform/graphics/blackberry/skia/PlatformSupport.cpp:198
> +        }
> +        int weight;

add blank line between these two

> Source/WebCore/platform/graphics/blackberry/skia/PlatformSupport.cpp:203
> +            family->isBold = false;
> +        int slant;

ditto

> Source/WebCore/platform/graphics/blackberry/skia/PlatformSupport.cpp:208
> +            family->isItalic = false;
> +        FcFontSetDestroy(fontSet);

ditto

> Source/WebCore/platform/graphics/blackberry/skia/PlatformSupport.h:43
> +// This is a minimal version of the Chromium PlatformSupport/WebFontInfo classes used for font support
> +
> +class PlatformSupport {

drop blank line, add "." at the end
Comment 3 Robin Cao 2012-02-23 22:50:12 PST
Thanks for the review. Will address your comments before landing.
Comment 4 Robin Cao 2012-02-23 23:13:24 PST
Created attachment 128660 [details]
Updated patch for landing
Comment 5 Leo Yang 2012-02-23 23:23:24 PST
Comment on attachment 128660 [details]
Updated patch for landing

Sending to cq...
Comment 6 WebKit Review Bot 2012-02-24 05:03:29 PST
Comment on attachment 128660 [details]
Updated patch for landing

Clearing flags on attachment: 128660

Committed r108775: <http://trac.webkit.org/changeset/108775>
Comment 7 WebKit Review Bot 2012-02-24 05:03:34 PST
All reviewed patches have been landed.  Closing bug.