Skip to content

Allowing choices of point arrangements in CellsEffect and VoronoiDiagramEffect#1665

Merged
cameronwhite merged 22 commits into
PintaProject:masterfrom
Lehonti:improvement3
Aug 19, 2025
Merged

Allowing choices of point arrangements in CellsEffect and VoronoiDiagramEffect#1665
cameronwhite merged 22 commits into
PintaProject:masterfrom
Lehonti:improvement3

Conversation

@Lehonti

@Lehonti Lehonti commented Aug 13, 2025

Copy link
Copy Markdown
Contributor

Apart from a random arrangement, one can chose to position the points along a circular path and also in a phyllotaxis arrangement (think of how the seeds of a sunflower are arranged)

A few notes:

  • The Voronoi8 test is being ignored for now because it produces different results in ARM64 MacOS. I suspect the floating-point operations produce slightly different results in ARM64.
  • It seems I can't annotate the values of PointArrangement with CaptionAttribute, because it is in he Pinta.Gui.Widgets project. Maybe we should move these attributes to Pinta.Core after this? It would break the API, though. Or we could move SpatialPartition to Pinta.Effects, but add-in developers wouldn't be able to use it.

Sunflower-public-domain-4-sm

image

(Sunflower image from project Rhea https://www.projectrhea.org/rhea/index.php/MA279Fall2018Topic1_Phyllotaxis)

@Lehonti

Lehonti commented Aug 14, 2025

Copy link
Copy Markdown
Contributor Author

Note that I added the configuration file mentioned in #1668 to the second-to-last commit of this pull request. The result produced in ARM64 is still different.

@cameronwhite

Copy link
Copy Markdown
Member

Yeah, we do need to figure out the caption attribute since I'd rather not forget about making these menu options translatable :)
Maybe if we move it to Pinta.Core but keep it in its original namespace? A bit hacky but that might possibly keep existing add-ins from breaking

For the platform differences, looking at the log from the tests it seems like a diagonal line of pixels has a different color. My guess would be that GetColorForLocation is very sensitive to small differences if the location is at nearly the same distance from 2+ points.

@Lehonti

Lehonti commented Aug 15, 2025

Copy link
Copy Markdown
Contributor Author

@cameronwhite yeah, I don't know. I am thinking that existing add-ins would break anyway if their compiled versions look for the attribute types in the Pinta.Gui.Widgets assembly instead of the Pinta.Core assembly, although code-wise you are most likely right.

I moved the types to Pinta.Core and annotated the values of the enum, but separated the different modifications over several commits, for convenience (you could choose which ones to keep and which ones to discard).

@cameronwhite

Copy link
Copy Markdown
Member

You're correct about it looking for the Pinta.Gui.Widgets assembly - I tried keeping the old namespace and add-ins from Pinta 3.0 threw some errors when launching the effect dialog

System.TypeLoadException: Could not load type 'Pinta.Gui.Widgets.CaptionAttribute' from assembly 'Pinta.Gui.Widgets, Version=3.1.0.0, Culture=neutral, PublicKeyToken=null'.

Moving it to Pinta.Core seems like the most reasonable place for it, especially since we can launch the effect dialog without an explicit Pinta.Gui.Widgets dependency using chrome.LaunchSimpleEffectDialog()
Maybe this would also allow removing the Pinta.Effects -> Pinta.Gui.Widgets dependency?

@Lehonti

Lehonti commented Aug 17, 2025

Copy link
Copy Markdown
Contributor Author

Unfortunately the dependency on Pinta.Gui.Widgets can't be removed just yet @cameronwhite. If you look at LevelsDialog and PosterizeDialog you will see that they use widgets declared and implemented in that assembly. One thing that can be done, though, is removing the unused namespace references (which I did in d906e12). That should make it easier to identify and remove the dependency in the future.

...And since we are at it we could sneak in a few stylistic improvements (7b89762)

@cameronwhite

Copy link
Copy Markdown
Member

Ah right, the dependency there still makes sense. It's good though that a "basic" effect would only need a Pinta.Core dependency now though

I think the only remaining thing to do here is to bump up the add-in compatibility version in PintaCore.cs now that we have a change that will require recompiling v3.0 addins (and note this in the changelog)

@Lehonti

Lehonti commented Aug 19, 2025

Copy link
Copy Markdown
Contributor Author

@cameronwhite I just bumped the version and updated the changelog 👍

@cameronwhite cameronwhite merged commit c666b4a into PintaProject:master Aug 19, 2025
6 checks passed
@Lehonti Lehonti deleted the improvement3 branch August 19, 2025 23:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants