[webkit-reviews] review granted: [Bug 82114] [chromium] Simplify and fix CCLayerSorter : [Attachment 133660] Patch

[bugzilla-daemon at webkit.org]

Kenneth Russell <kbr at google.com> has granted Vangelis Kokkevis
<vangelis at chromium.org>'s request for review:
Bug 82114: [chromium] Simplify and fix CCLayerSorter
https://bugs.webkit.org/show_bug.cgi?id=82114

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

------- Additional Comments from Kenneth Russell <kbr at google.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=133660&action=review


Looks good overall. A couple of minor comments. r=me

> Source/WebCore/platform/graphics/chromium/cc/CCLayerSorter.cpp:86
> +CCLayerSorter::ABCompareResult CCLayerSorter::checkOverlap(LayerShape* a,
LayerShape* b, float zThreshold, float* weight)

I'm pretty sure that the preferred style in WebKit would be to use a reference
rather than a pointer for the outgoing argument "weight".

> Source/WebCore/platform/graphics/chromium/cc/CCLayerSorter.cpp:242
> +    float zThreshold = m_zRange * 0.01;

Magic constant? Should this be defined somewhere above and named?

> Source/WebKit/chromium/tests/CCLayerSorterTest.cpp:37
> +TEST(CCLayerSorterTest, CheckOverlap)

These tests should be split up for finer grained reporting of results.