From: tonikitoo@webkit.org Date: Tue, 21 Feb 2012 19:49:21 +0000 (+0000) Subject: 2012-02-18 Antonio Gomes X-Git-Tag: 070512121124~12269 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=edf711ba779ce43ad6e5fc9a3542ec6b6c97e06b;p=profile%2Fivi%2Fwebkit-efl.git 2012-02-18 Antonio Gomes Fat fingers - cache the first rect-based hit test so we do not need to do it again https://bugs.webkit.org/show_bug.cgi?id=79115 Reviewed by Adam Treat. Our FatFingers implementation runs currently in two phases: the first checks for the elements intrinsically clickable; the second checks for elements made clickable by the page (for example, a div with a onclick event listener attached to it). For each phase, we perform a rect hittest, which is not needed since the result of each is the same. Patch introduces a caching mechanism so we avoid on rect hittest: when the first phase runs, it caches each nodeset per document in a hashmap. This second phase works with the cached results. No behavioral change, but performance is better since we avoid one (possibly expensive) rect hittest. I measured the performance gain on https://www.kvd.se/, and we save up to 0.04 seconds, by caching and re-using the results. * WebKitSupport/FatFingers.cpp: (BlackBerry::WebKit::dumpHitTestResult): (BlackBerry::WebKit::FatFingers::findBestPoint): (BlackBerry::WebKit::FatFingers::findIntersectingRegions): (BlackBerry::WebKit::FatFingers::cachingStrategy): (WebKit): (BlackBerry::WebKit::FatFingers::getNodesFromRect): * WebKitSupport/FatFingers.h: git-svn-id: http://svn.webkit.org/repository/webkit/trunk@108380 268f45cc-cd09-0410-ab3c-d52691b4dbfc --- diff --git a/Source/WebKit/blackberry/ChangeLog b/Source/WebKit/blackberry/ChangeLog index ed7dafd..9d6b63f 100644 --- a/Source/WebKit/blackberry/ChangeLog +++ b/Source/WebKit/blackberry/ChangeLog @@ -1,3 +1,36 @@ +2012-02-18 Antonio Gomes + + Fat fingers - cache the first rect-based hit test so we do not need to do it again + https://bugs.webkit.org/show_bug.cgi?id=79115 + + Reviewed by Adam Treat. + + Our FatFingers implementation runs currently in two phases: + the first checks for the elements intrinsically clickable; + the second checks for elements made clickable by the page + (for example, a div with a onclick event listener attached to it). + For each phase, we perform a rect hittest, which is not needed since + the result of each is the same. + + Patch introduces a caching mechanism so we avoid on rect hittest: + when the first phase runs, it caches each nodeset per document in + a hashmap. This second phase works with the cached results. + + No behavioral change, but performance is better since we + avoid one (possibly expensive) rect hittest. + + I measured the performance gain on https://www.kvd.se/, and we + save up to 0.04 seconds, by caching and re-using the results. + + * WebKitSupport/FatFingers.cpp: + (BlackBerry::WebKit::dumpHitTestResult): + (BlackBerry::WebKit::FatFingers::findBestPoint): + (BlackBerry::WebKit::FatFingers::findIntersectingRegions): + (BlackBerry::WebKit::FatFingers::cachingStrategy): + (WebKit): + (BlackBerry::WebKit::FatFingers::getNodesFromRect): + * WebKitSupport/FatFingers.h: + 2012-02-20 Antonio Gomes All default video/audio control elements should be rect-hit testable (Part II) diff --git a/Source/WebKit/blackberry/WebKitSupport/FatFingers.cpp b/Source/WebKit/blackberry/WebKitSupport/FatFingers.cpp index d67578b..016a426 100644 --- a/Source/WebKit/blackberry/WebKitSupport/FatFingers.cpp +++ b/Source/WebKit/blackberry/WebKitSupport/FatFingers.cpp @@ -170,9 +170,10 @@ FatFingers::~FatFingers() const FatFingersResult FatFingers::findBestPoint() { ASSERT(m_webPage); + ASSERT(m_webPage->m_mainFrame); + + m_cachedRectHitTestResults.clear(); - if (!m_webPage->m_mainFrame) - return m_contentPos; FatFingersResult result(m_contentPos); m_matchingApproach = ClickableByDefault; @@ -216,6 +217,7 @@ const FatFingersResult FatFingers::findBestPoint() } m_matchingApproach = Done; + m_cachedRectHitTestResults.clear(); if (!foundOne) return result; @@ -311,16 +313,15 @@ bool FatFingers::findIntersectingRegions(Document* document, m_debugFatFingerRect = m_webPage->mapToTransformed(m_webPage->mapFromContentsToViewport(fingerRect)); #endif - // Document::nodesFromRect needs viewport coordinates, but FatFinger::nodesFromRect gets contents coordinates - // for simplicity instead. bool foundOne = false; - HitTestResult result = nodesFromRect(document, frameContentPos); RenderLayer* lowestPositionedEnclosingLayerSoFar = 0; // Iterate over the list of nodes (and subrects of nodes where possible), for each saving the // intersection of the bounding box with the finger rect. - ListHashSet > intersectedNodes = result.rectBasedTestResult(); + ListHashSet > intersectedNodes; + getNodesFromRect(document, frameContentPos, intersectedNodes); + ListHashSet >::const_iterator it = intersectedNodes.begin(); ListHashSet >::const_iterator end = intersectedNodes.end(); for ( ; it != end; ++it) { @@ -461,8 +462,32 @@ void FatFingers::getPaddings(unsigned& top, unsigned& right, unsigned& bottom, u left = leftPadding / currentScale; } -HitTestResult FatFingers::nodesFromRect(Document* document, const IntPoint& contentPos) const +FatFingers::CachedResultsStrategy FatFingers::cachingStrategy() const +{ + switch (m_matchingApproach) { + case ClickableElement: + return GetFromRenderTree; + case MadeClickableByTheWebpage: + return GetFromCache; + case Done: + default: + ASSERT_NOT_REACHED(); + return GetFromRenderTree; + } +} + +void FatFingers::getNodesFromRect(Document* document, const IntPoint& contentPos, ListHashSet >& intersectedNodes) { + FatFingers::CachedResultsStrategy cacheResolvingStrategy = cachingStrategy(); + + if (cacheResolvingStrategy == GetFromCache) { + ASSERT(m_cachedRectHitTestResults.contains(document)); + intersectedNodes = m_cachedRectHitTestResults.get(document); + return; + } + + ASSERT(cacheResolvingStrategy == GetFromRenderTree); + unsigned topPadding, rightPadding, bottomPadding, leftPadding; getPaddings(topPadding, rightPadding, bottomPadding, leftPadding); @@ -470,7 +495,8 @@ HitTestResult FatFingers::nodesFromRect(Document* document, const IntPoint& cont HitTestResult result(contentPos, topPadding, rightPadding, bottomPadding, leftPadding, HitTestShadowDOM); document->renderView()->layer()->hitTest(request, result); - return result; + intersectedNodes = result.rectBasedTestResult(); + m_cachedRectHitTestResults.add(document, intersectedNodes); } void FatFingers::getRelevantInfoFromPoint(Document* document, const IntPoint& contentPos, Element*& elementUnderPoint, Element*& clickableElementUnderPoint) const diff --git a/Source/WebKit/blackberry/WebKitSupport/FatFingers.h b/Source/WebKit/blackberry/WebKitSupport/FatFingers.h index c17d7b4..6b847e1 100644 --- a/Source/WebKit/blackberry/WebKitSupport/FatFingers.h +++ b/Source/WebKit/blackberry/WebKitSupport/FatFingers.h @@ -25,6 +25,9 @@ #include #include + +#include +#include #include namespace WebCore { @@ -113,6 +116,10 @@ private: typedef std::pair IntersectingRegion; + enum CachedResultsStrategy { GetFromRenderTree = 0, GetFromCache }; + CachedResultsStrategy cachingStrategy() const; + typedef HashMap, ListHashSet > > CachedRectHitTestResults; + bool checkFingerIntersection(const Platform::IntRectRegion&, const Platform::IntRectRegion& remainingFingerRegion, WebCore::Node*, @@ -133,8 +140,7 @@ private: void setSuccessfulFatFingersResult(FatFingersResult&, WebCore::Node*, const WebCore::IntPoint&); - // It mimics Document::nodesFromRect, but has a different return value to fit our needs. - WebCore::HitTestResult nodesFromRect(WebCore::Document*, const WebCore::IntPoint&) const; + void getNodesFromRect(WebCore::Document*, const WebCore::IntPoint&, ListHashSet >&); // It mimics Document::elementFromPoint, but recursively hit-tests in case an inner frame is found. void getRelevantInfoFromPoint(WebCore::Document*, @@ -151,6 +157,7 @@ private: WebCore::IntPoint m_contentPos; TargetType m_targetType; MatchingApproachForClickable m_matchingApproach; + CachedRectHitTestResults m_cachedRectHitTestResults; }; }