49532 2010-11-15 04:19:27 -0800 [GTK] Use gdk_pixbuf_get_from_surface() when available 2010-11-16 10:08:19 -0800 1 1 1 Unclassified WebKit WebKitGTK 528+ (Nightly build) PC Linux RESOLVED FIXED P2 Normal --- 49549 0 cgarcia webkit-unassigned abarth commit-queue eric mrobinson webkit.review.bot oldest_to_newest 309234 0 cgarcia 2010-11-15 04:19:27 -0800 Since gtk3, there's gdk_pixbuf_get_from_surface() to convert an image cairo surface into a GdkPixbuf. 309235 1 73885 cgarcia 2010-11-15 04:23:31 -0800 Created attachment 73885 Patch to use gdk_pixbuf_get_from_surface() 309347 2 73885 mrobinson 2010-11-15 10:40:28 -0800 Comment on attachment 73885 Patch to use gdk_pixbuf_get_from_surface() View in context: https://bugs.webkit.org/attachment.cgi?id=73885&action=review > WebCore/platform/graphics/gtk/GdkCairoUtilities.cpp:37 > +#ifndef GTK_API_VERSION_2 > +GdkPixbuf* cairoImageSurfaceToGdkPixbuf(cairo_surface_t* surface) > +{ > + return gdk_pixbuf_get_from_surface(surface, 0, 0, > + cairo_image_surface_get_width(surface), > + cairo_image_surface_get_height(surface)); Is this new method used anywhere? Ideally a code change like this should be accompanied by a corresponding change in functionality somewhere. If there is some hidden functionality change it should be outlined in the ChangeLog. 309616 3 cgarcia 2010-11-16 00:08:06 -0800 This is not a new method: ./WebCore/platform/graphics/gtk/ImageBufferGtk.cpp: PlatformRefPtr<GdkPixbuf> pixbuf = cairoImageSurfaceToGdkPixbuf(m_data.m_surface); ./WebCore/platform/graphics/gtk/GdkCairoUtilities.cpp:GdkPixbuf* cairoImageSurfaceToGdkPixbuf(cairo_surface_t* surface) ./WebCore/platform/graphics/gtk/ImageGtk.cpp: return cairoImageSurfaceToGdkPixbuf(frameAtIndex(currentFrame())); and there isn't any new functionality hidden, it's just a new implementation for the method using gdk api to make the code simpler and easier tom maintain. This method is used when, for example, when creating the image during a drag and drop operation, or when saving an image as jpeg, png, etc. 309617 4 73970 cgarcia 2010-11-16 00:13:39 -0800 Created attachment 73970 Updated patch using GtkVersioning I had forgotten I already added gdk_pixbuf_get_from_surface() to GtkVersioning, we can use it unconditionally. 309733 5 73970 mrobinson 2010-11-16 08:25:04 -0800 Comment on attachment 73970 Updated patch using GtkVersioning Great. 309735 6 mrobinson 2010-11-16 08:27:46 -0800 (In reply to comment #3) > This is not a new method: > ./WebCore/platform/graphics/gtk/ImageBufferGtk.cpp: PlatformRefPtr<GdkPixbuf> pixbuf = cairoImageSurfaceToGdkPixbuf(m_data.m_surface); > ./WebCore/platform/graphics/gtk/GdkCairoUtilities.cpp:GdkPixbuf* cairoImageSurfaceToGdkPixbuf(cairo_surface_t* surface) > ./WebCore/platform/graphics/gtk/ImageGtk.cpp: return cairoImageSurfaceToGdkPixbuf(frameAtIndex(currentFrame())); Right. I was just a little confused that because it didn't remove the old implementation of the method. :) 309764 7 73970 commit-queue 2010-11-16 09:15:02 -0800 Comment on attachment 73970 Updated patch using GtkVersioning Clearing flags on attachment: 73970 Committed r72106: <http://trac.webkit.org/changeset/72106> 309765 8 commit-queue 2010-11-16 09:15:07 -0800 All reviewed patches have been landed. Closing bug. 309793 9 webkit.review.bot 2010-11-16 10:08:19 -0800 http://trac.webkit.org/changeset/72106 might have broken GTK Linux 64-bit Debug The following tests are not passing: editing/selection/move-past-trailing-space.html fast/blockflow/japanese-ruby-horizontal-bt.html fast/blockflow/japanese-ruby-vertical-lr.html fast/blockflow/japanese-ruby-vertical-rl.html 73885 2010-11-15 04:23:31 -0800 2010-11-16 00:13:39 -0800 Patch to use gdk_pixbuf_get_from_surface() pixbuf-from-surface.diff text/plain 1551 cgarcia ZGlmZiAtLWdpdCBhL1dlYkNvcmUvQ2hhbmdlTG9nIGIvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXgg OWMxZWRhZi4uNjE2OWNjOCAxMDA2NDQKLS0tIGEvV2ViQ29yZS9DaGFuZ2VMb2cKKysrIGIvV2Vi Q29yZS9DaGFuZ2VMb2cKQEAgLTEsMyArMSwxMyBAQAorMjAxMC0xMS0xNSAgQ2FybG9zIEdhcmNp YSBDYW1wb3MgIDxjZ2FyY2lhQGlnYWxpYS5jb20+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9C T0RZIChPT1BTISkuCisKKyAgICAgICAgW0dUS10gVXNlIGdka19waXhidWZfZ2V0X2Zyb21fc3Vy ZmFjZSgpIHdoZW4gYXZhaWxhYmxlCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3No b3dfYnVnLmNnaT9pZD00OTUzMgorCisgICAgICAgICogcGxhdGZvcm0vZ3JhcGhpY3MvZ3RrL0dk a0NhaXJvVXRpbGl0aWVzLmNwcDoKKyAgICAgICAgKGNhaXJvSW1hZ2VTdXJmYWNlVG9HZGtQaXhi dWYpOgorCiAyMDEwLTExLTE0ICBEYXZpZCBIeWF0dCAgPGh5YXR0QGFwcGxlLmNvbT4KIAogICAg ICAgICBCYWNrIG91dCB0aGUgaXRhbGljcyBwb3J0aW9uIG9mIHRoZSBwcmV2aW91cyBwYXRjaCB1 bnRpbCBJIGNhbiBmaWd1cmUgb3V0IHdoeSBpdApkaWZmIC0tZ2l0IGEvV2ViQ29yZS9wbGF0Zm9y bS9ncmFwaGljcy9ndGsvR2RrQ2Fpcm9VdGlsaXRpZXMuY3BwIGIvV2ViQ29yZS9wbGF0Zm9ybS9n cmFwaGljcy9ndGsvR2RrQ2Fpcm9VdGlsaXRpZXMuY3BwCmluZGV4IGQ3NjhjZTIuLmYwNjY4OGQg MTAwNjQ0Ci0tLSBhL1dlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3MvZ3RrL0dka0NhaXJvVXRpbGl0 aWVzLmNwcAorKysgYi9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL2d0ay9HZGtDYWlyb1V0aWxp dGllcy5jcHAKQEAgLTI5LDYgKzI5LDE0IEBACiAjaW5jbHVkZSA8Y2Fpcm8uaD4KICNpbmNsdWRl IDxndGsvZ3RrLmg+CiAKKyNpZm5kZWYgR1RLX0FQSV9WRVJTSU9OXzIKK0dka1BpeGJ1ZiogY2Fp cm9JbWFnZVN1cmZhY2VUb0dka1BpeGJ1ZihjYWlyb19zdXJmYWNlX3QqIHN1cmZhY2UpCit7Cisg ICAgcmV0dXJuIGdka19waXhidWZfZ2V0X2Zyb21fc3VyZmFjZShzdXJmYWNlLCAwLCAwLAorICAg ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgY2Fpcm9faW1hZ2Vfc3VyZmFjZV9n ZXRfd2lkdGgoc3VyZmFjZSksCisgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg ICBjYWlyb19pbWFnZV9zdXJmYWNlX2dldF9oZWlnaHQoc3VyZmFjZSkpOworfQorI2Vsc2UKIHN0 YXRpYyBpbmxpbmUgdW5zaWduZWQgY2hhciogZ2V0Q2Fpcm9TdXJmYWNlUGl4ZWwodW5zaWduZWQg Y2hhciogZGF0YSwgdW5zaWduZWQgeCwgdW5zaWduZWQgeSwgdW5zaWduZWQgcm93U3RyaWRlKQog ewogICAgIHJldHVybiBkYXRhICsgKHkgKiByb3dTdHJpZGUpICsgeCAqIDQ7CkBAIC05MiwzICsx MDAsNCBAQCBHZGtQaXhidWYqIGNhaXJvSW1hZ2VTdXJmYWNlVG9HZGtQaXhidWYoY2Fpcm9fc3Vy ZmFjZV90KiBzdXJmYWNlKQogICAgIHJldHVybiBkZXN0OwogfQogCisjZW5kaWYgLy8gR1RLX0FQ SV9WRVJTSU9OXzIK 73970 2010-11-16 00:13:39 -0800 2010-11-16 09:15:01 -0800 Updated patch using GtkVersioning gdkpixbuf-from-surface2.diff text/plain 3936 cgarcia ZGlmZiAtLWdpdCBhL1dlYkNvcmUvQ2hhbmdlTG9nIGIvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXgg OWMxZWRhZi4uZjk5ZGIyNCAxMDA2NDQKLS0tIGEvV2ViQ29yZS9DaGFuZ2VMb2cKKysrIGIvV2Vi Q29yZS9DaGFuZ2VMb2cKQEAgLTEsMyArMSwxMyBAQAorMjAxMC0xMS0xNiAgQ2FybG9zIEdhcmNp YSBDYW1wb3MgIDxjZ2FyY2lhQGlnYWxpYS5jb20+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9C T0RZIChPT1BTISkuCisKKyAgICAgICAgW0dUS10gVXNlIGdka19waXhidWZfZ2V0X2Zyb21fc3Vy ZmFjZSgpIHdoZW4gYXZhaWxhYmxlCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3No b3dfYnVnLmNnaT9pZD00OTUzMgorCisgICAgICAgICogcGxhdGZvcm0vZ3JhcGhpY3MvZ3RrL0dk a0NhaXJvVXRpbGl0aWVzLmNwcDoKKyAgICAgICAgKGNhaXJvSW1hZ2VTdXJmYWNlVG9HZGtQaXhi dWYpOgorCiAyMDEwLTExLTE0ICBEYXZpZCBIeWF0dCAgPGh5YXR0QGFwcGxlLmNvbT4KIAogICAg ICAgICBCYWNrIG91dCB0aGUgaXRhbGljcyBwb3J0aW9uIG9mIHRoZSBwcmV2aW91cyBwYXRjaCB1 bnRpbCBJIGNhbiBmaWd1cmUgb3V0IHdoeSBpdApkaWZmIC0tZ2l0IGEvV2ViQ29yZS9wbGF0Zm9y bS9ncmFwaGljcy9ndGsvR2RrQ2Fpcm9VdGlsaXRpZXMuY3BwIGIvV2ViQ29yZS9wbGF0Zm9ybS9n cmFwaGljcy9ndGsvR2RrQ2Fpcm9VdGlsaXRpZXMuY3BwCmluZGV4IGQ3NjhjZTIuLmQ4MDE4YTMg MTAwNjQ0Ci0tLSBhL1dlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3MvZ3RrL0dka0NhaXJvVXRpbGl0 aWVzLmNwcAorKysgYi9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL2d0ay9HZGtDYWlyb1V0aWxp dGllcy5jcHAKQEAgLTI1LDcwICsyNSwxNSBAQAogCiAjaW5jbHVkZSAiY29uZmlnLmgiCiAjaW5j bHVkZSAiR2RrQ2Fpcm9VdGlsaXRpZXMuaCIKKyNpbmNsdWRlICJHdGtWZXJzaW9uaW5nLmgiCiAK ICNpbmNsdWRlIDxjYWlyby5oPgogI2luY2x1ZGUgPGd0ay9ndGsuaD4KIAotc3RhdGljIGlubGlu ZSB1bnNpZ25lZCBjaGFyKiBnZXRDYWlyb1N1cmZhY2VQaXhlbCh1bnNpZ25lZCBjaGFyKiBkYXRh LCB1bnNpZ25lZCB4LCB1bnNpZ25lZCB5LCB1bnNpZ25lZCByb3dTdHJpZGUpCi17Ci0gICAgcmV0 dXJuIGRhdGEgKyAoeSAqIHJvd1N0cmlkZSkgKyB4ICogNDsKLX0KLQotc3RhdGljIGlubGluZSBn dWNoYXIqIGdldEdka1BpeGJ1ZlBpeGVsKGd1Y2hhciogZGF0YSwgdW5zaWduZWQgeCwgdW5zaWdu ZWQgeSwgdW5zaWduZWQgcm93U3RyaWRlKQotewotICAgIHJldHVybiBkYXRhICsgKHkgKiByb3dT dHJpZGUpICsgeCAqIDQ7Ci19Ci0KIEdka1BpeGJ1ZiogY2Fpcm9JbWFnZVN1cmZhY2VUb0dka1Bp eGJ1ZihjYWlyb19zdXJmYWNlX3QqIHN1cmZhY2UpCiB7Ci0gICAgaW50IHdpZHRoID0gY2Fpcm9f aW1hZ2Vfc3VyZmFjZV9nZXRfd2lkdGgoc3VyZmFjZSk7Ci0gICAgaW50IGhlaWdodCA9IGNhaXJv X2ltYWdlX3N1cmZhY2VfZ2V0X2hlaWdodChzdXJmYWNlKTsKLSAgICB1bnNpZ25lZCBjaGFyKiBz dXJmYWNlRGF0YSA9IGNhaXJvX2ltYWdlX3N1cmZhY2VfZ2V0X2RhdGEoc3VyZmFjZSk7Ci0gICAg aW50IHN1cmZhY2VSb3dTdHJpZGUgPSBjYWlyb19pbWFnZV9zdXJmYWNlX2dldF9zdHJpZGUoc3Vy ZmFjZSk7Ci0KLSAgICBHZGtQaXhidWYqIGRlc3QgPSBnZGtfcGl4YnVmX25ldyhHREtfQ09MT1JT UEFDRV9SR0IsIFRSVUUsIDgsIHdpZHRoLCBoZWlnaHQpOwotICAgIGlmICghZGVzdCkKLSAgICAg ICAgcmV0dXJuIDA7Ci0KLSAgICBndWNoYXIqIHBpeGJ1ZkRhdGEgPSBnZGtfcGl4YnVmX2dldF9w aXhlbHMoZGVzdCk7Ci0gICAgaW50IHBpeGJ1ZlJvd1N0cmlkZSA9IGdka19waXhidWZfZ2V0X3Jv d3N0cmlkZShkZXN0KTsKLQotICAgIC8qIEZyb206IGh0dHA6Ly9jYWlyb2dyYXBoaWNzLm9yZy9t YW51YWwvY2Fpcm8taW1hZ2Utc3VyZmFjZS5odG1sI2NhaXJvLWZvcm1hdC10Ci0gICAgICogIkNB SVJPX0ZPUk1BVF9BUkdCMzI6IGVhY2ggcGl4ZWwgaXMgYSAzMi1iaXQgcXVhbnRpdHksIHdpdGgg YWxwaGEgaW4KLSAgICAgKiB0aGUgdXBwZXIgOCBiaXRzLCB0aGVuIHJlZCwgdGhlbiBncmVlbiwg dGhlbiBibHVlLiBUaGUgMzItYml0Ci0gICAgICogcXVhbnRpdGllcyBhcmUgc3RvcmVkIG5hdGl2 ZS1lbmRpYW4uIFByZS1tdWx0aXBsaWVkIGFscGhhIGlzIHVzZWQuCi0gICAgICogKFRoYXQgaXMs IDUwJSB0cmFuc3BhcmVudCByZWQgaXMgMHg4MDgwMDAwMCwgbm90IDB4ODBmZjAwMDAuKSIKLSAg ICAgKgotICAgICAqIFNlZSBodHRwOi8vZGV2ZWxvcGVyLmdpbXAub3JnL2FwaS8yLjAvZ2RrLXBp eGJ1Zi9nZGstcGl4YnVmLWdkay1waXhidWYuaHRtbCNHZGtQaXhidWYKLSAgICAgKiBmb3IgaW5m b3JtYXRpb24gb24gdGhlIHN0cnVjdHVyZSBvZiBHZGtQaXhidWZzIHN0b3JlZCB3aXRoIEdES19D T0xPUlNQQUNFX1JHQi4KLSAgICAgKgotICAgICAqIFJHQiBjb2xvciBjaGFubmVscyBpbiBDQUlS T19GT1JNQVRfQVJHQjMyIGFyZSBzdG9yZWQgYmFzZWQgb24gdGhlCi0gICAgICogZW5kaWFubmVz cyBvZiB0aGUgbWFjaGluZSBhbmQgYXJlIGFsc28gbXVsdGlwbGllZCBieSB0aGUgYWxwaGEgY2hh bm5lbC4KLSAgICAgKiBUbyBwcm9wZXJseSB0cmFuc2ZlciB0aGUgZGF0YSBmcm9tIHRoZSBDYWly byBzdXJmYWNlIHdlIG11c3QgZGl2aWRlIGVhY2gKLSAgICAgKiBvZiB0aGUgUkdCIGNoYW5uZWxz IGJ5IHRoZSBhbHBoYSBjaGFubmVsIGFuZCB0aGVuIHJlb3JkZXIgYWxsIGNoYW5uZWxzCi0gICAg ICogaWYgdGhpcyBtYWNoaW5lIGlzIGxpdHRsZS1lbmRpYW4uCi0gICAgICovCi0gICAgZm9yIChp bnQgeSA9IDA7IHkgPCBoZWlnaHQ7IHkrKykgewotICAgICAgICBmb3IgKGludCB4ID0gMDsgeCA8 IHdpZHRoOyB4KyspIHsKLSAgICAgICAgICAgIHVuc2lnbmVkIGNoYXIqIHNvdXJjZSA9IGdldENh aXJvU3VyZmFjZVBpeGVsKHN1cmZhY2VEYXRhLCB4LCB5LCBzdXJmYWNlUm93U3RyaWRlKTsKLSAg ICAgICAgICAgIGd1Y2hhciogZGVzdCA9IGdldEdka1BpeGJ1ZlBpeGVsKHBpeGJ1ZkRhdGEsIHgs IHksIHBpeGJ1ZlJvd1N0cmlkZSk7Ci0KLSNpZiBHX0JZVEVfT1JERVIgPT0gR19MSVRUTEVfRU5E SUFOCi0gICAgICAgICAgICBndWNoYXIgYWxwaGEgPSBzb3VyY2VbM107Ci0gICAgICAgICAgICBk ZXN0WzBdID0gYWxwaGEgPyAoKHNvdXJjZVsyXSAqIDI1NSkgLyBhbHBoYSkgOiAwOwotICAgICAg ICAgICAgZGVzdFsxXSA9IGFscGhhID8gKChzb3VyY2VbMV0gKiAyNTUpIC8gYWxwaGEpIDogMDsK LSAgICAgICAgICAgIGRlc3RbMl0gPSBhbHBoYSA/ICgoc291cmNlWzBdICogMjU1KSAvIGFscGhh KSA6IDA7Ci0gICAgICAgICAgICBkZXN0WzNdID0gYWxwaGE7Ci0jZWxzZQotICAgICAgICAgICAg Z3VjaGFyIGFscGhhID0gc291cmNlWzBdOwotICAgICAgICAgICAgZGVzdFswXSA9IGFscGhhID8g KChzb3VyY2VbMV0gKiAyNTUpIC8gYWxwaGEpIDogMDsKLSAgICAgICAgICAgIGRlc3RbMV0gPSBh bHBoYSA/ICgoc291cmNlWzJdICogMjU1KSAvIGFscGhhKSA6IDA7Ci0gICAgICAgICAgICBkZXN0 WzJdID0gYWxwaGEgPyAoKHNvdXJjZVszXSAqIDI1NSkgLyBhbHBoYSkgOiAwOwotICAgICAgICAg ICAgZGVzdFszXSA9IGFscGhhOwotI2VuZGlmCi0gICAgICAgIH0KLSAgICB9Ci0KLSAgICByZXR1 cm4gZGVzdDsKKyAgICByZXR1cm4gZ2RrX3BpeGJ1Zl9nZXRfZnJvbV9zdXJmYWNlKHN1cmZhY2Us IDAsIDAsCisgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICBjYWlyb19pbWFn ZV9zdXJmYWNlX2dldF93aWR0aChzdXJmYWNlKSwKKyAgICAgICAgICAgICAgICAgICAgICAgICAg ICAgICAgICAgICAgIGNhaXJvX2ltYWdlX3N1cmZhY2VfZ2V0X2hlaWdodChzdXJmYWNlKSk7CiB9 CiAK