Get rid of computeFloatRectForPaintInvalidation

Get rid of computeFloatRectForPaintInvalidation

All (real) implementations of this - except the one in RenderSVGRoot -
forward to the SVGRenderSupport helper. The helper in turn just walks the
ancestors and applies the transforms - until it reaches the SVG root, at
which point it "crosses the border" and starts mapping the rect using the
non-float methods (mapRectToPaintInvalidationBacking).
The ancestors-within-SVG walk is easy to transform into an iteration until
the SVG root is reached. The final rect can then be computed by mapping the
rect through the SVG root.

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=185977

[fs, 2014-11-20 18:07:04]

fs@opera.com changed reviewers:
+ fmalita@chromium.org, pdr@chromium.org

[fs, 2014-11-20 18:07:05]

This just in from Crazytown (Tokköping), Sweden.

Pros:
* Reduced design debt (and subsequently reduced vtables...)
* In some sense closer/more similar to other paint inv. codepaths

Cons:
* Less flexible

[pdr., 2014-11-21 08:32:05]

Gardening all day has sapped my brain so I apologize for the not fantastic
review. My initial thought is this is an improvement already and Tokköping isn't
too crazy after all.

https://codereview.chromium.org/741123002/diff/1/Source/core/rendering/svg/Re...
File Source/core/rendering/svg/RenderSVGRoot.cpp (right):

https://codereview.chromium.org/741123002/diff/1/Source/core/rendering/svg/Re...
Source/core/rendering/svg/RenderSVGRoot.cpp:338:
rect.intersect(pixelSnappedBorderBoxRect());
PaintInvalidationState's ctor has something that looks a lot like this but is a
little different.

https://codereview.chromium.org/741123002/diff/1/Source/core/rendering/svg/Re...
Source/core/rendering/svg/RenderSVGRoot.cpp:338:
rect.intersect(pixelSnappedBorderBoxRect());
PaintInvalidationState's ctor has something that looks a lot like this but is a
little different. Should these be unified?

https://codereview.chromium.org/741123002/diff/1/Source/core/rendering/svg/SV...
File Source/core/rendering/svg/SVGRenderSupport.cpp (right):

https://codereview.chromium.org/741123002/diff/1/Source/core/rendering/svg/SV...
Source/core/rendering/svg/SVGRenderSupport.cpp:69: // FIXME: These are quirks
carried forward from RenderSVGRoot::computeFloatRectForPaintInvalidation.
This comment is no longer valid (similarly
RenderSVGRoot::clippedOverflowRectForPaintInvalidation's comment)

https://codereview.chromium.org/741123002/diff/1/Source/core/rendering/svg/SV...
File Source/core/rendering/svg/SVGRenderSupport.h (right):

https://codereview.chromium.org/741123002/diff/1/Source/core/rendering/svg/SV...
Source/core/rendering/svg/SVGRenderSupport.h:78: static const RenderSVGRoot*
mapRectToSVGRootForPaintInvalidation(const RenderObject*, const FloatRect&
localPaintInvalidationRect, LayoutRect&);
Can this return a reference?

[fs, 2014-11-21 11:23:15]

https://codereview.chromium.org/741123002/diff/1/Source/core/rendering/svg/Re...
File Source/core/rendering/svg/RenderSVGRoot.cpp (right):

https://codereview.chromium.org/741123002/diff/1/Source/core/rendering/svg/Re...
Source/core/rendering/svg/RenderSVGRoot.cpp:338:
rect.intersect(pixelSnappedBorderBoxRect());
On 2014/11/21 08:32:05, pdr wrote:
> PaintInvalidationState's ctor has something that looks a lot like this but is
a
> little different. Should these be unified?

They are the same thing, but in different coordinate spaces (w/ and w/o paint
offset). I think that unifying them would make for unnecessarily complicated
code - or at least I can't come up with something that feels like an
improvement. Ideas?

https://codereview.chromium.org/741123002/diff/1/Source/core/rendering/svg/SV...
File Source/core/rendering/svg/SVGRenderSupport.cpp (right):

https://codereview.chromium.org/741123002/diff/1/Source/core/rendering/svg/SV...
Source/core/rendering/svg/SVGRenderSupport.cpp:69: // FIXME: These are quirks
carried forward from RenderSVGRoot::computeFloatRectForPaintInvalidation.
On 2014/11/21 08:32:05, pdr wrote:
> This comment is no longer valid (similarly
> RenderSVGRoot::clippedOverflowRectForPaintInvalidation's comment)

Right. Updated.

https://codereview.chromium.org/741123002/diff/1/Source/core/rendering/svg/SV...
File Source/core/rendering/svg/SVGRenderSupport.h (right):

https://codereview.chromium.org/741123002/diff/1/Source/core/rendering/svg/SV...
Source/core/rendering/svg/SVGRenderSupport.h:78: static const RenderSVGRoot*
mapRectToSVGRootForPaintInvalidation(const RenderObject*, const FloatRect&
localPaintInvalidationRect, LayoutRect&);
On 2014/11/21 08:32:05, pdr wrote:
> Can this return a reference?

Sure can.

[fs, 2014-11-24 16:25:40]

Semi-tabletennis

[pdr., 2014-11-25 21:45:16]

The CQ bit was checked by pdr@chromium.org

[pdr., 2014-11-25 21:45:16]

LGTM.

Sorry for the delay (see gardening reports for why!) Today is going smoothly and
I got a chance to actually review this. This time the lack of comments is due to
good code, not a tired reviewer :)

[commit-bot: I haz the power, 2014-11-25 21:46:11]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/741123002/20001

[commit-bot: I haz the power, 2014-11-25 21:54:28]

Committed patchset #2 (id:20001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=185977