Bug 108961 - [GTK][AC] Implement opacity animation with clutter ac backend
Summary: [GTK][AC] Implement opacity animation with clutter ac backend
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks: 105699
  Show dependency treegraph
 
Reported: 2013-02-05 10:58 PST by ChangSeok Oh
Modified: 2013-02-08 08:09 PST (History)
4 users (show)

See Also:


Attachments
Patch (47.07 KB, patch)
2013-02-05 12:23 PST, ChangSeok Oh
gns: review+
gns: commit-queue-
Details | Formatted Diff | Diff
Patch (47.18 KB, patch)
2013-02-06 08:19 PST, ChangSeok Oh
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description ChangSeok Oh 2013-02-05 10:58:09 PST
In this bug, I plan to add opacity animation with clutter backend.
Comment 1 ChangSeok Oh 2013-02-05 12:23:38 PST
Created attachment 186675 [details]
Patch
Comment 2 Gustavo Noronha (kov) 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.
Comment 3 ChangSeok Oh 2013-02-06 08:19:51 PST
Created attachment 186859 [details]
Patch
Comment 4 ChangSeok Oh 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
Comment 5 WebKit Review Bot 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>
Comment 6 Sergio Villar Senin 2013-02-08 07:42:57 PST
forgot to close the bug?