Bug 78816 – [Qt] Previous web page appears outside content rect

• Component: WebKit2
• Status: RESOLVED FIXED

[Kenneth Rohde Christiansen]

SSIA

[Kenneth Rohde Christiansen]

Created attachment 127382 [details]
Patch

[Jocelyn Turcotte]

Comment on attachment 127382 [details]
Patch

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

> Source/WebKit2/WebProcess/WebPage/qt/LayerTreeHostQt.cpp:450
> +    HashSet<WebCore::WebGraphicsLayer*>::iterator end = m_registeredLayers.end();
> +    for (HashSet<WebCore::WebGraphicsLayer*>::iterator it = m_registeredLayers.begin(); it != end; ++it)
> +        (*it)->didRenderFrame();

This assumes that all the tile creation (which happen one tile at a time on a 0-timer) and rendering is going to be completed within one rendering loop to and from the UI process. It might work since this delay can be long, but it might break and cause white-flashing while committing the scale in cases where this delay isn't long enough and the removeTile calls for the previous backing store get to the UI process before the createTile of the new backing store. There would need some weird situation for this to happen though.

One way this could work is if the tiledbackingstore became completely synchronous (which might not be a bad thing since we don't need it to be async in WebKit2 AFAIK), or I think we should rely on a trigger to destroy the previous tiles that will work 100% of the time.

Maybe for now a reliable way could be to check the main and previous backing store at the end of updateContentBuffers for TiledBackingStore::coverageRatio of the visible rect and decide if it is alright to clear the previous backing store.

[Kenneth Rohde Christiansen]

How can this coverageRatio work? You want to check whether it is more than 1.0 (100%) meaning that the previous buffer is larger than the new one. But even if we are loading a new page which is smaller than the previous one we might not always get a coverageRatio larger than 1.0 - or am I missing something?

[Jocelyn Turcotte]

(In reply to comment #3)

In reference to http://trac.webkit.org/changeset/94678
I was using "coverageRatio >= 1.0f" mainly because I've always had this weird fear of floats slipping through my fingers, but it should just be "== 1.0f". The method outputs a float between 0 and 1 being "how much percentage of the rect you gave me is covered by visible tiles in this backing store".

The ratio was always calculated on the visible rect since this is where the user would see the empty-content-flashing after committing a scale.
So in that case, the moment that "mainBackingStore.coverageRatio(visibleRect) == 1.0" I would clear the previous tiles.
There was also a special case to handle where doing two quick scale commit would replace a full previousBackingStore with an almost empty mainBackingStore.

[Noam Rosenthal]

Created attachment 127999 [details]
Patch

[WebKit Review Bot]

Comment on attachment 127999 [details]
Patch

Clearing flags on attachment: 127999

Committed r108407: <http://trac.webkit.org/changeset/108407>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Lars Knudsen]

Created attachment 131581 [details]
Test page exposing what happens when clipping is done with no respect for 3D transforms.

To me, it seems wrong to do the clipping here - at least when it's done with no respect for 3D transforms.  I might be wrong though as I am just getting into this part of the code.

[Jocelyn Turcotte]

(In reply to comment #8)
Could you add a screenshot or explain the result?
It seems to work fine on my build.