Bug 41036 – REGRESSION (r61341): Image interpolation quality changes during scrolling and when activating / deactivating application

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Mark Rowe (bdash)]

Scrolling down the page on <http://epguides.com/AshestoAshes/> and then back to the top results in the image briefly being painted with a lower quality interpolation before snapping in to the higher quality interpolation.  My understanding was that we were only dropping to lower quality interpolation when the image was changing size.  That is not happening on this page.

Steps to reproduce:
1. Load <http://epguides.com/AshestoAshes/>.
2. Scroll to the bottom of the page, then back to the top.  Watch the photo at the top closely while doing so.

Alternate steps to reproduce:
1. Load <http://epguides.com/AshestoAshes/>.
2. Cmd-Tab to a different application, then Cmd-Tab back to Safari.  Watch the photo at the top closely while doing so.


I bisected and determined this to be caused by <http://trac.webkit.org/changeset/61341>, the fix for bug 38233.

[Mark Rowe (bdash)]

<rdar://problem/8120955>

[Stephen White]

Created attachment 59517 [details]
Scrolling example

This is actually by design.  The low-quality resize is always done first, since even scrolling may be affected by the speed of high-quality resize.  For example, I've attached a static page, which when scrolled, shows very poor scrolling performance without my fix (these are all the same image, of course, but it would also happen for many small thumbnails of different images).

If this new behaviour is still not acceptable to you, I do have an idea for a fix which will preserve most of the performance gains of the original fix.  It will definitely regress on pages like the attached, however.

[Mark Rowe (bdash)]

The attached page still shows poor performance with your change: all of the images change interpolation quality at once, leading to hangs of between half and one and a half seconds as every image is resampled at once.  Maybe this is better than it was, but it hardly seems like ideal performance.

I have to confess that I don’t really understand what is going on here.  It appears that any time we’re asked to repaint the image we first repaint it using the lower interpolation quality before repainting it at the higher interpolation quality.  This leads to a very visible shift in the image after changing tabs, scrolling, deactivating the window, activating the window, forcing the window to repaint via the Debug menu, etc.  I don’t think that this shifting is acceptable on pages that are clearly not doing anything heavyweight (whether it be resizing images rapidly or having many tens of resized images).  Whatever heuristic is being used to determine when to use the lower-quality interpolation needs to be smarter than just blindly doing it whenever a resized image repaints.

[Stephen White]

(In reply to comment #3)
> The attached page still shows poor performance with your change: all of the images change interpolation quality at once, leading to hangs of between half and one and a half seconds as every image is resampled at once.  Maybe this is better than it was, but it hardly seems like ideal performance.

The main difference is that the delay/hang happens only when you stop scrolling, rather than on every mouse move.  Without the patch, scrolling is unusable.  You're right that it's not ideal, but it's better than it was.

> I have to confess that I don’t really understand what is going on here.  It appears that any time we’re asked to repaint the image we first repaint it using the lower interpolation quality before repainting it at the higher interpolation quality.  This leads to a very visible shift in the image after changing tabs, scrolling, deactivating the window, activating the window, forcing the window to repaint via the Debug menu, etc.  I don’t think that this shifting is acceptable on pages that are clearly not doing anything heavyweight (whether it be resizing images rapidly or having many tens of resized images).  Whatever heuristic is being used to determine when to use the lower-quality interpolation needs to be smarter than just blindly doing it whenever a resized image repaints.

Actually, it sounds like you understand exactly what's going on.  You just don't like it, which is fair.

I've put together another patch that's more like the original behaviour, and I'll attach it to this bug so we can talk about it.  I'm still more happy with the original patch, since it's less janky, but I think this might be an acceptable compromise.

[Stephen White]

Created attachment 59550 [details]
Patch

[Mark Rowe (bdash)]

(In reply to comment #4)
> Actually, it sounds like you understand exactly what's going on.  You just don't like it, which is fair.

I phrased it like that because my understanding is based entirely on observing the behavior.  I’ve not looked at the code change that you made, so it would be easy for me to be missing something.

[Dave Hyatt]

Comment on attachment 59550 [details]
Patch

This looks good to me.  We definitely only want to kick to low quality if an image changes size.  We never want to kick to low quality if an image is statically scaled and not changing size (but repainting rapidly).  Graphics systems will have a cache of the image in that case, so there's no need for low quality.  If the case where you have multiple copies of the same image at different static scales all repainting rapidly is slow, that's fine. That's not really a case we need to be concerned about.

[Dave Hyatt]

Comment on attachment 59550 [details]
Patch

Let's land this new patch.  We talked a long time in IRC, and this patch can be refined further to have a threshold that has to be exceeded before it kicks the static scaled images into low quality.  That will happen in a followup.

[Stephen White]

Committed r61710: <http://trac.webkit.org/changeset/61710>

[Mark Rowe (bdash)]

*** Bug 40904 has been marked as a duplicate of this bug. ***

[Simon Fraser (smfr)]

See bug 42390.

[Nico Weber]

See http://code.google.com/p/chromium/issues/detail?id=55495 (affects safari as well).