My Mandelbrot sketching program in c# isn't working

Question

I have to make a c# program which creates a form and one of its controls has to be a control that makes a sketch of a Mandelbrot set in which you can zoom in and tell on which point you want to centre. In my opinion i'm doing the right thing, but when I want to start the program, I don't get the desired image of a standard Mandelbrot set. For the Mandelbrot sketch control I've got the next bit of my Mandelbrot class:

class Mandelbrotsketchscreen : UserControl
{
    //a method that draws every point with a certain paint
    //a method that chooses the right colour
    //a method that translates the pixel coordinates of the control to the actual coordinates

    //gives the number how many times the function has to be used
    private static int Mandelnumber(PointF p, PointF middle, double scale, int max, Size size) 
    {
        PointF realpoint = new PointF();
        realpoint.X = (float)(middle.X + (p.X - size.Width / 2) * scale);
        realpoint.Y = (float)(middle.Y + (p.Y - size.Height / 2) * scale);
        PointF help = new PointF(0, 0);
        int i;
        for (i = 1; i <= max; i++)
        {
            help.X = help.X * help.X - help.Y * help.Y + realpoint.X;
            help.Y = 2 * help.X * help.Y + realpoint.Y;

            if (Math.Sqrt(help.X * help.X + help.Y * help.Y) > 2)
                break;
        }
        return i;
    }
}

Can someone tell me if I'm doing the calculation wrong or that maybe my loops are wrong?

The result I'm getting now is:

What do you get now? Knowing what result you currently get can possibly help pinpointing the problem. (Screenshot of what the program currently produces currently could perhaps be helpful.) — , Dec 13 '18 at 21:19
Probably nothing to do with your problem but Brushes are disposable resources. You may be painting thousands of little rectangles and youre creating a new brush for each one. You only have 3 possible brushes, so I'd create the 3 brush instances as fields and the `Mandelbrotcolour` method would return a reference to one of those 3, rather than creating a new instance each time. Then make sure you dispose of them when your control is disposed. — Richardissimo, Dec 13 '18 at 21:22
@elgonzo realpoint.X and realpoint.Y won't be always less than or equal to zero, at least what i want it to be. realpoint.X and realpoint.Y have to depend on which point you on the control you're with sketching and what the middle point is and what the scale is. — Joes de Jonge, Dec 13 '18 at 21:41
@JoesdeJonge, yes i noticed i made a mistake (and deleted my comment already) I only saw `p.X - width` and completely missed that you actually do `p.X - width/2`... sorry for the mishap... — , Dec 13 '18 at 21:43
OK now we can see what you've got... What were you expecting? / What were you expecting to be different? — Richardissimo, Dec 13 '18 at 21:46
The fundamental problem here is that you treated a value as though it was mutable. This is sometimes necessary for performance, but it creates opportunities for bugs like the one you wrote. Instead of mutating the existing point, compute a new point, and the problem will go away. Points are cheap; you can make lots of them. — Eric Lippert, Dec 13 '18 at 22:24
Also, while we are looking at your code: **do not compute the square root**. That is, do not write `Math.Sqrt(help.X * help.X + help.Y * help.Y) > 2`. Write `help.X * help.X + help.Y * help.Y > 4`. If the square root of a positive value is greater than two, then the value will be greater than 4, and that is much, much cheaper to compute! — Eric Lippert, Dec 14 '18 at 17:59

score 2 · Accepted Answer · 2018-12-14T19:21:29.660

The new values for help.X and help.Y need to be calculated based on the previous values of help.X and help.Y.

Your code first calculates the new help.X value based on the previous help.X and help.Y values. So far, so good. But then, your code calculates help.Y using the new help.X value instead of the previous help.X value.

Thus, a solution/fix for your problem can be as simple as:

    for (i = 1; i <= max; i++)
    {
        var newX = help.X * help.X - help.Y * help.Y + realpoint.X;
        var newY = 2 * help.X * help.Y + realpoint.Y;

        help.X = newX;
        help.Y = newY;

        if (Math.Sqrt(help.X * help.X + help.Y * help.Y) > 2)
            break;
    }

(Side note: The newY variable in my example here is not strictly necessary. I chose to use it to clearly illustrate the distinction between previous and new values for help.X and help.Y.)

A different (and shorter) solution has been mentioned by Eric Lippert in the comments: Just create a new help point instead of mutating/modifying the existing help point:

    for (i = 1; i <= max; i++)
    {
        help = new PointF(
            help.X * help.X - help.Y * help.Y + realpoint.X,
            2 * help.X * help.Y + realpoint.Y
        );

        if (help.X * help.X + help.Y * help.Y > 4)
            break;
    }

This shorter solution also eliminates the rather slow square root computation by comparing against the square of 2 (= 4).

Also, the computation of the square root is unnecessary and slows down the loop enormously. — Eric Lippert, Dec 14 '18 at 18:02

My Mandelbrot sketching program in c# isn't working

1 Answers1