CSS Motion Path: calculate transform for simple ray() paths

CSS Motion Path: calculate transform for simple ray() paths

ray(<angle> <size>) indicates a ray with a bearing of the specified
angle. For example, ray(0 closest-side) indicates a ray towards the
top of the containing box.

Spec:
https://drafts.fxtf.org/motion-1/#offset-path-property

Not yet implemented: calculating the transform when ray uses 'contain',
or when offset-distance contains percentage, and thus the ray length
must be determined.

BUG=641245, 722757
Review-Url: https://codereview.chromium.org/2884943002
Cr-Commit-Position: refs/heads/master@{#472409}
Committed: https://chromium.googlesource.com/chromium/src/+/fb7632da4263f6fc7c97f120d2792282baf56770

[Eric Willigers, 2017-05-16 11:08:29]

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

[commit-bot: I haz the power, 2017-05-16 11:08:40]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Eric Willigers, 2017-05-16 11:26:19]

ericwilligers@chromium.org changed reviewers:
+ pdr@chromium.org

[commit-bot: I haz the power, 2017-05-16 12:30:32]

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

[commit-bot: I haz the power, 2017-05-16 12:30:33]

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

[pdr., 2017-05-16 22:51:19]

Overall this looks good! Do you think we could get someone more familiar with
CSS to help review this? Possibly meade?

https://codereview.chromium.org/2884943002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/external/wpt/css/motion-1/offset-path-ray.html
(right):

https://codereview.chromium.org/2884943002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/external/wpt/css/motion-1/offset-path-ray.html:11:
position: absolute;
Do these have to be position: absolute?

https://codereview.chromium.org/2884943002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/external/wpt/css/motion-1/offset-path-ray.html:18:
offset-path: ray(150deg closest-side);
Naive question: when I try the examples from the spec such as example 6 with the
circles, they don't work. For example, http://jsbin.com/xorofav. Is there
something I'm missing, or is this just lack of <size> support?

https://codereview.chromium.org/2884943002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/style/ComputedStyle.cpp (right):

https://codereview.chromium.org/2884943002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/style/ComputedStyle.cpp:1346: const StylePath&
motion_path = ToStylePath(*motion_data.path_);
Would it be useful to add the following?
DCHECK(motion_data.path_->GetType() == kStylePathType);

[Eric Willigers, 2017-05-17 05:46:59]

ericwilligers@chromium.org changed reviewers:
+ meade@chromium.org

[Eric Willigers, 2017-05-17 05:47:00]

https://codereview.chromium.org/2884943002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/external/wpt/css/motion-1/offset-path-ray.html
(right):

https://codereview.chromium.org/2884943002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/external/wpt/css/motion-1/offset-path-ray.html:11:
position: absolute;
On 2017/05/16 22:51:19, pdr. wrote:
> Do these have to be position: absolute?

left and top only have effect on positioned elements.
https://developer.mozilla.org/en/docs/Web/CSS/left

We could omit position: absolute and left and top, but then the elements would
be partially rotated off the screen.

https://codereview.chromium.org/2884943002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/external/wpt/css/motion-1/offset-path-ray.html:18:
offset-path: ray(150deg closest-side);
On 2017/05/16 22:51:19, pdr. wrote:
> Naive question: when I try the examples from the spec such as example 6 with
the
> circles, they don't work. For example, http://jsbin.com/xorofav. Is there
> something I'm missing, or is this just lack of <size> support?

There are three issues.

1. The circles appear below each other because of block layout. The example
should have
  .circleElement {
    position: absolute;
    left: 50%;
    top: 50%;
  }
and
  body {
    transform-style: preserve-3d;
  }
so that the elements are placed on rays from the body's center.

2. The offset-path values omit <size>, which is now required.

3. The example uses percentage offset-distance, which is yet to be implemented.
For now, use px:
https://jsbin.com/poseha

I have created https://github.com/w3c/fxtf-drafts/pull/164 for the first two
issues.

https://codereview.chromium.org/2884943002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/style/ComputedStyle.cpp (right):

https://codereview.chromium.org/2884943002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/style/ComputedStyle.cpp:1346: const StylePath&
motion_path = ToStylePath(*motion_data.path_);
On 2017/05/16 22:51:19, pdr. wrote:
> Would it be useful to add the following?
> DCHECK(motion_data.path_->GetType() == kStylePathType);

ToStylePath is already a debug checked cast, just like ToSVGPath.

[meade_UTC10, 2017-05-17 05:54:05]

this lgtm for ComputedStyle. cc'ing shend since he's currently working on
ComputedStyle.

[Eric Willigers, 2017-05-17 10:14:40]

The CQ bit was checked by ericwilligers@chromium.org

[commit-bot: I haz the power, 2017-05-17 10:15:04]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-17 10:21:15]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1495016080576390, "parent_rev":
"3b4c47659ea48f66c256654e99950e9511b520ec", "commit_rev":
"fb7632da4263f6fc7c97f120d2792282baf56770"}

[commit-bot: I haz the power, 2017-05-17 10:21:26]

Description was changed from

==========
CSS Motion Path: calculate transform for simple ray() paths

ray(<angle> <size>) indicates a ray with a bearing of the specified
angle. For example, ray(0 closest-side) indicates a ray towards the
top of the containing box.

Spec:
https://drafts.fxtf.org/motion-1/#offset-path-property

Not yet implemented: calculating the transform when ray uses 'contain',
or when offset-distance contains percentage, and thus the ray length
must be determined.

BUG=641245,722757
==========

to

==========
CSS Motion Path: calculate transform for simple ray() paths

ray(<angle> <size>) indicates a ray with a bearing of the specified
angle. For example, ray(0 closest-side) indicates a ray towards the
top of the containing box.

Spec:
https://drafts.fxtf.org/motion-1/#offset-path-property

Not yet implemented: calculating the transform when ray uses 'contain',
or when offset-distance contains percentage, and thus the ray length
must be determined.

BUG=641245,722757

Review-Url: https://codereview.chromium.org/2884943002
Cr-Commit-Position: refs/heads/master@{#472409}
Committed:
https://chromium.googlesource.com/chromium/src/+/fb7632da4263f6fc7c97f120d279...
==========

[commit-bot: I haz the power, 2017-05-17 10:21:27]

Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/fb7632da4263f6fc7c97f120d279...

[Yiorsi, 2017-05-18 03:09:02]

On 2017/05/17 05:47:00, Eric Willigers wrote:
>
https://codereview.chromium.org/2884943002/diff/1/third_party/WebKit/LayoutTe...
> File
> third_party/WebKit/LayoutTests/external/wpt/css/motion-1/offset-path-ray.html
> (right):
> 
>
https://codereview.chromium.org/2884943002/diff/1/third_party/WebKit/LayoutTe...
>
third_party/WebKit/LayoutTests/external/wpt/css/motion-1/offset-path-ray.html:11:
> position: absolute;
> On 2017/05/16 22:51:19, pdr. wrote:
> > Do these have to be position: absolute?
> 
> left and top only have effect on positioned elements.
> https://developer.mozilla.org/en/docs/Web/CSS/left
> 
> We could omit position: absolute and left and top, but then the elements would
> be partially rotated off the screen.
> 
>
https://codereview.chromium.org/2884943002/diff/1/third_party/WebKit/LayoutTe...
>
third_party/WebKit/LayoutTests/external/wpt/css/motion-1/offset-path-ray.html:18:
> offset-path: ray(150deg closest-side);
> On 2017/05/16 22:51:19, pdr. wrote:
> > Naive question: when I try the examples from the spec such as example 6 with
> the
> > circles, they don't work. For example, http://jsbin.com/xorofav. Is there
> > something I'm missing, or is this just lack of <size> support?
> 
> There are three issues.
> 
> 1. The circles appear below each other because of block layout. The example
> should have
>   .circleElement {
>     position: absolute;
>     left: 50%;
>     top: 50%;
>   }
> and
>   body {
>     transform-style: preserve-3d;
>   }
> so that the elements are placed on rays from the body's center.
> 
> 2. The offset-path values omit <size>, which is now required.
> 
> 3. The example uses percentage offset-distance, which is yet to be
implemented.
> For now, use px:
> https://jsbin.com/poseha
> 
> I have created https://github.com/w3c/fxtf-drafts/pull/164 for the first two
> issues.
> 
>
https://codereview.chromium.org/2884943002/diff/1/third_party/WebKit/Source/c...
> File third_party/WebKit/Source/core/style/ComputedStyle.cpp (right):
> 
>
https://codereview.chromium.org/2884943002/diff/1/third_party/WebKit/Source/c...
> third_party/WebKit/Source/core/style/ComputedStyle.cpp:1346: const StylePath&
> motion_path = ToStylePath(*motion_data.path_);
> On 2017/05/16 22:51:19, pdr. wrote:
> > Would it be useful to add the following?
> > DCHECK(motion_data.path_->GetType() == kStylePathType);
> 
> ToStylePath is already a debug checked cast, just like ToSVGPath.

Why can't we omit <size>?

[Eric Willigers, 2017-05-18 04:11:45]

On 2017/05/18 03:09:02, Yiorsi wrote:
> Why can't we omit <size>?

There was disagreement about the default: the spec defaulted to 'closest-side'
so we had polar positioning with distances independent of angle.

fantasai preferred for the default to be that a 100% ray extended to the
containing block edge in the direction of the current angle.
https://github.com/w3c/fxtf-drafts/issues/73

A compromise was agreed at the recent CSS Working Group meeting in Tokyo: <size>
became required, and fantasai's preferred behavior becomes available with a new
keyword, 'sides'.
https://github.com/w3c/fxtf-drafts/issues/73#issuecomment-296084456

[Yiorsi, 2017-05-18 05:06:56]

On 2017/05/18 04:11:45, Eric Willigers wrote:
> On 2017/05/18 03:09:02, Yiorsi wrote:
> > Why can't we omit <size>?
> 
> There was disagreement about the default: the spec defaulted to 'closest-side'
> so we had polar positioning with distances independent of angle.
> 
> fantasai preferred for the default to be that a 100% ray extended to the
> containing block edge in the direction of the current angle.
> https://github.com/w3c/fxtf-drafts/issues/73
> 
> A compromise was agreed at the recent CSS Working Group meeting in Tokyo:
<size>
> became required, and fantasai's preferred behavior becomes available with a
new
> keyword, 'sides'.
> https://github.com/w3c/fxtf-drafts/issues/73#issuecomment-296084456

Thanks!

BTW, animation is not supported yet?

[Eric Willigers, 2017-05-19 00:11:26]

> BTW, animation is not supported yet?

Intent:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/_DPl-JG6bV8/y2_GUwFV...

I need to update an animations spec to document path string interpolation, and
we need consensus on the serialization format for path strings.

Currently only offset-distance and offset-rotate have animation shipped.