[Webkit-unassigned] [Bug 80121] [BlackBerry] Upstream Texture and TextureCache

[bugzilla-daemon at webkit.org]

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


Rob Buis <rwlbuis at gmail.com> changed:

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




--- Comment #2 from Rob Buis <rwlbuis at gmail.com>  2012-03-04 16:14:10 PST ---
(From update of attachment 129847)
View in context: https://bugs.webkit.org/attachment.cgi?id=129847&action=review

Looks good overall, but can be improved some more.

> Source/WebCore/platform/graphics/blackberry/Texture.h:24
> +#include "Color.h"

A forward reference could be enough.

> Source/WebCore/platform/graphics/blackberry/Texture.h:28
> +#include <SkBitmap.h>

Ditto.

> Source/WebCore/platform/graphics/blackberry/Texture.h:50
> +    }

I think that this would be clearer with an enum. So you could do Texture::create(ColorTexture|NormalTexture) or something and only need one create.

> Source/WebCore/platform/graphics/blackberry/Texture.h:78
> +    // The following method is only called by our dear friend.

Ha ha :) Not the best of comments, I think you can remove it without losing anything.

> Source/WebCore/platform/graphics/blackberry/Texture.h:79
> +    friend class TextureCacheCompositingThread;

Usually this is put just under private.

> Source/WebCore/platform/graphics/blackberry/TextureCacheCompositingThread.cpp:30
> +static const int defaultMemoryLimit = 64 * 1024 * 1024; // Bytes

Comment is a bit short... Maybe // Measured in bytes.

> Source/WebCore/platform/graphics/blackberry/TextureCacheCompositingThread.cpp:97
> +    int delta = (texture->width() * texture->height() - oldSize.width() * oldSize.height()) * texture->bytesPerPixel();

bytesPerPixel is staic, so could as well use Texturee: bytesPerPixel() to make it consistent

> Source/WebCore/platform/graphics/blackberry/TextureCacheCompositingThread.h:77
> +    // The following methods are called by Texture class.

Is this comment really relevant?

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