Bug 83764

Summary: [Qt] Two canvas tests fail with QT_IMAGE_DECODER=0 setup
Product: WebKit Reporter: Zoltan Horvath <zoltan>
Component: ImagesAssignee: Zoltan Horvath <zoltan>
Status: RESOLVED FIXED    
Severity: Normal CC: noel.gordon, ossy, webkit.review.bot
Priority: P2 Keywords: Qt
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 32410    
Bug Blocks:    
Attachments:
Description Flags
Patch none

Zoltan Horvath
Reported 2012-04-12 02:57:44 PDT
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!
Attachments
Patch (1.81 KB, patch)
2012-04-12 03:40 PDT, Zoltan Horvath 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Zoltan Horvath
Comment 1 2012-04-12 03:40:16 PDT
Created attachment 136871 [details] Patch
Csaba Osztrogonác
Comment 2 2012-04-12 03:45:54 PDT
Comment on attachment 136871 [details] Patch r=me
Csaba Osztrogonác
Comment 3 2012-04-12 03:52:26 PDT
Comment on attachment 136871 [details] Patch Clearing flags on attachment: 136871 Committed r113960: <http://trac.webkit.org/changeset/113960>
Csaba Osztrogonác
Comment 4 2012-04-12 03:52:33 PDT
All reviewed patches have been landed. Closing bug.
noel gordon
Comment 5 2012-04-12 04:37:52 PDT
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?
Zoltan Horvath
Comment 6 2012-04-12 04:54:07 PDT
(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.
noel gordon
Comment 7 2012-04-12 05:15:57 PDT
(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.
Zoltan Horvath
Comment 8 2012-04-13 04:18:34 PDT
(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).
noel gordon
Comment 9 2012-04-14 04:43:57 PDT
(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.
Note You need to log in before you can comment on or make changes to this bug.