stylo: Some distance calculation and interpolated results are not correct for {box|text}-shadow in test_transitions_per_property.html

RESOLVED FIXED in Firefox 57

Status

()

Core
CSS Parsing and Computation
P2
normal
RESOLVED FIXED
3 months ago
3 months ago

People

(Reporter: boris, Assigned: hiro)

Tracking

unspecified
mozilla57
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox57 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(2 attachments, 1 obsolete attachment)

For example:

1. box-shadow:
TEST-UNEXPECTED-FAIL | layout/style/test/test_transitions_per_property.html | property 'box-shadow': distance 0 from quarter 'rgb(255, 0, 0) 8px 8px 8px 2px inset' to end '8px 8px 8px 8px red inset' should be three quarters distance 0 from start '8px 8px 8px red inset' to end '8px 8px 8px 8px red inset'
TEST-UNEXPECTED-FAIL | layout/style/test/test_transitions_per_property.html | shadow-valued property box-shadow: non-interpolable cases - got "rgba(255, 0, 0, 0.75) 6.5px 6.5px 6.5px 0px", expected "rgb(0, 0, 255) 2px 2px 2px 0px"
TEST-UNEXPECTED-FAIL | layout/style/test/test_transitions_per_property.html | shadow-valued property box-shadow: interpolation without color - got "rgba(255, 0, 0, 0.56) 6.38333px 8.38333px 7.38333px 0px", expected "rgb(0, 0, 255) 3px 5px 4px 0px"
... and others

2. text-shadow:
TEST-UNEXPECTED-FAIL | layout/style/test/test_transitions_per_property.html | property 'text-shadow': distance 0 from start 'none' to quarter 'rgba(255, 0, 0, 0.25) 1px 2px 0.75px' should be quarter distance 0 from start 'none' to end '4px 8px 3px red'
TEST-UNEXPECTED-FAIL | layout/style/test/test_transitions_per_property.html | property 'text-shadow': distance 0 from quarter 'rgba(255, 0, 0, 0.25) 1px 2px 0.75px' to end '4px 8px 3px red' should be three quarters distance 0 from start 'none' to end '4px 8px 3px red'
TEST-UNEXPECTED-FAIL | layout/style/test/test_transitions_per_property.html | shadow-valued property text-shadow: non-interpolable cases - got "rgba(66, 96, 0, 0.75) 4.25px 4.25px 2px, rgba(0, 0, 255, 0.56) 1.13333px 1.13333px 0px", expected "rgb(0, 0, 255) 2px 2px 2px"
TEST-UNEXPECTED-FAIL | layout/style/test/test_transitions_per_property.html | shadow-valued property text-shadow: interpolation without color - got "rgba(66, 96, 0, 0.56) 4.68333px 6.68333px 4px, rgba(0, 0, 255, 0.42) 0.85px 0.85px 0px", expected "rgb(0, 0, 255) 3px 5px 4px"
... and others
Blocks: 1388220
Mark this P2 because this may affect a real site issue.
Priority: -- → P2
(Assignee)

Updated

3 months ago
No longer blocks: 1388220
(Assignee)

Comment 2

3 months ago
Interpolation failures need bug 1388220.
Depends on: 1388220
(Assignee)

Comment 3

3 months ago
(In reply to Boris Chiou [:boris] from comment #0)
> 1. box-shadow:
> TEST-UNEXPECTED-FAIL | layout/style/test/test_transitions_per_property.html
> | shadow-valued property box-shadow: non-interpolable cases - got "rgba(255,
> 0, 0, 0.75) 6.5px 6.5px 6.5px 0px", expected "rgb(0, 0, 255) 2px 2px 2px 0px"
> TEST-UNEXPECTED-FAIL | layout/style/test/test_transitions_per_property.html
> | shadow-valued property box-shadow: interpolation without color - got
> "rgba(255, 0, 0, 0.56) 6.38333px 8.38333px 7.38333px 0px", expected "rgb(0,
> 0, 255) 3px 5px 4px 0px"
> ... and others
> 
> 2. text-shadow:
> TEST-UNEXPECTED-FAIL | layout/style/test/test_transitions_per_property.html
> | shadow-valued property text-shadow: non-interpolable cases - got "rgba(66,
> 96, 0, 0.75) 4.25px 4.25px 2px, rgba(0, 0, 255, 0.56) 1.13333px 1.13333px
> 0px", expected "rgb(0, 0, 255) 2px 2px 2px"
> TEST-UNEXPECTED-FAIL | layout/style/test/test_transitions_per_property.html
> | shadow-valued property text-shadow: interpolation without color - got
> "rgba(66, 96, 0, 0.56) 4.68333px 6.68333px 4px, rgba(0, 0, 255, 0.42) 0.85px
> 0.85px 0px", expected "rgb(0, 0, 255) 3px 5px 4px"
> ... and others

As for these interpolation failures reason is similar to bug 1387946.  The cause is gecko does not yet interpolate between shadows without color and with color (bug 726550).
(Assignee)

Comment 4

3 months ago
As for the distance failures, we do not implement yet. Emilio called me in a servo PR [1], but I totally missed that!

[1] https://github.com/servo/servo/pull/18058#discussion_r132844534
(Assignee)

Comment 5

3 months ago
(In reply to Hiroyuki Ikezoe (:hiro) from comment #4)
> As for the distance failures, we do not implement yet. Emilio called me in a
> servo PR [1], but I totally missed that!
> 
> [1] https://github.com/servo/servo/pull/18058#discussion_r132844534

The place might be different.  The problem is for the shadow distance is;
https://hg.mozilla.org/mozilla-central/file/d25db0546c92/servo/components/style/values/animated/effects.rs#l109
(Assignee)

Comment 6

3 months ago
I am going to defer currentColor interpolation issue to bug 1390697.
Assignee: nobody → hikezoe
Status: NEW → ASSIGNED
(Assignee)

Comment 7

3 months ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=f7375653ad36d08ab65f57287fd162f96f4cad17
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 10

3 months ago
mozreview-review
Comment on attachment 8897699 [details]
Bug 1387973 - Modify transition test cases for {text,box}-shadow without color.

https://reviewboard.mozilla.org/r/168992/#review174324
Attachment #8897699 - Flags: review?(bbirtles) → review+
(Reporter)

Comment 11

3 months ago
mozreview-review
Comment on attachment 8897698 [details]
Bug 1387973 - Implement distance for shadow list.

https://reviewboard.mozilla.org/r/168990/#review174326

nice.
Attachment #8897698 - Flags: review?(boris.chiou) → review+
(Assignee)

Comment 12

3 months ago
Created attachment 8897731 [details] [review]
Servo PR
Comment hidden (mozreview-request)
(Assignee)

Updated

3 months ago
Attachment #8897698 - Attachment is obsolete: true

Comment 14

3 months ago
Pushed by hikezoe@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/9a03b7830de9
Modify transition test cases for {text,box}-shadow without color. r=birtles

Comment 15

3 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/9a03b7830de9
Status: ASSIGNED → RESOLVED
Last Resolved: 3 months ago
status-firefox57: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57

Comment 16

3 months ago
Bonus - talos is indicating performance improvements as a result of this patch, good stuff!

== Change summary for alert #8839 (as of August 17 2017 00:21 UTC) ==

Improvements:

  4%  sessionrestore_many_windows linux64 pgo e10s     1,683.42 -> 1,620.42
  3%  sessionrestore_many_windows linux64 opt e10s     1,958.83 -> 1,908.58

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=8839
Huh, that's something really surprising...
You need to log in before you can comment on or make changes to this bug.