Use fragmented bounding box for hit-test/paint clipping.

Use fragmented bounding box for hit-test/paint clipping.

When calculating the bounding box used for hit testing and paint clipping,
use the visual bounding box from the union of the fragments instead of the
flow thread coordinates.

The physicalBoundingBox method returned a bounding box in flow thread
coordinates and not visual coordinates. Rename it to boundingBox and
introduce a fragmentedBoundingBox to find the visual one.

Fixed a bug negation bug in previously unused convertToLayerCoords that
takes a LayoutRect.

This fixes the regressions introduced by 18e0c42d.

R=mstensho@opera.com
BUG=359877
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=183251

[rune, 2014-10-02 10:41:25]

rune@opera.com changed reviewers:
+ abarth@chromium.org, jchaffraix@chromium.org

[mstensho (USE GERRIT), 2014-10-02 10:57:52]

lgtm, but should wait for feedback from abarth and/or jchaffraix.

[andersr, 2014-10-02 11:01:21]

andersr@opera.com changed reviewers:
+ andersr@opera.com

[andersr, 2014-10-02 11:01:22]

https://codereview.chromium.org/612323011/diff/1/Source/core/rendering/Render...
File Source/core/rendering/RenderLayer.cpp (right):

https://codereview.chromium.org/612323011/diff/1/Source/core/rendering/Render...
Source/core/rendering/RenderLayer.cpp:2248: LayoutRect
RenderLayer::fragmentedBoundingBox(const RenderLayer* ancestorLayer, const
LayoutPoint* offsetFromRoot) const
Is fragmentsBoundingBox a better name? (Like
RenderFlowThread::fragmentsBoundingBox). That way it doesn't sound like the
bounding box itself is fragmented.

[rune, 2014-10-02 11:17:34]

https://codereview.chromium.org/612323011/diff/1/Source/core/rendering/Render...
File Source/core/rendering/RenderLayer.cpp (right):

https://codereview.chromium.org/612323011/diff/1/Source/core/rendering/Render...
Source/core/rendering/RenderLayer.cpp:2248: LayoutRect
RenderLayer::fragmentedBoundingBox(const RenderLayer* ancestorLayer, const
LayoutPoint* offsetFromRoot) const
On 2014/10/02 11:01:21, andersr wrote:
> Is fragmentsBoundingBox a better name? (Like
> RenderFlowThread::fragmentsBoundingBox). That way it doesn't sound like the
> bounding box itself is fragmented.

Yes.

[mstensho (USE GERRIT), 2014-10-02 12:43:59]

Anders found a regression. Please disregard my previous "looks good to me".

https://codereview.chromium.org/612323011/diff/1/Source/core/rendering/Render...
File Source/core/rendering/RenderLayer.cpp (right):

https://codereview.chromium.org/612323011/diff/1/Source/core/rendering/Render...
Source/core/rendering/RenderLayer.cpp:2263: 
At this point, |result| is a nice visual rectangle, relative to the multicol
container. You have already added the offset between the layer and the flow
thread (pagination layer). The code that follows will add the offset from the
layer to some ancestor, which means that the offset from the layer to the flow
thread will be added twice.

The cheapest solution is to add a result.moveBy(-offsetWithinPaginationLayer)
here.

[rune, 2014-10-03 09:15:46]

https://codereview.chromium.org/612323011/diff/1/Source/core/rendering/Render...
File Source/core/rendering/RenderLayer.cpp (right):

https://codereview.chromium.org/612323011/diff/1/Source/core/rendering/Render...
Source/core/rendering/RenderLayer.cpp:2248: LayoutRect
RenderLayer::fragmentedBoundingBox(const RenderLayer* ancestorLayer, const
LayoutPoint* offsetFromRoot) const
On 2014/10/02 11:17:34, rune wrote:
> On 2014/10/02 11:01:21, andersr wrote:
> > Is fragmentsBoundingBox a better name? (Like
> > RenderFlowThread::fragmentsBoundingBox). That way it doesn't sound like the
> > bounding box itself is fragmented.
> 
> Yes.

Done.

https://codereview.chromium.org/612323011/diff/1/Source/core/rendering/Render...
Source/core/rendering/RenderLayer.cpp:2263: 
On 2014/10/02 12:43:59, mstensho wrote:

> The cheapest solution is to add a result.moveBy(-offsetWithinPaginationLayer)
> here.

Done.

I also removed the offsetFromRoot pointer, since that was passed as 0 from the
only caller.

[Julien - ping for review, 2014-10-03 21:38:49]

lgtm

https://codereview.chromium.org/612323011/diff/40001/Source/core/rendering/Re...
File Source/core/rendering/RenderLayer.h (left):

https://codereview.chromium.org/612323011/diff/40001/Source/core/rendering/Re...
Source/core/rendering/RenderLayer.h:232: LayoutRect
physicalBoundingBoxIncludingReflectionAndStackingChildren(const RenderLayer*
ancestorLayer, const LayoutPoint& offsetFromRoot) const;
Those 2 renamings are basically undoing what 18e0c42d did. While I prefer the
name without the physical prefix, I don't think it warrants the history churn
that we are introducing.

[rune, 2014-10-06 07:09:46]

https://codereview.chromium.org/612323011/diff/40001/Source/core/rendering/Re...
File Source/core/rendering/RenderLayer.h (left):

https://codereview.chromium.org/612323011/diff/40001/Source/core/rendering/Re...
Source/core/rendering/RenderLayer.h:232: LayoutRect
physicalBoundingBoxIncludingReflectionAndStackingChildren(const RenderLayer*
ancestorLayer, const LayoutPoint& offsetFromRoot) const;
On 2014/10/03 21:38:49, Julien Chaffraix - PST wrote:
> Those 2 renamings are basically undoing what 18e0c42d did. While I prefer the
> name without the physical prefix, I don't think it warrants the history churn
> that we are introducing.

Done.

[rune, 2014-10-06 07:10:39]

The CQ bit was checked by rune@opera.com

[commit-bot: I haz the power, 2014-10-06 07:11:31]

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

[commit-bot: I haz the power, 2014-10-06 08:10:52]

Committed patchset #4 (id:60001) as 183251