37621 2010-04-14 16:13:11 -0700 Chromium/mac: Image thumbnails don't work for indexed images 2010-04-15 16:59:02 -0700 1 1 1 Unclassified WebKit WebCore Misc. 528+ (Nightly build) PC OS X 10.5 RESOLVED FIXED P2 Normal --- 1 thakis thakis avi commit-queue oldest_to_newest 212638 0 thakis 2010-04-14 16:13:11 -0700 WebKit bug for http://crbug.com/41512 212642 1 53382 thakis 2010-04-14 16:18:01 -0700 Created attachment 53382 Patch 212643 2 thakis 2010-04-14 16:19:29 -0700 The CoreGraphics docs say that CGCreateContext doesn't work with indexed color spaces. Always using RGB is probably better for CMYK images too. 212651 3 avi 2010-04-14 16:26:24 -0700 > Index: WebCore/platform/chromium/DragImageChromiumMac.cpp > =================================================================== > + CGColorSpaceRef deviceRGB = CGColorSpaceCreateDeviceRGB(); RetainPtr? > + CGContextRef context = CGBitmapContextCreate(0, width, height, 8, width * 4, deviceRGB, kCGImageAlphaPremultipliedFirst | kCGBitmapByteOrder32Host); > + CGColorSpaceRelease(deviceRGB); > + BTW, the comment about CGCreateContext not working for indexed color spaces is IMHO comment-worthy in the code. Just me. 212851 4 avi 2010-04-15 07:50:08 -0700 > Index: WebCore/platform/chromium/DragImageChromiumMac.cpp > =================================================================== > + CGColorSpaceRef deviceRGB = CGColorSpaceCreateDeviceRGB(); BTW, CGColorSpaceCreateDeviceRGB hasn't created a device RGB color space since 10.3, so the variable name, technically speaking, is inaccurate. Since CGColorSpaceCreateDeviceRGB() == CGColorSpaceCreateWithName(kCGColorSpaceGenericRGB), you should switch to the latter as it's clearer what's it's doing. 212989 5 53464 thakis 2010-04-15 13:01:52 -0700 Created attachment 53464 Patch Did the RetainPtr thing and even the CGColorSpaceCreateWithName(kCGColorSpaceGenericRGB) suggestion. I didn't add a comment though, because just saying that createcontext doesn't work with indexed images misses the point. We really want RGB here, and not whatever weird color space the image might be in. (Also, the WebKit way apparently is to do `svn annotate` and then look at revision log/bug afaiu). 212992 6 avi 2010-04-15 13:08:56 -0700 (In reply to comment #5) > (Also, the WebKit way apparently is to do `svn annotate` and then look at > revision log/bug afaiu). Fine with me. Code LGTM. 212993 7 53464 dglazkov 2010-04-15 13:10:47 -0700 Comment on attachment 53464 Patch motownavi can't be wrong. rs=me. 213112 8 53464 commit-queue 2010-04-15 16:58:58 -0700 Comment on attachment 53464 Patch Clearing flags on attachment: 53464 Committed r57687: <http://trac.webkit.org/changeset/57687> 213113 9 commit-queue 2010-04-15 16:59:02 -0700 All reviewed patches have been landed. Closing bug. 53382 2010-04-14 16:18:01 -0700 2010-04-15 13:01:52 -0700 Patch webkit_dragindexed.diff text/plain 2379 thakis SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvQ2hhbmdlTG9n CShyZXZpc2lvbiA1NzYxNikKKysrIFdlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBA IC0xLDMgKzEsMTQgQEAKKzIwMTAtMDQtMTQgIE5pY29sYXMgV2ViZXIgIDx0aGFraXNAY2hyb21p dW0ub3JnPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAg IEZpeCBkcmFnIGltYWdlIHRodW1ibmFpbHMgZm9yIGluZGV4ZWQgaW1hZ2VzLgorICAgICAgICBo dHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9Mzc2MjEKKworICAgICAgICAq IHBsYXRmb3JtL2Nocm9taXVtL0RyYWdJbWFnZUNocm9taXVtTWFjLmNwcDoKKyAgICAgICAgKFdl YkNvcmU6OnNjYWxlRHJhZ0ltYWdlKTogQWx3YXlzIHVzZSBSR0IgY29sb3Igc3BhY2UuCisgICAg ICAgIChXZWJDb3JlOjpkaXNzb2x2ZURyYWdJbWFnZVRvRnJhY3Rpb24pOiBBbHdheXMgdXNlIFJH QiBjb2xvciBzcGFjZS4KKwogMjAxMC0wNC0xNCAgU2hlcmlmZiBCb3QgIDx3ZWJraXQucmV2aWV3 LmJvdEBnbWFpbC5jb20+CiAKICAgICAgICAgVW5yZXZpZXdlZCwgcm9sbGluZyBvdXQgcjU3NjA5 LgpJbmRleDogV2ViQ29yZS9wbGF0Zm9ybS9jaHJvbWl1bS9EcmFnSW1hZ2VDaHJvbWl1bU1hYy5j cHAKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PQotLS0gV2ViQ29yZS9wbGF0Zm9ybS9jaHJvbWl1bS9EcmFnSW1hZ2VDaHJv bWl1bU1hYy5jcHAJKHJldmlzaW9uIDU3NjE1KQorKysgV2ViQ29yZS9wbGF0Zm9ybS9jaHJvbWl1 bS9EcmFnSW1hZ2VDaHJvbWl1bU1hYy5jcHAJKHdvcmtpbmcgY29weSkKQEAgLTU3LDcgKzU3LDEx IEBAIERyYWdJbWFnZVJlZiBzY2FsZURyYWdJbWFnZShEcmFnSW1hZ2VSZWYKICAgICAgICAgcmV0 dXJuIDA7CiAgICAgc2l6ZV90IHdpZHRoID0gcm91bmRmKENHSW1hZ2VHZXRXaWR0aChpbWFnZSkg KiBzY2FsZS53aWR0aCgpKTsKICAgICBzaXplX3QgaGVpZ2h0ID0gcm91bmRmKENHSW1hZ2VHZXRI ZWlnaHQoaW1hZ2UpICogc2NhbGUuaGVpZ2h0KCkpOwotICAgIENHQ29udGV4dFJlZiBjb250ZXh0 ID0gQ0dCaXRtYXBDb250ZXh0Q3JlYXRlKDAsIHdpZHRoLCBoZWlnaHQsIDgsIHdpZHRoICogNCwg Q0dJbWFnZUdldENvbG9yU3BhY2UoaW1hZ2UpLCBrQ0dJbWFnZUFscGhhUHJlbXVsdGlwbGllZEZp cnN0IHwga0NHQml0bWFwQnl0ZU9yZGVyMzJIb3N0KTsKKworICAgIENHQ29sb3JTcGFjZVJlZiBk ZXZpY2VSR0IgPSBDR0NvbG9yU3BhY2VDcmVhdGVEZXZpY2VSR0IoKTsKKyAgICBDR0NvbnRleHRS ZWYgY29udGV4dCA9IENHQml0bWFwQ29udGV4dENyZWF0ZSgwLCB3aWR0aCwgaGVpZ2h0LCA4LCB3 aWR0aCAqIDQsIGRldmljZVJHQiwga0NHSW1hZ2VBbHBoYVByZW11bHRpcGxpZWRGaXJzdCB8IGtD R0JpdG1hcEJ5dGVPcmRlcjMySG9zdCk7CisgICAgQ0dDb2xvclNwYWNlUmVsZWFzZShkZXZpY2VS R0IpOworCiAgICAgaWYgKCFjb250ZXh0KQogICAgICAgICByZXR1cm4gMDsKICAgICBDR0NvbnRl eHREcmF3SW1hZ2UoY29udGV4dCwgQ0dSZWN0TWFrZSgwLCAwLCB3aWR0aCwgaGVpZ2h0KSwgaW1h Z2UpOwpAQCAtNzQsNyArNzgsMTEgQEAgRHJhZ0ltYWdlUmVmIGRpc3NvbHZlRHJhZ0ltYWdlVG9G cmFjdGlvbgogICAgICAgICByZXR1cm4gMDsKICAgICBzaXplX3Qgd2lkdGggPSBDR0ltYWdlR2V0 V2lkdGgoaW1hZ2UpOwogICAgIHNpemVfdCBoZWlnaHQgPSBDR0ltYWdlR2V0SGVpZ2h0KGltYWdl KTsKLSAgICBDR0NvbnRleHRSZWYgY29udGV4dCA9IENHQml0bWFwQ29udGV4dENyZWF0ZSgwLCB3 aWR0aCwgaGVpZ2h0LCA4LCB3aWR0aCAqIDQsIENHSW1hZ2VHZXRDb2xvclNwYWNlKGltYWdlKSwg a0NHSW1hZ2VBbHBoYVByZW11bHRpcGxpZWRGaXJzdCB8IGtDR0JpdG1hcEJ5dGVPcmRlcjMySG9z dCk7CisKKyAgICBDR0NvbG9yU3BhY2VSZWYgZGV2aWNlUkdCID0gQ0dDb2xvclNwYWNlQ3JlYXRl RGV2aWNlUkdCKCk7CisgICAgQ0dDb250ZXh0UmVmIGNvbnRleHQgPSBDR0JpdG1hcENvbnRleHRD cmVhdGUoMCwgd2lkdGgsIGhlaWdodCwgOCwgd2lkdGggKiA0LCBkZXZpY2VSR0IsIGtDR0ltYWdl QWxwaGFQcmVtdWx0aXBsaWVkRmlyc3QgfCBrQ0dCaXRtYXBCeXRlT3JkZXIzMkhvc3QpOworICAg IENHQ29sb3JTcGFjZVJlbGVhc2UoZGV2aWNlUkdCKTsKKwogICAgIGlmICghY29udGV4dCkKICAg ICAgICAgcmV0dXJuIDA7CiAgICAgLy8gRnJvbSBDR0NvbnRleHQuaDoK 53464 2010-04-15 13:01:52 -0700 2010-04-15 16:58:57 -0700 Patch webkit_dragindexed.diff text/plain 2586 thakis SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvQ2hhbmdlTG9n CShyZXZpc2lvbiA1NzYxNikKKysrIFdlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBA IC0xLDMgKzEsMTQgQEAKKzIwMTAtMDQtMTQgIE5pY29sYXMgV2ViZXIgIDx0aGFraXNAY2hyb21p dW0ub3JnPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAg IEZpeCBkcmFnIGltYWdlIHRodW1ibmFpbHMgZm9yIGluZGV4ZWQgaW1hZ2VzLgorICAgICAgICBo dHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9Mzc2MjEKKworICAgICAgICAq IHBsYXRmb3JtL2Nocm9taXVtL0RyYWdJbWFnZUNocm9taXVtTWFjLmNwcDoKKyAgICAgICAgKFdl YkNvcmU6OnNjYWxlRHJhZ0ltYWdlKTogQWx3YXlzIHVzZSBSR0IgY29sb3Igc3BhY2UuCisgICAg ICAgIChXZWJDb3JlOjpkaXNzb2x2ZURyYWdJbWFnZVRvRnJhY3Rpb24pOiBBbHdheXMgdXNlIFJH QiBjb2xvciBzcGFjZS4KKwogMjAxMC0wNC0xNCAgU2hlcmlmZiBCb3QgIDx3ZWJraXQucmV2aWV3 LmJvdEBnbWFpbC5jb20+CiAKICAgICAgICAgVW5yZXZpZXdlZCwgcm9sbGluZyBvdXQgcjU3NjA5 LgpJbmRleDogV2ViQ29yZS9wbGF0Zm9ybS9jaHJvbWl1bS9EcmFnSW1hZ2VDaHJvbWl1bU1hYy5j cHAKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PQotLS0gV2ViQ29yZS9wbGF0Zm9ybS9jaHJvbWl1bS9EcmFnSW1hZ2VDaHJv bWl1bU1hYy5jcHAJKHJldmlzaW9uIDU3NjE1KQorKysgV2ViQ29yZS9wbGF0Zm9ybS9jaHJvbWl1 bS9EcmFnSW1hZ2VDaHJvbWl1bU1hYy5jcHAJKHdvcmtpbmcgY29weSkKQEAgLTMzLDYgKzMzLDcg QEAKIAogI2luY2x1ZGUgIkltYWdlLmgiCiAjaW5jbHVkZSAiTm90SW1wbGVtZW50ZWQuaCIKKyNp bmNsdWRlIDx3dGYvUmV0YWluUHRyLmg+CiAKICNpbmNsdWRlIDxDb3JlR3JhcGhpY3MvQ0dCaXRt YXBDb250ZXh0Lmg+CiAjaW5jbHVkZSA8Q29yZUdyYXBoaWNzL0NHSW1hZ2UuaD4KQEAgLTU3LDcg KzU4LDEwIEBAIERyYWdJbWFnZVJlZiBzY2FsZURyYWdJbWFnZShEcmFnSW1hZ2VSZWYKICAgICAg ICAgcmV0dXJuIDA7CiAgICAgc2l6ZV90IHdpZHRoID0gcm91bmRmKENHSW1hZ2VHZXRXaWR0aChp bWFnZSkgKiBzY2FsZS53aWR0aCgpKTsKICAgICBzaXplX3QgaGVpZ2h0ID0gcm91bmRmKENHSW1h Z2VHZXRIZWlnaHQoaW1hZ2UpICogc2NhbGUuaGVpZ2h0KCkpOwotICAgIENHQ29udGV4dFJlZiBj b250ZXh0ID0gQ0dCaXRtYXBDb250ZXh0Q3JlYXRlKDAsIHdpZHRoLCBoZWlnaHQsIDgsIHdpZHRo ICogNCwgQ0dJbWFnZUdldENvbG9yU3BhY2UoaW1hZ2UpLCBrQ0dJbWFnZUFscGhhUHJlbXVsdGlw bGllZEZpcnN0IHwga0NHQml0bWFwQnl0ZU9yZGVyMzJIb3N0KTsKKworICAgIFJldGFpblB0cjxD R0NvbG9yU3BhY2VSZWY+IGRldmljZVJHQihXVEY6OkFkb3B0Q0YsIENHQ29sb3JTcGFjZUNyZWF0 ZVdpdGhOYW1lKGtDR0NvbG9yU3BhY2VHZW5lcmljUkdCKSk7CisgICAgQ0dDb250ZXh0UmVmIGNv bnRleHQgPSBDR0JpdG1hcENvbnRleHRDcmVhdGUoMCwgd2lkdGgsIGhlaWdodCwgOCwgd2lkdGgg KiA0LCBkZXZpY2VSR0IuZ2V0KCksIGtDR0ltYWdlQWxwaGFQcmVtdWx0aXBsaWVkRmlyc3QgfCBr Q0dCaXRtYXBCeXRlT3JkZXIzMkhvc3QpOworCiAgICAgaWYgKCFjb250ZXh0KQogICAgICAgICBy ZXR1cm4gMDsKICAgICBDR0NvbnRleHREcmF3SW1hZ2UoY29udGV4dCwgQ0dSZWN0TWFrZSgwLCAw LCB3aWR0aCwgaGVpZ2h0KSwgaW1hZ2UpOwpAQCAtNzQsNyArNzgsMTAgQEAgRHJhZ0ltYWdlUmVm IGRpc3NvbHZlRHJhZ0ltYWdlVG9GcmFjdGlvbgogICAgICAgICByZXR1cm4gMDsKICAgICBzaXpl X3Qgd2lkdGggPSBDR0ltYWdlR2V0V2lkdGgoaW1hZ2UpOwogICAgIHNpemVfdCBoZWlnaHQgPSBD R0ltYWdlR2V0SGVpZ2h0KGltYWdlKTsKLSAgICBDR0NvbnRleHRSZWYgY29udGV4dCA9IENHQml0 bWFwQ29udGV4dENyZWF0ZSgwLCB3aWR0aCwgaGVpZ2h0LCA4LCB3aWR0aCAqIDQsIENHSW1hZ2VH ZXRDb2xvclNwYWNlKGltYWdlKSwga0NHSW1hZ2VBbHBoYVByZW11bHRpcGxpZWRGaXJzdCB8IGtD R0JpdG1hcEJ5dGVPcmRlcjMySG9zdCk7CisKKyAgICBSZXRhaW5QdHI8Q0dDb2xvclNwYWNlUmVm PiBkZXZpY2VSR0IoV1RGOjpBZG9wdENGLCBDR0NvbG9yU3BhY2VDcmVhdGVXaXRoTmFtZShrQ0dD b2xvclNwYWNlR2VuZXJpY1JHQikpOworICAgIENHQ29udGV4dFJlZiBjb250ZXh0ID0gQ0dCaXRt YXBDb250ZXh0Q3JlYXRlKDAsIHdpZHRoLCBoZWlnaHQsIDgsIHdpZHRoICogNCwgZGV2aWNlUkdC LmdldCgpLCBrQ0dJbWFnZUFscGhhUHJlbXVsdGlwbGllZEZpcnN0IHwga0NHQml0bWFwQnl0ZU9y ZGVyMzJIb3N0KTsKKwogICAgIGlmICghY29udGV4dCkKICAgICAgICAgcmV0dXJuIDA7CiAgICAg Ly8gRnJvbSBDR0NvbnRleHQuaDoK