Closed Bug 1387973 Opened 7 years ago Closed 7 years ago

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

Categories

(Core :: CSS Parsing and Computation, defect, P2)

Product:
Core
Component:
CSS Parsing and Computation
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla57
Tracking Flags:
Tracking Status
firefox57 --- fixed

People

(Reporter: boris, Assigned: hiro)

References

Details

Attachments

(2 files, 1 obsolete file)

Bug 1387973 - Modify transition test cases for {text,box}-shadow without color.
59 bytes, text/x-review-board-request
birtles
: review+
Details
Servo PR
41 bytes, text/x-github-pull-request
Details | Review 
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
Mark this P2 because this may affect a real site issue.
Priority: -- → P2
No longer blocks: 1388220
Interpolation failures need bug 1388220.
Depends on: 1388220
(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).
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
(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
I am going to defer currentColor interpolation issue to bug 1390697.
Assignee: nobody → hikezoe
Status: NEW → ASSIGNED
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+
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+
Attached file Servo PR — 

    
    

    
      
  

    
  

        Attachment #8897698 -
        Attachment is obsolete: true

    
    

    
  
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

    
    

    
  
https://hg.mozilla.org/mozilla-central/rev/9a03b7830de9
Status: ASSIGNED → RESOLVED
Closed: 7 years ago

          status-firefox57:
          --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57

    
    

    
  
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.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: