Changeset 160774 in webkit

Timestamp:

Dec 18, 2013 10:14:12 AM (10 years ago)

Author:

commit-queue@webkit.org

Message:

Fix ASSERTION FAILED in WebCore::SVGLengthContext::determineViewport
​https://bugs.webkit.org/show_bug.cgi?id=120284

Patch by Tamas Gergely <​tgergely.u-szeged@partner.samsung.com> on 2013-12-18
Reviewed by Philip Rogers.

Source/WebCore:

Added handling of root <svg> elements.
Blink merge: ​https://chromium.googlesource.com/chromium/blink/+/a7dedf81eb7008276bb6854f0e46465e039788f8

SVGLengthContext::determineViewport() currently asserts that we're not
resolving lengths for the topmost element, but there's nothing to
prevent such calls.

The patch updates determineViewport() to handle root elements geracefully
(using their current viewport). It also changes the signature slightly
to operate directly on a FloatSize, reducing some of the boiler-plate
client code.

Tests: svg/custom/svg-length-value-handled.svg

svg/dom/svg-root-lengths.html

• svg/SVGLengthContext.cpp:

(WebCore::SVGLengthContext::convertValueFromUserUnitsToPercentage):
(WebCore::SVGLengthContext::convertValueFromPercentageToUserUnits):
(WebCore::SVGLengthContext::determineViewport):

• svg/SVGLengthContext.h:
• svg/graphics/filters/SVGFEImage.cpp:

(WebCore::FEImage::platformApplySoftware):

LayoutTests:

Added tests of handling root <svg> elements.
Blink merge: ​https://chromium.googlesource.com/chromium/blink/+/a7dedf81eb7008276bb6854f0e46465e039788f8

• svg/custom/svg-length-value-handled-expected.txt: Added.
• svg/custom/svg-length-value-handled.svg: Added.

Tests whether root svg elements sizes are handled.

• svg/dom/svg-root-lengths-expected.txt: Added.
• svg/dom/svg-root-lengths.html: Added.

Tests the correct handling of root svg elements sizes.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/svg/custom/svg-length-value-handled-expected.txt (added)
• LayoutTests/svg/custom/svg-length-value-handled.svg (added)
• LayoutTests/svg/dom/svg-root-lengths-expected.txt (added)
• LayoutTests/svg/dom/svg-root-lengths.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/svg/SVGLengthContext.cpp (modified) (7 diffs)
• Source/WebCore/svg/SVGLengthContext.h (modified) (1 diff)
• Source/WebCore/svg/graphics/filters/SVGFEImage.cpp (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2013-12-18  Tamas Gergely  <tgergely.u-szeged@partner.samsung.com>
+
+        Fix ASSERTION FAILED in WebCore::SVGLengthContext::determineViewport
+        https://bugs.webkit.org/show_bug.cgi?id=120284
+
+        Reviewed by Philip Rogers.
+
+        Added tests of handling root <svg> elements.
+        Blink merge: https://chromium.googlesource.com/chromium/blink/+/a7dedf81eb7008276bb6854f0e46465e039788f8
+
+        * svg/custom/svg-length-value-handled-expected.txt: Added.
+        * svg/custom/svg-length-value-handled.svg: Added.
+            Tests whether root svg elements sizes are handled.
+        * svg/dom/svg-root-lengths-expected.txt: Added.
+        * svg/dom/svg-root-lengths.html: Added.
+            Tests the correct handling of root svg elements sizes.
+
 2013-12-18  Darin Adler  <darin@apple.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2013-12-18  Tamas Gergely  <tgergely.u-szeged@partner.samsung.com>
+
+        Fix ASSERTION FAILED in WebCore::SVGLengthContext::determineViewport
+        https://bugs.webkit.org/show_bug.cgi?id=120284
+
+        Reviewed by Philip Rogers.
+
+        Added handling of root <svg> elements.
+        Blink merge: https://chromium.googlesource.com/chromium/blink/+/a7dedf81eb7008276bb6854f0e46465e039788f8
+
+        SVGLengthContext::determineViewport() currently asserts that we're not
+        resolving lengths for the topmost element, but there's nothing to
+        prevent such calls.
+
+        The patch updates determineViewport() to handle root elements geracefully
+        (using their current viewport). It also changes the signature slightly
+        to operate directly on a FloatSize, reducing some of the boiler-plate
+        client code.
+
+        Tests: svg/custom/svg-length-value-handled.svg
+               svg/dom/svg-root-lengths.html
+
+        * svg/SVGLengthContext.cpp:
+        (WebCore::SVGLengthContext::convertValueFromUserUnitsToPercentage):
+        (WebCore::SVGLengthContext::convertValueFromPercentageToUserUnits):
+        (WebCore::SVGLengthContext::determineViewport):
+        * svg/SVGLengthContext.h:
+        * svg/graphics/filters/SVGFEImage.cpp:
+        (WebCore::FEImage::platformApplySoftware):
+
 2013-12-18  Darin Adler  <darin@apple.com>
 

trunk/Source/WebCore/svg/SVGLengthContext.cpp

@@ -162 +162 @@
 float SVGLengthContext::convertValueFromUserUnitsToPercentage(float value, SVGLengthMode mode, ExceptionCode& ec) const
 {
-    float width = 0;
-    float height = 0;
-    if (!determineViewport(width, height)) {
+    FloatSize viewportSize;
+    if (!determineViewport(viewportSize)) {
         ec = NOT_SUPPORTED_ERR;
         return 0;
@@ -171 +170 @@
     switch (mode) {
     case LengthModeWidth:
-        return value / width * 100;
+        return value / viewportSize.width() * 100;
     case LengthModeHeight:
-        return value / height * 100;
+        return value / viewportSize.height() * 100;
     case LengthModeOther:
-        return value / (sqrtf((width * width + height * height) / 2)) * 100;
+        return value / (sqrtf(viewportSize.diagonalLengthSquared() / 2)) * 100;
     };
 
@@ -184 +183 @@
 float SVGLengthContext::convertValueFromPercentageToUserUnits(float value, SVGLengthMode mode, ExceptionCode& ec) const
 {
-    float width = 0;
-    float height = 0;
-    if (!determineViewport(width, height)) {
+    FloatSize viewportSize;
+    if (!determineViewport(viewportSize)) {
         ec = NOT_SUPPORTED_ERR;
         return 0;
@@ -193 +191 @@
     switch (mode) {
     case LengthModeWidth:
-        return value * width;
+        return value * viewportSize.width();
     case LengthModeHeight:
-        return value * height;
+        return value * viewportSize.height();
     case LengthModeOther:
-        return value * sqrtf((width * width + height * height) / 2);
+        return value * sqrtf(viewportSize.diagonalLengthSquared() / 2);
     };
 
@@ -281 +279 @@
 }
 
-bool SVGLengthContext::determineViewport(float& width, float& height) const
+bool SVGLengthContext::determineViewport(FloatSize& viewportSize) const
 {
     if (!m_context)
@@ -288 +286 @@
     // If an overriden viewport is given, it has precedence.
     if (!m_overridenViewport.isEmpty()) {
-        width = m_overridenViewport.width();
-        height = m_overridenViewport.height();
+        viewportSize = m_overridenViewport.size();
         return true;
     }
 
-    // SVGLengthContext should NEVER be used to resolve width/height values for <svg> elements,
-    // as they require special treatment, due the relationship with the CSS width/height properties.
-    ASSERT(m_context->document().documentElement() != m_context);
+    // Root <svg> element lengths are resolved against the top level viewport.
+    if (m_context->isOutermostSVGSVGElement()) {
+        viewportSize = toSVGSVGElement(m_context)->currentViewportSize();
+        return true;
+    }
 
     // Take size from nearest viewport element.
@@ -301 +300 @@
     if (!viewportElement || !isSVGSVGElement(viewportElement))
         return false;
-    
+
     const SVGSVGElement* svg = toSVGSVGElement(viewportElement);
-    FloatSize viewportSize = svg->currentViewBoxRect().size();
+    viewportSize = svg->currentViewBoxRect().size();
     if (viewportSize.isEmpty())
         viewportSize = svg->currentViewportSize();
 
-    width = viewportSize.width();
-    height = viewportSize.height();
     return true;
 }

trunk/Source/WebCore/svg/SVGLengthContext.h

@@ -69 +69 @@
     float convertValueFromUserUnits(float, SVGLengthMode, SVGLengthType toUnit, ExceptionCode&) const;
 
-    bool determineViewport(float& width, float& height) const;
+    bool determineViewport(FloatSize&) const;
 
 private:

trunk/Source/WebCore/svg/graphics/filters/SVGFEImage.cpp

@@ -129 +129 @@
         if (contextNode->hasRelativeLengths()) {
             SVGLengthContext lengthContext(contextNode);
-            float width = 0;
-            float height = 0;
+            FloatSize viewportSize;
 
             // If we're referencing an element with percentage units, eg. <rect with="30%"> those values were resolved against the viewport.
             // Build up a transformation that maps from the viewport space to the filter primitive subregion.
-            if (lengthContext.determineViewport(width, height))
-                resultImage->context()->concatCTM(makeMapBetweenRects(FloatRect(0, 0, width, height), destRect));
+            if (lengthContext.determineViewport(viewportSize))
+                resultImage->context()->concatCTM(makeMapBetweenRects(FloatRect(FloatPoint(), viewportSize), destRect));
         }