Bug 33408 – Add flag IGNORE_FIXED_BACKGROUNDS (disabled by default) to ignore fixed background images and accelerate web page scrolling on low-powered/mobile devices

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Daniel Bates]

Proposed optimization to ignore fixed background images (i.e. background-attachment: fixed) when a page does not contain any fixed position elements so as to allow fast repaints (via bit blit) when scrolling a page.

On low-powered/mobile devices slow repaints can cause noticeable delays when scrolling a page with a fixed background image. Currently, if a page has elements that specify either a fixed background or a fixed position then we perform a slow repaint (i.e disable blitting) so as to avoid rendering artifacts. By sacrificing support for fixed background images when there are no fixed elements on the page and with come care, we don't need to force slow repaints and can avoid rendering artifacts. Hence, on such devices we can improve the responsiveness of scrolling a page with a fixed background image.

[Daniel Bates]

Created attachment 46176 [details]
Patch 1 with test cases

[Daniel Bates]

Created attachment 46177 [details]
Patch 2

Add IGNORE_FIXED_BACKGROUNDS flag to the build files and tools.

[Daniel Bates]

Disabled by default.

(In reply to comment #2)
> Created an attachment (id=46177) [details]
> Patch 2
> 
> Add IGNORE_FIXED_BACKGROUNDS flag to the build files and tools.

[Simon Fraser (smfr)]

Comment on attachment 46176 [details]
Patch 1 with test cases

This seems like the wrong approach. Why not just change CSSStyleSelector to override FixedBackgroundAttachment with ScrollBackgroundAttachment?

Also, having a whole new build setting, IGNORE_FIXED_BACKGROUNDS, for this seems like overkill. What if you find some other CSS property that you'd like to ignore to get fast scrolling? It seems like it should be ALLOW_FAST_SCROLLING or something that indicates that this is for a limited-capacity mobile device.

[Adam Treat]

(In reply to comment #4)
> (From update of attachment 46176 [details])
> This seems like the wrong approach. Why not just change CSSStyleSelector to
> override FixedBackgroundAttachment with ScrollBackgroundAttachment?

Because if the page has fixed position elements then the scroll will be slow anyways so we might as well honor the fixed background?

> Also, having a whole new build setting, IGNORE_FIXED_BACKGROUNDS, for this
> seems like overkill. What if you find some other CSS property that you'd like
> to ignore to get fast scrolling? It seems like it should be
> ALLOW_FAST_SCROLLING or something that indicates that this is for a
> limited-capacity mobile device.

Hmm, then it isn't as descriptive to what it actually does.  We could certainly do this, but I guess it should have 'MOBILE' in the name or so...  MOBILE_FAST_SCROLLING ?

[David Kilzer (:ddkilzer)]

Comment on attachment 46177 [details]
Patch 2

> Index: WebCore/WebCore.IgnoreFixedBackgrounds.exp
> ===================================================================
> --- WebCore/WebCore.IgnoreFixedBackgrounds.exp	(revision 0)
> +++ WebCore/WebCore.IgnoreFixedBackgrounds.exp	(revision 0)
> @@ -0,0 +1 @@
> +_WebCoreIgnoreFixedBackgrounds

I don't see this method (or variable) implemented anywhere in the patch.

If this is a platform decision, why not just check in platform-specific results?  Why is a build setting needed at all since you're not actually changing any code?

It seems like an awful lot of work just to add a flag that is used by run-webkit-tests to check results for a given platform.

[David Kilzer (:ddkilzer)]

(In reply to comment #6)
> (From update of attachment 46177 [details])
> > Index: WebCore/WebCore.IgnoreFixedBackgrounds.exp
> > ===================================================================
> > --- WebCore/WebCore.IgnoreFixedBackgrounds.exp	(revision 0)
> > +++ WebCore/WebCore.IgnoreFixedBackgrounds.exp	(revision 0)
> > @@ -0,0 +1 @@
> > +_WebCoreIgnoreFixedBackgrounds
> 
> I don't see this method (or variable) implemented anywhere in the patch.

Sorry, I though Patch 2 was in response to Patch 1 having its review- flag set instead of a series.

[David Kilzer (:ddkilzer)]

Comment on attachment 46177 [details]
Patch 2

(In reply to comment #6)
> If this is a platform decision, why not just check in platform-specific
> results?

Since this is a platform-specific, compile-time decision, I think it would be best to simply check in "failing" test results as expected results instead of trying to do something fancy in run-webkit-tests.  That way, you still get test coverage for the tests, and you'll know if the results ever change, e.g., if someone accidentally turns the flag back on for your port!

[Daniel Bates]

Created attachment 46509 [details]
Patch with test cases

Based on the suggestions from both Simon Fraser and David Kilzer, I removed the build system- and tools- related changes. Also, included in the patch expected failure results for the Mac build.

I renamed IGNORE_FIXED_BACKGROUNDS to FAST_MOBILE_SCROLLING to be more descriptive of what this change does. That is, accelerate scrolling for low-powered/mobile devices.

[Adam Treat]

Comment on attachment 46509 [details]
Patch with test cases

r=me based on modifications suggested from ddkilzer and smfr. please watch buildbot for any problems.

[Eric Seidel (no email)]

Attachment 46509 [details] was posted by a committer and has review+, assigning to Daniel Bates for commit.

[Daniel Bates]

Comment on attachment 46509 [details]
Patch with test cases

Clearing flags on attachment: 46509

Committed r53476: <http://trac.webkit.org/changeset/53476>

[Daniel Bates]

All reviewed patches have been landed.  Closing bug.

[Simon Hausmann]

Cherry-picked into qtwebkit-4.6 with commit cd258386aa3564cd4c961933401d0f86a4c872f8