Bug 64151 – [skia] turn off bitmap filtering for RESAMPLE_NONE

• Component: New Bugs
• Status: RESOLVED WONTFIX

[Mike Lawther]

[skia] turn off bitmap filtering for RESAMPLE_NONE

[Mike Lawther]

Created attachment 100082 [details]
Patch

[Mike Lawther]

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.

[Stephen White]

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.

[Mike Lawther]

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.

[Stephen White]

(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.

[WebKit Review Bot]

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

[WebKit Review Bot]

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

[Mike Lawther]

Created attachment 103200 [details]
Patch

[Mike Lawther]

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) ?

[Mike Lawther]

Created attachment 103202 [details]
Patch

[Mike Lawther]

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.

[Stephen White]

This seems like a lot more tests than I would have expected.  Are these all now rendering with nearest-neighbour filtering?

[Mike Lawther]

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.

[James Robinson]

Comment on attachment 103202 [details]
Patch

R=me

[Mike Lawther]

Comment on attachment 103202 [details]
Patch

Thanks James - setting cq+ (hoping the test_expectations patches cleanly ...)

[WebKit Review Bot]

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

[Mike Lawther]

Created attachment 103576 [details]
Patch for landing

[WebKit Review Bot]

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