| Summary: | [Qt] Two canvas tests fail with QT_IMAGE_DECODER=0 setup | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Zoltan Horvath <zoltan> | ||||
| Component: | Images | Assignee: | 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: |
|
||||||
Created attachment 136871 [details]
Patch
Comment on attachment 136871 [details]
Patch
r=me
Comment on attachment 136871 [details] Patch Clearing flags on attachment: 136871 Committed r113960: <http://trac.webkit.org/changeset/113960> All reviewed patches have been landed. Closing bug. 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? (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. (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. (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). (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. |
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!