Use localToContainerPoint when checking mapToContainer result

Use localToContainerPoint when checking mapToContainer result

The mapToContainer method is calculating the value in relation
to the container, but when we verify the result we calculate
the localToAbsolute value. This doesn't take the container
into account so we get the wrong values.

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

[dsinclair, 2014-05-30 16:56:07]

PTAL.

[hartmanng, 2014-05-30 17:03:56]

non-OWNER lgtm

[leviw_travelin_and_unemployed, 2014-05-30 17:17:50]

What differences are you seeing?

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

https://codereview.chromium.org/310483002/diff/1/Source/core/rendering/Render...
Source/core/rendering/RenderObject.cpp:2332:
mapLocalToContainer(repaintContainer, transformState, mode |
ApplyContainerFlip);
I don't think you actually want this. mapLocalToAbsolute intentionally passes in
zero, which tells mapLocalToContainer to map it to the outermost container (and
hence, absolute). Passing a repaintContainer means you're actually implementing
localToContainer, not localToAbsolute.

[dsinclair, 2014-05-30 17:30:06]

On 2014/05/30 17:17:50, leviw wrote:
> What differences are you seeing?
> 
>
https://codereview.chromium.org/310483002/diff/1/Source/core/rendering/Render...
> File Source/core/rendering/RenderObject.cpp (right):
> 
>
https://codereview.chromium.org/310483002/diff/1/Source/core/rendering/Render...
> Source/core/rendering/RenderObject.cpp:2332:
> mapLocalToContainer(repaintContainer, transformState, mode |
> ApplyContainerFlip);
> I don't think you actually want this. mapLocalToAbsolute intentionally passes
in
> zero, which tells mapLocalToContainer to map it to the outermost container
(and
> hence, absolute). Passing a repaintContainer means you're actually
implementing
> localToContainer, not localToAbsolute.

These failures all come from me using the geometryMap in updateLayerPositions to
determine the positionFromRepaintContainer. Just to take one example:

compositing/contents-opaque/background-clip.html

localToAbsolute: 18.000000,8.000000
renderGeometryMap: 5.000000,0.000000

ASSERTION FAILED: roundedIntPoint(rendererMappedResult) ==
roundedIntPoint(result)


So, is the correct solution that the assert should be using localToContainer?

[dsinclair, 2014-05-30 18:19:40]

PTAL. Switched to localToContainerPoint.

[leviw_travelin_and_unemployed, 2014-05-30 18:21:42]

This actually makes sense. LGTM.

[dsinclair, 2014-05-30 18:25:02]

The CQ bit was checked by dsinclair@chromium.org

[commit-bot: I haz the power, 2014-05-30 18:25:48]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/dsinclair@chromium.org/310483002/20001

[commit-bot: I haz the power, 2014-05-30 19:19:03]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  blink_presubmit on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/blink_presubmit/builds/6544)
  linux_blink_dbg on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/linux_blink_dbg/builds/1...)
  linux_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/1...)
  linux_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...)

[commit-bot: I haz the power, 2014-05-30 19:40:43]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-05-30 19:40:43]

Try jobs failed on following builders:
  blink_presubmit on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/blink_presubmit/builds/6556)
  linux_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/1...)

[dsinclair, 2014-05-30 19:54:39]

The CQ bit was checked by dsinclair@chromium.org

[commit-bot: I haz the power, 2014-05-30 19:55:51]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/dsinclair@chromium.org/310483002/20001

[commit-bot: I haz the power, 2014-05-30 22:44:42]

Change committed as 175163