Bug 98778

Summary: [Chromium-Android] Color profiles are not supported on Android
Product: WebKit Reporter: Peter Beverloo <peter>
Component: ImagesAssignee: Nobody <webkit-unassigned>
Status: RESOLVED WONTFIX    
Severity: Normal CC: abarth, benjamin, code.vineet, hclam, ojan.autocc, qinmin, schenney, skyostil, tomhudson, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 96398    
Attachments:
Description Flags
proposed_patch
none
updated_patch abarth: review+

Attachments
proposed_patch (1.18 KB, patch)
2013-01-15 14:20 PST, Vineet Chaudhary (vineetc) 		no flags
DetailsFormatted DiffDiff
updated_patch (3.26 KB, patch)
2013-01-15 16:18 PST, Vineet Chaudhary (vineetc) 		abarth: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Vineet Chaudhary (vineetc)
Comment 1 2013-01-15 14:20:02 PST
Created attachment 182844 [details] proposed_patch Color Profiles are now supported on chromium platforms except Android. This patch enables building code for qcm and iccjpeg for android. I have tested basic test case attached in the bug which passes in content shell. Are there any concerns enabling these flags?
Adam Barth
Comment 2 2013-01-15 14:45:24 PST
Comment on attachment 182844 [details] proposed_patch View in context: https://bugs.webkit.org/attachment.cgi?id=182844&action=review > Source/WTF/wtf/Platform.h:489 > +#define WTF_USE_ICCJPEG 1 > +#define WTF_USE_QCMSLIB 1 Should we just bring these out of the #else clause since they're now enabled on all platforms?
Vineet Chaudhary (vineetc)
Comment 3 2013-01-15 14:57:19 PST
(In reply to comment #2) > (From update of attachment 182844 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=182844&action=review > > > Source/WTF/wtf/Platform.h:489 > > +#define WTF_USE_ICCJPEG 1 > > +#define WTF_USE_QCMSLIB 1 > > Should we just bring these out of the #else clause since they're now enabled on all platforms? Yes. I think we can even pull out #define WTF_USE_SKIA 1 along with above, as this is also defined for all ports for Chromium, in same patch or another.
Vineet Chaudhary (vineetc)
Comment 4 2013-01-15 16:18:32 PST
Created attachment 182867 [details] updated_patch I have removed the failing tests from TestExpectations, and checked these tests manually in content shell. As I have some issues with setup to run /run-webkit-tests --chromium-android
Adam Barth
Comment 5 2013-01-15 22:35:31 PST
Comment on attachment 182867 [details] updated_patch This patch LGTM, but we might want Peter Beverloo to sign off before landing.
Peter Beverloo
Comment 6 2013-01-16 04:14:05 PST
Hi Vineet, this is awesome, thank you so much! I'll run some performance tests to measure what this means for image decoding time, and will reply with them on the bug once they're complete. I'm also adding Sami and Alpha as they know much more about image decoding performance on Chromium for Android, and might be able to provide additional insights.
Vineet Chaudhary (vineetc)
Comment 7 2013-01-22 11:06:48 PST
(In reply to comment #6) > Hi Vineet, this is awesome, thank you so much! I'll run some performance tests to measure what this means for image decoding time, and will reply with them on the bug once they're complete. > > I'm also adding Sami and Alpha as they know much more about image decoding performance on Chromium for Android, and might be able to provide additional insights. Hello Peter Beverloo, Did you got chance to run performance tests? I did dry run for the link http://regex.info/blog/2012-03-27/1964 found no glitches, but w/o performance data. If you can point tests which can give perf results I can test that. Thanks.
Note You need to log in before you can comment on or make changes to this bug.