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

stylo: Bug 1341372 - Detect new transitions and let it run. #16496

Merged
merged 7 commits into from Apr 17, 2017

Conversation

@BorisChiou
Copy link
Contributor

BorisChiou commented Apr 17, 2017

These are interdependent patches of Bug 1341372. We let animation-only restyle also work for RESTYLE_CSS_TRANSITIONS, and check if we need to update transitions by each transition property. If it is necessary to create/replace/cancel transitions, we create a SequentialTask for CSS_TRANSITIONS.


  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes fix Bug 1341372
  • These changes do not require tests because there are tests in Gecko already.

This change is Reviewable

BorisChiou added 7 commits Apr 17, 2017
Animation-only restyle should include both Animation and Transition
cascade levels.

MozReview-Commit-ID: 5l6gaJKbixM
Add one FFI to check if there is any transition in CSSTransitionCollection.
This will be used to check if we need to update transition and if we
should compute the after-change style.

MozReview-Commit-ID: 6HpVAtrx6Rc
We will add another function, needs_update_transitions, to check if we need
to update transitions, so factor this out.

MozReview-Commit-ID: 5LYkyi4aDri
It is possible to call get_after_change_style if there is no transition
rule. In order to avoid cloning the token computed values, we just return None.
The caller can use this Option to know which computed values should be
used.

MozReview-Commit-ID: 7fcgSVEtXWh
TransitionProperty::any returns true if one of its closure returns true.

MozReview-Commit-ID: 4YsKkHaWCYq
1. We need to call get_after_change_style, which is the computed styles
   without transition rules, while process_animations.
2. If we have after-change style, we may replace the new computed values with
   after-change style, according to whether we really need to update
   transitions.
3. There are some cases we don't update transitions, so we need to early
   return. might_needs_transitions_update() will check it first and it
   will filter out most common cases.
4. needs_transitions_update() will check each property and existing running
   transitions to make sure we really don't need to update transitions.
   The logic of this function is similar with that of
   nsTransitionManager::DoUpdateTransitions().

MozReview-Commit-ID: 2ccdPjgrxKz
MozReview-Commit-ID: 2bJlBbdX543
@highfive
Copy link

highfive commented Apr 17, 2017

Heads up! This PR modifies the following files:

  • @bholley: components/style/gecko_bindings/bindings.rs, components/style/properties/gecko.mako.rs, components/style/data.rs, components/style/restyle_hints.rs, components/style/dom.rs and 4 more
  • @KiChjang: components/script/layout_wrapper.rs
  • @fitzgen: components/script/layout_wrapper.rs
  • @emilio: components/style/gecko_bindings/bindings.rs, components/style/properties/gecko.mako.rs, ports/geckolib/glue.rs, components/style/data.rs, components/style/restyle_hints.rs and 5 more
@highfive
Copy link

highfive commented Apr 17, 2017

warning Warning warning

  • These commits modify unsafe code. Please review it carefully!
  • These commits modify style and script code, but no tests are modified. Please consider adding a test!
@BorisChiou
Copy link
Contributor Author

BorisChiou commented Apr 17, 2017

I would like to wait the try results (gecko side).

@BorisChiou
Copy link
Contributor Author

BorisChiou commented Apr 17, 2017

@highfive highfive assigned heycam and unassigned metajack Apr 17, 2017
@BorisChiou
Copy link
Contributor Author

BorisChiou commented Apr 17, 2017

@BorisChiou
Copy link
Contributor Author

BorisChiou commented Apr 17, 2017

@bors-servo r=heycam

@bors-servo
Copy link
Contributor

bors-servo commented Apr 17, 2017

📌 Commit 5a1582c has been approved by heycam

@bors-servo
Copy link
Contributor

bors-servo commented Apr 17, 2017

Testing commit 5a1582c with merge 4d8c9c1...

bors-servo added a commit that referenced this pull request Apr 17, 2017
stylo: Bug 1341372 - Detect new transitions and let it run.

These are interdependent patches of Bug 1341372. We let animation-only restyle also work for RESTYLE_CSS_TRANSITIONS, and check if we need to update transitions by each transition property. If it is necessary to create/replace/cancel transitions, we create a SequentialTask for CSS_TRANSITIONS.

---
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix Bug 1341372
- [X] These changes do not require tests because there are tests in Gecko already.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/16496)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Apr 17, 2017

☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css, mac-rel-wpt1, mac-rel-wpt2, windows-msvc-dev
Approved by: heycam
Pushing 4d8c9c1 to master...

@bors-servo bors-servo merged commit 5a1582c into servo:master Apr 17, 2017
3 checks passed
3 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.