We should do the following (1) fix the bug, of course (2) re-name the transform conversion functions in the m_tiler - contentRectToLayerRect and layerRectToContentRect - so that they are clearer (3) add testing that the updateRect is computed correctly for the case where contentBounds != bounds.
Created attachment 123054 [details] Patch
Note that (2) on the list above was already done by Enne in a previous patch. This patch addresses (1) and (3).
Comment on attachment 123054 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=123054&action=review In general, looks good. Can you also leave a comment on LayerChromium::m_updateRect about what space it's supposed to be in? > Source/WebKit/chromium/tests/TiledLayerChromiumTest.cpp:367 > + // The updateRect (that indicates what was actually painted) should be in > + // layer space, not the "content space". This is quite minor, but I'm kind of curious why content space gets scare quotes everywhere in this patch. Do you want to be calling it something else?
Created attachment 123228 [details] addressed reviewer comments, removed unnecessary code in FakeTiledLayerEtc Yeah, the reason I put quotes there is obsolete now anyway. and I tend to overuse quotes, too.
Enne and James, could you please review? Should be a relatively quick and easy review... thanks!
Comment on attachment 123228 [details] addressed reviewer comments, removed unnecessary code in FakeTiledLayerEtc Thanks for the extra comment. LGTM.
Comment on attachment 123228 [details] addressed reviewer comments, removed unnecessary code in FakeTiledLayerEtc 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
Created attachment 123414 [details] Patch for landing
Comment on attachment 123414 [details] Patch for landing Rejecting attachment 123414 [details] from commit-queue. shawnsingh@chromium.org does not have committer permissions according to http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/common/config/committers.py. - If you do not have committer rights please read http://webkit.org/coding/contributing.html for instructions on how to use bugzilla flags. - If you have committer rights please correct the error in Tools/Scripts/webkitpy/common/config/committers.py by adding yourself to the file (no review needed). The commit-queue restarts itself every 2 hours. After restart the commit-queue will correctly respect your committer rights.
Created attachment 123501 [details] Patch for landing
Comment on attachment 123501 [details] Patch for landing Clearing flags on attachment: 123501 Committed r105680: <http://trac.webkit.org/changeset/105680>
All reviewed patches have been landed. Closing bug.