Title: [110624] trunk/Source
Revision
110624
Author
[email protected]
Date
2012-03-13 15:35:16 -0700 (Tue, 13 Mar 2012)

Log Message

Region::contains(IntPoint) is slow
https://bugs.webkit.org/show_bug.cgi?id=81008

Patch by Dana Jansens <[email protected]> on 2012-03-13
Reviewed by Anders Carlsson.

Source/WebCore:

Speed up Region::contains(IntPoint) by directly testing if the point
is inside the Region's shape, rather than using a temporary 1x1 Region
for the test.

Unit test: RegionTest.containsPoint

* platform/graphics/Region.cpp:
(WebCore::Region::contains):

Source/WebKit/chromium:

* WebKit.gypi:
* tests/RegionTest.cpp: Added.
(WebCore):
(WebCore::TEST):

Modified Paths

Added Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (110623 => 110624)


--- trunk/Source/WebCore/ChangeLog	2012-03-13 22:32:49 UTC (rev 110623)
+++ trunk/Source/WebCore/ChangeLog	2012-03-13 22:35:16 UTC (rev 110624)
@@ -1,3 +1,19 @@
+2012-03-13  Dana Jansens  <[email protected]>
+
+        Region::contains(IntPoint) is slow
+        https://bugs.webkit.org/show_bug.cgi?id=81008
+
+        Reviewed by Anders Carlsson.
+
+        Speed up Region::contains(IntPoint) by directly testing if the point
+        is inside the Region's shape, rather than using a temporary 1x1 Region
+        for the test.
+
+        Unit test: RegionTest.containsPoint
+
+        * platform/graphics/Region.cpp:
+        (WebCore::Region::contains):
+
 2012-03-13  Adrienne Walker  <[email protected]>
 
         [chromium] Mark root layer scrollbars as always opaque to disable blending

Modified: trunk/Source/WebCore/platform/graphics/Region.cpp (110623 => 110624)


--- trunk/Source/WebCore/platform/graphics/Region.cpp	2012-03-13 22:32:49 UTC (rev 110623)
+++ trunk/Source/WebCore/platform/graphics/Region.cpp	2012-03-13 22:35:16 UTC (rev 110624)
@@ -72,9 +72,30 @@
 
 bool Region::contains(const IntPoint& point) const
 {
-    // FIXME: This is inefficient. We should be able to iterate over the spans and find
-    // out if the region contains the point.
-    return contains(IntRect(point, IntSize(1, 1)));
+    if (!m_bounds.contains(point))
+        return false;
+
+    for (Shape::SpanIterator span = m_shape.spans_begin(), end = m_shape.spans_end(); span != end && span + 1 != end; ++span) {
+        int y = span->y;
+        int maxY = (span + 1)->y;
+
+        if (y > point.y())
+            break;
+        if (maxY <= point.y())
+            continue;
+
+        for (Shape::SegmentIterator segment = m_shape.segments_begin(span), end = m_shape.segments_end(span); segment != end && segment + 1 != end; segment += 2) {
+            int x = *segment;
+            int maxX = *(segment + 1);
+
+            if (x > point.x())
+                break;
+            if (maxX > point.x())
+                return true;
+        }
+    }
+
+    return false;
 }
 
 bool Region::intersects(const Region& region) const

Modified: trunk/Source/WebKit/chromium/ChangeLog (110623 => 110624)


--- trunk/Source/WebKit/chromium/ChangeLog	2012-03-13 22:32:49 UTC (rev 110623)
+++ trunk/Source/WebKit/chromium/ChangeLog	2012-03-13 22:35:16 UTC (rev 110624)
@@ -1,3 +1,15 @@
+2012-03-13  Dana Jansens  <[email protected]>
+
+        Region::contains(IntPoint) is slow
+        https://bugs.webkit.org/show_bug.cgi?id=81008
+
+        Reviewed by Anders Carlsson.
+
+        * WebKit.gypi:
+        * tests/RegionTest.cpp: Added.
+        (WebCore):
+        (WebCore::TEST):
+
 2012-03-13  Gavin Peters  <[email protected]>
 
         Implement ApplicationCacheHost::canCacheInPageCache() for chromium.

Modified: trunk/Source/WebKit/chromium/WebKit.gypi (110623 => 110624)


--- trunk/Source/WebKit/chromium/WebKit.gypi	2012-03-13 22:32:49 UTC (rev 110623)
+++ trunk/Source/WebKit/chromium/WebKit.gypi	2012-03-13 22:35:16 UTC (rev 110624)
@@ -116,6 +116,7 @@
             'tests/PODArenaTest.cpp',
             'tests/PODIntervalTreeTest.cpp',
             'tests/PODRedBlackTreeTest.cpp',
+            'tests/RegionTest.cpp',
             'tests/RenderTableCellTest.cpp',
             'tests/ScrollbarLayerChromiumTest.cpp',
             'tests/TextureManagerTest.cpp',

Added: trunk/Source/WebKit/chromium/tests/RegionTest.cpp (0 => 110624)


--- trunk/Source/WebKit/chromium/tests/RegionTest.cpp	                        (rev 0)
+++ trunk/Source/WebKit/chromium/tests/RegionTest.cpp	2012-03-13 22:35:16 UTC (rev 110624)
@@ -0,0 +1,115 @@
+/*
+ * Copyright (C) 2012 Google Inc. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1.  Redistributions of source code must retain the above copyright
+ *     notice, this list of conditions and the following disclaimer.
+ * 2.  Redistributions in binary form must reproduce the above copyright
+ *     notice, this list of conditions and the following disclaimer in the
+ *     documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS'' AND ANY
+ * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
+ * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+ * DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS BE LIABLE FOR ANY
+ * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
+ * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
+ * ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+ * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include "config.h"
+
+#include "Region.h"
+
+#include <gtest/gtest.h>
+
+using namespace WebCore;
+
+namespace {
+
+#define TEST_INSIDE_RECT(r, x, y, w, h)                      \
+    EXPECT_TRUE(r.contains(IntPoint(x, y)));                 \
+    EXPECT_TRUE(r.contains(IntPoint(x + w - 1, y)));         \
+    EXPECT_TRUE(r.contains(IntPoint(x, y + h - 1)));         \
+    EXPECT_TRUE(r.contains(IntPoint(x + w - 1, y + h - 1))); \
+    EXPECT_TRUE(r.contains(IntPoint(x, y + h / 2)));         \
+    EXPECT_TRUE(r.contains(IntPoint(x + w - 1, y + h / 2))); \
+    EXPECT_TRUE(r.contains(IntPoint(x + w / 2, y)));         \
+    EXPECT_TRUE(r.contains(IntPoint(x + w / 2, y + h - 1))); \
+    EXPECT_TRUE(r.contains(IntPoint(x + w / 2, y + h / 2))); \
+
+#define TEST_LEFT_OF_RECT(r, x, y, w, h)                     \
+    EXPECT_FALSE(r.contains(IntPoint(x - 1, y)));            \
+    EXPECT_FALSE(r.contains(IntPoint(x - 1, y + h - 1)));    \
+
+#define TEST_RIGHT_OF_RECT(r, x, y, w, h)                 \
+    EXPECT_FALSE(r.contains(IntPoint(x + w, y)));         \
+    EXPECT_FALSE(r.contains(IntPoint(x + w, y + h - 1))); \
+
+#define TEST_TOP_OF_RECT(r, x, y, w, h)                   \
+    EXPECT_FALSE(r.contains(IntPoint(x, y - 1)));         \
+    EXPECT_FALSE(r.contains(IntPoint(x + w - 1, y - 1))); \
+
+#define TEST_BOTTOM_OF_RECT(r, x, y, w, h)                \
+    EXPECT_FALSE(r.contains(IntPoint(x, y + h)));         \
+    EXPECT_FALSE(r.contains(IntPoint(x + w - 1, y + h))); \
+
+TEST(RegionTest, containsPoint)
+{
+    Region r;
+
+    EXPECT_FALSE(r.contains(IntPoint(0, 0)));
+
+    r.unite(IntRect(35, 35, 1, 1));
+    TEST_INSIDE_RECT(r, 35, 35, 1, 1);
+    TEST_LEFT_OF_RECT(r, 35, 35, 1, 1);
+    TEST_RIGHT_OF_RECT(r, 35, 35, 1, 1);
+    TEST_TOP_OF_RECT(r, 35, 35, 1, 1);
+    TEST_BOTTOM_OF_RECT(r, 35, 35, 1, 1);
+
+    r.unite(IntRect(30, 30, 10, 10));
+    TEST_INSIDE_RECT(r, 30, 30, 10, 10);
+    TEST_LEFT_OF_RECT(r, 30, 30, 10, 10);
+    TEST_RIGHT_OF_RECT(r, 30, 30, 10, 10);
+    TEST_TOP_OF_RECT(r, 30, 30, 10, 10);
+    TEST_BOTTOM_OF_RECT(r, 30, 30, 10, 10);
+
+    r.unite(IntRect(31, 40, 10, 10));
+    EXPECT_FALSE(r.contains(IntPoint(30, 40)));
+    EXPECT_TRUE(r.contains(IntPoint(31, 40)));
+    EXPECT_FALSE(r.contains(IntPoint(40, 39)));
+    EXPECT_TRUE(r.contains(IntPoint(40, 40)));
+
+    TEST_INSIDE_RECT(r, 30, 30, 10, 10);
+    TEST_LEFT_OF_RECT(r, 30, 30, 10, 10);
+    TEST_RIGHT_OF_RECT(r, 30, 30, 10, 10);
+    TEST_TOP_OF_RECT(r, 30, 30, 10, 10);
+    TEST_INSIDE_RECT(r, 31, 40, 10, 10);
+    TEST_LEFT_OF_RECT(r, 31, 40, 10, 10);
+    TEST_RIGHT_OF_RECT(r, 31, 40, 10, 10);
+    TEST_BOTTOM_OF_RECT(r, 31, 40, 10, 10);
+
+    r.unite(IntRect(42, 40, 10, 10));
+
+    TEST_INSIDE_RECT(r, 42, 40, 10, 10);
+    TEST_LEFT_OF_RECT(r, 42, 40, 10, 10);
+    TEST_RIGHT_OF_RECT(r, 42, 40, 10, 10);
+    TEST_TOP_OF_RECT(r, 42, 40, 10, 10);
+    TEST_BOTTOM_OF_RECT(r, 42, 40, 10, 10);
+
+    TEST_INSIDE_RECT(r, 30, 30, 10, 10);
+    TEST_LEFT_OF_RECT(r, 30, 30, 10, 10);
+    TEST_RIGHT_OF_RECT(r, 30, 30, 10, 10);
+    TEST_TOP_OF_RECT(r, 30, 30, 10, 10);
+    TEST_INSIDE_RECT(r, 31, 40, 10, 10);
+    TEST_LEFT_OF_RECT(r, 31, 40, 10, 10);
+    TEST_RIGHT_OF_RECT(r, 31, 40, 10, 10);
+    TEST_BOTTOM_OF_RECT(r, 31, 40, 10, 10);
+}
+
+} // namespace
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to