[webkit-reviews] review granted: [Bug 72919] [chromium] updateRect is probably incorrect when contentBounds != bounds : [Attachment 123228] addressed reviewer comments, removed unnecessary code in FakeTiledLayerEtc

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Jan 20 14:40:10 PST 2012


James Robinson <jamesr at chromium.org> has granted Shawn Singh
<shawnsingh at chromium.org>'s request for review:
Bug 72919: [chromium] updateRect is probably incorrect when contentBounds !=
bounds
https://bugs.webkit.org/show_bug.cgi?id=72919

Attachment 123228: addressed reviewer comments, removed unnecessary code in
FakeTiledLayerEtc
https://bugs.webkit.org/attachment.cgi?id=123228&action=review

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


R=me, some minor cleanups suggested

> Source/WebCore/platform/graphics/chromium/TiledLayerChromium.cpp:245
> +    float widthScale = bounds().width() /
static_cast<double>(contentBounds().width());
> +    float heightScale = bounds().height() /
static_cast<double>(contentBounds().height());

nit: if we're assigning to a float, why static_cast<> to double? why not float?


> Source/WebCore/platform/graphics/chromium/TiledLayerChromium.cpp:246
> +    m_updateRect = FloatRect(m_paintRect.x() * widthScale, m_paintRect.y() *
heightScale, m_paintRect.width() * widthScale, m_paintRect.height() *
heightScale);

nit: i think i'd weakly prefer doing something like
FloatRect(m_paintRect).scale(widthScale, heightScale); just to make the line a
bit shorter and have less risk of silly copy/paste errors


More information about the webkit-reviews mailing list