[Webkit-unassigned] [Bug 99850] [EFL][WK2] Use the port independent PageViewportController

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=99850





--- Comment #17 from Yael <yael.aharon.m at gmail.com>  2012-10-23 07:08:51 PST ---
(In reply to comment #15)
> (From update of attachment 169940 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=169940&action=review
> 
Thanks for continuing to review this :)
> > Source/WebKit2/UIProcess/PageViewportController.h:26
> > +#if USE(COORDINATED_GRAPHICS)
> > +
> 
> Why not change all of them to USE(TILED_BACKING_STORE)?
> COORDINATED_GRAPHICS is about rendering while most of this patch is about viewport and event handling.
> 
Thanks for the explanation, I was very confused about the flags.
> > Source/WebKit2/UIProcess/API/efl/PageViewportControllerClientEfl.cpp:-86
> > -    // Move visibleContentRect inside contentsRect when visibleContentRect goes outside contentsRect. 
> 
> Are you sure you don't need that code anymore?
> 
WebCore already handles this. Once we add support for gestures and scroll animation, we might need to bring some logic back, Right now it is not needed. The UI side never sees negative coordinated when running script like window.scrollTo(-100, -100), or when trying to over-scroll.

> > Source/WebKit2/UIProcess/API/efl/PageViewportControllerClientEfl.cpp:86
> > +    m_scrollPosition = newScrollPosition;
> 
> Shouldn't didChangeContentsVisibility get called here too?
> 
will fix in next version
> > Source/WebKit2/UIProcess/API/efl/PageViewportControllerClientEfl.cpp:94
> > +    ViewportUpdateDeferrer deferrer(m_pageViewportController);
> 
> Is there a reason why you need a deferrer here?
> 
Removing it does not seem to have any effect. Will remove in next version.
> > Source/WebKit2/UIProcess/API/efl/PageViewportControllerClientEfl.cpp:110
> > +    drawingArea()->layerTreeCoordinatorProxy()->setContentsSize(WebCore::FloatSize(m_contentsSize.width(), m_contentsSize.height()));
> 
> You forgot to remove this...
>
Oops... 
> > Source/WebKit2/UIProcess/efl/WebPageProxyEfl.cpp:107
> > +#if USE(TILED_BACKING_STORE) 
> > +void WebPageProxy::didRenderFrame(const WebCore::IntSize& contentsSize, const WebCore::IntRect& coveredRect)
> > +{
> > +    m_pageClient->didRenderFrame(contentsSize, coveredRect);
> > +}
> > +
> > +void WebPageProxy::pageTransitionViewportReady()
> > +{
> > +    m_pageClient->pageTransitionViewportReady();
> > +}
> > +#endif
> > +
> 
> I would prefer that you remove that code from WebPageProxyQt.cpp and move it to WebPageProxy.cpp so that we share it instead of duplicating it.
> 
ok

> > Source/WebKit2/UIProcess/WebPageProxy.h:861
> > +#if USE(TILED_BACKING_STORE) 
> > +    void pageTransitionViewportReady();
> > +#endif
> 
> There is another USE(TILED_BACKING_STORE) block 2 lines above... please use it.
ok

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.