[Webkit-unassigned] [Bug 110314] [GTK][AC] Implement matrix keyframe animations with clutter ac backend

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=110314





--- Comment #5 from ChangSeok Oh <kevin.cs.oh at gmail.com>  2013-04-03 02:48:54 PST ---
(From update of attachment 189267)
View in context: https://bugs.webkit.org/attachment.cgi?id=189267&action=review

Thank you for the comment, but would you consider it again if you don't mind?
I leave some notes, please point out if there is my misunderstanding. :)

>> Source/WebCore/platform/graphics/clutter/PlatformClutterAnimation.cpp:540
>> +    GOwnPtr<GValue> keyValues(g_new0(GValue, nKeyframes));
> 
> This will go wrong. GOwnPtr will expect a single GValue pointer here but you want an array. We either need a GOwnArrayPtr like the one used for OwnPtr above, or manual handling of this array. I suggest using manual handling.

Whether we allocate a single GValue or Gvalue array, we can and should free it by using g_free. Because we used g_new0 here to allocate it. Of course I know GOwnPtr<GValue> is not for array type but it works as we expected. It will call 'g_free' when finalizing this function.

>> Source/WebCore/platform/graphics/clutter/PlatformClutterAnimation.cpp:559
>> +}
> 
> Then here you'll have to free all of the values instead of unsetting.

I think we should call g_value_unset here since the GValues initialized and set CLUTTER_TYPE_MATRIX which is an object type not a primitive type. In my humble opinion, we should clear the deep allocation inside of each GValue before freeing it.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.