Bug 108961

Summary: [GTK][AC] Implement opacity animation with clutter ac backend
Product: WebKit Reporter: ChangSeok Oh <kevin.cs.oh>
Component: WebKitGTKAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: gustavo, joone, svillar, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 105699    
Attachments:
Description Flags
Patch
gustavo: review+, gustavo: commit-queue-
Patch none

ChangSeok Oh
Reported 2013-02-05 10:58:09 PST
In this bug, I plan to add opacity animation with clutter backend.
Attachments
Patch (47.07 KB, patch)
2013-02-05 12:23 PST, ChangSeok Oh 		gustavo: review+
gustavo: commit-queue-
DetailsFormatted DiffDiff
Patch (47.18 KB, patch)
2013-02-06 08:19 PST, ChangSeok Oh 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
ChangSeok Oh
Comment 1 2013-02-05 12:23:38 PST
Created attachment 186675 [details] Patch
Gustavo Noronha (kov)
Comment 2 2013-02-06 04:26:26 PST
Comment on attachment 186675 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=186675&action=review Looks OK in general, just a few nits. > Source/WebCore/ChangeLog:9 > + Almost implementations of GraphicsLayerClutter are based on mac port's one. This sentence seems to be missing a word. "Almost all"? > Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.cpp:32 > > +#include <wtf/text/CString.h> No blank line. > Source/WebCore/platform/graphics/clutter/GraphicsLayerClutter.cpp:45 > + > +#include <limits.h> Ditto. > Source/WebCore/platform/graphics/clutter/GraphicsLayerClutter.cpp:53 > +// If we send a duration of 0 to CA, then it will use the default duration Talking about CA here doesn't make much sense =) > Source/WebCore/platform/graphics/clutter/GraphicsLayerClutter.cpp:678 > + // CoreAnimation does not handle the steps() timing function. Fall back Another CA-related comment here. > Source/WebCore/platform/graphics/clutter/PlatformClutterAnimation.cpp:398 > + notImplemented(); > + else if (m_animatedPropertyType == BackgroundColor) > + notImplemented(); Make these ASSERT_NOT_REACHED() for now? You should be refusing to create these kinds of animations, so they should really not be reached.
ChangSeok Oh
Comment 3 2013-02-06 08:19:51 PST
Created attachment 186859 [details] Patch
ChangSeok Oh
Comment 4 2013-02-06 08:20:37 PST
Comment on attachment 186675 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=186675&action=review Thanks! :D >> Source/WebCore/ChangeLog:9 >> + Almost implementations of GraphicsLayerClutter are based on mac port's one. > > This sentence seems to be missing a word. "Almost all"? Oops. Sorry. >> Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.cpp:32 >> +#include <wtf/text/CString.h> > > No blank line. Done >> Source/WebCore/platform/graphics/clutter/GraphicsLayerClutter.cpp:45 >> +#include <limits.h> > > Ditto. yeap. >> Source/WebCore/platform/graphics/clutter/GraphicsLayerClutter.cpp:53 >> +// If we send a duration of 0 to CA, then it will use the default duration > > Talking about CA here doesn't make much sense =) Updated the comment talking about clutterTimeline. >> Source/WebCore/platform/graphics/clutter/GraphicsLayerClutter.cpp:678 >> + // CoreAnimation does not handle the steps() timing function. Fall back > > Another CA-related comment here. I rewrote this comment. >> Source/WebCore/platform/graphics/clutter/PlatformClutterAnimation.cpp:398 >> + notImplemented(); > > Make these ASSERT_NOT_REACHED() for now? You should be refusing to create these kinds of animations, so they should really not be reached. Done
WebKit Review Bot
Comment 5 2013-02-07 04:23:20 PST
Comment on attachment 186859 [details] Patch Clearing flags on attachment: 186859 Committed r142094: <http://trac.webkit.org/changeset/142094>
Sergio Villar Senin
Comment 6 2013-02-08 07:42:57 PST
forgot to close the bug?
Note You need to log in before you can comment on or make changes to this bug.