Bug 48395

Summary: Accelerated animation with missing values in keyframes is broken
Product: WebKit Reporter: Simon Fraser (smfr) <simon.fraser>
Component: Layout and RenderingAssignee: Simon Fraser (smfr) <simon.fraser>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, ariya.hidayat, cmarrin, eric, jarred, ossy, simon.fraser, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Mac   
OS: OS X 10.5   
URL: http://dev.sencha.com/animator/demos/healthy-choice/
Attachments:
Description Flags
Testcase
none
Patch
none
Patch mitz: review+

Description Simon Fraser (smfr) 2010-10-26 20:44:36 PDT 
Created attachment 71976 [details]
Testcase

http://dev.sencha.com/animator/demos/healthy-choice/ shows problems when running with accelerated compositing; the second set of divs do not become visible at all.
Comment 1 Simon Fraser (smfr) 2010-10-26 20:46:26 PDT 
The workaround is to explicitly specify opacity: 1 in keyframes that need it.
Comment 2 Simon Fraser (smfr) 2010-10-26 20:49:22 PDT 
This probably regressed in r66339.
Comment 3 Simon Fraser (smfr) 2010-10-26 22:13:42 PDT 
Created attachment 71983 [details]
Patch
Comment 4 Jarred Nicholls 2010-10-26 23:18:25 PDT 
Looks good to me.
Comment 5 Simon Fraser (smfr) 2010-10-27 08:26:12 PDT 
Comment on attachment 71983 [details]
Patch

Removing ? until I figure out if I have to stuff the values into the first keyframe also.
Comment 6 Simon Fraser (smfr) 2010-10-27 09:35:46 PDT 
Created attachment 72045 [details]
Patch
Comment 7 WebKit Review Bot 2010-10-27 09:37:15 PDT 
Attachment 72045 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
WebCore/rendering/RenderLayerBacking.cpp:1121:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Total errors found: 1 in 9 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 8 Jarred Nicholls 2010-10-27 09:46:15 PDT 
bool isFirstOrLastKeyframe = !key || key == 1; // per style check... ?
Comment 9 Simon Fraser (smfr) 2010-10-27 09:49:01 PDT 
http://trac.webkit.org/changeset/70657

(In reply to comment #8)
> bool isFirstOrLastKeyframe = !key || key == 1; // per style check... ?

I think what I have is more readable. I am purposefully ignoring the style check.
Comment 10 Jarred Nicholls 2010-10-27 09:54:58 PDT 
Indeed.  Good deal.
Comment 11 WebKit Review Bot 2010-10-27 14:03:24 PDT 
http://trac.webkit.org/changeset/70657 might have broken GTK Linux 32-bit Debug and Qt Linux Release
Comment 12 Csaba Osztrogonác 2010-10-27 14:32:00 PDT 
(In reply to comment #11)
> http://trac.webkit.org/changeset/70657 might have broken GTK Linux 32-bit Debug and Qt Linux Release

FYI: Qt specifix expected results landed in: http://trac.webkit.org/changeset
/70710

Please don't ignore commnts of sheriffbot! He usually tell the truth.