Bug 524104. Markers should be affected by group opacity on the marked element. r=longsonr@gmail.com

layout/reftests/svg/filters-and-group-opacity-01-ref.svg

@@ -0,0 +1,4 @@
+<svg xmlns="http://www.w3.org/2000/svg">
+  <rect width="100%" height="200%" fill="white"/>
+  <rect width="100%" height="200%" fill="blue" fill-opacity=".5"/>
+</svg>

layout/reftests/svg/filters-and-group-opacity-01.svg

@@ -0,0 +1,17 @@
+<!--
+     Any copyright is dedicated to the Public Domain.
+     http://creativecommons.org/licenses/publicdomain/
+-->
+<svg xmlns="http://www.w3.org/2000/svg">
+
+  <title>Test that filters and group opacity are applied in the correct order on an element</title>
+
+  <!-- From https://bugzilla.mozilla.org/show_bug.cgi?id=524104 -->
+
+  <filter id="filter" filterUnits="userSpaceOnUse" x="0" y="0" width="100%" height="100%"> 
+    <feFlood in="SourceGraphic" flood-color="blue"/>
+  </filter>
+
+  <rect width="1" height="1" filter="url(#filter)" opacity=".5"/>
+
+</svg>

layout/reftests/svg/markers-and-group-opacity-01-ref.svg

@@ -0,0 +1,4 @@
+<svg xmlns="http://www.w3.org/2000/svg">
+  <rect width="100%" height="200%" fill="white"/>
+  <rect width="100%" height="200%" fill="blue" fill-opacity=".5"/>
+</svg>

layout/reftests/svg/markers-and-group-opacity-01.svg

@@ -0,0 +1,17 @@
+<!--
+     Any copyright is dedicated to the Public Domain.
+     http://creativecommons.org/licenses/publicdomain/
+-->
+<svg xmlns="http://www.w3.org/2000/svg">
+
+  <title>Test that markers are affected by group opacity on the marked element</title>
+
+  <!-- From https://bugzilla.mozilla.org/show_bug.cgi?id=524104 -->
+
+  <marker id="mark" markerUnits="userSpaceOnUse" markerContentUnits="userSpaceOnUse" markerWidth="100%" markerHeight="200%" refX="0" refY="0">
+    <rect width="100%" height="200%" fill="blue"/>
+  </marker>
+
+  <line x1="0" y1="-1" x2="1" y2="-1" fill="none" stroke="black" opacity=".5" marker-start="url(#mark)"/>
+
+</svg>

layout/reftests/svg/reftest.list

@@ -66,6 +66,7 @@ include moz-only/reftest.list
 == filter-foreignObject-01.svg pass.svg
 == filter-invalidation-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
@@ -85,6 +86,7 @@ fails == inline-in-xul-basic-01.xul pass.svg
 == invalid-text-01.svg pass.svg
 == linearGradient-basic-01.svg pass.svg
 == linearGradient-basic-02.svg pass.svg
+== markers-and-group-opacity-01.svg markers-and-group-opacity-01-ref.svg
 == marker-attribute-01.svg pass.svg
 == mask-containing-masked-content-01.svg pass.svg
 # Bug 456323

layout/svg/base/src/nsSVGGeometryFrame.cpp

@@ -192,12 +192,13 @@ SetupCairoColor(gfxContext *aContext, nscolor aRGB, float aOpacity)
 }
 
 float
-nsSVGGeometryFrame::MaybeOptimizeOpacity(float aOpacity)
+nsSVGGeometryFrame::MaybeOptimizeOpacity(float aFillOrStrokeOpacity)
 {
-  if (nsSVGUtils::CanOptimizeOpacity(this)) {
-    aOpacity *= GetStyleDisplay()->mOpacity;
+  float opacity = GetStyleDisplay()->mOpacity;
+  if (opacity < 1 && nsSVGUtils::CanOptimizeOpacity(this)) {
+    return aFillOrStrokeOpacity * opacity;
   }
-  return aOpacity;
+  return aFillOrStrokeOpacity;
 }
 
 PRBool
@@ -237,7 +238,9 @@ nsSVGGeometryFrame::SetupCairoFill(gfxContext *aContext)
 PRBool
 nsSVGGeometryFrame::HasStroke()
 {
-  return GetStyleSVG()->mStroke.mType != eStyleSVGPaintType_None && 
+  const nsStyleSVG *style = GetStyleSVG();
+  return style->mStroke.mType != eStyleSVGPaintType_None &&
+         style->mStrokeOpacity > 0 &&
          GetStrokeWidth() > 0;
 }
 

layout/svg/base/src/nsSVGGeometryFrame.h

@@ -107,13 +107,14 @@ private:
   nsresult GetStrokeDashArray(double **arr, PRUint32 *count);
   float GetStrokeDashoffset();
 
-  // Returns opacity that should be used in rendering this primitive.
-  // In the general case the return value is just the passed opacity.
-  // If we can avoid the expense of a specified group opacity, we
-  // multiply the passed opacity by the value of the 'opacity'
-  // property, and elsewhere pretend the 'opacity' property has a
-  // value of 1.
-  float MaybeOptimizeOpacity(float aOpacity);
+  /**
+   * Returns the given 'fill-opacity' or 'stroke-opacity' value multiplied by
+   * the value of the 'opacity' property if it's possible to avoid the expense
+   * of creating and compositing an offscreen surface for 'opacity' by
+   * combining 'opacity' with the 'fill-opacity'/'stroke-opacity'. If not, the
+   * given 'fill-opacity'/'stroke-opacity' is returned unmodified.
+   */
+  float MaybeOptimizeOpacity(float aFillOrStrokeOpacity);
 };
 
 #endif // __NS_SVGGEOMETRYFRAME_H__

layout/svg/base/src/nsSVGUtils.cpp

@@ -995,9 +995,11 @@ nsSVGUtils::PaintFrameWithEffects(nsSVGRenderState *aContext,
 
   /* SVG defines the following rendering model:
    *
-   *  1. Render geometry
-   *  2. Apply filter
-   *  3. Apply clipping, masking, group opacity
+   *  1. Render fill
+   *  2. Render stroke
+   *  3. Render markers
+   *  4. Apply filter
+   *  5. Apply clipping, masking, group opacity
    *
    * We follow this, but perform a couple of optimizations:
    *
@@ -1382,16 +1384,26 @@ nsSVGUtils::GetRelativeRect(PRUint16 aUnits, const nsSVGLength2 *aXYWH,
 PRBool
 nsSVGUtils::CanOptimizeOpacity(nsIFrame *aFrame)
 {
-  if (!aFrame->GetStyleSVGReset()->mFilter) {
-    nsIAtom *type = aFrame->GetType();
-    if (type == nsGkAtoms::svgImageFrame)
-      return PR_TRUE;
-    if (type == nsGkAtoms::svgPathGeometryFrame) {
-      const nsStyleSVG *style = aFrame->GetStyleSVG();
-      if (style->mFill.mType == eStyleSVGPaintType_None ||
-          !static_cast<nsSVGPathGeometryFrame*>(aFrame)->HasStroke())
-        return PR_TRUE;
-    }
+  nsIAtom *type = aFrame->GetType();
+  if (type != nsGkAtoms::svgImageFrame &&
+      type != nsGkAtoms::svgPathGeometryFrame) {
+    return PR_FALSE;
+  }
+  if (aFrame->GetStyleSVGReset()->mFilter) {
+    return PR_FALSE;
+  }
+  // XXX The SVG WG is intending to allow fill, stroke and markers on <image>
+  if (type == nsGkAtoms::svgImageFrame) {
+    return PR_TRUE;
+  }
+  const nsStyleSVG *style = aFrame->GetStyleSVG();
+  if (style->mMarkerStart || style->mMarkerMid || style->mMarkerEnd) {
+    return PR_FALSE;
+  }
+  if (style->mFill.mType == eStyleSVGPaintType_None ||
+      style->mFillOpacity <= 0 ||
+      !static_cast<nsSVGPathGeometryFrame*>(aFrame)->HasStroke()) {
+    return PR_TRUE;
   }
   return PR_FALSE;
 }