Mandelbrot image generator and viewer

This is a C++ program which outputs a Mandelbrot fractal image in a graphics window and lets the user zoom and pan around the image, generating a new image each time the user does so. I'm using SFML for the graphics part.

Because it generates a new image after every zoom or pan, it is pretty slow, especially with a 1000x600 image (it can take up to half a second for the new image to load).

```
#include
#include

const int IMAGE_WIDTH = 1000;
const int IMAGE_HEIGHT = 600;
double zoom = 0.004; // allow the user to zoom in and out...
double offsetX = -0.7; // and move around
double offsetY = 0.0;
const int MAX = 127; // maximum number of iterations for mandelbrot()
// don't increase MAX or the colouring will look strange

int mandelbrot(double, double, int);
sf::Color getColor(int);

int main() {
sf::RenderWindow window(sf::VideoMode(IMAGE_WIDTH, IMAGE_HEIGHT), "Mandelbrot");
window.setFramerateLimit(30);

sf::Image image;
image.create(IMAGE_WIDTH, IMAGE_HEIGHT, sf::Color(0, 0, 0));
sf::Texture texture;
sf::Sprite sprite;

bool stateChanged = true; // track whether the image needs to be regenerated

while (window.isOpen()) {
sf::Event event;
while (window.pollEvent(event)) {
switch (event.type) {
case sf::Event::Closed:
window.close();
break;
case sf::Event::KeyPressed:
stateChanged = true; // image needs to be recreated when the user changes zoom or offset
switch (event.key.code) {
case sf::Keyboard::Escape:
window.close();
break;
case sf::Keyboard::Equal:
zoom *= 0.9;
break;
case sf::Keyboard::Dash:
zoom /= 0.9;
break;

I see some things that may help you improve your code.

Move loop invariants outside the loop

To maximize performance, moving loop invariants outside the loop often helps. This is an optimization that many compilers can make on their own, but it helps to write it explicitly. In particular, the nested loop within main where the image is recalculated. Right now the loop looks like this:

for (int x = 0; x < IMAGE_WIDTH; x++) {
    for (int y = 0; y < IMAGE_HEIGHT; y++) {
        // convert x and y to the appropriate complex number
        double real = (x - IMAGE_WIDTH / 2.0) * zoom + offsetX;
        double imag = (y - IMAGE_HEIGHT / 2.0) * zoom + offsetY;
        int value = mandelbrot(real, imag, MAX);
        image.setPixel(x, y, getColor(value));
    }
}

However, with a little bit of algebra, we see that the real and imag calculations could be rewritten as

double real = x * zoom - IMAGE_WIDTH / 2.0 * zoom + offsetX;
double imag = y * zoom - IMAGE_HEIGHT / 2.0 * zoom + offsetY;

Since only x and y are actually changing within the loop, it's actually simple to replace all of that with this:

double real = 0 * zoom - IMAGE_WIDTH / 2.0 * zoom + offsetX;
double imagstart = 0 * zoom - IMAGE_HEIGHT / 2.0 * zoom + offsetY;
for (int x = 0; x < IMAGE_WIDTH; x++, real += zoom) {
    double imag = imagstart;
    for (int y = 0; y < IMAGE_HEIGHT; y++, imag += zoom) {
        // convert x and y to the appropriate complex number
        int value = mandelbrot(real, imag, MAX);
        image.setPixel(x, y, getColor(value));
    }
}

Avoid special cases

The value of -1 is used to represent points within the Mandelbrot set, but then this value is explicitly checked when assigning a color and is equivalent to MAX. Why not simply assign the value of MAX and avoid the special case?

Prefer simple lookup to code

The getColor() function gets a single number in the range of 0 to MAX (if the previous suggestion is taken) and returns a color. Why not replace this with a table lookup? Compute the table values once (either at compile-time or run-time) and then use the table. At the beginning of main you could have this:

std::array colors;
for (int i=0; i <= MAX; ++i) {
    colors[i] = getColor(i);
}

Then the loop would use this:

image.setPixel(x, y, colors[value]);

Don't use == for floating point numbers

Using == with floating point numbers is generally not a good idea because if they differ only by some small amount (say 1e-99), they are not equal. In fact, if you modify the code to count the number of times that statement evaluates to true, I think you will find that it never does and is therefore simply wasting time.

For more depth about floating point issues, I'd recommend the excellent article "What every computer scientist should know about floating-point arithmetic" by David Goldberg.

Optimize loops for early bailout

The mandelbrot loop can be rewritten more simply like this:

int mandelbrot(double startReal, double startImag) {
    double zReal = startReal;
    double zImag = startImag;

    for (int counter = 0; counter  4.0) {
            return counter;
        }
        zImag = 2.0 * zReal * zImag + startImag;
        zReal = r2 - i2 + startReal;
    }
    return MAX;
}

Several changes were made here. First, since the original third parameter maximum was always passed in with the value of MAX, it was simply eliminated from the parameter list and the value MAX substituted directly.

Next, the loop is now a for loop instead of a while loop which makes the structure more obvious.

Next, the r2 and i2 values are precomputed, making the code a little shorter and simpler (and probably more efficient).

Lastly, the early bailout is done within the loop. Only if we get to the end of the loop does the value MAX get returned.

Omit work not needed

The window.clear() call near the end of main is not needed since the following line writes the entire window anyway. That line can simply be eliminated.

Eliminate global variables

All but MAX can be made local to within main.

Eliminate framerate limit

Unless there is a reason to do so, you might find that eliminating the call to window.setFramerateLimit or setting the value to 0 allows the program to be more responsive. After I applied all of the suggestions mentioned above, I found that the framerate limit was the thing preventing further speed increases on my machine.

Don't update the screen if not needed

Right now any key (even those that have no effect, cause the screen to recalculate. This is easily fixed by adding stateChanged = false; to the default case in the key event handling switch statement.

Use C++11 threads

One can fairly painlessly parallelize this code. I have changed the end of main so that it now looks like this:

```
if (stateChanged) {
updateImage(zoom, offsetX, offsetY, image);
texture.loadF

StackExchange Code Review Q#124358, answer score: 48