110314 2013-02-20 01:42:11 -0800 [GTK][AC] Implement matrix keyframe animations with clutter ac backend 2013-04-03 08:31:05 -0700 1 1 1 Unclassified WebKit WebKitGTK 528+ (Nightly build) All Linux RESOLVED FIXED P2 Normal --- 105699 0 kevin.cs.oh webkit-unassigned gustavo joone mrobinson webkit.review.bot oldest_to_newest 837260 0 kevin.cs.oh 2013-02-20 01:42:11 -0800 This bug is for adding matrix keyframe animation implementation. 837280 1 189267 kevin.cs.oh 2013-02-20 02:13:15 -0800 Created attachment 189267 Patch 847748 2 kevin.cs.oh 2013-03-05 09:31:06 -0800 You can find the same patch for webkit-clutter here, http://cgit.collabora.com/git/webkit-clutter.git/commit/?h=wip/changseok/unreviewed&id=c4a0149b278963106d6d33c50438873a938ac85a 866552 3 kevin.cs.oh 2013-03-29 22:14:28 -0700 Maybe the address changed for webkit-clutter. Here is a new one. http://cgit.collabora.com/git/webkit-clutter.git/commit/?h=wip/changseok/unreviewed&id=f609e018f18232c1c4cd40c7fdb0f7751aa5eb65 867763 4 189267 gustavo 2013-04-02 06:02:02 -0700 Comment on attachment 189267 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=189267&action=review > 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. > Source/WebCore/platform/graphics/clutter/PlatformClutterAnimation.cpp:559 > + for (unsigned i = 0; i < nKeyframes; ++i) > + g_value_unset(&keyValues.get()[i]); > +} Then here you'll have to free all of the values instead of unsetting. 868567 5 189267 kevin.cs.oh 2013-04-03 02:48:54 -0700 Comment on attachment 189267 Patch 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. 868654 6 189267 gustavo 2013-04-03 05:51:33 -0700 Comment on attachment 189267 Patch I think that's abusing GOwnPtr, makes things a bit less hackable, but I don't feel too strongly, so OK, let's go with your change. 868676 7 189267 webkit.review.bot 2013-04-03 06:21:52 -0700 Comment on attachment 189267 Patch Clearing flags on attachment: 189267 Committed r147549: <http://trac.webkit.org/changeset/147549> 868677 8 webkit.review.bot 2013-04-03 06:21:55 -0700 All reviewed patches have been landed. Closing bug. 868735 9 kevin.cs.oh 2013-04-03 08:31:05 -0700 (In reply to comment #6) > (From update of attachment 189267 [details]) > I think that's abusing GOwnPtr, makes things a bit less hackable, but I don't feel too strongly, so OK, let's go with your change. Thanks :) Let me improve those lines to make less hackable later! ;) 189267 2013-02-20 02:13:15 -0800 2013-04-03 06:21:51 -0700 Patch bug-110314-20130220190937.patch text/plain 6304 kevin.cs.oh U3VidmVyc2lvbiBSZXZpc2lvbjogMTQzNDQwCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggNDNjZjZhNGVkYjkwMjBj MmUyMDE2OGFkNzQyOGE5MTAzNGNlZjcxNC4uMWJkYTczOWMxNDhmN2Q2Nzk3ZDE5YjAyMWNlZGJj OWJmYjkzMzY2YSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDI1IEBACisyMDEzLTAyLTIwICBDaGFu Z1Nlb2sgT2ggIDxjaGFuZ3Nlb2sub2hAY29sbGFib3JhLmNvbT4KKworICAgICAgICBbR1RLXVtB Q10gSW1wbGVtZW50IG1hdHJpeCBrZXlmcmFtZSBhbmltYXRpb25zIHdpdGggY2x1dHRlciBhYyBi YWNrZW5kCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0x MTAzMTQKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBD bHV0dGVyIDEuMTIgZG9lc24ndCBzdXBwb3J0IGFkZGl0aXZlIHRyYW5zZm9ybSBhbmltYXRpb25z IHlldCwgU28gY2x1dHRlciBhYyBiYWNrZW5kCisgICAgICAgIHVzZXMgbWF0cml4IGFuaW1hdGlv bnMgZm9yIHRoZSBjYXNlLiBUaGlzIHBhdGNoIGZvbGxvd3MgY2hhbmdlc2V0IDE0MzM2OSwgMTQz MzQzCisgICAgICAgIHRvIHN1cHBvcnQgbWF0cml4IGtleWZyYW1lIGFuaW1hdGlvbnMuIEkgYmVs aWV2ZSB0aGlzIGNoYW5nZSBpcyBlYXN5IGlmIHlvdSB1bmRlcnN0YW5kCisgICAgICAgIGFib3Zl IHR3byBjaGFuZ2VzZXRzLgorCisgICAgICAgIENvdmVyZWQgYnkgZXhpc3RpbmcgYW5pbWF0aW9u IHRlc3RzLgorCisgICAgICAgICogcGxhdGZvcm0vZ3JhcGhpY3MvY2x1dHRlci9QbGF0Zm9ybUNs dXR0ZXJBbmltYXRpb24uY3BwOgorICAgICAgICAoV2ViQ29yZTo6UGxhdGZvcm1DbHV0dGVyQW5p bWF0aW9uOjpzZXRWYWx1ZXMpOgorICAgICAgICAoV2ViQ29yZTo6UGxhdGZvcm1DbHV0dGVyQW5p bWF0aW9uOjphZGRDbHV0dGVyS2V5ZnJhbWVUcmFuc2l0aW9uRm9yUHJvcGVydHkpOgorICAgICAg ICAoV2ViQ29yZSk6CisgICAgICAgIChXZWJDb3JlOjpQbGF0Zm9ybUNsdXR0ZXJBbmltYXRpb246 OmFkZFRyYW5zZm9ybVRyYW5zaXRpb24pOgorICAgICAgICAqIHBsYXRmb3JtL2dyYXBoaWNzL2Ns dXR0ZXIvUGxhdGZvcm1DbHV0dGVyQW5pbWF0aW9uLmg6CisgICAgICAgIChQbGF0Zm9ybUNsdXR0 ZXJBbmltYXRpb24pOgorCiAyMDEzLTAyLTIwICBBbmRyZXkgTHVzaG5pa292ICA8bHVzaG5pa292 QGNocm9taXVtLm9yZz4KIAogICAgICAgICBXZWIgSW5zcGVjdG9yOiBoaWdobGlnaHQgdW5kZWZp bmVkIHdvcmQgaW4gSmF2YVNjcmlwdApkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUvcGxhdGZv cm0vZ3JhcGhpY3MvY2x1dHRlci9QbGF0Zm9ybUNsdXR0ZXJBbmltYXRpb24uY3BwIGIvU291cmNl L1dlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3MvY2x1dHRlci9QbGF0Zm9ybUNsdXR0ZXJBbmltYXRp b24uY3BwCmluZGV4IDU3NTFhMzBkODFjNGVkMmZiNTdiZTkyNmQ0ZWQ4ZTY3OTBkMGI5OTcuLmE2 MTY2NjMyN2UwYTUwZWY5YmZmNTZjOWVmZGM1NjYxNGM3NzVkMDkgMTAwNjQ0Ci0tLSBhL1NvdXJj ZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL2NsdXR0ZXIvUGxhdGZvcm1DbHV0dGVyQW5pbWF0 aW9uLmNwcAorKysgYi9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy9jbHV0dGVyL1Bs YXRmb3JtQ2x1dHRlckFuaW1hdGlvbi5jcHAKQEAgLTM2OCw3ICszNjgsOSBAQCB2b2lkIFBsYXRm b3JtQ2x1dHRlckFuaW1hdGlvbjo6c2V0VmFsdWVzKGNvbnN0IFZlY3RvcjxmbG9hdD4mIHZhbHVl KQogCiB2b2lkIFBsYXRmb3JtQ2x1dHRlckFuaW1hdGlvbjo6c2V0VmFsdWVzKGNvbnN0IFZlY3Rv cjxXZWJDb3JlOjpUcmFuc2Zvcm1hdGlvbk1hdHJpeD4mIHZhbHVlKQogewotICAgIG5vdEltcGxl bWVudGVkKCk7CisgICAgQVNTRVJUKGFuaW1hdGlvblR5cGUoKSA9PSBLZXlmcmFtZSk7CisKKyAg ICBtX3ZhbHVlc01hdHJpeCA9IHZhbHVlOwogfQogCiB2b2lkIFBsYXRmb3JtQ2x1dHRlckFuaW1h dGlvbjo6c2V0VmFsdWVzKGNvbnN0IFZlY3RvcjxGbG9hdFBvaW50M0Q+JiB2YWx1ZSkKQEAgLTUx OSw2ICs1MjEsNDMgQEAgdm9pZCBQbGF0Zm9ybUNsdXR0ZXJBbmltYXRpb246OmFkZENsdXR0ZXJL ZXlmcmFtZVRyYW5zaXRpb25Gb3JQcm9wZXJ0eShjb25zdCBTdHIKICAgICAgICAgZ192YWx1ZV91 bnNldCgma2V5VmFsdWVzLmdldCgpW2ldKTsKIH0KIAordm9pZCBQbGF0Zm9ybUNsdXR0ZXJBbmlt YXRpb246OmFkZENsdXR0ZXJLZXlmcmFtZVRyYW5zaXRpb25Gb3JQcm9wZXJ0eShjb25zdCBTdHJp bmcmIHByb3BlcnR5LCBjb25zdCBWZWN0b3I8V2ViQ29yZTo6VHJhbnNmb3JtYXRpb25NYXRyaXg+ JiB2YWx1ZXMpCit7CisgICAgQVNTRVJUKHByb3BlcnR5ICE9ICJOb1Byb3BlcnR5Iik7CisKKyAg ICBWZWN0b3I8Q29nbE1hdHJpeD4gY29nbE1hdHJpY2VzOworICAgIGZvciAodW5zaWduZWQgaSA9 IDA7IGkgPCB2YWx1ZXMuc2l6ZSgpOyArK2kpCisgICAgICAgIGNvZ2xNYXRyaWNlcy5hcHBlbmQo dmFsdWVzW2ldKTsKKworICAgIEdSZWZQdHI8Q2x1dHRlclRyYW5zaXRpb24+IHRyYW5zaXRpb24g PSBhZG9wdEdSZWYoY2x1dHRlcl9rZXlmcmFtZV90cmFuc2l0aW9uX25ldyhwcm9wZXJ0eS51dGY4 KCkuZGF0YSgpKSk7CisgICAgY2x1dHRlcl90cmFuc2l0aW9uX3NldF9mcm9tKHRyYW5zaXRpb24u Z2V0KCksIENMVVRURVJfVFlQRV9NQVRSSVgsIGNvZ2xNYXRyaWNlcy5maXJzdCgpKTsKKyAgICBj bHV0dGVyX3RyYW5zaXRpb25fc2V0X3RvKHRyYW5zaXRpb24uZ2V0KCksIENMVVRURVJfVFlQRV9N QVRSSVgsIGNvZ2xNYXRyaWNlcy5sYXN0KCkpOworCisgICAgLy8gSWdub3JlIHRoZSBmaXJzdCBr ZXlmcmFtZSwgc2luY2UgaXQncyBhICcwJyBmcmFtZSwgbWVhbmluZ2xlc3MuCisgICAgY29uc3Qg dW5zaWduZWQgbktleWZyYW1lcyA9IHZhbHVlcy5zaXplKCkgLSAxOworICAgIE93bkFycmF5UHRy PENsdXR0ZXJBbmltYXRpb25Nb2RlPiBhbmltYXRpb25Nb2RlcyA9IGFkb3B0QXJyYXlQdHIobmV3 IENsdXR0ZXJBbmltYXRpb25Nb2RlW25LZXlmcmFtZXNdKTsKKyAgICBPd25BcnJheVB0cjxkb3Vi bGU+IGtleVRpbWVzID0gYWRvcHRBcnJheVB0cihuZXcgZG91YmxlW25LZXlmcmFtZXNdKTsKKyAg ICBHT3duUHRyPEdWYWx1ZT4ga2V5VmFsdWVzKGdfbmV3MChHVmFsdWUsIG5LZXlmcmFtZXMpKTsK KworICAgIGZvciAodW5zaWduZWQgaSA9IDA7IGkgPCBuS2V5ZnJhbWVzOyArK2kpIHsKKyAgICAg ICAga2V5VGltZXNbaV0gPSBzdGF0aWNfY2FzdDxkb3VibGU+KG1fa2V5VGltZXNbaSArIDFdKTsK KyAgICAgICAgYW5pbWF0aW9uTW9kZXNbaV0gPSB0b0NsdXR0ZXJBbmltYXRpb25Nb2RlKG1fdGlt aW5nRnVuY3Rpb25zW2ldKTsKKyAgICAgICAgZ192YWx1ZV9pbml0KCZrZXlWYWx1ZXMuZ2V0KClb aV0sIENMVVRURVJfVFlQRV9NQVRSSVgpOworICAgICAgICBnX3ZhbHVlX3NldF9ib3hlZCgma2V5 VmFsdWVzLmdldCgpW2ldLCAmY29nbE1hdHJpY2VzW2kgKyAxXSk7CisgICAgfQorCisgICAgY2x1 dHRlcl9rZXlmcmFtZV90cmFuc2l0aW9uX3NldF9rZXlfZnJhbWVzKENMVVRURVJfS0VZRlJBTUVf VFJBTlNJVElPTih0cmFuc2l0aW9uLmdldCgpKSwgbktleWZyYW1lcywga2V5VGltZXMuZ2V0KCkp OworICAgIGNsdXR0ZXJfa2V5ZnJhbWVfdHJhbnNpdGlvbl9zZXRfdmFsdWVzKENMVVRURVJfS0VZ RlJBTUVfVFJBTlNJVElPTih0cmFuc2l0aW9uLmdldCgpKSwgbktleWZyYW1lcywga2V5VmFsdWVz LmdldCgpKTsKKyAgICBjbHV0dGVyX2tleWZyYW1lX3RyYW5zaXRpb25fc2V0X21vZGVzKENMVVRU RVJfS0VZRlJBTUVfVFJBTlNJVElPTih0cmFuc2l0aW9uLmdldCgpKSwgbktleWZyYW1lcywgYW5p bWF0aW9uTW9kZXMuZ2V0KCkpOworCisgICAgY2x1dHRlcl90cmFuc2l0aW9uX2dyb3VwX2FkZF90 cmFuc2l0aW9uKENMVVRURVJfVFJBTlNJVElPTl9HUk9VUChtX2FuaW1hdGlvbi5nZXQoKSksIHRy YW5zaXRpb24uZ2V0KCkpOworCisgICAgY2x1dHRlcl9pbnRlcnZhbF9yZWdpc3Rlcl9wcm9ncmVz c19mdW5jKENMVVRURVJfVFlQRV9NQVRSSVgsIGNsdXR0ZXJNYXRyaXhQcm9ncmVzcyk7CisKKyAg ICBmb3IgKHVuc2lnbmVkIGkgPSAwOyBpIDwgbktleWZyYW1lczsgKytpKQorICAgICAgICBnX3Zh bHVlX3Vuc2V0KCZrZXlWYWx1ZXMuZ2V0KClbaV0pOworfQorCiB2b2lkIFBsYXRmb3JtQ2x1dHRl ckFuaW1hdGlvbjo6YWRkQ2x1dHRlcktleWZyYW1lVHJhbnNpdGlvbkZvclByb3BlcnR5KGNvbnN0 IFN0cmluZyYgcHJvcGVydHksIGNvbnN0IFZlY3RvcjxGbG9hdFBvaW50M0Q+JiB2YWx1ZXMpCiB7 CiAgICAgQVNTRVJUKHByb3BlcnR5ICE9ICJOb1Byb3BlcnR5Iik7CkBAIC01OTcsNyArNjM2LDEw IEBAIHZvaWQgUGxhdGZvcm1DbHV0dGVyQW5pbWF0aW9uOjphZGRUcmFuc2Zvcm1UcmFuc2l0aW9u KCkKICAgICAgICAgICAgIGFkZENsdXR0ZXJUcmFuc2l0aW9uRm9yUHJvcGVydHkodG9DbHV0dGVy QWN0b3JQcm9wZXJ0eVN0cmluZyhtX3ZhbHVlRnVuY3Rpb25UeXBlKSwgbV9mcm9tVmFsdWUzRCwg bV90b1ZhbHVlM0QpOwogICAgICAgICBicmVhazsKICAgICBjYXNlIE1hdHJpeDoKLSAgICAgICAg YWRkQ2x1dHRlclRyYW5zaXRpb25Gb3JQcm9wZXJ0eSh0b0NsdXR0ZXJBY3RvclByb3BlcnR5U3Ry aW5nKG1fdmFsdWVGdW5jdGlvblR5cGUpLCBtX2Zyb21WYWx1ZU1hdHJpeCwgbV90b1ZhbHVlTWF0 cml4KTsKKyAgICAgICAgaWYgKGlzS2V5ZnJhbWUpCisgICAgICAgICAgICBhZGRDbHV0dGVyS2V5 ZnJhbWVUcmFuc2l0aW9uRm9yUHJvcGVydHkodG9DbHV0dGVyQWN0b3JQcm9wZXJ0eVN0cmluZyht X3ZhbHVlRnVuY3Rpb25UeXBlKSwgbV92YWx1ZXNNYXRyaXgpOyAKKyAgICAgICAgZWxzZQorICAg ICAgICAgICAgYWRkQ2x1dHRlclRyYW5zaXRpb25Gb3JQcm9wZXJ0eSh0b0NsdXR0ZXJBY3RvclBy b3BlcnR5U3RyaW5nKG1fdmFsdWVGdW5jdGlvblR5cGUpLCBtX2Zyb21WYWx1ZU1hdHJpeCwgbV90 b1ZhbHVlTWF0cml4KTsKICAgICAgICAgYnJlYWs7CiAgICAgZGVmYXVsdDoKICAgICAgICAgQVNT RVJUX05PVF9SRUFDSEVEKCk7CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9n cmFwaGljcy9jbHV0dGVyL1BsYXRmb3JtQ2x1dHRlckFuaW1hdGlvbi5oIGIvU291cmNlL1dlYkNv cmUvcGxhdGZvcm0vZ3JhcGhpY3MvY2x1dHRlci9QbGF0Zm9ybUNsdXR0ZXJBbmltYXRpb24uaApp bmRleCBlOGRiZDMxMDFhMWEwZjIyNjFhMGNmYzkyOGQzM2I2YWJmMjhiM2Y5Li5jNzI4MTVjN2Vm M2Y5NGIzYzk0NjhiZDE5MTc3YjVmOTU2MGQzMGEwIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29y ZS9wbGF0Zm9ybS9ncmFwaGljcy9jbHV0dGVyL1BsYXRmb3JtQ2x1dHRlckFuaW1hdGlvbi5oCisr KyBiL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL2NsdXR0ZXIvUGxhdGZvcm1DbHV0 dGVyQW5pbWF0aW9uLmgKQEAgLTE0NCw2ICsxNDQsNyBAQCBwcml2YXRlOgogICAgIHZvaWQgYWRk Q2x1dHRlclRyYW5zaXRpb25Gb3JQcm9wZXJ0eShjb25zdCBTdHJpbmcmIHByb3BlcnR5LCBjb25z dCBGbG9hdFBvaW50M0QmIGZyb21WYWx1ZSwgY29uc3QgRmxvYXRQb2ludDNEJiB0b1ZhbHVlKTsK IAogICAgIHZvaWQgYWRkQ2x1dHRlcktleWZyYW1lVHJhbnNpdGlvbkZvclByb3BlcnR5KGNvbnN0 IFN0cmluZyYgcHJvcGVydHksIGNvbnN0IFZlY3RvcjxmbG9hdD4mIHZhbHVlcyk7CisgICAgdm9p ZCBhZGRDbHV0dGVyS2V5ZnJhbWVUcmFuc2l0aW9uRm9yUHJvcGVydHkoY29uc3QgU3RyaW5nJiBw cm9wZXJ0eSwgY29uc3QgVmVjdG9yPFdlYkNvcmU6OlRyYW5zZm9ybWF0aW9uTWF0cml4PiYgdmFs dWVzKTsKICAgICB2b2lkIGFkZENsdXR0ZXJLZXlmcmFtZVRyYW5zaXRpb25Gb3JQcm9wZXJ0eShj b25zdCBTdHJpbmcmIHByb3BlcnR5LCBjb25zdCBWZWN0b3I8RmxvYXRQb2ludDNEPiYgdmFsdWVz KTsKIAogICAgIHZvaWQgYWRkT3BhY2l0eVRyYW5zaXRpb24oKTsKQEAgLTE3Niw2ICsxNzcsNyBA QCBwcml2YXRlOgogCiAgICAgVmVjdG9yPGZsb2F0PiBtX3ZhbHVlczsKICAgICBWZWN0b3I8Rmxv YXRQb2ludDNEPiBtX3ZhbHVlczNEOworICAgIFZlY3RvcjxXZWJDb3JlOjpUcmFuc2Zvcm1hdGlv bk1hdHJpeD4gbV92YWx1ZXNNYXRyaXg7CiB9OwogCiB9Cg==