83764 2012-04-12 02:57:44 -0700 [Qt] Two canvas tests fail with QT_IMAGE_DECODER=0 setup 2012-04-14 04:43:57 -0700 1 1 1 Unclassified WebKit Images 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED Qt P2 Normal --- 32410 1 zoltan zoltan noel.gordon ossy webkit.review.bot oldest_to_newest 601045 0 zoltan 2012-04-12 02:57:44 -0700 fast/canvas/canvas-toDataURL-case-insensitive-mimetype.html --- /home/zoltan/prog/local/WebKit/WebKitBuild/Release/layout-test-results/fast/canvas/canvas-toDataURL-case-insensitive-mimetype-expected.txt +++ /home/zoltan/prog/local/WebKit/WebKitBuild/Release/layout-test-results/fast/canvas/canvas-toDataURL-case-insensitive-mimetype-actual.txt @@ -6,9 +6,6 @@ PASS tryMimeType('image/png') is 'image/png' PASS tryMimeType('iMAge/Png') is 'image/png' PASS tryMimeType('IMAGE/PNG') is 'image/png' -PASS tryMimeType('image/jpeg') is 'image/jpeg' -PASS tryMimeType('imAgE/jPEg') is 'image/jpeg' -PASS tryMimeType('IMAGE/JPEG') is 'image/jpeg' PASS successfullyParsed is true TEST COMPLETE fast/canvas/toDataURL-supportedTypes.html --- /home/zoltan/prog/local/WebKit/WebKitBuild/Release/layout-test-results/fast/canvas/toDataURL-supportedTypes-expected.txt +++ /home/zoltan/prog/local/WebKit/WebKitBuild/Release/layout-test-results/fast/canvas/toDataURL-supportedTypes-actual.txt @@ -4,8 +4,8 @@ MIME types are the SAME. Given MIMEType: image/jpeg -Used MIMEType: image/jpeg -MIME types are the SAME. +Used MIMEType: image/png +MIME types DIFFER. Given MIMEType: image/gif Used MIMEType: image/png Let's check this out! 601076 1 136871 zoltan 2012-04-12 03:40:16 -0700 Created attachment 136871 Patch 601080 2 136871 ossy 2012-04-12 03:45:54 -0700 Comment on attachment 136871 Patch r=me 601082 3 136871 ossy 2012-04-12 03:52:26 -0700 Comment on attachment 136871 Patch Clearing flags on attachment: 136871 Committed r113960: <http://trac.webkit.org/changeset/113960> 601083 4 ossy 2012-04-12 03:52:33 -0700 All reviewed patches have been landed. Closing bug. 601096 5 noel.gordon 2012-04-12 04:37:52 -0700 Looks ok to me. If I were doing it, I'd consider changing http://trac.webkit.org/browser/trunk/Source/WebCore/platform/MIMETypeRegistry.cpp#L290 to read #elif PLATFORM(QT) is you need to support all the "encoding" formats you did prior to this change. If the goal is to reduce the number of supported formats, well I'm fine with that also. And is canvas/philip/tests toDataURL.jpeg.alpha.html passing or skipped on Qt? 601104 6 zoltan 2012-04-12 04:54:07 -0700 (In reply to comment #5) > Looks ok to me. If I were doing it, I'd consider changing > http://trac.webkit.org/browser/trunk/Source/WebCore/platform/MIMETypeRegistry.cpp#L290 > > to read > #elif PLATFORM(QT) > > is you need to support all the "encoding" formats you did prior to this change. If the goal is to reduce the number of supported formats, well I'm fine with that also. Thanks! I need to extend my patch in bug #80400 with MIMETypeRegistry changes. Our goal is to change to webcore imagedecoders and let a fallback case to qtimagedecoder for other types. > And is canvas/philip/tests toDataURL.jpeg.alpha.html passing or skipped on Qt? It's not skipped and passing. 601117 7 noel.gordon 2012-04-12 05:15:57 -0700 (In reply to comment #6) > (In reply to comment #5) > > Looks ok to me. If I were doing it, I'd consider changing > > http://trac.webkit.org/browser/trunk/Source/WebCore/platform/MIMETypeRegistry.cpp#L290 > > > > to read > > #elif PLATFORM(QT) > > > > is you need to support all the "encoding" formats you did prior to this change. If the goal is to reduce the number of supported formats, well I'm fine with that also. > > Thanks! I need to extend my patch in bug #80400 with MIMETypeRegistry changes. Our goal is to change to webcore imagedecoders and let a fallback case to qtimagedecoder for other types. That's fine for decoders -- here I'm talking about image "encoders". > > And is canvas/philip/tests toDataURL.jpeg.alpha.html passing or skipped on Qt? > > It's not skipped and passing. Good, so many ports get this wrong. You must pass premultipled color components from the 2d canvas to the JPEG encoder to pass this test and so conform to the <canvas> spec -- for output images that do not have alpha, form a Porter-Duff composite src-over black. For the 3d WebGL <cavnas>.toDataURL() cases, things are a little more complicated. Sometimes you must pass premultipled color, and sometimes you must not. Not sure if the Qt implementation has that ability yet, but there is a test for it in the webgl test suite. fast/canvas/webgl/premultiplyalpha-test.html Refer to bug 68366. 601968 8 zoltan 2012-04-13 04:18:34 -0700 (In reply to comment #7) > (In reply to comment #6) > > (In reply to comment #5) > > > Looks ok to me. If I were doing it, I'd consider changing > > > http://trac.webkit.org/browser/trunk/Source/WebCore/platform/MIMETypeRegistry.cpp#L290 > > > > > > to read > > > #elif PLATFORM(QT) > > > > > > is you need to support all the "encoding" formats you did prior to this change. If the goal is to reduce the number of supported formats, well I'm fine with that also. > > > > Thanks! I need to extend my patch in bug #80400 with MIMETypeRegistry changes. Our goal is to change to webcore imagedecoders and let a fallback case to qtimagedecoder for other types. > > That's fine for decoders -- here I'm talking about image "encoders". I would modify that when we get rid of QT_IMAGE_DECODER macro, in other case we won't hit elif in L297. > > > And is canvas/philip/tests toDataURL.jpeg.alpha.html passing or skipped on Qt? > > > > It's not skipped and passing. > > Good, so many ports get this wrong. You must pass premultipled color components from the 2d canvas to the JPEG encoder to pass this test and so conform to the <canvas> spec -- for output images that do not have alpha, form a Porter-Duff composite src-over black. > > For the 3d WebGL <cavnas>.toDataURL() cases, things are a little more complicated. Sometimes you must pass premultipled color, and sometimes you must not. Not sure if the Qt implementation has that ability yet, but there is a test for it in the webgl test suite. > > fast/canvas/webgl/premultiplyalpha-test.html > > Refer to bug 68366. Unfortunately, we skip all webgl tests for Qt-port at http://trac.webkit.org/browser/trunk/LayoutTests/platform/qt/Skipped#L385 It would be good to check these test why they are failing (only few passes). 602650 9 noel.gordon 2012-04-14 04:43:57 -0700 (In reply to comment #8) > > > Thanks! I need to extend my patch in bug #80400 with MIMETypeRegistry changes. Our goal is to change to webcore imagedecoders and let a fallback case to qtimagedecoder for other types. > > > > That's fine for decoders -- here I'm talking about image "encoders". > > I would modify that when we get rid of QT_IMAGE_DECODER macro, in other case we won't hit elif in L297. Sounds good Zoltan. Added myself to bug 80400 to follow along. > > Refer to bug 68366. > Unfortunately, we skip all webgl tests for Qt-port ... I see. 136871 2012-04-12 03:40:16 -0700 2012-04-12 03:52:26 -0700 Patch bug-83764-20120412123923.patch text/plain 1858 zoltan U3VidmVyc2lvbiBSZXZpc2lvbjogMTEzOTU4CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggODg0YjgwNzFhNGFhMjkw OWMzNTU2ZmFiNTA1ZDJmOTRjMjJlOTg2My4uMTZkMzk4OWVlNjgwOGE3YmMxMTcxMDg4ZGQ0NDIz YjE4YTViMDIzMSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDIxIEBACisyMDEyLTA0LTEyICBab2x0 YW4gSG9ydmF0aCAgPHpvbHRhbkB3ZWJraXQub3JnPgorCisgICAgICAgIFtRdF0gVHdvIGNhbnZh cyB0ZXN0cyBmYWlsIHdpdGggUVRfSU1BR0VfREVDT0RFUj0wIHNldHVwCisgICAgICAgIGh0dHBz Oi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD04Mzc2NAorCisgICAgICAgIFRoZSBm b2xsb3dpbmcgdGVzdHMgd2VyZSBmYWlsZWQgYWZ0ZXIgcjEwODc5MjoKKyAgICAgICAgLSBjYW52 YXMtdG9EYXRhVVJMLWNhc2UtaW5zZW5zaXRpdmUtbWltZXR5cGUuaHRtbAorICAgICAgICAtIHRv RGF0YVVSTC1zdXBwb3J0ZWRUeXBlcy5odG1sIAorICAgICAgICBiZWNhdXNlIHN1cHBvcnRlZElt YWdlTUlNRVR5cGVzRm9yRW5jb2Rpbmcgd2Fzbid0IGZpbGxlZAorICAgICAgICB3aXRoIE1JTUUg dHlwZXMgaW4gY2FzZSBvZiBRVF9JTUFHRV9ERUNPREVSPTAgYnVpbGQgc2V0dXAuCisKKyAgICAg ICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgRml4ZXMgZXhpc3Rpbmcg dGVzdHMuCisKKyAgICAgICAgKiBwbGF0Zm9ybS9NSU1FVHlwZVJlZ2lzdHJ5LmNwcDoKKyAgICAg ICAgKFdlYkNvcmU6OmluaXRpYWxpemVTdXBwb3J0ZWRJbWFnZU1JTUVUeXBlc0ZvckVuY29kaW5n KToKKwogMjAxMi0wNC0xMiAgQ2hhcmxlcyBXZWkgIDxjaGFybGVzLndlaUB0b3JjaG1vYmlsZS5j b20uY24+CiAKICAgICAgICAgW0JsYWNrQmVycnldIFVwc3RyZWFtaW5nIEJsYWNrQmVycnktc3Bl Y2lmaWMgY2hhbmdlcyB0byBQbHVnaW5WaWV3IC0tIHJlbW92ZSB1bnVzZWQgZnVuY3Rpb25zCmRp ZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9NSU1FVHlwZVJlZ2lzdHJ5LmNwcCBi L1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL01JTUVUeXBlUmVnaXN0cnkuY3BwCm9sZCBtb2RlIDEw MDY0NApuZXcgbW9kZSAxMDA3NTUKaW5kZXggMjNjNjk3MTVhZDQ1MDY0NGM3YTc3YzhmM2VjNGNh MTAxNGE3MTZjMS4uYjQ4MzBjZDU1OTExMjdiZWQ4MWQ0M2I3ZjQ0ZjI3MDU4MWY1NWVmZgotLS0g YS9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9NSU1FVHlwZVJlZ2lzdHJ5LmNwcAorKysgYi9Tb3Vy Y2UvV2ViQ29yZS9wbGF0Zm9ybS9NSU1FVHlwZVJlZ2lzdHJ5LmNwcApAQCAtMjk0LDcgKzI5NCw3 IEBAIHN0YXRpYyB2b2lkIGluaXRpYWxpemVTdXBwb3J0ZWRJbWFnZU1JTUVUeXBlc0ZvckVuY29k aW5nKCkKICAgICAgICAgaWYgKCFtaW1lVHlwZS5pc0VtcHR5KCkpCiAgICAgICAgICAgICBzdXBw b3J0ZWRJbWFnZU1JTUVUeXBlc0ZvckVuY29kaW5nLT5hZGQobWltZVR5cGUpOwogICAgIH0KLSNl bGlmIFBMQVRGT1JNKEdUSykKKyNlbGlmIFBMQVRGT1JNKEdUSykgfHwgKFBMQVRGT1JNKFFUKSAm JiAhVVNFKFFUX0lNQUdFX0RFQ09ERVIpKQogICAgIHN1cHBvcnRlZEltYWdlTUlNRVR5cGVzRm9y RW5jb2RpbmctPmFkZCgiaW1hZ2UvcG5nIik7CiAgICAgc3VwcG9ydGVkSW1hZ2VNSU1FVHlwZXNG b3JFbmNvZGluZy0+YWRkKCJpbWFnZS9qcGVnIik7CiAgICAgc3VwcG9ydGVkSW1hZ2VNSU1FVHlw ZXNGb3JFbmNvZGluZy0+YWRkKCJpbWFnZS90aWZmIik7Cg==