Bug 678250 - Filter bounds rounded before scaling so small filters may be too large. r=roc
authorRobert Longson <longsonr@gmail.com>
Mon, 15 Aug 2011 19:41:57 +0100
changeset 75321 485f0838da930c169ee790875405dfc0760b754e
parent 75320 083551a45a833bb34e8d0ef68e7fcbd49dee6851
child 75322 f9be98f4a042f3f6a6e8779d3f61ff314bb551e9
push id2
push userbsmedberg@mozilla.com
push dateFri, 19 Aug 2011 14:38:13 +0000
reviewersroc
bugs678250
milestone8.0a1
Bug 678250 - Filter bounds rounded before scaling so small filters may be too large. r=roc
layout/reftests/svg/filter-scaled-01.svg
layout/reftests/svg/filters/filter-nested-filtering-01.svg
layout/reftests/svg/objectBoundingBox-and-fePointLight-01-ref.svg
layout/reftests/svg/objectBoundingBox-and-fePointLight-01.svg
layout/reftests/svg/reftest.list
layout/svg/base/src/nsSVGFilterFrame.cpp
layout/svg/base/src/nsSVGFilterInstance.cpp
new file mode 100644
--- /dev/null
+++ b/layout/reftests/svg/filter-scaled-01.svg
@@ -0,0 +1,18 @@
+<svg width="100%" height="100%" preserveAspectRatio="none" 
+     xmlns="http://www.w3.org/2000/svg"
+     xmlns:xlink="http://www.w3.org/1999/xlink">
+ 
+  <title>Testcase for scaled filter with non-integer x and y and objectBoundingBox</title> 
+  <defs>
+    <filter id="filter" filterUnits="objectBoundingBox" x="0.5" y="0.5" width="0.25" height="0.25">
+      <feFlood flood-color="red" width="1" height="1" />
+    </filter>
+  </defs>
+
+  <rect width="100%" height="100%" fill="lime"/>
+  <g transform="scale(400)">
+    <rect width="1" height="1" filter="url(#filter)" />
+  </g>
+  <rect x="200" y="200" width="100" height="100" fill="lime" />
+
+</svg>
--- a/layout/reftests/svg/filters/filter-nested-filtering-01.svg
+++ b/layout/reftests/svg/filters/filter-nested-filtering-01.svg
@@ -1,16 +1,16 @@
 <!--
      Any copyright is dedicated to the Public Domain.
      http://creativecommons.org/publicdomain/zero/1.0/
 -->
 <svg xmlns="http://www.w3.org/2000/svg" version="1.0">
   <!-- From https://bugzilla.mozilla.org/show_bug.cgi?id=461863 -->
   <desc>
-    This test checks that transforms and filtering a decedent of a
+    This test checks that transforms and filtering a descendant of a
     filtered element interact correctly.
   </desc>
   <filter id="a"><feGaussianBlur stdDeviation="0.001"/></filter>
   <filter id="b"><feGaussianBlur stdDeviation="0.001"/></filter>
   <rect width="100%" height="100%" fill="lime"/>
   <circle fill="red" cx="200" cy="200" r="98"/>
   <g id="g" filter="url(#a)" transform="translate(100, 0)">
     <circle fill="lime" filter="url(#b)" cx="100" cy="200" r="100"/>
--- a/layout/reftests/svg/objectBoundingBox-and-fePointLight-01-ref.svg
+++ b/layout/reftests/svg/objectBoundingBox-and-fePointLight-01-ref.svg
@@ -1,24 +1,23 @@
 <!--
      Any copyright is dedicated to the Public Domain.
      http://creativecommons.org/publicdomain/zero/1.0/
 -->
-<svg width="100%" height="100%"
-  viewBox="0 0 480 360" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">
+<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">
   <title>Reference for objectBoundingBox with fePointLight</title>
   <!-- From https://bugzilla.mozilla.org/show_bug.cgi?id=544742 -->
 
   <defs>
     <filter id="light" primitiveUnits="userSpaceOnUse">
       <feSpecularLighting lighting-color="blue" surfaceScale="5" specularConstant="10" specularExponent="6">
         <fePointLight x="30" y="30" z="-5"/>
       </feSpecularLighting>
       <feComposite operator="in" in2="SourceGraphic"/>
     </filter>
   </defs>
 
-  <g stroke="black" transform="translate(20 20)">
+  <g transform="translate(20 20)">
     <rect width="40" height="20" filter="url(#light)" fill="black" stroke="none"/>
   </g>
 
 </svg>
 
--- a/layout/reftests/svg/objectBoundingBox-and-fePointLight-01.svg
+++ b/layout/reftests/svg/objectBoundingBox-and-fePointLight-01.svg
@@ -1,14 +1,13 @@
 <!--
      Any copyright is dedicated to the Public Domain.
      http://creativecommons.org/publicdomain/zero/1.0/
 -->
-<svg width="100%" height="100%"
-  viewBox="0 0 480 360" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">
+<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">
   <title>Testcase for objectBoundingBox with fePointLight</title>
   <!-- From https://bugzilla.mozilla.org/show_bug.cgi?id=544742 -->
 
   <defs>
     <filter id="light" primitiveUnits="objectBoundingBox">
       <feSpecularLighting lighting-color="blue" surfaceScale="5" specularConstant="10" specularExponent="6">
         <!-- Note: for z  the scalefactor is  31,622776601683793319988935444327
                      sqrt(bbox.width*bbox.width + bbox.height*bbox.height)/sqrt(2) -->
--- a/layout/reftests/svg/reftest.list
+++ b/layout/reftests/svg/reftest.list
@@ -106,16 +106,17 @@ random == dynamic-use-nested-01.svg dyna
 == fallback-color-02a.svg fallback-color-02-ref.svg
 == fallback-color-02b.svg fallback-color-02-ref.svg
 == fallback-color-03.svg pass.svg
 == filter-basic-01.svg pass.svg
 == filter-basic-02.svg pass.svg
 == filter-basic-03.svg pass.svg
 == filter-foreignObject-01.svg pass.svg
 == filter-invalidation-01.svg pass.svg
+== filter-scaled-01.svg pass.svg
 == filter-translated-01.svg filter-translated-01-ref.svg
 == filters-and-group-opacity-01.svg filters-and-group-opacity-01-ref.svg
 == foreignObject-01.svg pass.svg
 == foreignObject-02.svg foreignObject-02-ref.svg
 == foreignObject-ancestor-style-change-01.svg foreignObject-ancestor-style-change-01-ref.svg
 == foreignObject-change-transform-01.svg pass.svg
 == foreignObject-display-01.svg pass.svg
 == foreignObject-move-repaint-01.svg pass.svg
--- a/layout/svg/base/src/nsSVGFilterFrame.cpp
+++ b/layout/svg/base/src/nsSVGFilterFrame.cpp
@@ -130,59 +130,63 @@ nsAutoFilterInstance::nsAutoFilterInstan
   // a factor of the bbox width/height). We should also send a warning if the
   // user uses a number without units (a future SVG spec should really
   // deprecate that, since it's too confusing for a bare number to be sometimes
   // interpreted as a fraction of the bounding box and sometimes as user-space
   // units). So really only percentage values should be used in this case.
   
   gfxRect filterRegion = nsSVGUtils::GetRelativeRect(filterUnits,
     filter->mLengthAttributes, bbox, aTarget);
-  filterRegion.RoundOut();
 
   if (filterRegion.Width() <= 0 || filterRegion.Height() <= 0) {
     // 0 disables rendering, < 0 is error. dispatch error console warning
     // or error as appropriate.
     return;
   }
 
   gfxMatrix userToDeviceSpace = nsSVGUtils::GetCanvasTM(aTarget);
   
   // Calculate filterRes (the width and height of the pixel buffer of the
   // temporary offscreen surface that we'll paint into):
 
   gfxIntSize filterRes;
-  if (filter->mIntegerPairAttributes[nsSVGFilterElement::FILTERRES].IsExplicitlySet()) {
-    PRInt32 filterResX =
-      filter->mIntegerPairAttributes[nsSVGFilterElement::FILTERRES].GetAnimValue(nsSVGIntegerPair::eFirst);
-    PRInt32 filterResY =
-      filter->mIntegerPairAttributes[nsSVGFilterElement::FILTERRES].GetAnimValue(nsSVGIntegerPair::eSecond);
-    // XXX what if the 'filterRes' attribute has a bad value? error console warning?
+  const nsSVGIntegerPair& filterResAttrs =
+    filter->mIntegerPairAttributes[nsSVGFilterElement::FILTERRES];
+  if (filterResAttrs.IsExplicitlySet()) {
+    PRInt32 filterResX = filterResAttrs.GetAnimValue(nsSVGIntegerPair::eFirst);
+    PRInt32 filterResY = filterResAttrs.GetAnimValue(nsSVGIntegerPair::eSecond);
+    if (filterResX <= 0 || filterResY <= 0) {
+      // 0 disables rendering, < 0 is error. dispatch error console warning?
+      return;
+    }
 
+    filterRegion.Scale(filterResX, filterResY);
+    filterRegion.RoundOut();
+    filterRegion.Scale(1.0 / filterResX, 1.0 / filterResY);
     // We don't care if this overflows, because we can handle upscaling/
     // downscaling to filterRes
     PRBool overflow;
     filterRes =
       nsSVGUtils::ConvertToSurfaceSize(gfxSize(filterResX, filterResY),
                                        &overflow);
     // XXX we could send a warning to the error console if the author specified
     // filterRes doesn't align well with our outer 'svg' device space.
   } else {
     // Match filterRes as closely as possible to the pixel density of the nearest
     // outer 'svg' device space:
     float scale = nsSVGUtils::MaxExpansion(userToDeviceSpace);
+
+    filterRegion.Scale(scale);
+    filterRegion.RoundOut();
     // We don't care if this overflows, because we can handle upscaling/
     // downscaling to filterRes
     PRBool overflow;
-    filterRes = nsSVGUtils::ConvertToSurfaceSize(filterRegion.Size() * scale,
+    filterRes = nsSVGUtils::ConvertToSurfaceSize(filterRegion.Size(),
                                                  &overflow);
-  }
-
-  if (filterRes.width <= 0 || filterRes.height <= 0) {
-    // 0 disables rendering, < 0 is error. dispatch error console warning?
-    return;
+    filterRegion.Scale(1.0 / scale);
   }
 
   // XXX we haven't taken account of the fact that filterRegion may be
   // partially or entirely outside the current clip region. :-/
 
   // Convert the dirty rects to filter space, and create our nsSVGFilterInstance:
 
   gfxMatrix filterToUserSpace(filterRegion.Width() / filterRes.width, 0.0f,
--- a/layout/svg/base/src/nsSVGFilterInstance.cpp
+++ b/layout/svg/base/src/nsSVGFilterInstance.cpp
@@ -366,17 +366,17 @@ nsSVGFilterInstance::BuildSourceImages()
 
     // SVG graphics paint to device space, so we need to set an initial device
     // space to filter space transform on the gfxContext that SourceGraphic
     // and SourceAlpha will paint to.
     //
     // (In theory it would be better to minimize error by having filtered SVG
     // graphics temporarily paint to user space when painting the sources and
     // only set a user space to filter space transform on the gfxContext
-    // (since that would elliminate the transform multiplications from user
+    // (since that would eliminate the transform multiplications from user
     // space to device space and back again). However, that would make the
     // code more complex while being hard to get right without introducing
     // subtle bugs, and in practice it probably makes no real difference.)
     gfxMatrix deviceToFilterSpace = GetFilterSpaceToDeviceSpaceTransform().Invert();
     tmpState.GetGfxContext()->Multiply(deviceToFilterSpace);
     mPaintCallback->Paint(&tmpState, mTargetFrame, &dirty);
 
     gfxContext copyContext(sourceColorAlpha);