Fix: Incorrect clip region issue (#492)

- Small refactoring in SvgClipPath - Fix incorrect clip region issue, fixes #363 - Add test images
svg-net · Jun 20, 2019 · a424206 · a424206
1 parent e68fc46
commit a424206
Show file tree

Hide file tree

Showing 6 changed files with 42 additions and 40 deletions.
diff --git a/Source/Basic Shapes/SvgVisualElement.cs b/Source/Basic Shapes/SvgVisualElement.cs
@@ -363,7 +363,7 @@ protected internal virtual void SetClip(ISvgRenderer renderer)
                 if (this.ClipPath != null)
                 {
                     SvgClipPath clipPath = this.OwnerDocument.GetElementById<SvgClipPath>(this.ClipPath.ToString());
-                    if (clipPath != null) renderer.SetClip(clipPath.GetClipRegion(this), CombineMode.Intersect);
+                    if (clipPath != null) renderer.SetClip(clipPath.GetClipRegion(this, renderer), CombineMode.Intersect);
                 }
 
                 var clip = this.Clip;

diff --git a/Source/Clipping and Masking/SvgClipPath.cs b/Source/Clipping and Masking/SvgClipPath.cs
@@ -1,9 +1,5 @@
-using System;
-using System.Collections.Generic;
-using System.Text;
 using System.Drawing;
 using System.Drawing.Drawing2D;
-using Svg.Transforms;
 
 namespace Svg
 {
@@ -15,7 +11,7 @@ public sealed class SvgClipPath : SvgElement
     {
         private SvgCoordinateUnits _clipPathUnits = SvgCoordinateUnits.Inherit;
 
-        private bool _pathDirty = true;
+        private GraphicsPath _path;
 
         /// <summary>
         /// Specifies the coordinate system for the clipping path.
@@ -27,30 +23,28 @@ public SvgCoordinateUnits ClipPathUnits
             set { _clipPathUnits = value; Attributes["clipPathUnits"] = value; }
         }
 
-        private GraphicsPath cachedClipPath = null;
-
         /// <summary>
         /// Gets this <see cref="SvgClipPath"/>'s region to be used as a clipping region.
         /// </summary>
+        /// <param name="owner"></param>
+        /// <param name="renderer"></param>
         /// <returns>A new <see cref="Region"/> containing the <see cref="Region"/> to be used for clipping.</returns>
-        public Region GetClipRegion(SvgVisualElement owner)
+        public Region GetClipRegion(SvgVisualElement owner, ISvgRenderer renderer)
         {
-            if (cachedClipPath == null || this._pathDirty)
+            if (_path == null || IsPathDirty)
             {
-                cachedClipPath = new GraphicsPath();
+                _path = new GraphicsPath();
 
-                foreach (SvgElement element in this.Children)
-                {
-                    this.CombinePaths(cachedClipPath, element);
-                }
+                foreach (var element in Children)
+                    CombinePaths(_path, element, renderer);
 
-                this._pathDirty = false;
+                IsPathDirty = false;
             }
 
-            var result = cachedClipPath;
+            var result = _path;
             if (ClipPathUnits == SvgCoordinateUnits.ObjectBoundingBox)
             {
-                result = (GraphicsPath)cachedClipPath.Clone();
+                result = (GraphicsPath)_path.Clone();
                 using (var transform = new Matrix())
                 {
                     var bounds = owner.Bounds;
@@ -68,31 +62,28 @@ public Region GetClipRegion(SvgVisualElement owner)
         /// </summary>
         /// <param name="path"></param>
         /// <param name="element"></param>
-        private void CombinePaths(GraphicsPath path, SvgElement element)
+        /// <param name="renderer"></param>
+        private void CombinePaths(GraphicsPath path, SvgElement element, ISvgRenderer renderer)
         {
             var graphicsElement = element as SvgVisualElement;
-
-            if (graphicsElement != null && graphicsElement.Path(null) != null)
+            if (graphicsElement != null)
             {
-                path.FillMode = (graphicsElement.ClipRule == SvgClipRule.NonZero) ? FillMode.Winding : FillMode.Alternate;
+                var childPath = graphicsElement.Path(renderer);
+                if (childPath != null)
+                {
+                    path.FillMode = graphicsElement.ClipRule == SvgClipRule.NonZero ? FillMode.Winding : FillMode.Alternate;
 
-                GraphicsPath childPath = graphicsElement.Path(null);
+                    if (graphicsElement.Transforms != null)
+                        foreach (var transform in graphicsElement.Transforms)
+                            childPath.Transform(transform.Matrix);
 
-                if (graphicsElement.Transforms != null)
-                {
-                    foreach (SvgTransform transform in graphicsElement.Transforms)
-                    {
-                        childPath.Transform(transform.Matrix);
-                    }
+                    if (childPath.PointCount > 0)
+                        path.AddPath(childPath, false);
                 }
-
-                if (childPath.PointCount > 0) path.AddPath(childPath, false);
             }
 
-            foreach (SvgElement child in element.Children)
-            {
-                this.CombinePaths(path, child);
-            }
+            foreach (var child in element.Children)
+                CombinePaths(path, child, renderer);
         }
 
         /// <summary>
@@ -104,7 +95,7 @@ private void CombinePaths(GraphicsPath path, SvgElement element)
         protected override void AddElement(SvgElement child, int index)
         {
             base.AddElement(child, index);
-            this._pathDirty = true;
+            IsPathDirty = true;
         }
 
         /// <summary>
@@ -115,7 +106,7 @@ protected override void AddElement(SvgElement child, int index)
         protected override void RemoveElement(SvgElement child)
         {
             base.RemoveElement(child);
-            this._pathDirty = true;
+            IsPathDirty = true;
         }
 
         /// <summary>
@@ -124,10 +115,8 @@ protected override void RemoveElement(SvgElement child)
         /// <param name="renderer">The <see cref="ISvgRenderer"/> object to render to.</param>
         protected override void Render(ISvgRenderer renderer)
         {
-            // Do nothing
         }
 
-
         public override SvgElement DeepCopy()
         {
             return DeepCopy<SvgClipPath>();
@@ -136,7 +125,7 @@ public override SvgElement DeepCopy()
         public override SvgElement DeepCopy<T>()
         {
             var newObj = base.DeepCopy<T>() as SvgClipPath;
-            newObj.ClipPathUnits = this.ClipPathUnits;
+            newObj.ClipPathUnits = ClipPathUnits;
             return newObj;
         }
     }

diff --git a/Tests/Svg.UnitTests/PassingTests.csv b/Tests/Svg.UnitTests/PassingTests.csv
@@ -206,6 +206,7 @@ __issue-338-01_stroke_width
 __issue-342-01
 __issue-345-01
 __issue-354-01
+__issue-363-01
 __issue-385-01_Test_text-anchor-middle
 __issue-398-01
 __issue-419-01
@@ -222,5 +223,6 @@ __pull_request-433-01
 __pull_request-444-01
 __pull_request-462-01
 __pull_request-471-01
+__pull_request-492-01
 __Telefunken_FuBK_test_pattern
 __title
diff --git a/Tests/W3CTestSuite/PassingTests.txt b/Tests/W3CTestSuite/PassingTests.txt
@@ -236,6 +236,7 @@ __issue-342-01.svg
 __issue-345-01.svg
 __issue-354-01.svg
 __issue-385-01_Test_text-anchor-middle.svg
+__issue-363-01.svg
 __issue-419-01.svg
 __issue-436-01.svg
 __issue-437-01.svg
@@ -248,6 +249,7 @@ __pull_request-374-01.svg
 __pull_request-414-01.svg
 __pull_request-433-01.svg
 __pull_request-444-01.svg
+__pull_request-492-01.svg
 __AJ_Digital_Camera.svg
 __Telefunken_FuBK_test_pattern.svg
 __tiger.svg

diff --git a/Tests/W3CTestSuite/png/__pull_request-492-01.png b/Tests/W3CTestSuite/png/__pull_request-492-01.png
diff --git a/Tests/W3CTestSuite/svg/__pull_request-492-01.svg b/Tests/W3CTestSuite/svg/__pull_request-492-01.svg