[Qt] Unify the PageViewportController<->Client interface regarding positions
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 25 Sep 2012 16:15:12 +0000 (16:15 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 25 Sep 2012 16:15:12 +0000 (16:15 +0000)
https://bugs.webkit.org/show_bug.cgi?id=97220

Patch by Jocelyn Turcotte <jocelyn.turcotte@digia.com> on 2012-09-25
Reviewed by Kenneth Rohde Christiansen.

- Make sure that css units are used for all position arguments
- Make sure that all positions represent the viewport relatively to the contents
  rather than the other way around
- Delay clamping the viewport to the contents size in the controller rather than in the client

* UIProcess/API/qt/qquickwebview.cpp:
(QQuickWebViewFlickablePrivate::updateViewportSize):
* UIProcess/PageViewportController.cpp:
(WebKit::PageViewportController::pageDidRequestScroll):
(WebKit::PageViewportController::didChangeViewportSize):
(WebKit::PageViewportController::didChangeContentsVisibility):
(WebKit::PageViewportController::syncVisibleContents):
(WebKit::PageViewportController::positionRangeForViewportAtScale):
* UIProcess/PageViewportController.h:
(PageViewportController):
* UIProcess/PageViewportControllerClient.h:
(PageViewportControllerClient):
* UIProcess/qt/PageViewportControllerClientQt.cpp:
(WebKit::PageViewportControllerClientQt::animateContentRectVisible):
(WebKit::PageViewportControllerClientQt::focusEditableArea):
(WebKit::PageViewportControllerClientQt::zoomToAreaGestureEnded):
(WebKit::PageViewportControllerClientQt::nearestValidVisibleContentsRect):
(WebKit::PageViewportControllerClientQt::setViewportPosition):
(WebKit::PageViewportControllerClientQt::updateViewportController):
* UIProcess/qt/PageViewportControllerClientQt.h:
(PageViewportControllerClientQt):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@129515 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebKit2/ChangeLog
Source/WebKit2/UIProcess/API/qt/qquickwebview.cpp
Source/WebKit2/UIProcess/PageViewportController.cpp
Source/WebKit2/UIProcess/PageViewportController.h
Source/WebKit2/UIProcess/PageViewportControllerClient.h
Source/WebKit2/UIProcess/qt/PageViewportControllerClientQt.cpp
Source/WebKit2/UIProcess/qt/PageViewportControllerClientQt.h

index 6cc6934..de85b01 100644 (file)
@@ -1,5 +1,39 @@
 2012-09-25  Jocelyn Turcotte  <jocelyn.turcotte@digia.com>
 
+        [Qt] Unify the PageViewportController<->Client interface regarding positions
+        https://bugs.webkit.org/show_bug.cgi?id=97220
+
+        Reviewed by Kenneth Rohde Christiansen.
+
+        - Make sure that css units are used for all position arguments
+        - Make sure that all positions represent the viewport relatively to the contents
+          rather than the other way around
+        - Delay clamping the viewport to the contents size in the controller rather than in the client
+
+        * UIProcess/API/qt/qquickwebview.cpp:
+        (QQuickWebViewFlickablePrivate::updateViewportSize):
+        * UIProcess/PageViewportController.cpp:
+        (WebKit::PageViewportController::pageDidRequestScroll):
+        (WebKit::PageViewportController::didChangeViewportSize):
+        (WebKit::PageViewportController::didChangeContentsVisibility):
+        (WebKit::PageViewportController::syncVisibleContents):
+        (WebKit::PageViewportController::positionRangeForViewportAtScale):
+        * UIProcess/PageViewportController.h:
+        (PageViewportController):
+        * UIProcess/PageViewportControllerClient.h:
+        (PageViewportControllerClient):
+        * UIProcess/qt/PageViewportControllerClientQt.cpp:
+        (WebKit::PageViewportControllerClientQt::animateContentRectVisible):
+        (WebKit::PageViewportControllerClientQt::focusEditableArea):
+        (WebKit::PageViewportControllerClientQt::zoomToAreaGestureEnded):
+        (WebKit::PageViewportControllerClientQt::nearestValidVisibleContentsRect):
+        (WebKit::PageViewportControllerClientQt::setViewportPosition):
+        (WebKit::PageViewportControllerClientQt::updateViewportController):
+        * UIProcess/qt/PageViewportControllerClientQt.h:
+        (PageViewportControllerClientQt):
+
+2012-09-25  Jocelyn Turcotte  <jocelyn.turcotte@digia.com>
+
         [Qt] Make sure that desktop pages honour the devicePixelRatio
         https://bugs.webkit.org/show_bug.cgi?id=97215
 
index 9306d42..b78cae0 100644 (file)
@@ -868,7 +868,7 @@ void QQuickWebViewFlickablePrivate::updateViewportSize()
     Q_Q(QQuickWebView);
 
     if (m_pageViewportController)
-        m_pageViewportController->setViewportSize(QSizeF(q->width(), q->height()));
+        m_pageViewportController->didChangeViewportSize(QSizeF(q->width(), q->height()));
 }
 
 void QQuickWebViewFlickablePrivate::pageDidRequestScroll(const QPoint& pos)
index ee615a6..0ae5d2f 100644 (file)
@@ -128,15 +128,13 @@ void PageViewportController::pageDidRequestScroll(const IntPoint& cssPosition)
     if (m_activeDeferrerCount)
         return;
 
-    FloatRect endPosRange = positionRangeForContentAtScale(m_effectiveScale);
-    FloatPoint endPosition(cssPosition);
-    endPosition.scale(m_effectiveScale, m_effectiveScale);
-    endPosition = boundPosition(endPosRange.minXMinYCorner(), endPosition, endPosRange.maxXMaxYCorner());
+    FloatRect endPosRange = positionRangeForViewportAtScale(m_effectiveScale);
+    FloatPoint endPosition = boundPosition(endPosRange.minXMinYCorner(), cssPosition, endPosRange.maxXMaxYCorner());
 
-    m_client->setContentsPosition(endPosition);
+    m_client->setViewportPosition(endPosition);
 }
 
-void PageViewportController::setViewportSize(const FloatSize& newSize)
+void PageViewportController::didChangeViewportSize(const FloatSize& newSize)
 {
     if (newSize.isEmpty())
         return;
@@ -150,9 +148,9 @@ void PageViewportController::setViewportSize(const FloatSize& newSize)
     syncVisibleContents();
 }
 
-void PageViewportController::setVisibleContentsRect(const FloatRect& visibleContentsRect, float viewportScale, const FloatPoint& trajectoryVector)
+void PageViewportController::didChangeContentsVisibility(const FloatPoint& viewportPos, float viewportScale, const FloatPoint& trajectoryVector)
 {
-    m_visibleContentsRect = visibleContentsRect;
+    m_viewportPos = viewportPos;
     m_effectiveScale = viewportScale;
     syncVisibleContents(trajectoryVector);
 }
@@ -160,10 +158,15 @@ void PageViewportController::setVisibleContentsRect(const FloatRect& visibleCont
 void PageViewportController::syncVisibleContents(const FloatPoint& trajectoryVector)
 {
     DrawingAreaProxy* const drawingArea = m_webPageProxy->drawingArea();
-    if (!drawingArea || m_viewportSize.isEmpty() || m_contentsSize.isEmpty() || m_visibleContentsRect.isEmpty())
+    if (!drawingArea || m_viewportSize.isEmpty() || m_contentsSize.isEmpty())
         return;
 
-    drawingArea->setVisibleContentsRect(m_visibleContentsRect, m_effectiveScale, trajectoryVector);
+    FloatRect endPosRange = positionRangeForViewportAtScale(m_effectiveScale);
+    FloatPoint endPosition = boundPosition(endPosRange.minXMinYCorner(), m_viewportPos, endPosRange.maxXMaxYCorner());
+
+    FloatRect visibleContentsRect(endPosition, m_viewportSize / m_effectiveScale);
+    visibleContentsRect.intersect(FloatRect(FloatPoint::zero(), m_contentsSize));
+    drawingArea->setVisibleContentsRect(visibleContentsRect, m_effectiveScale, trajectoryVector);
 
     m_client->didChangeVisibleContents();
 }
@@ -222,12 +225,10 @@ void PageViewportController::updateMinimumScaleToFit()
     }
 }
 
-FloatRect PageViewportController::positionRangeForContentAtScale(float viewportScale) const
+FloatRect PageViewportController::positionRangeForViewportAtScale(float viewportScale) const
 {
-    const FloatSize contentSize = m_contentsSize * viewportScale;
-
-    const float horizontalRange = contentSize.width() - m_viewportSize.width();
-    const float verticalRange = contentSize.height() - m_viewportSize.height();
+    const float horizontalRange = m_contentsSize.width() - m_viewportSize.width() / viewportScale;
+    const float verticalRange = m_contentsSize.height() - m_viewportSize.height() / viewportScale;
 
     return FloatRect(0, 0, horizontalRange, verticalRange);
 }
index 1d73784..d7fb3a0 100644 (file)
@@ -74,7 +74,7 @@ public:
     void suspendContent();
     void resumeContent();
 
-    WebCore::FloatRect positionRangeForContentAtScale(float viewportScale) const;
+    WebCore::FloatRect positionRangeForViewportAtScale(float viewportScale) const;
 
     float innerBoundedViewportScale(float) const;
     float outerBoundedViewportScale(float) const;
@@ -91,9 +91,9 @@ public:
 
     void setHadUserInteraction(bool didUserInteract) { m_hadUserInteraction = didUserInteract; }
 
-    // Notifications to the WebProcess.
-    void setViewportSize(const WebCore::FloatSize& newSize);
-    void setVisibleContentsRect(const WebCore::FloatRect& visibleContentsRect, float viewportScale, const WebCore::FloatPoint& trajectoryVector = WebCore::FloatPoint::zero());
+    // Notifications from the viewport.
+    void didChangeViewportSize(const WebCore::FloatSize& newSize);
+    void didChangeContentsVisibility(const WebCore::FloatPoint& viewportPos, float viewportScale, const WebCore::FloatPoint& trajectoryVector = WebCore::FloatPoint::zero());
 
     // Notifications from the WebProcess.
     void didChangeContentsSize(const WebCore::IntSize& newSize);
@@ -118,9 +118,9 @@ private:
     bool m_hasSuspendedContent;
     bool m_hadUserInteraction;
 
+    WebCore::FloatPoint m_viewportPos;
     WebCore::FloatSize m_viewportSize;
     WebCore::FloatSize m_contentsSize;
-    WebCore::FloatRect m_visibleContentsRect;
     float m_effectiveScale; // Should always be cssScale * devicePixelRatio.
 
     friend class ViewportUpdateDeferrer;
index 6a50b42..3d91b9a 100644 (file)
@@ -33,8 +33,8 @@ public:
     PageViewportControllerClient() { }
     virtual ~PageViewportControllerClient() { }
 
-    virtual void setContentsPosition(const WebCore::FloatPoint& localPoint) = 0;
-    virtual void setContentsScale(float localScale, bool treatAsInitialValue) = 0;
+    virtual void setViewportPosition(const WebCore::FloatPoint& contentsPoint) = 0;
+    virtual void setContentsScale(float, bool treatAsInitialValue) = 0;
 
     virtual void didResumeContent() = 0;
     virtual void didChangeContentsSize() = 0;
index 62f1ed5..3b9f666 100644 (file)
@@ -108,7 +108,7 @@ void PageViewportControllerClientQt::animateContentRectVisible(const QRectF& con
 
     // Inform the web process about the requested visible content rect immediately so that new tiles
     // are rendered at the final destination during the animation.
-    m_controller->setVisibleContentsRect(contentRect, viewportScaleForRect(contentRect));
+    m_controller->didChangeContentsVisibility(contentRect.topLeft(), viewportScaleForRect(contentRect));
 
     // Since we have to animate scale and position at the same time the scale animation interpolates
     // from the current viewport rect in content coordinates to a visible rect of the content.
@@ -206,12 +206,12 @@ void PageViewportControllerClientQt::focusEditableArea(const QRectF& caretArea,
     const QPointF hotspot = QPointF(targetArea.x(), targetArea.center().y());
     const QPointF viewportHotspot = QPointF(x, /* FIXME: visibleCenter */ viewportRect.center().y());
 
-    QPointF endPosition = hotspot * targetScale - viewportHotspot;
-    QRectF endPosRange = m_controller->positionRangeForContentAtScale(targetScale);
+    QPointF endPosition = hotspot - viewportHotspot / targetScale;
+    QRectF endPosRange = m_controller->positionRangeForViewportAtScale(targetScale);
 
     endPosition = boundPosition(endPosRange.topLeft(), endPosition, endPosRange.bottomRight());
 
-    QRectF endVisibleContentRect(endPosition / targetScale, viewportRect.size() / targetScale);
+    QRectF endVisibleContentRect(endPosition, viewportRect.size() / targetScale);
 
     animateContentRectVisible(endVisibleContentRect);
 }
@@ -243,12 +243,12 @@ void PageViewportControllerClientQt::zoomToAreaGestureEnded(const QPointF& touch
     const QPointF hotspot = QPointF(endArea.center().x(), touchPoint.y());
     const QPointF viewportHotspot = viewportRect.center();
 
-    QPointF endPosition = hotspot * targetScale - viewportHotspot;
+    QPointF endPosition = hotspot - viewportHotspot / targetScale;
 
-    QRectF endPosRange = m_controller->positionRangeForContentAtScale(targetScale);
+    QRectF endPosRange = m_controller->positionRangeForViewportAtScale(targetScale);
     endPosition = boundPosition(endPosRange.topLeft(), endPosition, endPosRange.bottomRight());
 
-    QRectF endVisibleContentRect(endPosition / targetScale, viewportRect.size() / targetScale);
+    QRectF endVisibleContentRect(endPosition, viewportRect.size() / targetScale);
 
     enum { ZoomIn, ZoomBack, ZoomOut, NoZoom } zoomAction = ZoomIn;
 
@@ -258,7 +258,7 @@ void PageViewportControllerClientQt::zoomToAreaGestureEnded(const QPointF& touch
         // Use fuzzy compare with a fixed error to be able to deal with largish differences due to pixel rounding.
         if (fuzzyCompare(targetScale, currentScale, 0.01)) {
             // If moving the viewport would expose more of the targetRect and move at least 40 pixels, update position but do not scale out.
-            QRectF currentContentRect(visibleContentsRect());
+            QRectF currentContentRect(m_viewportItem->mapRectToWebContent(viewportRect));
             QRectF targetIntersection = endVisibleContentRect.intersected(targetArea);
             if (!currentContentRect.contains(targetIntersection)
                     && (qAbs(endVisibleContentRect.top() - currentContentRect.top()) >= 40
@@ -274,18 +274,18 @@ void PageViewportControllerClientQt::zoomToAreaGestureEnded(const QPointF& touch
 
     switch (zoomAction) {
     case ZoomIn:
-        m_scaleStack.append(ScaleStackItem(currentScale, m_viewportItem->contentPos().x()));
+        m_scaleStack.append(ScaleStackItem(currentScale, m_viewportItem->contentPos().x() / currentScale));
         m_zoomOutScale = targetScale;
         break;
     case ZoomBack: {
         ScaleStackItem lastScale = m_scaleStack.takeLast();
         targetScale = lastScale.scale;
         // Recalculate endPosition and bound it according to new scale.
-        endPosition.setY(hotspot.y() * targetScale - viewportHotspot.y());
+        endPosition.setY(hotspot.y() - viewportHotspot.y() / targetScale);
         endPosition.setX(lastScale.xPosition);
-        endPosRange = m_controller->positionRangeForContentAtScale(targetScale);
+        endPosRange = m_controller->positionRangeForViewportAtScale(targetScale);
         endPosition = boundPosition(endPosRange.topLeft(), endPosition, endPosRange.bottomRight());
-        endVisibleContentRect = QRectF(endPosition / targetScale, viewportRect.size() / targetScale);
+        endVisibleContentRect = QRectF(endPosition, viewportRect.size() / targetScale);
         break;
     }
     case ZoomOut:
@@ -307,19 +307,18 @@ QRectF PageViewportControllerClientQt::nearestValidVisibleContentsRect() const
 
     const QRectF viewportRect = m_viewportItem->boundingRect();
     QPointF viewportHotspot = viewportRect.center();
-    QPointF endPosition = m_viewportItem->mapToWebContent(viewportHotspot) * targetScale - viewportHotspot;
+    // Keep the center at the position of the old center, and substract viewportHotspot / targetScale to get the top left position.
+    QPointF endPosition = m_viewportItem->mapToWebContent(viewportHotspot) - viewportHotspot / targetScale;
 
-    FloatRect endPosRange = m_controller->positionRangeForContentAtScale(targetScale);
+    FloatRect endPosRange = m_controller->positionRangeForViewportAtScale(targetScale);
     endPosition = boundPosition(endPosRange.minXMinYCorner(), endPosition, endPosRange.maxXMaxYCorner());
 
-    QRectF endVisibleContentRect(endPosition / targetScale, viewportRect.size() / targetScale);
-
-    return endVisibleContentRect;
+    return QRectF(endPosition, viewportRect.size() / targetScale);
 }
 
-void PageViewportControllerClientQt::setContentsPosition(const FloatPoint& localPoint)
+void PageViewportControllerClientQt::setViewportPosition(const FloatPoint& contentsPoint)
 {
-    QPointF newPosition(m_pageItem->pos() + QPointF(localPoint));
+    QPointF newPosition((m_pageItem->pos() + QPointF(contentsPoint)) * m_pageItem->contentsScale());
     m_viewportItem->setContentPos(newPosition);
     updateViewportController();
 }
@@ -478,12 +477,6 @@ void PageViewportControllerClientQt::pinchGestureCancelled()
     m_scaleUpdateDeferrer.reset();
 }
 
-QRectF PageViewportControllerClientQt::visibleContentsRect() const
-{
-    const QRectF visibleRect(m_viewportItem->boundingRect().intersected(m_pageItem->boundingRect()));
-    return m_viewportItem->mapRectToWebContent(visibleRect);
-}
-
 void PageViewportControllerClientQt::didChangeContentsSize()
 {
     // Emit for testing purposes, so that it can be verified that
@@ -517,9 +510,9 @@ void PageViewportControllerClientQt::didChangeViewportAttributes()
 
 void PageViewportControllerClientQt::updateViewportController(const QPointF& trajectory, qreal scale)
 {
-    FloatRect currentVisibleRect(visibleContentsRect());
-    float viewportScale = (scale < 0) ? viewportScaleForRect(currentVisibleRect) : scale;
-    m_controller->setVisibleContentsRect(currentVisibleRect, viewportScale, trajectory);
+    FloatPoint viewportPos = m_viewportItem->mapToWebContent(QPointF());
+    float viewportScale = (scale < 0) ? m_pageItem->contentsScale() : scale;
+    m_controller->didChangeContentsVisibility(viewportPos, viewportScale, trajectory);
 }
 
 void PageViewportControllerClientQt::scaleContent(qreal itemScale, const QPointF& centerInCSSCoordinates)
index 19b27ae..bd0e173 100644 (file)
@@ -47,8 +47,8 @@ public:
     PageViewportControllerClientQt(QQuickWebView*, QQuickWebPage*);
     ~PageViewportControllerClientQt();
 
-    virtual void setContentsPosition(const WebCore::FloatPoint& localPoint);
-    virtual void setContentsScale(float localScale, bool treatAsInitialValue);
+    virtual void setViewportPosition(const WebCore::FloatPoint& contentsPoint);
+    virtual void setContentsScale(float scale, bool treatAsInitialValue);
 
     virtual void didResumeContent();
     virtual void didChangeContentsSize();
@@ -121,7 +121,6 @@ private:
     QQuickWebPage* const m_pageItem;
 
     float viewportScaleForRect(const QRectF&) const;
-    QRectF visibleContentsRect() const;
     QRectF nearestValidVisibleContentsRect() const;
 
     void setContentsRectToNearestValidBounds();