Bug 119263

Summary: [Qt] Images are down-scaled badly
Product: WebKit Reporter: Allan Sandfeld Jensen <allan.jensen>
Component: WebKit QtAssignee: Allan Sandfeld Jensen <allan.jensen>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, jturcotte, noam, webkit-ews
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 119392    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch jturcotte: review+

Allan Sandfeld Jensen
Reported 2013-07-30 09:06:21 PDT
If images are scaled down more than 2x the scaled result becomes increasingly worse. This is probably caused by QPainter using bilinear sampling, see https://bugreports.qt-project.org/browse/QTBUG-30682 . To avoid the issue we can prescale the image before sending it to QPainter.
Attachments
Patch (3.53 KB, patch)
2013-07-31 03:01 PDT, Allan Sandfeld Jensen 		no flags
DetailsFormatted DiffDiff
Patch (3.68 KB, patch)
2013-07-31 03:15 PDT, Allan Sandfeld Jensen 		no flags
DetailsFormatted DiffDiff
Patch (3.71 KB, patch)
2013-07-31 03:42 PDT, Allan Sandfeld Jensen 		no flags
DetailsFormatted DiffDiff
Patch (4.03 KB, patch)
2013-07-31 06:30 PDT, Allan Sandfeld Jensen 		jturcotte: review+
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Allan Sandfeld Jensen
Comment 1 2013-07-31 03:01:50 PDT
Created attachment 207823 [details] Patch
WebKit Commit Bot
Comment 2 2013-07-31 03:03:04 PDT
Attachment 207823 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCore/platform/graphics/qt/ImageQt.cpp']" exit_code: 1 Source/WebCore/platform/graphics/qt/ImageQt.cpp:273: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/platform/graphics/qt/ImageQt.cpp:274: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/platform/graphics/qt/ImageQt.cpp:275: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/platform/graphics/qt/ImageQt.cpp:276: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Total errors found: 4 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style.
Early Warning System Bot
Comment 3 2013-07-31 03:07:48 PDT
Comment on attachment 207823 [details] Patch Attachment 207823 [details] did not pass qt-ews (qt): Output: http://webkit-queues.appspot.com/results/1292609
Early Warning System Bot
Comment 4 2013-07-31 03:08:59 PDT
Comment on attachment 207823 [details] Patch Attachment 207823 [details] did not pass qt-wk2-ews (qt-wk2): Output: http://webkit-queues.appspot.com/results/1293620
Allan Sandfeld Jensen
Comment 5 2013-07-31 03:15:47 PDT
Created attachment 207824 [details] Patch
WebKit Commit Bot
Comment 6 2013-07-31 03:17:47 PDT
Attachment 207824 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCore/platform/graphics/qt/ImageQt.cpp']" exit_code: 1 Source/WebCore/platform/graphics/qt/ImageQt.cpp:273: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/platform/graphics/qt/ImageQt.cpp:274: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/platform/graphics/qt/ImageQt.cpp:275: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/platform/graphics/qt/ImageQt.cpp:276: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Total errors found: 4 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style.
Early Warning System Bot
Comment 7 2013-07-31 03:21:24 PDT
Comment on attachment 207824 [details] Patch Attachment 207824 [details] did not pass qt-ews (qt): Output: http://webkit-queues.appspot.com/results/1289715
Early Warning System Bot
Comment 8 2013-07-31 03:21:30 PDT
Comment on attachment 207824 [details] Patch Attachment 207824 [details] did not pass qt-wk2-ews (qt-wk2): Output: http://webkit-queues.appspot.com/results/1275021
Allan Sandfeld Jensen
Comment 9 2013-07-31 03:24:24 PDT
(In reply to comment #8) > (From update of attachment 207824 [details]) > Attachment 207824 [details] did not pass qt-wk2-ews (qt-wk2): > Output: http://webkit-queues.appspot.com/results/1275021 Seems the EWS bot is building with Qt 5.0.
Allan Sandfeld Jensen
Comment 10 2013-07-31 03:42:59 PDT
Created attachment 207828 [details] Patch Fix build with Qt 5.0
Jocelyn Turcotte
Comment 11 2013-07-31 05:41:57 PDT
Comment on attachment 207828 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=207828&action=review > Source/WebCore/ChangeLog:8 > + Prescale images before painting and cache the result in the pixmap cache. Please add a note here or in the code explaining the difference of behavior between QPixmap and QPainter when down-scaling. > Source/WebCore/platform/graphics/qt/ImageQt.cpp:272 > + && (normalizedDst.width() * 1.5 * pixelRatio < normalizedSrc.width() > + || normalizedDst.height() * 1.5 * pixelRatio < normalizedSrc.height())) { 1.0 / 0.5 == 2.0, I'm not following why you're using 1.5 here.
Allan Sandfeld Jensen
Comment 12 2013-07-31 06:16:06 PDT
(In reply to comment #11) > (From update of attachment 207828 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=207828&action=review > > > Source/WebCore/ChangeLog:8 > > + Prescale images before painting and cache the result in the pixmap cache. > > Please add a note here or in the code explaining the difference of behavior between QPixmap and QPainter when down-scaling. > I will update the comments and ChangeLog. > > Source/WebCore/platform/graphics/qt/ImageQt.cpp:272 > > + && (normalizedDst.width() * 1.5 * pixelRatio < normalizedSrc.width() > > + || normalizedDst.height() * 1.5 * pixelRatio < normalizedSrc.height())) { > > 1.0 / 0.5 == 2.0, I'm not following why you're using 1.5 here. It is mentioned in the comment above. // We prescale before hitting 0.5x because prescaling quality is already better at 0.5x, and with caching often // faster than transforms in the raster paint engine.
Allan Sandfeld Jensen
Comment 13 2013-07-31 06:30:57 PDT
Created attachment 207848 [details] Patch Improve ChangeLog, and restrict prescaling to 0.5x and below. Quality can be better before, but the pixelation issues only trigger after.
Allan Sandfeld Jensen
Comment 14 2013-07-31 07:38:30 PDT
Committed r153522: <http://trac.webkit.org/changeset/153522>
Note You need to log in before you can comment on or make changes to this bug.