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: Fix compute_squared_distance for AnimatedFilterList #17898

Merged
merged 2 commits into from Jul 28, 2017

Conversation

@BorisChiou
Copy link
Contributor

BorisChiou commented Jul 28, 2017

We implement compute_distance for Angle to avoid returning Err(()) from it, and then rewrite compute_squared_distance of AnimatedFilterLIst to avoid using unwrap() and make it simpler.


  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes fix Bug 1384014.
  • These changes do not require tests because there is a DevTools test for this already.

This change is Reviewable

When computing the distance between two filter functions, e.g.
HueRotate(0deg) and HueRotate(-360deg), we got an Err(()) and unwrap it
immediately in compute_filter_square_distance(), which causes a crash.
The root-cause is that we don't implement computed_distance of Angle, and we
assume we always get a valid result from compute_squared_distance() for
each filter function.
@highfive
Copy link

highfive commented Jul 28, 2017

Heads up! This PR modifies the following files:

  • @bholley: components/style/Cargo.toml, components/style/lib.rs, components/style/properties/helpers/animated_properties.mako.rs
  • @canaltinova: components/style/Cargo.toml, components/style/lib.rs, components/style/properties/helpers/animated_properties.mako.rs
  • @emilio: components/style/Cargo.toml, components/style/lib.rs, components/style/properties/helpers/animated_properties.mako.rs
@highfive
Copy link

highfive commented Jul 28, 2017

warning Warning warning

  • These commits modify style code, but no tests are modified. Please consider adding a test!
@BorisChiou BorisChiou changed the title stylo: Fix compute_square_distance for AnimatedFilterList stylo: Fix compute_squared_distance for AnimatedFilterList Jul 28, 2017
@BorisChiou
Copy link
Contributor Author

BorisChiou commented Jul 28, 2017

@bors-servo r=birtles

@bors-servo
Copy link
Contributor

bors-servo commented Jul 28, 2017

📌 Commit d6ddc0a has been approved by birtles

@BorisChiou
Copy link
Contributor Author

BorisChiou commented Jul 28, 2017

@bors-servo
Copy link
Contributor

bors-servo commented Jul 28, 2017

Testing commit d6ddc0a with merge 715e6f2...

bors-servo added a commit that referenced this pull request Jul 28, 2017
…birtles

stylo: Fix compute_squared_distance for AnimatedFilterList

We implement compute_distance for Angle to avoid returning Err(()) from it, and then rewrite compute_squared_distance of AnimatedFilterLIst to avoid using unwrap() and make it simpler.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix [Bug 1384014](https://bugzilla.mozilla.org/show_bug.cgi?id=1384014).
- [X] These changes do not require tests because there is a DevTools test for this 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/17898)
<!-- Reviewable:end -->
@BorisChiou BorisChiou force-pushed the BorisChiou:stylo/animation/filter_distance branch from d6ddc0a to 9028195 Jul 28, 2017
@BorisChiou
Copy link
Contributor Author

BorisChiou commented Jul 28, 2017

@bors-servo r=birtles

Fix cargo lock

@bors-servo
Copy link
Contributor

bors-servo commented Jul 28, 2017

📌 Commit 9028195 has been approved by birtles

@bors-servo
Copy link
Contributor

bors-servo commented Jul 28, 2017

💡 This pull request was already approved, no need to approve it again.

  • There's another pull request that is currently being tested, blocking this pull request: #17892
@bors-servo
Copy link
Contributor

bors-servo commented Jul 28, 2017

📌 Commit 9028195 has been approved by birtles

@BorisChiou
Copy link
Contributor Author

BorisChiou commented Jul 28, 2017

@bors-servo
Copy link
Contributor

bors-servo commented Jul 28, 2017

Testing commit 9028195 with merge 3ab6b1c...

bors-servo added a commit that referenced this pull request Jul 28, 2017
…birtles

stylo: Fix compute_squared_distance for AnimatedFilterList

We implement compute_distance for Angle to avoid returning Err(()) from it, and then rewrite compute_squared_distance of AnimatedFilterLIst to avoid using unwrap() and make it simpler.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix [Bug 1384014](https://bugzilla.mozilla.org/show_bug.cgi?id=1384014).
- [X] These changes do not require tests because there is a DevTools test for this 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/17898)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Jul 28, 2017

@bors-servo bors-servo merged commit 9028195 into servo:master Jul 28, 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

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