Add a transform paint property for local SVG transforms

Add a transform paint property for local SVG transforms

This patch adds a transform node for local SVG transforms which lets us
distinguish between SVG and CSS transforms. With SVG and CSS transforms
separated, the SVG root supports both types of transforms simultaneously.
This will let us support compositor-driven CSS transform animations in
the future.

With this patch we can remove the paint offset quirk for SVG. I've just
added TODOs to make this easier to review, but I plan it as an immediate
followup.

BUG=600618
Committed: https://crrev.com/143503e05b2d40592e84ec4213af92428dfe3964
Cr-Commit-Position: refs/heads/master@{#386095}

[pdr., 2016-04-07 02:32:18]

pdr@chromium.org changed reviewers:
+ jbroman@chromium.org, trchen@chromium.org

[pdr., 2016-04-07 02:32:21]

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

[commit-bot: I haz the power, 2016-04-07 02:32:40]

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

[commit-bot: I haz the power, 2016-04-07 03:21:37]

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

[commit-bot: I haz the power, 2016-04-07 03:21:38]

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

[pdr., 2016-04-07 03:58:07]

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

[commit-bot: I haz the power, 2016-04-07 03:58:34]

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

[commit-bot: I haz the power, 2016-04-07 05:00:41]

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

[commit-bot: I haz the power, 2016-04-07 05:00:42]

Dry run: This issue passed the CQ dry run.

[jbroman, 2016-04-07 14:37:11]

https://codereview.chromium.org/1865093004/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/paint/ObjectPaintProperties.h (right):

https://codereview.chromium.org/1865093004/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/paint/ObjectPaintProperties.h:58: //     |  
+---[ scrollTranslation ]    The space created by overflow clip.
Can an element have both an SVG local transform and a scroll translation? (If
so, can you confirm that this is the right hierarchy?)

[pdr., 2016-04-07 18:19:07]

On 2016/04/07 at 14:37:11, jbroman wrote:
>
https://codereview.chromium.org/1865093004/diff/20001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/core/paint/ObjectPaintProperties.h (right):
> 
>
https://codereview.chromium.org/1865093004/diff/20001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/paint/ObjectPaintProperties.h:58: //     |  
+---[ scrollTranslation ]    The space created by overflow clip.
> Can an element have both an SVG local transform and a scroll translation? (If
so, can you confirm that this is the right hierarchy?)

Good question. SVG doesn't support any scrolling so this shouldn't be a problem
today.

[jbroman, 2016-04-07 18:47:38]

https://codereview.chromium.org/1865093004/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/paint/ObjectPaintProperties.h (right):

https://codereview.chromium.org/1865093004/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/paint/ObjectPaintProperties.h:58: //     |  
+---[ scrollTranslation ]    The space created by overflow clip.
On 2016/04/07 at 18:19:07, pdr wrote:
> On 2016/04/07 at 14:37:11, jbroman wrote:
> > Can an element have both an SVG local transform and a scroll translation?
(If so, can you confirm that this is the right hierarchy?)
> 
> Good question. SVG doesn't support any scrolling so this shouldn't be a
problem today.

Would you mind clarifying this? The code in question updates the context when it
creates both of them, so if both could be created, then they would be parent and
child, not siblings.

If you don't want to draw it as parent-child, perhaps something like:

   +--[ svgLocalTransform ]
         OR
   +--[ scrollTranslation ]

With a DCHECK after creating both that they don't exist simultaneously, i.e.

DCHECK(!newTransformNodeForSvgLocalTransform ||
!newTransformNodeForScrollTranslation)
    << "SVG elements cannot have scroll translation";

[pdr., 2016-04-08 02:36:20]

On 2016/04/07 at 18:47:38, jbroman wrote:
>
https://codereview.chromium.org/1865093004/diff/20001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/core/paint/ObjectPaintProperties.h (right):
> 
>
https://codereview.chromium.org/1865093004/diff/20001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/paint/ObjectPaintProperties.h:58: //     |  
+---[ scrollTranslation ]    The space created by overflow clip.
> On 2016/04/07 at 18:19:07, pdr wrote:
> > On 2016/04/07 at 14:37:11, jbroman wrote:
> > > Can an element have both an SVG local transform and a scroll translation?
(If so, can you confirm that this is the right hierarchy?)
> > 
> > Good question. SVG doesn't support any scrolling so this shouldn't be a
problem today.
> 
> Would you mind clarifying this? The code in question updates the context when
it creates both of them, so if both could be created, then they would be parent
and child, not siblings.
> 
> If you don't want to draw it as parent-child, perhaps something like:
> 
>    +--[ svgLocalTransform ]
>          OR
>    +--[ scrollTranslation ]
> 
> With a DCHECK after creating both that they don't exist simultaneously, i.e.
> 
> DCHECK(!newTransformNodeForSvgLocalTransform ||
!newTransformNodeForScrollTranslation)
>     << "SVG elements cannot have scroll translation";

Good idea, done.

My first time using DCHECK. It's pretty cool, I could get used to this.

[pdr., 2016-04-08 02:36:25]

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

[commit-bot: I haz the power, 2016-04-08 02:37:18]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1865093004/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1865093004/40001

[commit-bot: I haz the power, 2016-04-08 03:35:09]

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

[commit-bot: I haz the power, 2016-04-08 03:35:10]

Dry run: This issue passed the CQ dry run.

[jbroman, 2016-04-08 15:24:00]

lgtm

[pdr., 2016-04-08 16:18:01]

The CQ bit was checked by pdr@chromium.org

[commit-bot: I haz the power, 2016-04-08 16:18:28]

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

[commit-bot: I haz the power, 2016-04-08 16:23:35]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-04-08 16:24:53]

Description was changed from

==========
Add a transform paint property for local SVG transforms

This patch adds a transform node for local SVG transforms which lets us
distinguish between SVG and CSS transforms. With SVG and CSS transforms
separated, the SVG root supports both types of transforms simultaneously.
This will let us support compositor-driven CSS transform animations in
the future.

With this patch we can remove the paint offset quirk for SVG. I've just
added TODOs to make this easier to review, but I plan it as an immediate
followup.

BUG=600618
==========

to

==========
Add a transform paint property for local SVG transforms

This patch adds a transform node for local SVG transforms which lets us
distinguish between SVG and CSS transforms. With SVG and CSS transforms
separated, the SVG root supports both types of transforms simultaneously.
This will let us support compositor-driven CSS transform animations in
the future.

With this patch we can remove the paint offset quirk for SVG. I've just
added TODOs to make this easier to review, but I plan it as an immediate
followup.

BUG=600618

Committed: https://crrev.com/143503e05b2d40592e84ec4213af92428dfe3964
Cr-Commit-Position: refs/heads/master@{#386095}
==========

[commit-bot: I haz the power, 2016-04-08 16:24:54]

Patchset 3 (id:??) landed as
https://crrev.com/143503e05b2d40592e84ec4213af92428dfe3964
Cr-Commit-Position: refs/heads/master@{#386095}