[skia] turn off bitmap filtering for RESAMPLE_NONE
Created attachment 100082 [details] Patch
I came across this as part of figuring out why optimize-contrast didn't work with Skia (note this patch will not fix optimize-contrast - I'll do that in a followup patch if this one goes in). It seems that Skia always applies bilinear scaling, even when computeResamplingMode calculates that it should use RESAMPLE_NONE. Looking at line 422 of ImageSkia.cpp, setFilterBitmap() is set similarly to this patch.
Comment on attachment 100082 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=100082&action=review > Source/WebCore/platform/graphics/skia/ImageSkia.cpp:281 > + paint.setFilterBitmap(resampling == RESAMPLE_LINEAR); I'm pretty sure this is going to break the timer-based resize stuff, in the sense that it will now go from Lanczos3 to nearest, rather than Lanczos3 to linear. I realize that seems to be what Chrome/Mac is currently doing, but that's wrong IMHO. This should be fixed in GraphicsContext::drawImage() (see the FIXME there about useLowQuality mapping to InterpolationNone). If this is not desired by the Safari folks, we should provide a port-specific define that defines what "useLowQualityScale" to InterpolationLow on Chrome, and InterpolationNone elsewhere.
Actually, this won't break the timer based stuff, since Skia ignores the InterpolationQuality hint from the GraphicsContext anyway. This patch makes Skia respect its own decision made in computeSamplingMode :) However, you've anticipated my next patch, which was to make Skia pay attention to the hint, since that's how optimize-contrast selects a nearest-neighbour interpolation (only works on Mac thus far). So it sounds like before I do that, I'll need to resolve the FIXME in GraphicsContext::drawImage() - which is about to celebrate its 3rd birthday. I might even get an opportunity to resolve https://bugs.webkit.org/show_bug.cgi?id=58495 at the same time.
(In reply to comment #4) > Actually, this won't break the timer based stuff, since Skia ignores the InterpolationQuality hint from the GraphicsContext anyway. This patch makes Skia respect its own decision made in computeSamplingMode :) Actually, computeResamplingMode *does* pay attention to the hint. It uses it to "gate" the Lanczos3 implementation (if it's anything other than "high", it'll fall back to bilinear). See ImageSkia.cpp:147. But you're right, I think it will be ok in this case, since computeFilterBitmap() will still return linear in the animated case, and setFilterBitmap() will still be set to true. So it should be fine.
Comment on attachment 100082 [details] Patch Rejecting attachment 100082 [details] from commit-queue. Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=ec2-cq-02', '--port..." exit_code: 2 Last 500 characters of output: rotate-transform.html = IMAGE fast/borders/border-image-scale-transform.html = IMAGE fast/borders/inline-mask-overlay-image.html = IMAGE fast/forms/input-appearance-height.html = IMAGE fast/forms/search-vertical-alignment.html = IMAGE fast/reflections/reflection-masks-opacity.html = IMAGE fast/reflections/reflection-masks.html = IMAGE fast/repaint/background-misaligned.html = IMAGE fast/replaced/001.html = IMAGE fast/replaced/002.html = IMAGE fast/replaced/003.html = IMAGE Full output: http://queues.webkit.org/results/9023810
Created attachment 100752 [details] Archive of layout-test-results from ec2-cq-02 The attached test failures were seen while running run-webkit-tests on the commit-queue. Bot: ec2-cq-02 Port: Chromium Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
Created attachment 103200 [details] Patch
Hi Stephen - I've added in all the tests that'll need rebaselining to test_expectations as a result of this change. Can you take another look (the code is that same as before) ?
Created attachment 103202 [details] Patch
Comment on attachment 103202 [details] Patch Darn - the cr-linux EWS is failing some tests that I'll have to check out. The chromium try-bots were passing with this patch. I'm clearing the review flag for now.
This seems like a lot more tests than I would have expected. Are these all now rendering with nearest-neighbour filtering?
Comment on attachment 103202 [details] Patch Yes. In some cases, it's only part of the image that has used nearest-neighbor. For example, in fast/borders/border-image-01.html, only the border images that are stretched a lot in one dimension are NN, the central image is still linear. This seems like an intended effect of computeResamplingMode, eg // Don't resample if it is being stretched a lot in only one direction. // This is trying to catch cases where somebody has created a border // (which might be large) and then is stretching it to fill some part // of the page. All of these heuristics that returned RESAMPLE_NONE are currently not being acted on, so if they are wrong, or no longer desired, then they really should be deleted. I can do that in another patch. btw - the cr-linux bot passed the patch (looks like it was a unrelated failure), so I'm adding the review flag back.
Comment on attachment 103202 [details] Patch R=me
Comment on attachment 103202 [details] Patch Thanks James - setting cq+ (hoping the test_expectations patches cleanly ...)
Comment on attachment 103202 [details] Patch Rejecting attachment 103202 [details] from commit-queue. Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=ec2-cq-01', '--port..." exit_code: 2 Last 500 characters of output: 2 lines). Hunk #2 succeeded at 2843 (offset -5 lines). Hunk #3 FAILED at 3686. 1 out of 3 hunks FAILED -- saving rejects to file LayoutTests/platform/chromium/test_expectations.txt.rej patching file Source/WebCore/ChangeLog Hunk #1 succeeded at 1 with fuzz 3. patching file Source/WebCore/platform/graphics/skia/ImageSkia.cpp Hunk #1 succeeded at 278 (offset 1 line). Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--reviewer', u'James Robinson', u'--f..." exit_code: 1 Full output: http://queues.webkit.org/results/9346012
Created attachment 103576 [details] Patch for landing
Comment on attachment 103576 [details] Patch for landing Rejecting attachment 103576 [details] from commit-queue. Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=ec2-cq-03', '--port..." exit_code: 2 Last 500 characters of output: ressions: Unexpected image and text mismatch : (1) svg/custom/svg-fonts-word-spacing.html = IMAGE+TEXT Regressions: Unexpected image mismatch : (6) fast/text/atsui-multiple-renderers.html = IMAGE fast/text/international/danda-space.html = IMAGE fast/text/international/thai-baht-space.html = IMAGE fast/text/international/thai-line-breaks.html = IMAGE platform/chromium-linux/fast/text/international/complex-joining-using-gpos.html = IMAGE tables/mozilla/core/bloomberg.html = IMAGE Full output: http://queues.webkit.org/results/9353070