Fix positioning of CSS reference clips in composited layers.

Fix positioning of CSS reference clips in composited layers.

When painting into composited scrolling layers, clip paths must use
an offset from the root location to account for the origin of the
the layer's paint coordinate system. SVG clips referenced by CSS,
with the default UserSpaceOnUse coordinate system were not applying
this offset as required. When the clip is in ObjectBoundingBox
coordinates the offset was applied as a side effect of the object's
bounding box.

This patch corrects the position by adding the necessary offset when
the clip coordinate system is UserSpaceOnUse.

R=fs@opera.com
BUG=617265
Committed: https://crrev.com/e6cc64ba85ee2f7927578efb8d91641db367c2f0
Cr-Commit-Position: refs/heads/master@{#401642}

[Stephen Chennney, 2016-06-21 20:16:55]

The CQ bit was checked by schenney@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-06-21 20:17:49]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2078343005/1

[Stephen Chennney, 2016-06-21 20:18:13]

This is a bit ugly, but I don't see a better way to do it. I'm running the try
bot to see what breaks, and to get baselines for what will be at least 1 failing
test.

The behavior now matches Firefox.

I modified the tests to work on both browsers. How would you feel about
un-prefixing -webkit-clip-path?

[fs, 2016-06-21 20:42:55]

On 2016/06/21 at 20:18:13, schenney wrote:
> This is a bit ugly, but I don't see a better way to do it. I'm running the try
bot to see what breaks, and to get baselines for what will be at least 1 failing
test.

Yes, this looks like the right thing to do, but we'd need to make sure the
"non-asPath" code-path gets it right as well (a <clipPath> with <text> or so).

> The behavior now matches Firefox.
> 
> I modified the tests to work on both browsers. How would you feel about
un-prefixing -webkit-clip-path?

I was actually thinking we should put that in the Q3 OKR =)

[Stephen Chennney, 2016-06-21 20:52:28]

On 2016/06/21 20:42:55, fs wrote:
> On 2016/06/21 at 20:18:13, schenney wrote:
> > This is a bit ugly, but I don't see a better way to do it. I'm running the
try
> bot to see what breaks, and to get baselines for what will be at least 1
failing
> test.
> 
> Yes, this looks like the right thing to do, but we'd need to make sure the
> "non-asPath" code-path gets it right as well (a <clipPath> with <text> or so).

The test we have for shape-based clipping with a polygon was already working.
That uses a different code path for path conversion and does not rely on the SVG
code to set up the clip. And we already match Firefox on that test.

> 
> > The behavior now matches Firefox.
> > 
> > I modified the tests to work on both browsers. How would you feel about
> un-prefixing -webkit-clip-path?
> 
> I was actually thinking we should put that in the Q3 OKR =)

So let's make sure Chris adds that in. I thought about it some and I don't see
any major issue, or at least none different to those with the filter
un-prefixing.

[fs, 2016-06-21 20:57:52]

On 2016/06/21 at 20:52:28, schenney wrote:
> On 2016/06/21 20:42:55, fs wrote:
> > On 2016/06/21 at 20:18:13, schenney wrote:
> > > This is a bit ugly, but I don't see a better way to do it. I'm running the
try
> > bot to see what breaks, and to get baselines for what will be at least 1
failing
> > test.
> > 
> > Yes, this looks like the right thing to do, but we'd need to make sure the
> > "non-asPath" code-path gets it right as well (a <clipPath> with <text> or
so).
> 
> The test we have for shape-based clipping with a polygon was already working.
That uses a different code path for path conversion and does not rely on the SVG
code to set up the clip. And we already match Firefox on that test.

I was thinking about "clipperState = ClipperAppliedMask" branch/case in the
SVGClipPainter, that won't be covered by a <shape>-based (polygon(...)) test,
no?

[Stephen Chennney, 2016-06-21 21:01:52]

On 2016/06/21 20:57:52, fs wrote:
> On 2016/06/21 at 20:52:28, schenney wrote:
> > On 2016/06/21 20:42:55, fs wrote:
> > > On 2016/06/21 at 20:18:13, schenney wrote:
> > > Yes, this looks like the right thing to do, but we'd need to make sure the
> > > "non-asPath" code-path gets it right as well (a <clipPath> with <text> or
> so).
> > 
> > The test we have for shape-based clipping with a polygon was already
working.
> That uses a different code path for path conversion and does not rely on the
SVG
> code to set up the clip. And we already match Firefox on that test.
> 
> I was thinking about "clipperState = ClipperAppliedMask" branch/case in the
> SVGClipPainter, that won't be covered by a <shape>-based (polygon(...)) test,
> no?

I get you now. Yes, I'll make sure we get a test to cover that case.

[commit-bot: I haz the power, 2016-06-21 22:14:21]

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

[commit-bot: I haz the power, 2016-06-21 22:14:23]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Stephen Chennney, 2016-06-22 20:14:59]

On 2016/06/21 22:14:23, commit-bot: I haz the power wrote:
> Dry run: Try jobs failed on following builders:
>   mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

Broke a css3 masking test, apparently. Otherwise, svg text clipping works fine
and matches Firefox.

[Stephen Chennney, 2016-06-22 20:55:36]

The CQ bit was checked by schenney@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-06-22 20:56:21]

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

[Stephen Chennney, 2016-06-22 20:57:19]

The broken CSS test was assuming userSpaceOnUse meant an original at the page
origin, which I think we agreed on the bug was incorrect. That is,
userSpaceOnUse units have their origin at the using objects border box origin.

Otherwise things seem to be on track. I'm running CQ to get expectations.

[fs, 2016-06-22 21:04:19]

On 2016/06/22 at 20:57:19, schenney wrote:
> The broken CSS test was assuming userSpaceOnUse meant an original at the page
origin, which I think we agreed on the bug was incorrect. That is,
userSpaceOnUse units have their origin at the using objects border box origin.

Sounds right. Could verify that Firefox agrees with our new interpretation.

> Otherwise things seem to be on track. I'm running CQ to get expectations.

LGTM then. Maybe we can figure out a prettier way to do it in the future.

[fs, 2016-06-22 21:07:07]

On 2016/06/22 at 21:04:19, fs wrote:
> On 2016/06/22 at 20:57:19, schenney wrote:
> > The broken CSS test was assuming userSpaceOnUse meant an original at the
page origin, which I think we agreed on the bug was incorrect. That is,
userSpaceOnUse units have their origin at the using objects border box origin.
> 
> Sounds right. Could verify that Firefox agrees with our new interpretation.
> 
> > Otherwise things seem to be on track. I'm running CQ to get expectations.
> 
> LGTM then. Maybe we can figure out a prettier way to do it in the future.

BTW, you should probably go through crbug.com/373358 and check the tests from
there - I suspect they'll pass with this fix and that we can dupe that bug to
this bug (crbug.com/617265.)

[commit-bot: I haz the power, 2016-06-22 22:22:40]

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

[commit-bot: I haz the power, 2016-06-22 22:22:41]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Stephen Chennney, 2016-06-23 14:41:18]

The CQ bit was checked by schenney@chromium.org

[Stephen Chennney, 2016-06-23 14:41:19]

The patchset sent to the CQ was uploaded after l-g-t-m from fs@opera.com
Link to the patchset: https://codereview.chromium.org/2078343005/#ps40001
(title: "Expectations")

[commit-bot: I haz the power, 2016-06-23 14:41:35]

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

[commit-bot: I haz the power, 2016-06-23 15:53:33]

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

[commit-bot: I haz the power, 2016-06-23 15:53:34]

Try jobs failed on following builders:
  linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Stephen Chennney, 2016-06-23 16:11:22]

The CQ bit was checked by schenney@chromium.org

[Stephen Chennney, 2016-06-23 16:11:22]

The patchset sent to the CQ was uploaded after l-g-t-m from fs@opera.com
Link to the patchset: https://codereview.chromium.org/2078343005/#ps60001
(title: "Add virtual expectation")

[commit-bot: I haz the power, 2016-06-23 16:11:41]

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

[commit-bot: I haz the power, 2016-06-23 17:25:55]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-06-23 17:42:01]

Description was changed from

==========
Fix positioning of CSS reference clips in composited layers.

When painting into composited scrolling layers, clip paths must use
an offset from the root location to account for the origin of the
the layer's paint coordinate system. SVG clips referenced by CSS,
with the default UserSpaceOnUse coordinate system were not applying
this offset as required. When the clip is in ObjectBoundingBox
coordinates the offset was applied as a side effect of the object's
bounding box.

This patch corrects the position by adding the necessary offset when
the clip coordinate system is UserSpaceOnUse.

R=fs@opera.com
BUG=617265
==========

to

==========
Fix positioning of CSS reference clips in composited layers.

When painting into composited scrolling layers, clip paths must use
an offset from the root location to account for the origin of the
the layer's paint coordinate system. SVG clips referenced by CSS,
with the default UserSpaceOnUse coordinate system were not applying
this offset as required. When the clip is in ObjectBoundingBox
coordinates the offset was applied as a side effect of the object's
bounding box.

This patch corrects the position by adding the necessary offset when
the clip coordinate system is UserSpaceOnUse.

R=fs@opera.com
BUG=617265

Committed: https://crrev.com/e6cc64ba85ee2f7927578efb8d91641db367c2f0
Cr-Commit-Position: refs/heads/master@{#401642}
==========

[commit-bot: I haz the power, 2016-06-23 17:42:02]

Patchset 4 (id:??) landed as
https://crrev.com/e6cc64ba85ee2f7927578efb8d91641db367c2f0
Cr-Commit-Position: refs/heads/master@{#401642}