Made fractal algorithms more flexible so that colors can be customized

[Lehonti]

There isn't a gradient control for use in SimpleEffectDialog yet, and the gradient type that was used is still undocumented, and is internal within Pinta.Effects. This is intentional, as we have to get it right before making it part of the public API.

... but until then we could add a combobox so that the user can choose among a few pre-defined color schemes. What do you think?

Here's with the original colors (which is now the default gradient):

Here's with the same gradient stops but different colors:

Here's with the original colors but different stops:

[cameronwhite]

I haven't looked through the code in depth, but one idea could be to have the EffectData take a gradient parameter rather than an enum, so that the effect's API doesn't change if we later add support for a widget that can specify a custom gradient.
The SimpleEffectDialog could then handle how to create a gradient widget, e.g. just showing a preset combo box for now

[Lehonti]

I understand that, but in order to do that we would have to clearly define the public interface of the gradient type, but since this isn't done yet what if we use an enum, and we make the enum-typed property and the gradient type internal? That's what I just did, and the API hasn't changed, because there is no new public properties yet. This way, we will be able to ship the feature (the effect would be customizable within Pinta), and third parties won't notice the change in API until we make the property (and gradient type) public. In this way, we'd get as much time as needed to work on the API part of the gradient

[cameronwhite]

Would this prevent writing a unit test since the property is internal?

I'm not sure if this is a high enough priority that it needs to be squeezed in right away though - why not take the time to implement a gradient API that you're happy with making public?

[Lehonti]

I was thinking that the feature would increase the popularity of the project and maybe bring more contributors, but yeah, we could wait to get it right first.

I am almost happy with the methods in my gradient class; although it may need some refinements, like methods for adding new gradient stops and such, changing the collection type (to something to which elements can be added and automatically keeps them sorted), and adding some static creational methods so that the API doesn’t break even if some feature is added to the class. But what do you think about the public interface of the class as it is now?

As for the testing, I think we could do [assembly: InternalsVisibleTo ("Pinta.Effects.Test")] so that any piece of code can be tested even if it's not part of the public API.

Edit: I've now edited the class as I've described, in such a way that the API won't break if more operations or members are added. Also, the colors in the Julia fractal are now customizable, too.

[Lehonti]

There seems to be some bug that is making the tests fail, even though the fractals look visually OK. I'll be looking into it. Otherwise, issues have been taken care of.

[Lehonti]

The problem was a comparison that was supposed to be greater or equal, but was only greater. I think it's ready for merging now.

[cameronwhite]

Thank you! I think the changes look good 👍
After taking a look over the unit tests, one final question is whether GetRandomColor() is necessary or if we can just use some arbitrary fixed colours like black or white. Having random values in a test makes me nervous in general :)

[Lehonti]

It's understandable (and very reasonable) that you want the tests to be deterministic and reproducible.

GetRandomColor() is not necessary as such, but it's more of a way to emphasize that a certain result holds no matter what color is chosen. Of course, it need not be non-deterministic; we can find some other way to test that.

In any case, I've now made the changes so that everything is deterministic.

[cameronwhite]

Looks good! Thank you for working on this