But today returns window coordinates, which sometimes happen to coincide with the pixel viewport coordinates, sometimes not. PR 312404
Created attachment 194974 [details] Patch
Comment on attachment 194974 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=194974&action=review > Source/WebCore/platform/graphics/blackberry/LayerRenderer.cpp:573 > + FloatRect result = toPixelCoordinates(rect, m_viewport, surfaceHeight); A more meaningful variable than result could be helpful here. pixelRect? As well, if the delta calculation was put ahead the calculation, adjustment and return would all be nicely grouped. > Source/WebCore/platform/graphics/blackberry/LayerRenderer.cpp:583 > + FloatRect result = toPixelCoordinates(rect, m_viewport, surfaceHeight); See other comment. Could this function be FloatRect pixelRect(toPixelViewCoordinates(rect); pixelRect.scale(1 / m_scale); return enclosingIntRect(pixelRect);
Comment on attachment 194974 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=194974&action=review >> Source/WebCore/platform/graphics/blackberry/LayerRenderer.cpp:573 >> + FloatRect result = toPixelCoordinates(rect, m_viewport, surfaceHeight); > > A more meaningful variable than result could be helpful here. pixelRect? > > As well, if the delta calculation was put ahead the calculation, adjustment and return would all be nicely grouped. Good point >> Source/WebCore/platform/graphics/blackberry/LayerRenderer.cpp:583 >> + FloatRect result = toPixelCoordinates(rect, m_viewport, surfaceHeight); > > See other comment. > > Could this function be > FloatRect pixelRect(toPixelViewCoordinates(rect); > pixelRect.scale(1 / m_scale); > return enclosingIntRect(pixelRect); The reason I opted for some code duplication here was to get floating point precision, unfortunately toPixelViewportCoordinates returns an IntPoint. However, the rounding should be harmless, I'll go for your suggestion instead to keep it DRY.
Created attachment 195086 [details] Patch
Comment on attachment 195086 [details] Patch Looks good to me.
Comment on attachment 195086 [details] Patch LGTM.
Comment on attachment 195086 [details] Patch Thanks Rob and Jakob!
(In reply to comment #7) > (From update of attachment 195086 [details]) > Thanks Rob and Jakob! Err, I mean Rob and Mike.
Comment on attachment 195086 [details] Patch Clearing flags on attachment: 195086 Committed r146906: <http://trac.webkit.org/changeset/146906>
All reviewed patches have been landed. Closing bug.