Bug 117451 – [BlackBerry] Smarter algorithm to determine the backingstore rect

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit BlackBerry

[Jakob Petsovits]

So far, the backingstore tile geometry allocation was pretty straightforward: We would start off from the current viewport and append all available tiles into the current scrolling direction from there. This will usually work well enough, but has the downside of discarding all the tiles in the opposite direction. Also, tiles very close to the viewport will often get discarded even if the user only scrolls very slowly.

The patch below completely revamps the algorithm for determining where the backingstore should be positioned.

The general idea is that we construct a "desired rect" based on the viewport and inflate it into all four directions according to the current scroll momentum. This rectangle will be similarly large as a backingstore tile geometry rectangle might be, by using the approximate number of pixels that are available in the given number of tiles. The proportions for extending the rectangle from the viewport are influenced by different factors, including scroll momentum, viewport ratio, available space in the overall contents rectangle, and natural bias for the "down" direction.

In practice, this results in a backingstore that is roughly evenly distributed around the viewport when no movement is happening, and will gradually narrow down and extend into the scroll direction at a higher momentum.

The final tile geometry is constructed by trying fit the tiles into the desired rect in a way that maximizes the area of its intersection. There are a few parameters that can be tweaked, the ones in this patch seem to handle most cases well enough to minimize checkerboarding. As an additional bonus, a rectangle-based tiling strategy can more easily be adopted for accelerated compositing, which currently operates on a simpler algorithm that also inflates the viewport but does not take scrolling into account.

[Jakob Petsovits]

Created attachment 204231 [details]
Patch

The style checker is complaining about a variable named 'l'. I believe that in the context (l/r/u/d for left/right/up/down), that's clear and optimal. Hopefully it'll still go through the bots (and review), or something.

[WebKit Commit Bot]

Attachment 204231 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit/blackberry/Api/BackingStore.cpp', u'Source/WebKit/blackberry/Api/BackingStore_p.h', u'Source/WebKit/blackberry/ChangeLog']" exit_code: 1
Source/WebKit/blackberry/Api/BackingStore.cpp:855:  l is incorrectly named. Don't use the single letter 'l' as an identifier name.  [readability/naming] [4]
Total errors found: 1 in 3 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Arvid Nilsson]

Comment on attachment 204231 [details]
Patch

This looks great! Like you said, this algorithm would apply to AC layer scrolling too, once the visibility patch lands, and visibility is represented as a simple rect. Perhaps we can extract this algorithm into a reusable class in the future and use it in the AC code too.

[Rob Buis]

Comment on attachment 204231 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=204231&action=review

Looks good, please think about the more verbose naming before landing.

>> Source/WebKit/blackberry/Api/BackingStore.cpp:855
>> +        const float l = expandLeft;
> 
> l is incorrectly named. Don't use the single letter 'l' as an identifier name.  [readability/naming] [4]

More verbose would be better but up to you.

[WebKit Commit Bot]

Comment on attachment 204231 [details]
Patch

Clearing flags on attachment: 204231

Committed r151503: <http://trac.webkit.org/changeset/151503>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.