Fix bounds computation bugs responsible for http://bugs.webkit.org/show_bug.cgi?id...
authoroliver@apple.com <oliver@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 5 Jan 2008 05:20:11 +0000 (05:20 +0000)
committeroliver@apple.com <oliver@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 5 Jan 2008 05:20:11 +0000 (05:20 +0000)
and other image repaint bugs.

Reviewed by Beth Dakin.

We now cache the full local bounds for the <image> element, as otherwise certain
combinations of attribute changes could result in incorrect dirty rects.
Additionally we no longer use any of the integer bounds fields on RenderObject for
determining repaint bounds (this was the principle cause of bug #16015).

I also removed the outline painting code as it was both wrong, and not correctly
repainted.  I feel safe doing this as no other browser or viewer supports outline
properties on svg elements.

I was unable to make a testcase for this unfortunately, despite seemingly deterministic
behaviour :(

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

LayoutTests/ChangeLog
LayoutTests/platform/mac/svg/carto.net/scrollbar-expected.txt
WebCore/ChangeLog
WebCore/rendering/RenderSVGImage.cpp
WebCore/rendering/RenderSVGImage.h

index 81dfb99..c442071 100644 (file)
@@ -1,3 +1,12 @@
+2008-01-04  Oliver Hunt  <oliver@apple.com>
+
+        Reviewed by Beth Dakin.
+
+        As a byproduct of correct the bounds computation of image elements
+        the metrics for the render tree changed slightly.
+
+        * platform/mac/svg/carto.net/scrollbar-expected.txt:
+
 2008-01-04  Beth Dakin  <bdakin@apple.com>
 
         Reviewed by Oliver.
index 780fe99..28867b5 100644 (file)
@@ -9,8 +9,8 @@ layer at (0,0) size 800x600
     RenderSVGText {text} at (567,30) size 383x17 contains 1 chunk(s)
       RenderSVGInlineText {#text} at (0,-14) size 383x17
         chunk 1 (end anchor) text run 1 at (567.00,30.00) startOffset 0 endOffset 59 width 383.00: "One can also drag the images and the scrollbars will adopt."
-    RenderSVGViewportContainer {svg} at (40,60) size 2885.60x336
-      RenderSVGContainer {g} at (40,60) size 2885.60x336
+    RenderSVGViewportContainer {svg} at (40,60) size 2885.60x336.40
+      RenderSVGContainer {g} at (40,60) size 2885.60x336.40
         RenderImage {image} at (0,0) size 3607x420
         RenderSVGContainer {g} at (370.40,112) size 1625.60x260
           RenderSVGText {text} at (413,88) size 86x18 contains 1 chunk(s)
@@ -58,8 +58,8 @@ layer at (0,0) size 800x600
     RenderSVGText {text} at (430,280) size 520x14 contains 1 chunk(s)
       RenderSVGInlineText {#text} at (0,-11) size 520x14
         chunk 1 (end anchor) text run 1 at (430.00,280.00) startOffset 0 endOffset 89 width 520.00: "Panorama Zervreilastausee, Vals, Graub\x{FC}nden, Switzerland, taken 2006-02-04 (\x{A9} A. Neumann)"
-    RenderSVGViewportContainer {svg} at (40,300) size 2848x551.20
-      RenderSVGContainer {g} at (40,300) size 2848x551.20
+    RenderSVGViewportContainer {svg} at (40,300) size 2848.60x551.20
+      RenderSVGContainer {g} at (40,300) size 2848.60x551.20
         RenderImage {image} at (0,0) size 3560x689
     RenderSVGContainer {g} at (40,300) size 728.80x168.80
       RenderSVGContainer {g} at (40,460) size 720x8.80
index 22311de..6666400 100644 (file)
@@ -1,3 +1,29 @@
+2008-01-04  Oliver Hunt  <oliver@apple.com>
+
+        Reviewed by Beth Dakin.
+
+        Fix bounds computation bugs responsible for http://bugs.webkit.org/show_bug.cgi?id=16015
+        and other image repaint bugs.
+
+        We now cache the full local bounds for the <image> element, as otherwise certain
+        combinations of attribute changes could result in incorrect dirty rects.
+        Additionally we no longer use any of the integer bounds fields on RenderObject for
+        determining repaint bounds (this was the principle cause of bug #16015).
+
+        I also removed the outline painting code as it was both wrong, and not correctly
+        repainted.  I feel safe doing this as no other browser or viewer supports outline
+        properties on svg elements.
+
+        I was unable to make a testcase for this unfortunately, despite seemingly deterministic 
+        behaviour :(
+
+        * rendering/RenderSVGImage.cpp:
+        (WebCore::RenderSVGImage::layout):
+        (WebCore::RenderSVGImage::paint):
+        (WebCore::RenderSVGImage::nodeAtPoint):
+        (WebCore::RenderSVGImage::calculateAbsoluteBounds):
+        * rendering/RenderSVGImage.h:
+
 2008-01-04  Beth Dakin  <bdakin@apple.com>
 
         Reviewed by Oliver.
index fbefc50..4986bbe 100644 (file)
@@ -148,9 +148,13 @@ void RenderSVGImage::layout()
     
     // minimum height
     m_height = errorOccurred() ? intrinsicSize().height() : 0;
-    
+
     calcWidth();
     calcHeight();
+
+    SVGImageElement* image = static_cast<SVGImageElement*>(node());
+    m_localBounds = FloatRect(image->x().value(), image->y().value(), image->width().value(), image->height().value());
+
     calculateAbsoluteBounds();
 
     if (checkForRepaint)
@@ -170,29 +174,22 @@ void RenderSVGImage::paint(PaintInfo& paintInfo, int, int)
     if (paintInfo.phase == PaintPhaseForeground) {
         SVGResourceFilter* filter = 0;
 
-        AffineTransform imageCtm(translationForAttributes());
         PaintInfo savedInfo(paintInfo);
 
-        FloatRect boundingBox = FloatRect(narrowPrecisionToFloat(imageCtm.e()), narrowPrecisionToFloat(imageCtm.f()), m_imageWidth, m_imageHeight);
-        prepareToRenderSVGContent(this, paintInfo, boundingBox, filter);
+        prepareToRenderSVGContent(this, paintInfo, m_localBounds, filter);
 
-        SVGImageElement* imageElt = static_cast<SVGImageElement*>(node());
-
-        FloatRect destRect(m_x, m_y, m_imageWidth, m_imageHeight);
+        FloatRect destRect = m_localBounds;
         FloatRect srcRect(0, 0, image()->width(), image()->height());
 
+        SVGImageElement* imageElt = static_cast<SVGImageElement*>(node());
         if (imageElt->preserveAspectRatio()->align() != SVGPreserveAspectRatio::SVG_PRESERVEASPECTRATIO_NONE)
             adjustRectsForAspectRatio(destRect, srcRect, imageElt->preserveAspectRatio());
 
-        paintInfo.context->concatCTM(imageCtm);
         paintInfo.context->drawImage(image(), destRect, srcRect);
 
-        finishRenderSVGContent(this, paintInfo, boundingBox, filter, savedInfo.context);
+        finishRenderSVGContent(this, paintInfo, m_localBounds, filter, savedInfo.context);
     }
     
-    if ((paintInfo.phase == PaintPhaseOutline || paintInfo.phase == PaintPhaseSelfOutline) && style()->outlineWidth())
-        paintOutline(paintInfo.context, 0, 0, width(), height(), style());
-
     paintInfo.context->restore();
 }
 
@@ -208,10 +205,9 @@ bool RenderSVGImage::nodeAtPoint(const HitTestRequest& request, HitTestResult& r
     if (isVisible || !hitRules.requireVisible) {
         double localX, localY;
         absoluteTransform().inverse().map(_x, _y, &localX, &localY);
-        translationForAttributes().inverse().map(localX, localY, &localX, &localY);
 
         if (hitRules.canHitFill) {
-            if (FloatRect(0.0f, 0.0f, m_width, m_height).contains(narrowPrecisionToFloat(localX), narrowPrecisionToFloat(localY))) {
+            if (m_localBounds.contains(narrowPrecisionToFloat(localX), narrowPrecisionToFloat(localY))) {
                 updateHitTestResult(result, IntPoint(_x, _y));
                 return true;
             }
@@ -226,10 +222,9 @@ bool RenderSVGImage::requiresLayer()
     return false;
 }
 
-FloatRect RenderSVGImage::relativeBBox(bool includeStroke) const
+FloatRect RenderSVGImage::relativeBBox(bool) const
 {
-    SVGImageElement* image = static_cast<SVGImageElement*>(node());
-    return FloatRect(image->x().value(), image->y().value(), width(), height());
+    return m_localBounds;
 }
 
 void RenderSVGImage::imageChanged(CachedImage* image)
@@ -252,7 +247,7 @@ void RenderSVGImage::calculateAbsoluteBounds()
 #endif
 
     if (!absoluteRect.isEmpty())
-        absoluteRect.inflate(1.5f); // inflate 1 pixel for antialiasing, 0.5 due to subpixel position or dimensions.
+        absoluteRect.inflate(1); // inflate 1 pixel for antialiasing
 
     m_absoluteBounds = enclosingIntRect(absoluteRect);
 }
@@ -274,26 +269,6 @@ void RenderSVGImage::absoluteRects(Vector<IntRect>& rects, int, int, bool)
     rects.append(absoluteClippedOverflowRect());
 }
 
-AffineTransform RenderSVGImage::translationForAttributes()
-{
-    SVGImageElement* image = static_cast<SVGImageElement*>(node());
-    return AffineTransform().translate(image->x().value(), image->y().value());
-}
-
-void RenderSVGImage::calcWidth()
-{
-    RenderImage::calcWidth();
-    SVGImageElement* image = static_cast<SVGImageElement*>(node());
-    m_imageWidth = image->width().value();
-}
-
-void RenderSVGImage::calcHeight()
-{
-    RenderImage::calcHeight();
-    SVGImageElement* image = static_cast<SVGImageElement*>(node());
-    m_imageHeight = image->height().value();
-}
-
 }
 
 #endif // ENABLE(SVG)
index 38330bc..77c9030 100644 (file)
@@ -27,6 +27,7 @@
 #if ENABLE(SVG)
 
 #include "AffineTransform.h"
+#include "FloatRect.h"
 #include "RenderImage.h"
 
 namespace WebCore {
@@ -43,7 +44,6 @@ namespace WebCore {
         
         virtual FloatRect relativeBBox(bool includeStroke = true) const;
         virtual IntRect absoluteClippedOverflowRect();
-        
         virtual void absoluteRects(Vector<IntRect>&, int tx, int ty, bool topLevel = true);
         virtual void addFocusRingRects(GraphicsContext*, int tx, int ty);
 
@@ -57,16 +57,12 @@ namespace WebCore {
 
         virtual bool nodeAtPoint(const HitTestRequest&, HitTestResult&, int _x, int _y, int _tx, int _ty, HitTestAction);
 
-        virtual void calcWidth();
-        virtual void calcHeight();
         bool calculateLocalTransform();
 
     private:
         void calculateAbsoluteBounds();
-        AffineTransform translationForAttributes();
         AffineTransform m_localTransform;
-        float m_imageWidth;
-        float m_imageHeight;
+        FloatRect m_localBounds;
         IntRect m_absoluteBounds;
     };