Skip to content

Commit

Permalink
And linq (#895)
Browse files Browse the repository at this point in the history
* Proposed clean-ups

This is a partial set of clean-ups. There could be others, but they may be controversial (e.g. adopting LINQ in some areas, which may or may not hurt performance). I've also tried to be mindful of target framework versions and also some edge cases (e.g. hashing) where seemingly redundant chunks of code, if removed or changed for another option, may blow things up.

See what you think.

* Adding LINQ on top of the other clean-ups

Pre-change:

// * Summary *

BenchmarkDotNet=v0.12.1, OS=endeavouros
AMD Ryzen 7 5800H with Radeon Graphics, 1 CPU, 16 logical and 8 physical cores
.NET Core SDK=8.0.108
  [Host]     : .NET Core 8.0.8 (CoreCLR 8.0.824.36612, CoreFX 8.0.824.36612), X64 RyuJIT
  Job-GRIUMJ : .NET Core 8.0.8 (CoreCLR 8.0.824.36612, CoreFX 8.0.824.36612), X64 RyuJIT

IterationCount=1  LaunchCount=1  WarmupCount=1

|         Method | EdgeCount |       Mean | Error |     Gen 0 |     Gen 1 |     Gen 2 | Allocated |
|--------------- |---------- |-----------:|------:|----------:|----------:|----------:|----------:|
| Intersection_N |      1000 |   119.9 ms |    NA | 1000.0000 |         - |         - |   9.04 MB |
| Intersection_N |      2000 |   542.1 ms |    NA | 1000.0000 |         - |         - |   16.2 MB |
| Intersection_N |      3000 | 1,431.1 ms |    NA | 5000.0000 | 4000.0000 | 1000.0000 |  37.28 MB |
| Intersection_N |      4000 | 3,008.9 ms |    NA | 6000.0000 | 5000.0000 | 1000.0000 |  47.41 MB |
| Intersection_N |      5000 | 6,173.7 ms |    NA | 9000.0000 | 8000.0000 | 1000.0000 |  74.05 MB |

// * Legends *
  EdgeCount : Value of the 'EdgeCount' parameter
  Mean      : Arithmetic mean of all measurements
  Error     : Half of 99.9% confidence interval
  Gen 0     : GC Generation 0 collects per 1000 operations
  Gen 1     : GC Generation 1 collects per 1000 operations
  Gen 2     : GC Generation 2 collects per 1000 operations
  Allocated : Allocated memory per single operation (managed only, inclusive, 1KB = 1024B)
  1 ms      : 1 Millisecond (0.001 sec)

// * Diagnostic Output - MemoryDiagnoser *

// ***** BenchmarkRunner: End *****
// ** Remained 0 benchmark(s) to run **
Run time: 00:00:47 (47.83 sec), executed benchmarks: 5

Global total time: 00:00:57 (57.28 sec), executed benchmarks: 5

Post-change:

// * Summary *

BenchmarkDotNet=v0.12.1, OS=endeavouros
AMD Ryzen 7 5800H with Radeon Graphics, 1 CPU, 16 logical and 8 physical cores
.NET Core SDK=8.0.108
  [Host]     : .NET Core 8.0.8 (CoreCLR 8.0.824.36612, CoreFX 8.0.824.36612), X64 RyuJIT
  Job-WWTCPL : .NET Core 8.0.8 (CoreCLR 8.0.824.36612, CoreFX 8.0.824.36612), X64 RyuJIT

IterationCount=1  LaunchCount=1  WarmupCount=1

|         Method | EdgeCount |       Mean | Error |     Gen 0 |     Gen 1 |     Gen 2 | Allocated |
|--------------- |---------- |-----------:|------:|----------:|----------:|----------:|----------:|
| Intersection_N |      1000 |   127.1 ms |    NA |         - |         - |         - |   6.85 MB |
| Intersection_N |      2000 |   475.1 ms |    NA | 2000.0000 | 1000.0000 |         - |  23.04 MB |
| Intersection_N |      3000 | 1,408.1 ms |    NA | 5000.0000 | 4000.0000 | 1000.0000 |  36.15 MB |
| Intersection_N |      4000 | 2,874.6 ms |    NA | 5000.0000 | 4000.0000 | 1000.0000 |  41.76 MB |
| Intersection_N |      5000 | 5,954.3 ms |    NA | 9000.0000 | 8000.0000 | 2000.0000 |  68.62 MB |

// * Legends *
  EdgeCount : Value of the 'EdgeCount' parameter
  Mean      : Arithmetic mean of all measurements
  Error     : Half of 99.9% confidence interval
  Gen 0     : GC Generation 0 collects per 1000 operations
  Gen 1     : GC Generation 1 collects per 1000 operations
  Gen 2     : GC Generation 2 collects per 1000 operations
  Allocated : Allocated memory per single operation (managed only, inclusive, 1KB = 1024B)
  1 ms      : 1 Millisecond (0.001 sec)

// * Diagnostic Output - MemoryDiagnoser *

// ***** BenchmarkRunner: End *****
// ** Remained 0 benchmark(s) to run **
Run time: 00:00:46 (46.34 sec), executed benchmarks: 5

Global total time: 00:00:51 (51.28 sec), executed benchmarks: 5

* Oops - roll back the net version for this project to work on GitHub
  • Loading branch information
philstopford authored Sep 30, 2024
1 parent 304235b commit 907d1fd
Show file tree
Hide file tree
Showing 8 changed files with 95 additions and 189 deletions.
19 changes: 5 additions & 14 deletions CSharp/Clipper2Lib/Clipper.Core.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#nullable enable
using System;
using System.Collections.Generic;
using System.Linq;
using System.Runtime.CompilerServices;

namespace Clipper2Lib
Expand Down Expand Up @@ -478,9 +479,7 @@ public Path64(int capacity = 0) : base(capacity) { }
public Path64(IEnumerable<Point64> path) : base(path) { }
public override string ToString()
{
string s = "";
foreach (Point64 p in this)
s = s + p.ToString() + ", ";
string s = this.Aggregate("", (current, p) => current + p.ToString() + ", ");
if (s != "") s = s.Remove(s.Length - 2);
return s;
}
Expand All @@ -493,10 +492,7 @@ public Paths64(int capacity = 0) : base(capacity) { }
public Paths64(IEnumerable<Path64> paths) : base(paths) { }
public override string ToString()
{
string s = "";
foreach (Path64 p in this)
s = s + p + "\n";
return s;
return this.Aggregate("", (current, p) => current + p + "\n");
}
}

Expand All @@ -507,9 +503,7 @@ public PathD(int capacity = 0) : base(capacity) { }
public PathD(IEnumerable<PointD> path) : base(path) { }
public string ToString(int precision = 2)
{
string s = "";
foreach (PointD p in this)
s = s + p.ToString(precision) + ", ";
string s = this.Aggregate("", (current, p) => current + p.ToString(precision) + ", ");
if (s != "") s = s.Remove(s.Length - 2);
return s;
}
Expand All @@ -522,10 +516,7 @@ public PathsD(int capacity = 0) : base(capacity) { }
public PathsD(IEnumerable<PathD> paths) : base(paths) { }
public string ToString(int precision = 2)
{
string s = "";
foreach (PathD p in this)
s = s + p.ToString(precision) + "\n";
return s;
return this.Aggregate("", (current, p) => current + p.ToString(precision) + "\n");
}
}

Expand Down
42 changes: 12 additions & 30 deletions CSharp/Clipper2Lib/Clipper.Engine.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
using System;
using System.Collections;
using System.Collections.Generic;
using System.Linq;
using System.Runtime.CompilerServices;

namespace Clipper2Lib
Expand Down Expand Up @@ -235,8 +236,7 @@ internal static void EnsureCapacity<T>(this List<T> list, int minCapacity)
internal static void AddPathsToVertexList(Paths64 paths, PathType polytype, bool isOpen,
List<LocalMinima> minimaList, List<Vertex> vertexList)
{
int totalVertCnt = 0;
foreach (Path64 path in paths) totalVertCnt += path.Count;
int totalVertCnt = paths.Sum(path => path.Count);
vertexList.EnsureCapacity(vertexList.Count + totalVertCnt);

foreach (Path64 path in paths)
Expand Down Expand Up @@ -2100,7 +2100,7 @@ private void AddToHorzSegList(OutPt op)
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
private OutPt GetLastOp(Active hotEdge)
private static OutPt GetLastOp(Active hotEdge)
{
OutRec outrec = hotEdge.outrec!;
return (hotEdge == outrec.frontEdge) ?
Expand Down Expand Up @@ -2514,7 +2514,7 @@ private static OutPt DuplicateOp(OutPt op, bool insert_after)
return result;
}

private int HorzSegSort(HorzSegment? hs1, HorzSegment? hs2)
private static int HorzSegSort(HorzSegment? hs1, HorzSegment? hs2)
{
if (hs1 == null || hs2 == null) return 0;
if (hs1.rightOp == null)
Expand All @@ -2528,9 +2528,7 @@ private int HorzSegSort(HorzSegment? hs1, HorzSegment? hs2)

private void ConvertHorzSegsToJoins()
{
int k = 0;
foreach (HorzSegment hs in _horzSegList)
if (UpdateHorzSegment(hs)) k++;
int k = _horzSegList.Count(UpdateHorzSegment);
if (k < 2) return;
_horzSegList.Sort(HorzSegSort);

Expand Down Expand Up @@ -3034,10 +3032,8 @@ private bool CheckBounds(OutRec outrec)

private bool CheckSplitOwner(OutRec outrec, List<int>? splits)
{
foreach (int i in splits!)
foreach (OutRec? split in splits!.Select(i => GetRealOutRec(_outrecList[i])).OfType<OutRec>().Where(split => split != outrec && split.recursiveSplit != outrec))
{
OutRec? split = GetRealOutRec(_outrecList[i]);
if (split == null || split == outrec || split.recursiveSplit == outrec) continue;
split.recursiveSplit = outrec; //#599
if (split.splits != null && CheckSplitOwner(outrec, split.splits)) return true;
if (!IsValidOwner(outrec, split) ||
Expand All @@ -3047,6 +3043,7 @@ private bool CheckSplitOwner(OutRec outrec, List<int>? splits)
outrec.owner = split; //found in split
return true;
}

return false;
}
private void RecursiveCheckOwners(OutRec outrec, PolyPathBase polypath)
Expand Down Expand Up @@ -3346,11 +3343,9 @@ public bool Execute(ClipType clipType, FillRule fillRule,
if (!success) return false;

solutionClosed.EnsureCapacity(solClosed64.Count);
foreach (Path64 path in solClosed64)
solutionClosed.Add(Clipper.ScalePathD(path, _invScale));
solutionClosed.AddRange(solClosed64.Select(path => Clipper.ScalePathD(path, _invScale)));
solutionOpen.EnsureCapacity(solOpen64.Count);
foreach (Path64 path in solOpen64)
solutionOpen.Add(Clipper.ScalePathD(path, _invScale));
solutionOpen.AddRange(solOpen64.Select(path => Clipper.ScalePathD(path, _invScale)));

return true;
}
Expand Down Expand Up @@ -3385,8 +3380,7 @@ public bool Execute(ClipType clipType, FillRule fillRule, PolyTreeD polytree, Pa
if (!success) return false;
if (oPaths.Count <= 0) return true;
openPaths.EnsureCapacity(oPaths.Count);
foreach (Path64 path in oPaths)
openPaths.Add(Clipper.ScalePathD(path, _invScale));
openPaths.AddRange(oPaths.Select(path => Clipper.ScalePathD(path, _invScale)));

return true;
}
Expand Down Expand Up @@ -3540,13 +3534,7 @@ public PolyPath64 Child(int index)
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public double Area()
{
double result = Polygon == null ? 0 : Clipper.Area(Polygon);
foreach (PolyPathBase polyPathBase in _childs)
{
PolyPath64 child = (PolyPath64) polyPathBase;
result += child.Area();
}
return result;
return (Polygon == null ? 0 : Clipper.Area(Polygon)) + _childs.Cast<PolyPath64>().Sum(child => child.Area());
}
}
public class PolyPathD : PolyPathBase
Expand Down Expand Up @@ -3589,13 +3577,7 @@ public PolyPathD this[int index]
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public double Area()
{
double result = Polygon == null ? 0 : Clipper.Area(Polygon);
foreach (PolyPathBase polyPathBase in _childs)
{
PolyPathD child = (PolyPathD) polyPathBase;
result += child.Area();
}
return result;
return (Polygon == null ? 0 : Clipper.Area(Polygon)) + _childs.Cast<PolyPathD>().Sum(child => child.Area());
}
}

Expand Down
7 changes: 3 additions & 4 deletions CSharp/Clipper2Lib/Clipper.Minkowski.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

#nullable enable
using System;
using System.Linq;

namespace Clipper2Lib
{
Expand All @@ -25,13 +26,11 @@ private static Paths64 MinkowskiInternal(Path64 pattern, Path64 path, bool isSum
Path64 path2 = new Path64(patLen);
if (isSum)
{
foreach (Point64 basePt in pattern)
path2.Add(pathPt + basePt);
path2.AddRange(pattern.Select(basePt => pathPt + basePt));
}
else
{
foreach (Point64 basePt in pattern)
path2.Add(pathPt - basePt);
path2.AddRange(pattern.Select(basePt => pathPt - basePt));
}
tmp.Add(path2);
}
Expand Down
27 changes: 8 additions & 19 deletions CSharp/Clipper2Lib/Clipper.Offset.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

using System;
using System.Collections.Generic;
using System.Linq;
using System.Runtime.CompilerServices;

namespace Clipper2Lib
Expand Down Expand Up @@ -140,22 +141,12 @@ public void AddPaths(Paths64 paths, JoinType joinType, EndType endType)

private int CalcSolutionCapacity()
{
int result = 0;
foreach (Group g in _groupList)
result += (g.endType == EndType.Joined) ? g.inPaths.Count * 2 : g.inPaths.Count;
return result;
return _groupList.Sum(g => (g.endType == EndType.Joined) ? g.inPaths.Count * 2 : g.inPaths.Count);
}

internal bool CheckPathsReversed()
{
bool result = false;
foreach (Group g in _groupList)
if (g.endType == EndType.Polygon)
{
result = g.pathsReversed;
break;
}
return result;
return (from g in _groupList where g.endType == EndType.Polygon select g.pathsReversed).FirstOrDefault();
}

private void ExecuteInternal(double delta)
Expand All @@ -166,9 +157,8 @@ private void ExecuteInternal(double delta)
// make sure the offset delta is significant
if (Math.Abs(delta) < 0.5)
{
foreach (Group group in _groupList)
foreach (Path64 path in group.inPaths)
_solution.Add(path);
foreach (Path64 path in _groupList.SelectMany(group => group.inPaths))
_solution.Add(path);
return;
}

Expand Down Expand Up @@ -240,10 +230,9 @@ internal static int GetLowestPathIdx(Paths64 paths)
Point64 botPt = new Point64(long.MaxValue, long.MinValue);
for (int i = 0; i < paths.Count; ++i)
{
foreach (Point64 pt in paths[i])
{
if ((pt.Y < botPt.Y) ||
((pt.Y == botPt.Y) && (pt.X >= botPt.X))) continue;
foreach (Point64 pt in paths[i].Where(pt => (pt.Y >= botPt.Y) &&
((pt.Y != botPt.Y) || (pt.X < botPt.X))))
{
result = i;
botPt.X = pt.X;
botPt.Y = pt.Y;
Expand Down
28 changes: 8 additions & 20 deletions CSharp/Clipper2Lib/Clipper.RectClip.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#nullable enable
using System;
using System.Collections.Generic;
using System.Linq;
using System.Runtime.CompilerServices;

namespace Clipper2Lib
Expand Down Expand Up @@ -91,10 +92,8 @@ private static bool Path1ContainsPath2(Path64 path1, Path64 path2)
// nb: occasionally, due to rounding, path1 may
// appear (momentarily) inside or outside path2.
int ioCount = 0;
foreach (Point64 pt in path2)
foreach (PointInPolygonResult pip in path2.Select(pt => InternalClipper.PointInPolygon(pt, path1)))
{
PointInPolygonResult pip =
InternalClipper.PointInPolygon(pt, path1);
switch(pip)
{
case PointInPolygonResult.IsInside:
Expand Down Expand Up @@ -482,7 +481,7 @@ protected void GetNextLocation(Path64 path,
} // switch
}

private bool StartLocsAreClockwise(List<Location> startLocs)
private static bool StartLocsAreClockwise(List<Location> startLocs)
{
int result = 0;
for (int i = 1; i < startLocs.Count; i++)
Expand Down Expand Up @@ -637,9 +636,8 @@ private void ExecuteInternal(Path64 path)
if (startLocs.Count > 0)
{
prev = loc;
foreach (Location loc2 in startLocs)
foreach (Location loc2 in startLocs.Where(loc2 => prev != loc2))
{
if (prev == loc2) continue;
AddCorner(ref prev, HeadingClockwise(prev, loc2));
prev = loc2;
}
Expand All @@ -654,9 +652,8 @@ public Paths64 Execute(Paths64 paths)
{
Paths64 result = new Paths64();
if (rect_.IsEmpty()) return result;
foreach (Path64 path in paths)
foreach (Path64 path in paths.Where(path => path.Count >= 3))
{
if (path.Count < 3) continue;
pathBounds_ = Clipper.GetBounds(path);
if (!rect_.Intersects(pathBounds_))
continue; // the path must be completely outside fRect
Expand All @@ -671,11 +668,7 @@ public Paths64 Execute(Paths64 paths)
for (int i = 0; i < 4; ++i)
TidyEdgePair(i, edges_[i * 2], edges_[i * 2 + 1]);

foreach (OutPt2? op in results_)
{
Path64 tmp = GetPath(op);
if (tmp.Count > 0) result.Add(tmp);
}
result.AddRange(results_.Select(GetPath).Where(tmp => tmp.Count > 0));

//clean up after every loop
results_.Clear();
Expand Down Expand Up @@ -962,9 +955,8 @@ internal RectClipLines64(Rect64 rect) : base(rect) { }
{
Paths64 result = new Paths64();
if (rect_.IsEmpty()) return result;
foreach (Path64 path in paths)
foreach (Path64 path in paths.Where(path => path.Count >= 2))
{
if (path.Count < 2) continue;
pathBounds_ = Clipper.GetBounds(path);
if (!rect_.Intersects(pathBounds_))
continue; // the path must be completely outside fRect
Expand All @@ -973,11 +965,7 @@ internal RectClipLines64(Rect64 rect) : base(rect) { }
// fRect, simply by comparing path bounds with fRect.
ExecuteInternal(path);

foreach (OutPt2? op in results_)
{
Path64 tmp = GetPath(op);
if (tmp.Count > 0) result.Add(tmp);
}
result.AddRange(results_.Select(GetPath).Where(tmp => tmp.Count > 0));

//clean up after every loop
results_.Clear();
Expand Down
Loading

0 comments on commit 907d1fd

Please sign in to comment.