-
Notifications
You must be signed in to change notification settings - Fork 275
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Voronoi diagrams #692
Voronoi diagrams #692
Changes from 16 commits
f0b9f96
543f89a
08dbde2
c9da51b
3a0a12d
4d3f34c
e146763
4cd5095
83c9136
c33fd4d
e000330
b7d6b14
368f2cb
2dfe1a8
bd4babb
c00bec6
1b452dc
3fa9d8f
4955709
53f5bcb
1aab816
38ca138
73946e6
4fa83a6
d85079e
63ed0cd
44b3fba
c1a2ce9
b8ca36a
313f7ee
0f772d2
aa26e5a
c91da94
e34c9b1
d354929
9b44f4e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,284 @@ | ||
using System; | ||
using System.Collections.Generic; | ||
using System.Collections.Immutable; | ||
using System.ComponentModel; | ||
using System.Linq; | ||
using Cairo; | ||
using Pinta.Core; | ||
using Pinta.Gui.Widgets; | ||
|
||
namespace Pinta.Effects; | ||
|
||
public sealed class VoronoiDiagramEffect : BaseEffect | ||
{ | ||
// TODO: Icon | ||
|
||
public override bool IsTileable => false; | ||
|
||
public override string Name => Translations.GetString ("Voronoi Diagram"); | ||
|
||
public override bool IsConfigurable => true; | ||
|
||
public override string EffectMenuCategory => Translations.GetString ("Render"); | ||
|
||
public VoronoiDiagramData Data => (VoronoiDiagramData) EffectData!; // NRT - Set in constructor | ||
|
||
public VoronoiDiagramEffect () | ||
{ | ||
EffectData = new VoronoiDiagramData (); | ||
} | ||
|
||
public override void LaunchConfiguration () | ||
=> EffectHelper.LaunchSimpleEffectDialog (this); | ||
|
||
private sealed record VoronoiSettings ( | ||
int w, | ||
int h, | ||
bool showPoints, | ||
ImmutableArray<PointI> points, | ||
ImmutableArray<ColorBgra> colors, | ||
Func<PointI, PointI, double> distanceCalculator); | ||
|
||
private VoronoiSettings CreateSettings (ImageSurface dst, RectangleI roi) | ||
{ | ||
ColorSorting colorSorting = Data.ColorSorting; | ||
|
||
IEnumerable<PointI> basePoints = CreatePoints (roi, Data.PointCount, Data.PointLocationsSeed); | ||
ImmutableArray<PointI> points = SortPoints (basePoints, colorSorting).ToImmutableArray (); | ||
|
||
IEnumerable<ColorBgra> baseColors = CreateColors (points.Length, Data.ColorsSeed); | ||
IEnumerable<ColorBgra> positionSortedColors = SortColors (baseColors, colorSorting); | ||
IEnumerable<ColorBgra> reversedSortingColors = Data.ReverseColorSorting ? positionSortedColors.Reverse () : positionSortedColors; | ||
|
||
return new ( | ||
w: dst.Width, | ||
h: dst.Height, | ||
showPoints: Data.ShowPoints, | ||
points: points, | ||
colors: reversedSortingColors.ToImmutableArray (), | ||
distanceCalculator: GetDistanceCalculator (Data.DistanceCalculationMethod) | ||
); | ||
} | ||
|
||
protected override void Render (ImageSurface src, ImageSurface dst, RectangleI roi) | ||
{ | ||
VoronoiSettings settings = CreateSettings (dst, roi); | ||
|
||
Span<ColorBgra> dst_data = dst.GetPixelData (); | ||
|
||
for (int y = roi.Top; y <= roi.Bottom; y++) { | ||
var dst_row = dst_data.Slice (y * settings.w, settings.w); | ||
for (int x = roi.Left; x <= roi.Right; x++) { | ||
PointI pixelLocation = new (x, y); | ||
double shortestDistance = double.MaxValue; | ||
int closestIndex = 0; | ||
for (var i = 0; i < settings.points.Length; i++) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is probably more something to leave a TODO comment for future work, but adding some acceleration structure here could improve performance when there is a large number of points |
||
var point = settings.points[i]; | ||
double distance = settings.distanceCalculator (point, pixelLocation); | ||
if (distance > shortestDistance) continue; | ||
shortestDistance = distance; | ||
closestIndex = i; | ||
} | ||
dst_row[x] = | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Only using the closest point can produce jagged edges, so you may want to consider doing some sort of blending / antialiasing There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe this would require either changing the algorithm used to make the calculations, or applying some sort edge blur effect (that is not yet developed, as far as I know)? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It could probably be done easily with supersampling, e.g. compute the color at multiple sub-pixel positions and blend - currently the sampling is done at only the upper left corner of the pixel Most pixels don't need this though since there are large solid regions, so this could be skipped if there isn't another voronoi point close enough to be the closest to any sub pixel location There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can this be done in a future PR? A change introduced in this PR (the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah I think this is fine to add in a future PR |
||
settings.showPoints && shortestDistance == 0 | ||
? ColorBgra.Black | ||
: settings.colors[closestIndex]; | ||
} | ||
} | ||
} | ||
|
||
private static IEnumerable<ColorBgra> SortColors (IEnumerable<ColorBgra> baseColors, ColorSorting colorSorting) | ||
|
||
=> colorSorting switch { | ||
|
||
ColorSorting.Random => baseColors, | ||
|
||
ColorSorting.HorizontalBGR | ||
or ColorSorting.VerticalBGR => baseColors.OrderBy (p => p.B).ThenBy (p => p.G).ThenBy (p => p.R), | ||
|
||
ColorSorting.HorizontalBRG | ||
or ColorSorting.VerticalBRG => baseColors.OrderBy (p => p.B).ThenBy (p => p.R).ThenBy (p => p.G), | ||
|
||
ColorSorting.HorizontalGBR | ||
or ColorSorting.VerticalGBR => baseColors.OrderBy (p => p.G).ThenBy (p => p.B).ThenBy (p => p.R), | ||
|
||
ColorSorting.HorizontalGRB | ||
or ColorSorting.VerticalGRB => baseColors.OrderBy (p => p.G).ThenBy (p => p.R).ThenBy (p => p.B), | ||
|
||
ColorSorting.HorizontalRBG | ||
or ColorSorting.VerticalRBG => baseColors.OrderBy (p => p.R).ThenBy (p => p.B).ThenBy (p => p.G), | ||
|
||
ColorSorting.HorizontalRGB | ||
or ColorSorting.VerticalRGB => baseColors.OrderBy (p => p.R).ThenBy (p => p.G).ThenBy (p => p.B), | ||
|
||
_ => throw new InvalidEnumArgumentException ( | ||
nameof (baseColors), | ||
(int) colorSorting, | ||
typeof (ColorSorting)), | ||
}; | ||
|
||
private static IEnumerable<PointI> SortPoints (IEnumerable<PointI> basePoints, ColorSorting colorSorting) | ||
|
||
=> colorSorting switch { | ||
|
||
ColorSorting.Random => basePoints, | ||
|
||
ColorSorting.HorizontalBGR | ||
or ColorSorting.HorizontalBRG | ||
or ColorSorting.HorizontalGBR | ||
or ColorSorting.HorizontalGRB | ||
or ColorSorting.HorizontalRBG | ||
or ColorSorting.HorizontalRGB => basePoints.OrderBy (p => p.X).ThenBy (p => p.Y), | ||
|
||
ColorSorting.VerticalBGR | ||
or ColorSorting.VerticalBRG | ||
or ColorSorting.VerticalGBR | ||
or ColorSorting.VerticalGRB | ||
or ColorSorting.VerticalRBG | ||
or ColorSorting.VerticalRGB => basePoints.OrderBy (p => p.Y).ThenBy (p => p.X), | ||
|
||
_ => throw new InvalidEnumArgumentException ( | ||
nameof (colorSorting), | ||
(int) colorSorting, | ||
typeof (ColorSorting)), | ||
}; | ||
|
||
private static Func<PointI, PointI, double> GetDistanceCalculator (DistanceCalculationMethod distanceCalculationMethod) | ||
{ | ||
return distanceCalculationMethod switch { | ||
DistanceCalculationMethod.Euclidean => Euclidean, | ||
DistanceCalculationMethod.Manhattan => Manhattan, | ||
DistanceCalculationMethod.Chebyshev => Chebyshev, | ||
_ => throw new InvalidEnumArgumentException ( | ||
nameof (distanceCalculationMethod), | ||
(int) distanceCalculationMethod, | ||
typeof (DistanceCalculationMethod)), | ||
}; | ||
|
||
static double Euclidean (PointI targetPoint, PointI pixelLocation) | ||
{ | ||
PointI difference = pixelLocation - targetPoint; | ||
return difference.Magnitude (); | ||
} | ||
|
||
static double Manhattan (PointI targetPoint, PointI pixelLocation) | ||
{ | ||
PointI difference = pixelLocation - targetPoint; | ||
return Math.Abs (difference.X) + Math.Abs (difference.Y); | ||
} | ||
|
||
static double Chebyshev (PointI targetPoint, PointI pixelLocation) | ||
{ | ||
PointI difference = pixelLocation - targetPoint; | ||
return Math.Max (Math.Abs (difference.X), Math.Abs (difference.Y)); | ||
} | ||
} | ||
|
||
private static ImmutableHashSet<PointI> CreatePoints (RectangleI roi, int pointCount, RandomSeed pointLocationsSeed) | ||
{ | ||
int effectivePointCount = Math.Min (pointCount, roi.Width * roi.Height); | ||
|
||
Random randomPositioner = new (pointLocationsSeed.Value); | ||
var result = ImmutableHashSet.CreateBuilder<PointI> (); // Ensures points' uniqueness | ||
|
||
while (result.Count < effectivePointCount) { | ||
|
||
PointI point = new ( | ||
X: randomPositioner.Next (roi.Left, roi.Right + 1), | ||
Y: randomPositioner.Next (roi.Top, roi.Bottom + 1) | ||
); | ||
|
||
result.Add (point); | ||
} | ||
|
||
return result.ToImmutable (); | ||
} | ||
|
||
private static IEnumerable<ColorBgra> CreateColors (int colorCount, RandomSeed colorsSeed) | ||
{ | ||
Random randomColorizer = new (colorsSeed.Value); | ||
HashSet<ColorBgra> uniquenessTracker = new (); | ||
while (uniquenessTracker.Count < colorCount) { | ||
ColorBgra candidateColor = randomColorizer.RandomColorBgra (); | ||
if (uniquenessTracker.Contains (candidateColor)) continue; | ||
uniquenessTracker.Add (candidateColor); | ||
yield return candidateColor; | ||
} | ||
} | ||
|
||
public sealed class VoronoiDiagramData : EffectData | ||
{ | ||
[Caption ("Distance Calculation Method")] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd suggest calling this There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I went ahead and called it |
||
public DistanceCalculationMethod DistanceCalculationMethod { get; set; } = DistanceCalculationMethod.Euclidean; | ||
|
||
[Caption ("Point Count"), MinimumValue (1), MaximumValue (1024)] | ||
public int PointCount { get; set; } = 100; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would something like There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I named it |
||
|
||
// Translators: The user can choose whether or not to render the points used in the calculation of a Voronoi diagram | ||
[Caption ("Show Points")] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are there good use cases for this to be exposed to users? Seems more like a debugging option to me at first glance There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It could be an educational tool, so that they see that the cells are centered around the points, and then it piques their curiosity and then they look it up |
||
public bool ShowPoints { get; set; } = false; | ||
|
||
[Caption ("Color Sorting")] | ||
public ColorSorting ColorSorting { get; set; } = ColorSorting.Random; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This might need some more thought on how to present it to the user. The effect is neat, but the huge menu of combinations seems intimidating |
||
|
||
// Translators: In this context, "reverse" is a verb, and the user can choose whether or not they want to reverse the color sorting | ||
[Caption ("Reverse Color Sorting")] | ||
public bool ReverseColorSorting { get; set; } = false; | ||
|
||
[Caption ("Colors Seed")] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd maybe just call this There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I went ahead and called that |
||
public RandomSeed ColorsSeed { get; set; } = new (0); | ||
|
||
[Caption ("Point Locations Seed")] | ||
public RandomSeed PointLocationsSeed { get; set; } = new (0); | ||
} | ||
|
||
public enum DistanceCalculationMethod | ||
{ | ||
Euclidean, | ||
Manhattan, | ||
Chebyshev, | ||
} | ||
|
||
public enum ColorSorting | ||
{ | ||
[Caption ("Random Color Sorting")] Random, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you change this just to "Random"? |
||
|
||
|
||
// Translators: Horizontal color sorting with B, then G as the leading terms | ||
[Caption ("Horizontal (B, G, R)")] HorizontalBGR, | ||
|
||
// Translators: Horizontal color sorting with B, then R as the leading terms | ||
[Caption ("Horizontal (B, R, G)")] HorizontalBRG, | ||
|
||
// Translators: Horizontal color sorting with G, then B as the leading terms | ||
[Caption ("Horizontal (G, B, R)")] HorizontalGBR, | ||
|
||
// Translators: Horizontal color sorting with G, then R as the leading terms | ||
[Caption ("Horizontal (G, R, B)")] HorizontalGRB, | ||
|
||
// Translators: Horizontal color sorting with R, then B as the leading terms | ||
[Caption ("Horizontal (R, B, G)")] HorizontalRBG, | ||
|
||
// Translators: Horizontal color sorting with R, then G as the leading terms | ||
[Caption ("Horizontal (R, G, B)")] HorizontalRGB, | ||
|
||
|
||
// Translators: Vertical color sorting with B, then G as the leading terms | ||
[Caption ("Vertical (B, G, R)")] VerticalBGR, | ||
|
||
// Translators: Vertical color sorting with B, then R as the leading terms | ||
[Caption ("Vertical (B, R, G)")] VerticalBRG, | ||
|
||
// Translators: Vertical color sorting with G, then B as the leading terms | ||
[Caption ("Vertical (G, B, R)")] VerticalGBR, | ||
|
||
// Translators: Vertical color sorting with G, then R as the leading terms | ||
[Caption ("Vertical (G, R, B)")] VerticalGRB, | ||
|
||
// Translators: Vertical color sorting with R, then B as the leading terms | ||
[Caption ("Vertical (R, B, G)")] VerticalRBG, | ||
|
||
// Translators: Vertical color sorting with R, then G as the leading terms | ||
[Caption ("Vertical (R, G, B)")] VerticalRGB, | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The effect is non-fileable because of
CreateSettings
, correct? This loop could still be done in parallel for better performanceThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not only that, there is more to it.
Most effects use the entire canvas as the area where the effect is rendered, and the ROIs as the parts of the area that they have to fill.
On the other hand, in this effect, the ROI is the area, and the calculations are done based on the ROI.
For a more concrete example, open a photo with Pinta, select a rectangle area, and apply the twist effect to it. The center of the twist won't be the center of the selection, but the center of the canvas.
Maybe we should re-engineer the rendering and the tileability mechanisms now that the rendering API can still be changed, and adapt the algorithms so that they can act on either the entire canvas or the overall ROI (while still being executable in parallel).
Also, just to clarify, I understand what you mean: once we have the list of points and the list of colors, the color of each pixel in the rendering can be calculated independently, which means it can all be done in parallel. That much is clear... I just think it's essential to address the other part of it as well.
I dare say that we could delay the plugin feature by one release if that means that we get the API right. The current one is just very limited, and just like support for non-tileable effects required a change in the API, I am sure there are many other things that just are not possible as of now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree that the API probably needs further thought to solve this in a better way.
For now I'd suggest still adding a Parallel.For loop to improve performance - the only downside is just that it won't be able to hook into the live preview to incrementally render tiles as they're completed.
FYI the next Pinta release is hopefully not too far away - basically once #674 is figured out I plan to start locking things down. There still can be API changes in the future if we decide they're necessary
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's being done with
Parallel
now (with some adjustments, as aReadOnlySpan<T>
cannot be referenced inside a lambda. There is a noticeable speed improvement in my machine.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
great, yeah it seems much faster on my machine too 👍