[webkit-reviews] review granted: [Bug 73059] [chromium] Refactor tile drawing to be more data-driven : [Attachment 119118] Fix ToT merge conflicts

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Dec 14 12:28:25 PST 2011


James Robinson <jamesr at chromium.org> has granted Adrienne Walker
<enne at google.com>'s request for review:
Bug 73059: [chromium] Refactor tile drawing to be more data-driven
https://bugs.webkit.org/show_bug.cgi?id=73059

Attachment 119118: Fix ToT merge conflicts
https://bugs.webkit.org/attachment.cgi?id=119118&action=review

------- Additional Comments from James Robinson <jamesr at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=119118&action=review


This looks awesome. R=me with some small comments.

> Source/WebCore/platform/graphics/chromium/LayerRendererChromium.cpp:443
> +    renderMatrix.scale3d(layerRect.width(), layerRect.height(), 1);

nit: scaleNonUniform(layerRect.width(), layerRect.height()) would be strictly
better, this is doing an unnecessary set of calculations on the 3rd column of
the matrix. it doesn't matter much here since this isn't performance sensitive
code but it's a good habit to get into.

> Source/WebCore/platform/graphics/chromium/cc/CCDebugBorderDrawQuad.cpp:43
> +    if (m_color.alpha() != 1)
> +	   m_quadOpaque = false;

i'm pretty sure all debug border quads are non-opaque since we just draw a line
loop around the edges. the middle is always empty

> Source/WebCore/platform/graphics/chromium/cc/CCLayerImpl.h:74
> +    PassOwnPtr<CCSharedQuadState> sharedQuadState() const;

nit: i think this should be called createSharedQuadState(). every call does
some calculations and returns a new value so it shouldn't look like a simple
getter

> Source/WebCore/platform/graphics/chromium/cc/CCRenderPass.h:56
> +    CCRenderPass(CCRenderSurface*);

explicit


More information about the webkit-reviews mailing list