Changeset 122970 in webkit

Timestamp:

Jul 18, 2012 7:47:40 AM (12 years ago)

Author:

commit-queue@webkit.org

Message:

TOUCH_ADJUSTMENT is too aggressive when snapping to large elements.
​https://bugs.webkit.org/show_bug.cgi?id=91262

Patch by Kevin Ellis <​kevers@chromium.org> on 2012-07-18
Reviewed by Antonio Gomes.

Source/WebCore:

Constrains the extent to which the touch point can be adjusted when
generating synthetic mouse events when TOUCH_ADJUSTEMNT is enabled.
Previously, the target position snapped to the center of the target
element, which can be far removed from the touch position when tapping
on or near a large element. The refined strategy is to leave the
adjusted position unchanged if tapping within the element or to snap
to the center of the overlap region if the touch point lies outside the
bounds of the element, but the touch area and element bounds overlap.
For non-rectilineary bounds, a point lying outside the element boundary
is pulled towards the center of the element, by an amount limited by
the radius of the touch area.

Tests: touchadjustment/big-div.html

touchadjustment/rotated-node.html

• page/TouchAdjustment.cpp:

(WebCore::TouchAdjustment::contentsToWindow):
(TouchAdjustment):
(WebCore::TouchAdjustment::snapTo):
(WebCore::TouchAdjustment::findNodeWithLowestDistanceMetric):

LayoutTests:

Adding a test case to ensure that the adjusted touch position is
within the bounds of the target element and the touch area.
Previously, the target position snapped to the center of the target
element, which can be far removed from the touch area.

The second test is for non-rectilinear elements, and verifies that
the touch area must overlap the true bounds of the element for an
adjustment to occur.

• touchadjustment/big-div-expected.txt: Added.
• touchadjustment/big-div.html: Added.
• touchadjustment/rotated-node-expected.txt: Added.
• touchadjustment/rotated-node.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/touchadjustment/big-div-expected.txt (added)
• LayoutTests/touchadjustment/big-div.html (added)
• LayoutTests/touchadjustment/rotated-node-expected.txt (added)
• LayoutTests/touchadjustment/rotated-node.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/page/TouchAdjustment.cpp (modified) (3 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2012-07-18  Kevin Ellis  <kevers@chromium.org>
+
+        TOUCH_ADJUSTMENT is too aggressive when snapping to large elements.
+        https://bugs.webkit.org/show_bug.cgi?id=91262
+
+        Reviewed by Antonio Gomes.
+
+        Adding a test case to ensure that the adjusted touch position is 
+        within the bounds of the target element and the touch area.
+        Previously, the target position snapped to the center of the target
+        element, which can be far removed from the touch area.
+
+        The second test is for non-rectilinear elements, and verifies that
+        the touch area must overlap the true bounds of the element for an
+        adjustment to occur.
+
+        * touchadjustment/big-div-expected.txt: Added.
+        * touchadjustment/big-div.html: Added.
+        * touchadjustment/rotated-node-expected.txt: Added.
+        * touchadjustment/rotated-node.html: Added.
+
 2012-07-18  Vsevolod Vlasov  <vsevik@chromium.org>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2012-07-18  Kevin Ellis  <kevers@chromium.org>
+
+        TOUCH_ADJUSTMENT is too aggressive when snapping to large elements.
+        https://bugs.webkit.org/show_bug.cgi?id=91262
+
+        Reviewed by Antonio Gomes.
+
+        Constrains the extent to which the touch point can be adjusted when
+        generating synthetic mouse events when TOUCH_ADJUSTEMNT is enabled.
+        Previously, the target position snapped to the center of the target
+        element, which can be far removed from the touch position when tapping
+        on or near a large element.  The refined strategy is to leave the
+        adjusted position unchanged if tapping within the element or to snap
+        to the center of the overlap region if the touch point lies outside the
+        bounds of the element, but the touch area and element bounds overlap.
+        For non-rectilineary bounds, a point lying outside the element boundary
+        is pulled towards the center of the element, by an amount limited by
+        the radius of the touch area.
+
+        Tests: touchadjustment/big-div.html
+               touchadjustment/rotated-node.html
+
+        * page/TouchAdjustment.cpp:
+        (WebCore::TouchAdjustment::contentsToWindow):
+        (TouchAdjustment):
+        (WebCore::TouchAdjustment::snapTo):
+        (WebCore::TouchAdjustment::findNodeWithLowestDistanceMetric):
+
 2012-07-18  Sergey Rogulenko  <rogulenko@google.com>
 

trunk/Source/WebCore/page/TouchAdjustment.cpp

@@ -240 +240 @@
 }
 
+FloatPoint contentsToWindow(FrameView *view, FloatPoint pt)
+{
+    int x = static_cast<int>(pt.x() + 0.5f);
+    int y = static_cast<int>(pt.y() + 0.5f);
+    IntPoint adjusted = view->contentsToWindow(IntPoint(x, y));
+    return FloatPoint(adjusted.x(), adjusted.y());
+}
+
+bool snapTo(const SubtargetGeometry& geom, const IntPoint& touchPoint, const IntRect& touchArea, IntPoint& adjustedPoint)
+{
+    FrameView* view = geom.node()->document()->view();
+    FloatQuad quad = geom.quad();
+
+    if (quad.isRectilinear()) {
+        IntRect contentBounds = geom.boundingBox();
+        // Convert from frame coordinates to window coordinates.
+        IntRect bounds = view->contentsToWindow(contentBounds);
+        if (bounds.contains(touchPoint)) {
+            adjustedPoint = touchPoint;
+            return true;
+        }
+        if (bounds.intersects(touchArea)) {
+            bounds.intersect(touchArea);
+            adjustedPoint = bounds.center();
+            return true;
+        }
+        return false;
+    }
+
+    // Non-rectilinear element.
+    // Convert quad from content to window coordinates.
+    FloatPoint p1 = contentsToWindow(view, quad.p1());
+    FloatPoint p2 = contentsToWindow(view, quad.p2());
+    FloatPoint p3 = contentsToWindow(view, quad.p3());
+    FloatPoint p4 = contentsToWindow(view, quad.p4());
+    quad = FloatQuad(p1, p2, p3, p4);
+
+    if (quad.containsPoint(touchPoint)) {
+        adjustedPoint = touchPoint;
+        return true;
+    }
+
+    // Pull point towards the center of the element.
+    float cx = 0.25 * (p1.x() + p2.x() + p3.x() + p4.x());
+    float cy = 0.25 * (p1.y() + p2.y() + p3.y() + p4.y());
+    FloatPoint center = FloatPoint(cx, cy);
+
+    FloatSize pullDirection = center - touchPoint;
+    float distanceToCenter = pullDirection.diagonalLength();
+
+    // Use distance from center to corner of touch area to limit adjustment distance.
+    float dx = 0.5f * touchArea.width();
+    float dy = 0.5f * touchArea.height();
+    float touchRadius = sqrt(dx * dx + dy * dy);
+
+    float scaleFactor = touchRadius / distanceToCenter;
+    if (scaleFactor > 1)
+        scaleFactor = 1;
+    pullDirection.scale(scaleFactor);
+
+    int x = static_cast<int>(touchPoint.x() + pullDirection.width());
+    int y = static_cast<int>(touchPoint.y() + pullDirection.height());
+    IntPoint point(x, y);
+
+    if (quad.containsPoint(point)) {
+        adjustedPoint = point;
+        return true;
+    }
+    return false;
+}
 
 // A generic function for finding the target node with the lowest distance metric. A distance metric here is the result
@@ -250 +320 @@
     SubtargetGeometryList::const_iterator it = subtargets.begin();
     const SubtargetGeometryList::const_iterator end = subtargets.end();
+    IntPoint adjustedPoint;
     for (; it != end; ++it) {
         Node* node = it->node();
         float distanceMetric = distanceFunction(touchHotspot, touchArea, *it);
         if (distanceMetric < bestDistanceMetric) {
-            targetPoint = roundedIntPoint(it->quad().center());
-            targetArea = it->boundingBox();
-            targetNode = node;
-            bestDistanceMetric = distanceMetric;
+            if (snapTo(*it, touchHotspot, touchArea, adjustedPoint)) {
+                targetPoint = adjustedPoint;
+                targetArea = it->boundingBox();
+                targetNode = node;
+                bestDistanceMetric = distanceMetric;
+            }
         } else if (distanceMetric == bestDistanceMetric) {
             // Try to always return the inner-most element.
-            if (node->isDescendantOf(targetNode)) {
+            if (node->isDescendantOf(targetNode) && snapTo(*it, touchHotspot, touchArea, adjustedPoint)) {
+                targetPoint = adjustedPoint;
                 targetNode = node;
                 targetArea = it->boundingBox();
@@ -268 +342 @@
     if (targetNode) {
         targetArea = targetNode->document()->view()->contentsToWindow(targetArea);
-        targetPoint = targetNode->document()->view()->contentsToWindow(targetPoint);
-    }
-
+    }
     return (targetNode);
 }