Voronoi diagrams #692

Lehonti · 2024-01-14T12:52:11Z

Here's with Euclidean distance calculation:

And here's with Manhattan distance calculation:

For now it's only raster-based, but maybe at some point we could add a different algorithm that allows us to draw it based on shapes, and then having Cairo put everything together.

~~There may be a merge conflict in CoreEffectsExtension if we were to merge both this one and #457~~

Thank you for adding support for non-tileable effects, which made this possible.

Lehonti · 2024-01-14T16:49:18Z

I just made sure there aren't any potential merge conflicts

Lehonti · 2024-01-14T17:18:16Z

I just noticed that, with the current code, some colors tend to be to the left and some to the right. This is unintended, but at the same time it's cool, so it could be added as an option (besides the option of truly randomizing the position of the colors)

Lehonti · 2024-01-14T18:11:40Z

...And now the option has been added, along with the relevant captions and comments for translators

`VoronoiSettings`

Not leaving the ordering of non-leading component to chance

cameronwhite

Looks cool!

I left a few comments from doing some initial testing. Please also add a unit test for this too when ready :)

cameronwhite · 2024-01-15T02:48:00Z

Pinta.Effects/Effects/VoronoiDiagramEffect.cs

+
+		Span<ColorBgra> dst_data = dst.GetPixelData ();
+
+		for (int y = roi.Top; y <= roi.Bottom; y++) {


The effect is non-fileable because of CreateSettings, correct? This loop could still be done in parallel for better performance

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.

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

It's being done with Parallel now (with some adjustments, as a ReadOnlySpan<T> cannot be referenced inside a lambda. There is a noticeable speed improvement in my machine.

great, yeah it seems much faster on my machine too 👍

cameronwhite · 2024-01-15T02:50:01Z

Pinta.Effects/Effects/VoronoiDiagramEffect.cs

+				PointI pixelLocation = new (x, y);
+				double shortestDistance = double.MaxValue;
+				int closestIndex = 0;
+				for (var i = 0; i < settings.points.Length; i++) {


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

cameronwhite · 2024-01-15T02:57:23Z

Pinta.Effects/Effects/VoronoiDiagramEffect.cs

+
+	public sealed class VoronoiDiagramData : EffectData
+	{
+		[Caption ("Distance Calculation Method")]


I'd suggest calling this Distance Metric or Distance Method to be a bit more concise

I went ahead and called it Distance Metric

cameronwhite · 2024-01-15T02:58:35Z

Pinta.Effects/Effects/VoronoiDiagramEffect.cs

+		public DistanceCalculationMethod DistanceCalculationMethod { get; set; } = DistanceCalculationMethod.Euclidean;
+
+		[Caption ("Point Count"), MinimumValue (1), MaximumValue (1024)]
+		public int PointCount { get; set; } = 100;


Would something like Number of Regions make more sense to a user, since they don't see the point positions?

I named it Number of Cells

cameronwhite · 2024-01-15T03:00:22Z

Pinta.Effects/Effects/VoronoiDiagramEffect.cs

+		public int PointCount { get; set; } = 100;
+
+		// Translators: The user can choose whether or not to render the points used in the calculation of a Voronoi diagram
+		[Caption ("Show Points")]


Are there good use cases for this to be exposed to users? Seems more like a debugging option to me at first glance

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

cameronwhite · 2024-01-15T03:02:00Z

Pinta.Effects/Effects/VoronoiDiagramEffect.cs

+		public bool ShowPoints { get; set; } = false;
+
+		[Caption ("Color Sorting")]
+		public ColorSorting ColorSorting { get; set; } = ColorSorting.Random;


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
The menu may change, but I'll also mention that Random Color Sorting could just be Random since it's inside a menu labelled Color Sorting

cameronwhite · 2024-01-15T03:03:36Z

Pinta.Effects/Effects/VoronoiDiagramEffect.cs

+		[Caption ("Reverse Color Sorting")]
+		public bool ReverseColorSorting { get; set; } = false;
+
+		[Caption ("Colors Seed")]


I'd maybe just call this Random Colors since the Seed is mentioned by the "Reseed" button underneath, and similar for the other seed below

I went ahead and called that Random Colors, and similarly for the points Random Point Locations

cameronwhite · 2024-01-15T03:09:57Z

Pinta.Effects/Effects/VoronoiDiagramEffect.cs

+					shortestDistance = distance;
+					closestIndex = i;
+				}
+				dst_row[x] =


Only using the closest point can produce jagged edges, so you may want to consider doing some sort of blending / antialiasing

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)?

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

Can this be done in a future PR? A change introduced in this PR (the Utility.GeneratePixelOffsets() method) is useful for other effects, too, and there's no risk of breaking an API if we add to this effect in the future.

Yeah I think this is fine to add in a future PR

as per suggestion

This reverts commit 4fa83a6.

Lehonti · 2024-01-18T11:47:38Z

Now the merge conflicts have been solved

cameronwhite · 2024-01-19T00:15:59Z

👍 I think this looks good, the only thing I'm not sure about is the big menu of color sorting options which still feels pretty awkward to me

Lehonti · 2024-01-19T00:18:20Z

@cameronwhite what changes do you suggest? I could just reduce them to 3 in each direction (r, g, or b horizontally, and the same vertically). Would that work?

cameronwhite · 2024-01-19T00:25:52Z

yeah something like that sounds simpler. I'm mostly just worried about it getting even more complicated if we want to add more options in the future
e.g. it could be useful to also just do an outline with the current palette color instead of filling with random colours

Lehonti · 2024-01-19T00:35:40Z

I've now reduced the number of sortings.

As for your last comment, do you mean an outline as in drawing a border around the cells? That is definitely harder than just filling the cells with colors.

It's possible that you mean filling the cells with the colors currently in the palette, and that could be done, it's just that it's very likely that at least a pair (or a group) of adjacent cells will have the same color.

cameronwhite · 2024-01-20T03:59:23Z

Pinta.Effects/Effects/VoronoiDiagramEffect.cs

+
+	public enum ColorSorting
+	{
+		[Caption ("Random Color Sorting")] Random,


Could you change this just to "Random"?
The "color sorting" isn't really necessary since the menu is also called "Color Sorting"

cameronwhite · 2024-01-20T04:00:30Z

Pinta.Effects/Effects/VoronoiDiagramEffect.cs

+	{
+		[Caption ("Random Color Sorting")] Random,
+
+		// Translators: Horizontal color sorting with B as the leading term


For this group of comments, could you change phrases like B to something like blue (B)?
Translators might not be fully aware of the context of what the individual R/G/B characters mean here

No problem, it has been done now

cameronwhite · 2024-01-20T04:03:25Z

As for your last comment, do you mean an outline as in drawing a border around the cells? That is definitely harder than just filling the cells with colors.

That is what I meant, but I agree it would be more work to implement and isn't in the scope of this PR

cameronwhite · 2024-01-21T03:55:42Z

Looks good, thanks for working on this!

Lehonti added 5 commits January 14, 2024 13:41

Initial implementation

f0b9f96

Added it to CoreEffectsExtension

543f89a

Assigned distance

08dbde2

Reduced number of points

c9da51b

Merge branch 'PintaProject:master' into feature/voronoi

3a0a12d

Lehonti added 2 commits January 14, 2024 17:56

Using immutable structures

4d3f34c

Added Chebyshev method of distance calculation

e146763

Lehonti added 3 commits January 14, 2024 18:44

Added color sorting

4cd5095

Added labels to sorting options

83c9136

Added notes for translators, and added option to invert sorting

c33fd4d

Lehonti added 6 commits January 14, 2024 19:13

Removed empty comment for translators

e000330

stored values in more variables

b7d6b14

Created new record type for settings

368f2cb

`VoronoiSettings`

Added option to show points

2dfe1a8

Local refactoring and reformatting

bd4babb

Ordering of points

c00bec6

Not leaving the ordering of non-leading component to chance

cameronwhite reviewed Jan 15, 2024

View reviewed changes

Lehonti added 10 commits January 15, 2024 09:01

Merge branch 'PintaProject:master' into feature/voronoi

1b452dc

Changed naming and added TODO comment

3fa9d8f

as per suggestion

Added tests

4955709

Removed property assignment

53f5bcb

Constructor now gets a service manager

1aab816

Created Utility.GeneratePixelOffsets

38ca138

Creating pixels in parallel

73946e6

Removed intermediate dictionary structure and added check

4fa83a6

Revert "Removed intermediate dictionary structure and added check"

d85079e

This reverts commit 4fa83a6.

With the dictionary it seems to work faster for some reason

63ed0cd

Lehonti added 4 commits January 17, 2024 00:02

Tried parallel (and removing intermediate structure) in some other way

44b3fba

A few adjustments

c1a2ce9

Solved merge conflicts

b8ca36a

Added chrome service to VoronoiDiagramEffect

313f7ee

Lehonti added 2 commits January 19, 2024 01:31

Reduced number of sortings

0f772d2

Removed a test

aa26e5a

Merge branch 'PintaProject:master' into feature/voronoi

c91da94

cameronwhite reviewed Jan 20, 2024

View reviewed changes

Lehonti added 3 commits January 20, 2024 14:14

Changed comments for translators

e34c9b1

Merge branch 'PintaProject:master' into feature/voronoi

d354929

Added two more tests

9b44f4e

cameronwhite merged commit f34897c into PintaProject:master Jan 21, 2024
5 checks passed

Lehonti deleted the feature/voronoi branch January 21, 2024 03:59

Lehonti mentioned this pull request Nov 14, 2024

Supersampling for smoother Voronoi diagrams #1128

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Voronoi diagrams #692

Voronoi diagrams #692

Lehonti commented Jan 14, 2024 •

edited

Loading

Lehonti commented Jan 14, 2024 •

edited

Loading

Lehonti commented Jan 14, 2024 •

edited

Loading

Lehonti commented Jan 14, 2024

cameronwhite left a comment

cameronwhite Jan 15, 2024

Lehonti Jan 15, 2024 •

edited

Loading

cameronwhite Jan 16, 2024

Lehonti Jan 16, 2024 •

edited

Loading

cameronwhite Jan 17, 2024

cameronwhite Jan 15, 2024

cameronwhite Jan 15, 2024

Lehonti Jan 17, 2024

cameronwhite Jan 15, 2024

Lehonti Jan 17, 2024

cameronwhite Jan 15, 2024

Lehonti Jan 15, 2024

cameronwhite Jan 15, 2024

cameronwhite Jan 15, 2024

Lehonti Jan 17, 2024

cameronwhite Jan 15, 2024

Lehonti Jan 15, 2024 •

edited

Loading

cameronwhite Jan 16, 2024

Lehonti Jan 16, 2024 •

edited

Loading

cameronwhite Jan 17, 2024

Lehonti commented Jan 18, 2024

cameronwhite commented Jan 19, 2024

Lehonti commented Jan 19, 2024

cameronwhite commented Jan 19, 2024

Lehonti commented Jan 19, 2024

cameronwhite Jan 20, 2024

cameronwhite Jan 20, 2024

Lehonti Jan 20, 2024

cameronwhite commented Jan 20, 2024

cameronwhite commented Jan 21, 2024


		Span<ColorBgra> dst_data = dst.GetPixelData ();

		for (int y = roi.Top; y <= roi.Bottom; y++) {

Voronoi diagrams #692

Voronoi diagrams #692

Conversation

Lehonti commented Jan 14, 2024 • edited Loading

Lehonti commented Jan 14, 2024 • edited Loading

Lehonti commented Jan 14, 2024 • edited Loading

Lehonti commented Jan 14, 2024

cameronwhite left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Lehonti Jan 15, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Lehonti Jan 16, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Lehonti Jan 15, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Lehonti Jan 16, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Lehonti commented Jan 18, 2024

cameronwhite commented Jan 19, 2024

Lehonti commented Jan 19, 2024

cameronwhite commented Jan 19, 2024

Lehonti commented Jan 19, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cameronwhite commented Jan 20, 2024

cameronwhite commented Jan 21, 2024

Lehonti commented Jan 14, 2024 •

edited

Loading

Lehonti commented Jan 14, 2024 •

edited

Loading

Lehonti commented Jan 14, 2024 •

edited

Loading

Lehonti Jan 15, 2024 •

edited

Loading

Lehonti Jan 16, 2024 •

edited

Loading

Lehonti Jan 15, 2024 •

edited

Loading

Lehonti Jan 16, 2024 •

edited

Loading