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 81dfb990f976a625a81188cfaa826bb1af5705cb..c442071ac6c8b4c52fef2731db058bcb84ff5dad 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 780fe9930d1b564e6855c2d78d00f70f52d8e517..28867b52953c12f20ef407020353459c8bdd5441 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 22311decd10c267973a1737d51c98e89ac462d80..66664004c73bc48324889b7dc29fc9699cebd7ef 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 fbefc506cbf5d33321c1901d314bc7afa6b652ca..4986bbe374b1e793c7e3de8475e9ac74a0f1e086 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 38330bc934ca3e7b062ddc9451cffcd77e4f7c4b..77c9030d35603681bff2e569217d777815dc6124 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;
     };