[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