Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[motion-1] Serialization of offset-rotate #340

Closed
BorisChiou opened this issue May 22, 2019 · 4 comments
Closed

[motion-1] Serialization of offset-rotate #340

BorisChiou opened this issue May 22, 2019 · 4 comments

Comments

@BorisChiou
Copy link

BorisChiou commented May 22, 2019

Per the spec definition [1] and its wpt [2], the serialization of the specified values seems like:
(template: input => serialization of the specified value)

  1. auto => auto
  2. auto 0deg => auto 0deg
  3. 0rad reverse => reverse 0rad
  4. 0deg => 0deg

However, based on the shortest serialization principle, it seems the expected results are:

  1. auto => auto
  2. auto 0deg => auto
  3. 0rad reverse => reverse
  4. 0deg => 0deg

Just wonder is it possible to make the serialization simpler? Or is there any specific reason to keep the 0deg angle in the serialization?

This rule also applies to the computed value, I think.
(template: input => specified value => computed value)

  1. auto => auto => auto
  2. auto 0deg => auto => auto
  3. reverse 0deg => reverse => auto 180deg

Besides, for computed value, it seems we always convert reverse xdeg into auto (180+x)deg, so it could be easier to do interpolation because no need to do an additional conversion, I guess.

[1] https://drafts.fxtf.org/motion-1/#offset-rotate-property
[2] https://github.com/web-platform-tests/wpt/blob/master/css/motion/parsing/offset-rotate-parsing-valid.html

cc @ericwilligers @emilio

@emilio
Copy link
Contributor

emilio commented May 22, 2019

So is reverse expected to go away at computed value time? Are reverse and auto values expected to be interpolable?

@emilio
Copy link
Contributor

emilio commented May 22, 2019

Oh it does look like it looking at the spec. That's fine, but I wonder why having reverse in the first place then, you can just do calc(180deg + angle). But sure.

BorisChiou added a commit to BorisChiou/gecko-dev that referenced this issue May 22, 2019
This includes style system and layout update. I add 3 extra reftest
because the original tests use ray() function as the offset-path, but we
don't support it. It'd be better to add tests using a different type of
offset-path.

The spec issue about the serialization:
w3c/fxtf-drafts#340

Differential Revision: https://phabricator.services.mozilla.com/D32212
BorisChiou added a commit to BorisChiou/gecko-dev that referenced this issue May 23, 2019
This includes style system and layout update. I add 3 extra reftest
because the original tests use ray() function as the offset-path, but we
don't support it. It'd be better to add tests using a different type of
offset-path.

The spec issue about the serialization:
w3c/fxtf-drafts#340

Differential Revision: https://phabricator.services.mozilla.com/D32212
moz-wptsync-bot pushed a commit to web-platform-tests/wpt that referenced this issue May 24, 2019
This includes style system and layout update. I add 3 extra reftests
because the original tests use ray() function as the offset-path, but we
don't support it. It'd be better to add tests using a different type of
offset-path.

The spec issue about the serialization:
w3c/fxtf-drafts#340

Differential Revision: https://phabricator.services.mozilla.com/D32212

bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1429301
gecko-commit: ae71e496d5873033d9696c54fb8b36297abbf8d0
gecko-integration-branch: autoland
gecko-reviewers: emilio
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue May 24, 2019
This includes style system and layout update. I add 3 extra reftests
because the original tests use ray() function as the offset-path, but we
don't support it. It'd be better to add tests using a different type of
offset-path.

The spec issue about the serialization:
w3c/fxtf-drafts#340

Differential Revision: https://phabricator.services.mozilla.com/D32212

--HG--
extra : moz-landing-system : lando
lissyx pushed a commit to lissyx/mozilla-central that referenced this issue May 24, 2019
This includes style system and layout update. I add 3 extra reftests
because the original tests use ray() function as the offset-path, but we
don't support it. It'd be better to add tests using a different type of
offset-path.

The spec issue about the serialization:
w3c/fxtf-drafts#340

Differential Revision: https://phabricator.services.mozilla.com/D32212
moz-wptsync-bot pushed a commit to web-platform-tests/wpt that referenced this issue May 24, 2019
This includes style system and layout update. I add 3 extra reftests
because the original tests use ray() function as the offset-path, but we
don't support it. It'd be better to add tests using a different type of
offset-path.

The spec issue about the serialization:
w3c/fxtf-drafts#340

Differential Revision: https://phabricator.services.mozilla.com/D32212

bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1429301
gecko-commit: ae71e496d5873033d9696c54fb8b36297abbf8d0
gecko-integration-branch: autoland
gecko-reviewers: emilio
emilio pushed a commit to emilio/servo that referenced this issue May 25, 2019
This includes style system and layout update. I add 3 extra reftests
because the original tests use ray() function as the offset-path, but we
don't support it. It'd be better to add tests using a different type of
offset-path.

The spec issue about the serialization:
w3c/fxtf-drafts#340

Differential Revision: https://phabricator.services.mozilla.com/D32212
emilio pushed a commit to emilio/servo that referenced this issue May 25, 2019
This includes style system and layout update. I add 3 extra reftests
because the original tests use ray() function as the offset-path, but we
don't support it. It'd be better to add tests using a different type of
offset-path.

The spec issue about the serialization:
w3c/fxtf-drafts#340

Differential Revision: https://phabricator.services.mozilla.com/D32212
@ewilligers
Copy link
Contributor

The reverse is motivated by SMIL animateMotion rotate.
Otherwise, I don't think it would have been included.

If we have animation from auto 20deg to reverse -60deg,
should this flip at 50% progress or should animation be smooth?
If smooth, should the computed value at 60% progress be auto 80deg
or reverse -100deg? Smooth animation is most useful, but note that we
don't usually (ever?) have smooth animation when a keyword in the
computed value changes between keyframes.

The spec answers these questions by normalizing all
computed values to auto? <angle>:
"Computed value: computed <angle> value, optionally preceded by auto".

We have smooth animation, with auto 80deg at 60% progress.

emilio pushed a commit to emilio/servo that referenced this issue May 29, 2019
This includes style system and layout update. I add 3 extra reftests
because the original tests use ray() function as the offset-path, but we
don't support it. It'd be better to add tests using a different type of
offset-path.

The spec issue about the serialization:
w3c/fxtf-drafts#340

Differential Revision: https://phabricator.services.mozilla.com/D32212
marcoscaceres pushed a commit to web-platform-tests/wpt that referenced this issue Jul 23, 2019
This includes style system and layout update. I add 3 extra reftests
because the original tests use ray() function as the offset-path, but we
don't support it. It'd be better to add tests using a different type of
offset-path.

The spec issue about the serialization:
w3c/fxtf-drafts#340

Differential Revision: https://phabricator.services.mozilla.com/D32212

bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1429301
gecko-commit: ae71e496d5873033d9696c54fb8b36297abbf8d0
gecko-integration-branch: autoland
gecko-reviewers: emilio
@tabatkins
Copy link
Member

Closing as invalid, because the spec doesn't specify serialization at all, and so the general CSSOM rules apply - shortest serialization, in grammar order.

As always, the "Computed Value" line says absolutely nothing about how to serialize the value; it describes the internal representation of the value, which is then used by serialization to produce a string that'll reproduce the same internal representation when re-parsed.

However, the WPT tests are thus wrong and need to be updated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants