[webkit-reviews] review granted: [Bug 114276] [BlackBerry] LayerTexture refactoring : [Attachment 197530] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Apr 11 02:54:15 PDT 2013


Carlos Garcia Campos <cgarcia at igalia.com> has granted Arvid Nilsson
<anilsson at rim.com>'s request for review:
Bug 114276: [BlackBerry] LayerTexture refactoring
https://bugs.webkit.org/show_bug.cgi?id=114276

Attachment 197530: Patch
https://bugs.webkit.org/attachment.cgi?id=197530&action=review

------- Additional Comments from Carlos Garcia Campos <cgarcia at igalia.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=197530&action=review


Nice cleanup!

> Source/WebCore/ChangeLog:11
> +	   now it's just BB::P::G::Buffer all around.

Better say BlackBerry::Platform::Graphics::Buffer to make it clearer, I think.

> Source/WebCore/platform/graphics/blackberry/LayerTexture.cpp:104
> +    // Force creation if it's 0

Nit: finish the comment with a period.

> Source/WebCore/platform/graphics/blackberry/LayerTexture.cpp:112
> +	   platformTexture =
reinterpret_cast<GLuint>(platformBufferHandle(m_buffer));

Shouldn't it be reinterpret_cast<Platform3DObject> instead?

> Source/WebCore/platform/graphics/blackberry/LayerTexture.h:61
> +    // This will ensure a buffer is allocated for this texture with the
requested backing and size

Period here too.

>
Source/WebCore/platform/graphics/blackberry/TextureCacheCompositingThread.cpp:1
10
> +	   Buffer* buffer = createBuffer(size, type);
> +	   if (!buffer)
>	       return false;

I think createBuffer is not expected to return 0, there's an assert in such
case, so maybe we can skip this check, or add an assert here too.

>
Source/WebCore/platform/graphics/blackberry/TextureCacheCompositingThread.cpp:1
30
> +    Buffer* buffer = createBuffer(size, type);
> +    if (!buffer)
>	   return false;

Ditto.

>
Source/WebCore/platform/graphics/blackberry/TextureCacheCompositingThread.h:54
> +    PassRefPtr<LayerTexture>
textureForContents(BlackBerry::Platform::Graphics::Buffer* contents);

I think we can omit the parameter name in this case.

>
Source/WebCore/platform/graphics/blackberry/TextureCacheCompositingThread.h:58
> +    PassRefPtr<LayerTexture> updateContents(const RefPtr<LayerTexture>&,
BlackBerry::Platform::Graphics::Buffer* contents);

Ditto.


More information about the webkit-reviews mailing list