Multithreaded Mandelbrot set code outputs incorrect image

Question

#include <chrono>
#include <cstdint>
#include <cstdlib>
#include <complex>
#include <fstream>
#include <iostream>
#include <thread>
#include <mutex>
#include <condition_variable>

using std::chrono::duration_cast;
using std::chrono::milliseconds;
using std::chrono::seconds;
using std::complex;
using std::cout;
using std::endl;
using std::this_thread::sleep_for;
using std::ofstream;
using std::thread;
using std::mutex;
using std::condition_variable;
using std::unique_lock;
mutex mutex1;
mutex mutex2;
std::condition_variable done;

typedef std::chrono::steady_clock the_clock;



const int WIDTH1 = 960;
const int HEIGHT1 = 600;
const int WIDTH2 = 1920;
const int HEIGHT2 = 1200;
int finished_threads = 0;


const int MAX_ITERATIONS = 500;


uint32_t image[HEIGHT2][WIDTH2];



struct ThreadArgs { int id; int delay; };
void myThreadFunc(ThreadArgs args)

{
    for (int i = 0; i < 1; i++) {
        sleep_for(seconds(args.delay));
        cout << args.id;
    }
}
void write_tga(const char *filename)
{
    unique_lock<mutex> lock(mutex2);
    while (finished_threads < 2) {
        done.wait(lock);
    }
    ofstream outfile(filename, ofstream::binary);

    uint8_t header[18] = {
        0, // no image ID
        0, // no colour map
        2, // uncompressed 24-bit image
        0, 0, 0, 0, 0, // empty colour map specification
        0, 0, // X origin
        0, 0, // Y origin
        WIDTH2 & 0xFF, (WIDTH2 >> 8) & 0xFF, // width
        HEIGHT2 & 0xFF, (HEIGHT2 >> 8) & 0xFF, // height
        24, // bits per pixel
        0, // image descriptor
    };
    outfile.write((const char *)header, 18);

    for (int y = 0; y < HEIGHT2; ++y)
    {
        for (int x = 0; x < WIDTH2; ++x)
        {
            uint8_t pixel[3] = {
                image[y][x] & 0xFF, // blue channel
                (image[y][x] >> 8) & 0xFF, // green channel
                (image[y][x] >> 16) & 0xFF, // red channel
            };
            outfile.write((const char *)pixel, 3);
        }
    }

    outfile.close();
    if (!outfile)
    {

        cout << "Error writing to " << filename << endl;
        exit(1);
    }
}


// Render the Mandelbrot set into the image array.
// The parameters specify the region on the complex plane to plot.
void compute_mandelbrot(double left, double right, double top, double bottom)

{
    unique_lock<mutex> lock(mutex2);

    for (int y = 0; y < HEIGHT1; ++y)
    {
        for (int x = 0; x < WIDTH1; ++x)
        {

            complex<double> c(left + (x * (right - left) / WIDTH2),
                top + (y * (bottom - top) / HEIGHT2));

            // Start off z at (0, 0).
            complex<double> z(0.0, 0.0);

            // Iterate z = z^2 + c until z moves more than 2 units
            // away from (0, 0), or we've iterated too many times.
            int iterations = 0;
            mutex1.lock();
            while (abs(z) < 2.0 && iterations < MAX_ITERATIONS)
            {
                z = (z * z) + c;

                ++iterations;
            }
            mutex1.unlock();
            if (iterations == MAX_ITERATIONS)
            {

                image[y][x] = 0x000000; // black
            }
            else
            {

                image[y][x] = 0xFFFFFF; // white
                finished_threads = finished_threads + 1;
                done.notify_all();
            }
        }
    }
}
void compute_mandelbrot2(double left2, double right2, double top2, double bottom2)

{
    unique_lock<mutex> lock(mutex2);
    //map <int, int> val = map<int, int>(0, MAX_ITERATIONS);
    //map <int, int> colourval = map<int, int>(0, MAX_ITERATIONS);
    for (int y = HEIGHT1; y < HEIGHT2; ++y)
    {
        for (int x = HEIGHT1; x < WIDTH2; ++x)
        {

            complex<double> c(left2 + (x * (right2 - left2) / WIDTH2),
                top2 + (y * (bottom2 - top2) / HEIGHT2));

            // Start off z at (0, 0).
            complex<double> z(0.0, 0.0);

            // Iterate z = z^2 + c until z moves more than 2 units

            int iterations = 0;
            mutex1.lock();
            while (abs(z) < 2.0 && iterations < MAX_ITERATIONS)
            {
                z = (z * z) + c;

                ++iterations;
            }
            mutex1.unlock();
            if (iterations == MAX_ITERATIONS)
            {
                // z didn't escape from the circle.
                // This point is in the Mandelbrot set.
                image[y][x] = 0x000000; // black
            }
            else
            {
                // z escaped within less than MAX_ITERATIONS
                // iterations. This point isn't in the set.
                image[y][x] = 0xFFFFFF; // white
                finished_threads = finished_threads + 1;
                done.notify_one();
            }
        }
    }
}


int main(int argc, char *argv[])
{

    std::thread myThread;
    std::thread myThread2;
    std::thread myThread3;

    ThreadArgs args;

    myThread = std::thread(compute_mandelbrot, -2.0, 1.0, 1.125, -1.125);
    myThread3 = std::thread(compute_mandelbrot2, -2.0, 1.0, 1.125, -1.125);
    myThread2 = std::thread(write_tga, "output.tga");

    cout << "Please wait..." << endl;

    // Start timing
    the_clock::time_point start = the_clock::now();



    myThread.join();
    myThread3.join();




    // Stop timing
    the_clock::time_point end = the_clock::now();

    // Compute the difference between the two times in milliseconds
    auto time_taken = duration_cast<milliseconds>(end - start).count();
    cout << "Computing the Mandelbrot set took " << time_taken << " ms." << endl;


    myThread2.join();

    return 0;
}

Above is the multithreaded version of the code and it outputs an incorrect version of the set where most of it is black but some is correct, so I don't know what the issue is:Mandlebrot threaded but the non-threaded version outputs a correct Mandelbrot setMandelbrot not threaded the problem is likely something to do with how I used multithreading: I just don't know what I did wrong. Any help is appreciated.

Answer 1

OK, 'simple' things first! You have a 'typo' in your compute_mandelbrot2 function on the control statement for your inner (x) loop; This line:

    for (int x = HEIGHT1; x < WIDTH2; ++x)

should (of course) be:

    for (int x = WIDTH1; x < WIDTH2; ++x) // WIDTH1 not HEIGHT1

Now for the more 'subtle' stuff. You are trying to split the calculation into two halves, by splitting both the 'x' and 'y' ranges into halves. This will not work, as that would require four threads, each dealing with the relevant quarter of the plot. To keep to using just two halves, the 'y' ranges in the two thread functions must cover the whole plot (but the 'x' ranges can properly be split in two).

Thus, your outer ('y') loop control statements should cover the entire range in both thread functions, and they should both be this:

for (int y = 0; y < HEIGHT2; ++y) {
    //...

I have tested your code with the aforementioned three changes and it produces the correct Mandelbrot set image. Feel free to ask for for further clarification and/or explanation.