Bug 98581 – [BlackBerry] Fetch blit rects from a viewport accessor

• Component: WebKit BlackBerry
• Status: RESOLVED FIXED

[Jakob Petsovits]

The long-standing userInterfaceBlittedVisibleContentsRect() method in WebPageClient has long been the bane of my existence, as it returns the source rect for WebKit contents, but in backingstore pixel coordinates. This makes it not only unwieldy but also terribly fragile; when the backingstore changes its zoom factor then the result of this method has to change as well, synchronously, even though many things happen on the user interface thread these days.

BlackBerry::Platform now exposes a ViewportAccessor interface, which can be used to get the same rectangle in document coordinates or target pixel coordinates, both being a better choice for being passed as an argument as they make the backingstore scale an implementation detail.

I started out on a smaller patch that would add a setScale() to BackingStoreGeometry and the single tiles. Not only does this provide safety against blitting a buffer with wrong scale, but it also lays the foundation for blitting multiple states at different scales, if we want that. (There is certainly a need for having both a low-res preview tile with wide coverage underlying the current tiles, or maybe rendering some tiles in a different resolution while we pinch-zoom.)

The disadvantage of my first approach was that it used the scale in the current BackingStoreGeometry (a.k.a. tile layout) even if we're operating without tiles. That made it necessary to access the front and back state BackingStoreGeometry objects in places where we wouldn't usually care about or update any BackingStoreGeometry at all, and where it's not threadsafe to do so. (Hat tip to Arvid for calling me out on this.)

The solution turned out to be a lot nicer, but also more wide-ranging in scope. The relevant backState()->setScale() calls are still in the patch, but I removed the bad ones. In turn, I modified blitVisibleContents() so that it doesn't use backingstore tile pixel measures at all except in the block where it actually blits from the tiles. Other code in blitVisibleContents(), including the WebPageCompositor part, now only refers to either document (WebCore) or user-visible pixel coordinates.

In order to achieve this, I had to change paintDefaultBackground() so that it doesn't take a backingstore-to-UI-pixel differential transformation. It's very easy to calculate those same areas with ViewportAccessor methods. However, that in turn broke the DEBUG_VISUALIZE blocks (which were slightly broken before); in order to fix this mode it seemed like a good solution to supply a different ViewportAccessor instead. The paintDefaultBackground() call in renderDirectToWindow() seemed to be broken too as it didn't take scroll information into account, and it happens on the WebKit thread so I can't use the UI thread ViewportAccessor there.

In the end, the patch introduced two new ViewportAccessors. The plan for the WebKitThreadViewportAccessor is to use it in lots more places and also replace the unloved transformed/untransformed nomenclature with the more intuitive pixel/document naming provided by ViewportAccessor.

== tl;dr == Never worry anymore about the resolution of backingstore tiles when asking for scaled blits.

Internally tracked as RIM PR 173292.
Patch below.

[Jakob Petsovits]

Created attachment 167437 [details]
Patch

[Arvid Nilsson]

(In reply to comment #1)
> Created an attachment (id=167437) [details]
> Patch

Looks very good to me! Great stuff!

[Jakob Petsovits]

Created attachment 167466 [details]
Patch, same but with internal reviewer line

I take Arvid's comment as a positive internal review, as he did the previous round already and is probably the most knowledgeable person about this code right now.
Still needs an upstream review, thanks!

[Jakob Petsovits]

Created attachment 168480 [details]
Patch

Adam suggested that the scale tracking in geometry and tiles was poorly explained by the commit message. Here's an attempt at making it better.

[George Staikos]

Comment on attachment 168480 [details]
Patch

Good to go!

[WebKit Review Bot]

Comment on attachment 168480 [details]
Patch

Clearing flags on attachment: 168480

Committed r131257: <http://trac.webkit.org/changeset/131257>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.