Bug #: 4459

Submitted by: Tobias Lidskog <tobiaslidskog@mac.com>
Reviewed by: eseidel
        * kcanvas/KCanvasContainer.cpp:
        (KCanvasContainer::bbox): avoid unnecessary computation.
        (KCanvasContainer::collisions):
        * kcanvas/device/quartz/KCanvasItemQuartz.mm:
        (KCanvasItemQuartz::draw):
        (KCanvasItemQuartz::hitsPath): use untranslated point
        * svg-tests/W3C-SVG-1.1/metadata-example-01-b-expected.txt:
        Performance improvements to bbox and hit-testing fix.
        http://bugzilla.opendarwin.org/show_bug.cgi?id=4459

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

diff --git a/LayoutTests/svg/W3C-SVG-1.1/metadata-example-01-b-expected.txt b/LayoutTests/svg/W3C-SVG-1.1/metadata-example-01-b-expected.txt
index d722564a06a2eef98c5cc98890f5434c0c4d5355..334dbd9e797f03df371ce20167573e483d32c212 100644 (file)
--- a/LayoutTests/svg/W3C-SVG-1.1/metadata-example-01-b-expected.txt
+++ b/LayoutTests/svg/W3C-SVG-1.1/metadata-example-01-b-expected.txt
@@ -54,7 +54,7 @@ KCanvasContainer at (0,0) size 479x359
         KCanvasItem {path} at (201,64) size 20x10 [stroke={server=UNIMPLEMENTED}] [fill={server=UNIMPLEMENTED}] [data="M221.00,210.00C295.00,210.00,497.00,210.00,595.00,210.00,0.00,69.60,0.00,-2.00,-2.00,-2.00"]
         KCanvasItem {path} at (201,67) size 16x10 [stroke={server=UNIMPLEMENTED}] [fill={server=UNIMPLEMENTED}] [data="M216.00,220.00C224.00,224.00,232.00,227.00,240.00,229.00,0.00,69.60,0.00,-2.00,-2.00,-2.00C334.00,252.00,444.00,261.00,506.00,264.00,0.00,69.60,0.00,-2.00,-2.00,-2.00"]
         KCanvasItem {path} at (200,65) size 23x16 [stroke={server=UNIMPLEMENTED}] [fill={server=UNIMPLEMENTED}] [data="M201.00,227.00C212.00,253.00,215.00,265.00,240.00,280.00,0.00,69.60,0.00,-2.00,-2.00,-2.00C352.00,344.00,508.00,337.00,592.00,325.00,0.00,69.60,0.00,-2.00,-2.00,-2.00"]
-      KCanvasContainer at (223,70) size 0x0 [stroke={server=UNIMPLEMENTED}] [fill={server=UNIMPLEMENTED}]
+      KCanvasContainer at (209,63) size 0x0 [stroke={server=UNIMPLEMENTED}] [fill={server=UNIMPLEMENTED}]
         KCanvasItem {polygon} at (209,63) size 0x0 [stroke={server=UNIMPLEMENTED}] [fill={server=UNIMPLEMENTED}] [data="M433.00,107.00L425.00,111.00L425.00,103.00"]
         KCanvasItem {polygon} at (210,66) size 0x0 [stroke={server=UNIMPLEMENTED}] [fill={server=UNIMPLEMENTED}] [data="M428.00,160.00L421.00,165.00L420.00,157.00"]
         KCanvasItem {polygon} at (220,64) size 0x0 [stroke={server=UNIMPLEMENTED}] [fill={server=UNIMPLEMENTED}] [data="M600.00,210.00L592.00,214.00L592.00,206.00"]

diff --git a/WebCore/ChangeLog-2005-08-23 b/WebCore/ChangeLog-2005-08-23
index 6c7826b5f750d3e3effebcc8fbd2b783318567bd..0d9744305de9bcde2c0c550b95f2eee85c4e278b 100644 (file)
--- a/WebCore/ChangeLog-2005-08-23
+++ b/WebCore/ChangeLog-2005-08-23
@@ -1,3 +1,18 @@
+2005-08-19  Eric Seidel  <eseidel@apple.com>
+        Change from Tobias Lidskog <tobiaslidskog@mac.com>
+
+        Reviewed by eseidel.
+
+        * kcanvas/KCanvasContainer.cpp:
+        (KCanvasContainer::bbox): avoid unnecessary computation.
+        (KCanvasContainer::collisions):
+        * kcanvas/device/quartz/KCanvasItemQuartz.mm:
+        (KCanvasItemQuartz::draw):
+        (KCanvasItemQuartz::hitsPath): use untranslated point
+        * svg-tests/W3C-SVG-1.1/metadata-example-01-b-expected.txt:
+        Performance improvements to bbox and hit-testing fix.
+        http://bugzilla.opendarwin.org/show_bug.cgi?id=4459
+
 2005-08-19  Eric Seidel  <eseidel@apple.com>
 
         Reviewed by darin.

diff --git a/WebCore/kcanvas/KCanvasContainer.cpp b/WebCore/kcanvas/KCanvasContainer.cpp
index 86c5925a94a1b621b35ddbff3b38b90e7aafb1c4..a57927c4c6993d71d6eb363af95dc2ead82bae45 100644 (file)
--- a/WebCore/kcanvas/KCanvasContainer.cpp
+++ b/WebCore/kcanvas/KCanvasContainer.cpp
@@ -209,18 +209,21 @@ void KCanvasContainer::invalidate() const
 
 QRect KCanvasContainer::bbox(bool includeStroke) const
 {
-       QRect rect;
-
-       KCanvasItem *current = d->first;
-       for(; current != 0; current = current->next())
-       {
-               if(!rect.isValid())
-                       rect = current->bbox(includeStroke);
-               else
-                       rect = rect.unite(current->bbox(includeStroke));
-       }
-
-       return rect;
+    KCanvasItem *current = d->first;
+    
+    QRect rect;
+    
+    if (current) {
+        rect = current->bbox(includeStroke);
+        current = current->next();
+        
+        for(; current != 0; current = current->next())
+        {
+            rect = rect.unite(current->bbox(includeStroke));
+        }
+    }
+    
+    return rect;
 }
 
 bool KCanvasContainer::raiseItem(KCanvasItem *item)
@@ -279,24 +282,25 @@ KCanvasItem *KCanvasContainer::last() const
 
 void KCanvasContainer::collisions(const QPoint &p, KCanvasItemList &hits) const
 {
-       if(p.x() < 0 || p.y() < 0)
-               return;
-
-       KCanvasItem *current = d->last;
-       for(; current != 0; current = current->prev())
-       {
-               if(current->isContainer())
-               {
-                       static_cast<const KCanvasContainer *>(current)->collisions(p, hits);
-                       return;
-               }
-               
-               QRect rect(current->bbox());
-
-               // TODO: is this logic correct?
-               if(((current->fillContains(p) && rect.contains(p))) || current->strokeContains(p))
-                       hits.append(current);
-       }
+    if(p.x() < 0 || p.y() < 0)
+        return;
+    
+    KCanvasItem *current = d->last;
+    for(; current != 0; current = current->prev())
+    {
+        if(current->isContainer())
+            static_cast<const KCanvasContainer *>(current)->collisions(p, hits);
+        else
+        {
+            QRect fillRect(current->bbox(false));
+            QRect strokeRect(current->bbox(true));
+            
+            // Test bounding boxes firsts for speed
+            if((fillRect.contains(p) && current->fillContains(p)) ||
+               (strokeRect.contains(p) && current->strokeContains(p)))
+                hits.append(current);
+        }
+    }
 }
 
 // vim:ts=4:noet

diff --git a/WebCore/kcanvas/device/quartz/KCanvasItemQuartz.mm b/WebCore/kcanvas/device/quartz/KCanvasItemQuartz.mm
index a6cd1ecbe97291eb11cc7a6924eca085aab2604f..314b43d9edd3a12cd3e1ca196c72cc8dedaa0df3 100644 (file)
--- a/WebCore/kcanvas/device/quartz/KCanvasItemQuartz.mm
+++ b/WebCore/kcanvas/device/quartz/KCanvasItemQuartz.mm
@@ -93,7 +93,8 @@ void KCanvasItemQuartz::draw(const QRect &rect) const
                context = quartzContext->cgContext();
        }
        
-       applyClipPathsForStyle(context, canvas()->registry(), style(), bboxPath(true)); // FIXME: need bbox when clipping.
+    if (!style()->clipPaths().isEmpty()) 
+        applyClipPathsForStyle(context, canvas()->registry(), style(), bboxPath(true)); // FIXME: need bbox when clipping.
        
        CGContextBeginPath(context);
        
@@ -214,7 +215,7 @@ bool KCanvasItemQuartz::hitsPath(const QPoint &hitPoint, bool fill) const
 //                             NSStringFromPoint(NSPoint(hitPoint)), CFStringFromCGPath(cgPath),
 //                             NSStringFromRect(*(NSRect *)&CGContextGetPathBoundingBox(sharedContext)));
        } else if (!fill && style()->strokePainter()->paintServer()) {
-               hitSuccess = CGContextPathContainsPoint(sharedContext, CGPoint(hitPoint), kCGPathStroke);
+               hitSuccess = CGContextPathContainsPoint(sharedContext, localHitPoint, kCGPathStroke);
        }
        
        CGContextRestoreGState(sharedContext);