[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 #7 from Jocelyn Turcotte <jocelyn.turcotte at digia.com>  2012-10-22 06:05:30 PST ---
(From update of attachment 169748)
View in context: https://bugs.webkit.org/attachment.cgi?id=169748&action=review

>>> Source/WebKit2/UIProcess/WebPageProxy.messages.in:76
>>> +#if PLATFORM(QT) || PLATFORM(EFL) && USE(COORDINATED_GRAPHICS)
>> 
>> Maybe we should have a different define?
> 
> We already have so many flags, I would hate to introduce a new one.
> I think USE(COORDINATED_GRAPHICS) is turned on by default in Qt port, so I will try to keep only that and see if something breaks.

This is not tied to COORDINATED_GRAPHICS in any way except for EFL, I would rather put it under USE(TILED_BACKING_STORE) in which case you also wouldn't have to restrict it to PLATFORM(EFL).
This however shows that we should probably rename/detach this define at some point, what it currently means is basically that we're doing UI process scrolling.

> Source/WebKit2/UIProcess/API/efl/PageViewportControllerClientEfl.cpp:91
>      FloatRect mapRectToWebContent = m_visibleContentRect;
>      mapRectToWebContent.scale(1 / m_scaleFactor);
>      drawingArea()->setVisibleContentsRect(enclosingIntRect(mapRectToWebContent), m_scaleFactor, trajectory);

This suggests that setVisibleContentsRect is passed a rect in Viewport coordinates (zoom applied), but you now use m_visibleContentRect everywhere directly to pass coordinates to PageViewportController which expects page coordinates. Something is fishy, make sure that you test this code properly by panning and navigating pages while the page is zoomed.

> Source/WebKit2/UIProcess/API/efl/PageViewportControllerClientEfl.cpp:109
> +    setVisibleContentsRect(IntPoint(contentsPoint.x(), contentsPoint.y()), m_scaleFactor, FloatPoint());
> +    m_pageViewportController->didChangeContentsVisibility(contentsPoint, m_scaleFactor, FloatPoint());

Both of them are calling drawingArea()->setVisibleContentsRect, this is wrong, only the viewportcontroller should be dealing with it.

> Source/WebKit2/UIProcess/API/efl/PageViewportControllerClientEfl.cpp:117
> +    drawingArea()->layerTreeCoordinatorProxy()->setContentsSize(WebCore::FloatSize(m_contentsSize.width(), m_contentsSize.height()));

What is the goal of that call here?

> Source/WebKit2/UIProcess/API/efl/PageViewportControllerClientEfl.cpp:122
> +    m_pageViewportController->didChangeContentsVisibility(m_visibleContentRect.location(), m_scaleFactor);

Viewport coordinates or page coordinates here?

> Source/WebKit2/UIProcess/API/efl/PageViewportControllerClientEfl.cpp:128
> +    IntRect rect = IntRect(IntPoint(), m_visibleContentRect.size());
> +    ewk_view_display(m_viewWidget, rect);

I would use m_viewportSize directly instead of m_visibleContentRect.size().

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