[Webkit-unassigned] [Bug 79875] [BlackBerry] Upstream classes that handle layer tiling

[bugzilla-daemon at webkit.org]

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


Rob Buis <rwlbuis at gmail.com> changed:

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




--- Comment #15 from Rob Buis <rwlbuis at gmail.com>  2012-03-08 08:02:13 PST ---
(From update of attachment 130806)
View in context: https://bugs.webkit.org/attachment.cgi?id=130806&action=review

Looks good, but needs one more round :)

> Source/WebCore/platform/graphics/blackberry/LayerTileIndex.h:68
> +template<> struct IntHash<WebCore::TileIndex> {

You probably don't need WebCore:: prefixes in this file.

> Source/WebCore/platform/graphics/blackberry/LayerTiler.cpp:153
> +        dirtyRect = IntRect(IntPoint(), requiredTextureSize);

IntPoint::zero

> Source/WebCore/platform/graphics/blackberry/LayerTiler.cpp:159
> +    if (requiredTextureSize.isEmpty() || dirtyRect == IntRect(IntPoint(), requiredTextureSize)) {

IntPoint::zero

> Source/WebCore/platform/graphics/blackberry/LayerTiler.cpp:648
> +IntSize LayerTiler::defaultTileSize()

It seems this is only used internally? If so you can make it a free, static function.

> Source/WebCore/platform/graphics/blackberry/LayerTiler.h:104
> +            : type(type)

This looks really confusing, can you add m_ to the member vars? This makes it clear which is which.

> Source/WebCore/platform/graphics/blackberry/LayerTiler.h:122
> +        static TextureJob setContents(const SkBitmap& contents, bool isOpaque) { return TextureJob(SetContents, contents, IntRect(IntPoint(), IntSize(contents.width(), contents.height())), isOpaque); }

IntPoint::zero

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