0
$\begingroup$

I'm having some issues with my multi-threaded ray tracing implementation. It seems to only render about half of the image. Here is the picture: enter image description here

Here is the code:

int max = (nx * ny); //width * height
volatile std::atomic<int> count(0);

int cores = std::thread::hardware_concurrency();
cores -= 1;
vector<future<vector<rgbpacket>>> future_vector;
cout << "Threads supported: " << cores << "\n";
while (cores--) {
    future_vector.emplace_back(
        std::async([=, &world, &cam, &dim, &orth, &count] {
            vector<rgbpacket> stack;
            while (true) {

                int index = count++;

                if (index >= max) {

                    return stack;
                } else {
                    int k = (index % nx); //mod width to get column
                    int j = (index / nx); //divide width to get row

                    rgbpacket curpx = render(j, k, dim, world, cam, orth);
                    curpx.k = (unsigned int)k;
                    curpx.j = (unsigned int)j;
                    stack.push_back(curpx);

                }
            }
        })
    );
}

//parse the image buffer
for (unsigned int i = 0; i < future_vector.size(); i++) {
    vector<rgbpacket> stack = future_vector[i].get();
    for (unsigned int k = 0; k < stack.size(); k++) {
        rgbpacket rgb_comp = stack.back();

        //Using the new compressed packet
        rgbaColor conv;
        conv.r = rgb_comp.r;
        conv.g = rgb_comp.g;
        conv.b = rgb_comp.b;

        hslaColor pix = rgb2hsl(conv);
        HSLAPixel newpixel(pix.h, pix.s, pix.l);

        HSLAPixel &imgpx = img.getPixel(rgb_comp.k, rgb_comp.j);
        imgpx = newpixel;

        stack.pop_back();
    }
}

For some preface, I am calculating the pixel with respect to the position with the index variable. Once I calculate it, I store it (along with the position) in the vector. After I calculate all of the pixel values, I parse them into an image buffer and later print that image.

I've narrowed the issue (I think) to where I am calculating the rendered pixel and storing it in the vector. For some reason, it does not store all of the pixels in the vector and I am not sure why? Any help is appreciated. Thanks!

Improve this question
$\endgroup$
6
  • 1
    $\begingroup$ I don't think std::vector<> is thread safe, you should be using something like a mutex when calling push_back() $\endgroup$
    – PaulHK
    Commented Apr 13, 2018 at 8:35
  • $\begingroup$ Hi @PaulHK, Could this be part of the reason for the half-picture behavior? I got some more information about the memory, The objects are stored in memory so when I call the render function, the "hits" itself might not be registering because of the asynchronous memory accesses. But I was thinking that this would cause more random behavior, like seeing a random distribution of pixels on the picture every time. $\endgroup$
    – Varun Govind
    Commented Apr 14, 2018 at 18:00
  • $\begingroup$ You could remove your completion queue (future_vector) and just have your workers directly write to an RGB array, you don't need thread synchronisation for that. You will need a mechanism to detect when all workers are completed though, maybe something like an atomic counter and semaphore. $\endgroup$
    – PaulHK
    Commented Apr 15, 2018 at 4:04
  • $\begingroup$ The half written image is probably caused by the main thread not waiting for workers to complete. In addition to that it is hazardous for multiple threads to access the vector<> and that could cause issues if 2 threads update the internal state of the vector<> at the same time. $\endgroup$
    – PaulHK
    Commented Apr 15, 2018 at 4:09
  • 1
    $\begingroup$ Hey PaulHK, thanks for helping me out, I fixed the bug, it was dumb, I posted an answer to anyone who might look at this. $\endgroup$
    – Varun Govind
    Commented Apr 24, 2018 at 6:38

1 Answer 1

Reset to default
0
$\begingroup$

I fixed it, It was just a coding mistake, there was nothing wrong with the implementation. So the error was in fact in the 2nd for-loop when I was parsing the image buffer. I was reaching for the last pixel in the vector and then popping it off from the back of the stack.

This means that in the for loop condition, the vector size will be changing but k will still be incrementing the same, so it will effectively only do half of the elements per vector. Because as K is incrementing stack.size() is decrementing so they meet in the middle and the loop is done.

To fix this, I just used index notation, so:

rgbpacket &rgbpixel = stack[index];

and then I deleted the line where I was popping off from the stack.

Even though this is a stupid bug, I hope this helps other people.

Improve this answer
$\endgroup$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.