Bug 74720

Summary: [Qt] Handle the layers visible rect calculation on the web process.
Product: WebKit Reporter: Jocelyn Turcotte <jturcotte>
Component: New BugsAssignee: Jocelyn Turcotte <jturcotte>
Status: RESOLVED FIXED    
Severity: Normal CC: kenneth, noam, rafael.lobo, webkit.review.bot, zoltan
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch noam: review+, noam: commit-queue-

Description Jocelyn Turcotte 2011-12-16 08:45:36 PST
[Qt] Handle the layers visible rect calculation on the web process.
Comment 1 Jocelyn Turcotte 2011-12-16 08:47:09 PST
Created attachment 119614 [details]
Patch
Comment 2 Jocelyn Turcotte 2011-12-16 08:49:38 PST
This patch works but makes the desktop view not show any content unless we resize the view. This is caused by the layertreehost not receiving the first visible rect because AC had not been completely entered yet. I'll have to fix this after Christmas before pushing any of this.

It would work previously since painting would update the visible rect later on.
Comment 3 Noam Rosenthal 2011-12-16 09:49:03 PST
Comment on attachment 119614 [details]
Patch

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

Awesome.

> Source/WebCore/platform/graphics/texmap/LayerTransform.cpp:80
> +        .translate3d(originX + m_position.x(), originY + m_position.y(), m_anchorPoint.z() )

Space before parentheses
Comment 4 Jocelyn Turcotte 2012-01-13 05:26:45 PST
Created attachment 122414 [details]
Patch

Resubmitting after rebase and addressing comments.
Comment 5 Noam Rosenthal 2012-01-13 06:15:05 PST
Comment on attachment 122414 [details]
Patch

Wait... I think you uploaded the wrong patch to the wrong bug.
Comment 6 Noam Rosenthal 2012-01-13 10:26:11 PST
btw if you're adding files for TextureMapper, don't forget to add them to the EFL/GTK builds (just grep for TextureMapperNode.cpp)
Comment 7 Jocelyn Turcotte 2012-01-18 06:55:06 PST
Created attachment 122920 [details]
Patch

Updated patch after rebasing and splitting the LayerTransform part into bug #76291.
Another patch will follow up to do a projection of the visible rect instead of mapping it like collectVisibleContentsRects was doing.
Comment 8 Noam Rosenthal 2012-01-18 07:05:18 PST
Comment on attachment 122920 [details]
Patch

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

> Source/WebKit2/UIProcess/API/qt/qquickwebview.cpp:335
> +    webPageProxy->drawingArea()->setVisibleContentsRectAndScale(IntRect(IntPoint(), viewportSize), 1.0);

You don't need the .0 (coding style)

> Source/WebKit2/WebProcess/WebCoreSupport/WebGraphicsLayer.cpp:109
> +        child->setVisibleContentRectAndScale(m_visiblePageContentsRect, m_contentsScale);

I doubt this compiles... you named the variable differently in the header :)

> Source/WebKit2/WebProcess/WebCoreSupport/WebGraphicsLayer.cpp:119
> +    toWebGraphicsLayer(layer)->setVisibleContentRectAndScale(m_visiblePageContentsRect, m_contentsScale);

Instead of doing all this propagation, can we add a function to LayerTreeTileClient?
Comment 9 Jocelyn Turcotte 2012-01-18 08:28:31 PST
(In reply to comment #8)
> I doubt this compiles... you named the variable differently in the header :)
Oh sorry, last minute fix.

> Instead of doing all this propagation, can we add a function to LayerTreeTileClient?
Humm we would still need a method to trigger the visible rect update that would then call the getter on the interface from each layer. The root cause is that layers can be created and added to the tree without the tree host knowing about them, and this method would have to be called instead of the setter in set/addChildren. Personally I prefer a straight setter for the rect and scale.
Comment 10 Noam Rosenthal 2012-01-18 09:05:02 PST
Comment on attachment 122920 [details]
Patch

Please fix variable name (I prefer m_pageVisibleRect or m_visibleRectInPageCoordinates) and maybe merge in patch 76541 before committing.
Comment 11 Jocelyn Turcotte 2012-01-19 07:38:44 PST
Committed r105413: <http://trac.webkit.org/changeset/105413>
Comment 12 Rafael Brandao 2012-01-19 14:46:38 PST
(In reply to comment #11)
> Committed r105413: <http://trac.webkit.org/changeset/105413>

It seems that this patch has broken something. I was running a layout test with: MiniBrowser --desktop 'file:///opt/git/webkit/LayoutTests/editing/selection/regional-indicators.html', and it was visible until the patch before this one (I have a release build). When I've applied this, then MiniBrowser usually loads but shows a blank page instead, I'm not sure what is going on. Even for "http://google.com" I still get nothing there.

Running MiniBrowser without --desktop, I still shows a blank page, but it seems to show up when I refresh it. Any ideas?
Comment 13 Jocelyn Turcotte 2012-01-20 01:28:06 PST
(In reply to comment #12)
> Running MiniBrowser without --desktop, I still shows a blank page, but it seems to show up when I refresh it. Any ideas?

Yep sorry about that, I should have waited to push this patch with the one in bug #76296 which fixes this issue. I'll try to get it in today.