Skip to main content
Tweeted twitter.com/StackCodeReview/status/1548819940010070016
edited body
Source Link

The goal is to write a simple example in which we split a frame into tiletiles (smaller chunks of say 32x32 pixels) and get the threads to render the pixels of each tile. I want to use the minimalist approach and thus avoid doing something like a task scheduler. Therefore I have chosen to calculate the number of tiles in the main program, store that in an atomic variable, and use that in the threads to provide each thread with a current tile number. Threads then decrement that number and keep doing so while that total number of tiles is greater or equal to 0.

To make the exercise more fun, I am currently assigning a random color to each tile before I implement the rest of the rendering process. I am using a random number generator from the standard lib for now. I declared the generator thread_local though to be honest unless I actually call the function render() reclusivelyrecursively or need other functions called from within the thread function to use the generator I don't think this is necessary. As long as each thread own its own random number generator I believe this is thread safe too.

What do you think.? Here is the code:

#include <thread>
#include <atomic>
#include <iostream>
#include <fstream>
#include <mutex>
#include <random>

struct render_info_t
{
    unsigned int width { 640 };
    unsigned int height { 480 };
    unsigned int tile_size { 32 };
    unsigned char *bufferint num_tiles_x{ nullptr}, num_tiles_y{};
    unsigned intchar num_tiles_x{},*buffer num_tiles_y{ nullptr };
};

struct thread_info_t
{
    unsigned int id;
    const render_info_t* render_info;
};

void render(const thread_info_t& thread_info, std::atomic_int &count)
{
    int curr_tile {};
    const render_info_t* ri = thread_info.render_info;
    thread_local std::mt19937 gen(std::hash<std::jthread::id>()(std::this_thread::get_id()));
    std::uniform_real_distribution<float> dist(0.0f, 1.f);
    unsigned char *buffer = (unsigned char*)malloc(ri->tile_size * ri->tile_size * 3);
    
    while ((curr_tile = --count) >= 0) {
        unsigned char *curr_pixel = buffer;
        float r = dist(gen);
        float g = dist(gen);
        float b = dist(gen);

        unsigned int curr_tile_y = curr_tile / ri->num_tiles_x;
        unsigned int curr_tile_x = curr_tile - curr_tile_y * ri->num_tiles_x;
        unsigned int x0 = curr_tile_x * ri->tile_size;
        unsigned int x1 = std::min((curr_tile_x + 1) * ri->tile_size, ri->width);
        unsigned int y0 = curr_tile_y * ri->tile_size;
        unsigned int y1 = std::min((curr_tile_y + 1) * ri->tile_size, ri->height);
        for (unsigned int y = y0; y < y1 ; ++y) {
            for (unsigned int x = x0; x < x1; ++x, curr_pixel += 3) {
                /*
                ** TODO trace ray at pixel coordinate x, y
                */
                curr_pixel[0] = (unsigned char)(r * 255);
                curr_pixel[1] = (unsigned char)(g * 255);
                curr_pixel[2] = (unsigned char)(b * 255);
            }
        }

        // copy the tile's pixels into the image buffer
        unsigned char *row = buffer;
        unsigned char *from = ri->buffer + (y0 * ri->width + x0) * 3;
        for (unsigned int y = y0; y < y1 ; ++y, row += ri->tile_size * 3, from += ri->width * 3) { // , from += ri->width * 3
            // the data pointed by the pointer ri->buffer is not part of the struct, so it can be change
            memcpy(from, row, ri->tile_size * 3);
        }
    }
    free(buffer);
}

int main(int argc, char **argv)
{
    unsigned int num_threads = std::jthread::hardware_concurrency();
    std::cout << "Rendering with " << num_threads << " threads" << std::endl;
    std::vector<std::jthread> threads;
    render_info_t ri;
    ri.num_tiles_x = (ri.width + ri.tile_size - 1) / ri.tile_size;
    ri.num_tiles_y = (ri.height + ri.tile_size - 1) / ri.tile_size;
    ri.buffer = (unsigned char*)malloc(ri.width * ri.height * 3);
    std::atomic_int num_tiles = ri.num_tiles_x * ri.num_tiles_y;
    for (unsigned int n = 0; n < num_threads; ++n) {
        thread_info_t thread_info;
        thread_info.id = n;
        thread_info.render_info = &ri;
        threads.emplace_back(render, std::move(thread_info), std::ref(num_tiles));
    }

    for (auto& thread : threads)
        thread.join();

    std::ofstream ofs;
    ofs.open("./output.ppm", std::ios::binary);
    ofs << "P6\n" << ri.width << " " << ri.height << "\n255\n";
    ofs.write((char*)ri.buffer, ri.width * ri.height * 3);
    ofs.close();
    free(ri.buffer);

    return EXIT_SUCCESS;
}

The goal is to write a simple example in which we split a frame into tile (smaller chunks of say 32x32 pixels) and get the threads to render the pixels of each tile. I want to use the minimalist approach and thus avoid doing something like a task scheduler. Therefore I have chosen to calculate the number of tiles in the main program, store that in an atomic variable, and use that in the threads to provide each thread with a current tile number. Threads then decrement that number and keep doing so while that total number of tiles is greater or equal to 0.

To make the exercise more fun, I am currently assigning a random color to each tile before I implement the rest of the rendering process. I am using a random number generator from the standard lib for now. I declared the generator thread_local though to be honest unless I actually call the function render() reclusively or need other functions called from within the thread function to use the generator I don't think this is necessary. As long as each thread own its own random number generator I believe this is thread safe too.

What do you think. Here is the code:

#include <thread>
#include <atomic>
#include <iostream>
#include <fstream>
#include <mutex>
#include <random>

struct render_info_t
{
    unsigned int width { 640 };
    unsigned int height { 480 };
    unsigned int tile_size { 32 };
    unsigned char *buffer { nullptr };
    unsigned int num_tiles_x{}, num_tiles_y{};
};

struct thread_info_t
{
    unsigned int id;
    const render_info_t* render_info;
};

void render(const thread_info_t& thread_info, std::atomic_int &count)
{
    int curr_tile {};
    const render_info_t* ri = thread_info.render_info;
    thread_local std::mt19937 gen(std::hash<std::jthread::id>()(std::this_thread::get_id()));
    std::uniform_real_distribution<float> dist(0.0f, 1.f);
    unsigned char *buffer = (unsigned char*)malloc(ri->tile_size * ri->tile_size * 3);
    
    while ((curr_tile = --count) >= 0) {
        unsigned char *curr_pixel = buffer;
        float r = dist(gen);
        float g = dist(gen);
        float b = dist(gen);

        unsigned int curr_tile_y = curr_tile / ri->num_tiles_x;
        unsigned int curr_tile_x = curr_tile - curr_tile_y * ri->num_tiles_x;
        unsigned int x0 = curr_tile_x * ri->tile_size;
        unsigned int x1 = std::min((curr_tile_x + 1) * ri->tile_size, ri->width);
        unsigned int y0 = curr_tile_y * ri->tile_size;
        unsigned int y1 = std::min((curr_tile_y + 1) * ri->tile_size, ri->height);
        for (unsigned int y = y0; y < y1 ; ++y) {
            for (unsigned int x = x0; x < x1; ++x, curr_pixel += 3) {
                /*
                ** TODO trace ray at pixel coordinate x, y
                */
                curr_pixel[0] = (unsigned char)(r * 255);
                curr_pixel[1] = (unsigned char)(g * 255);
                curr_pixel[2] = (unsigned char)(b * 255);
            }
        }

        // copy the tile's pixels into the image buffer
        unsigned char *row = buffer;
        unsigned char *from = ri->buffer + (y0 * ri->width + x0) * 3;
        for (unsigned int y = y0; y < y1 ; ++y, row += ri->tile_size * 3, from += ri->width * 3) { // , from += ri->width * 3
            // the data pointed by the pointer ri->buffer is not part of the struct, so it can be change
            memcpy(from, row, ri->tile_size * 3);
        }
    }
    free(buffer);
}

int main(int argc, char **argv)
{
    unsigned int num_threads = std::jthread::hardware_concurrency();
    std::cout << "Rendering with " << num_threads << " threads" << std::endl;
    std::vector<std::jthread> threads;
    render_info_t ri;
    ri.num_tiles_x = (ri.width + ri.tile_size - 1) / ri.tile_size;
    ri.num_tiles_y = (ri.height + ri.tile_size - 1) / ri.tile_size;
    ri.buffer = (unsigned char*)malloc(ri.width * ri.height * 3);
    std::atomic_int num_tiles = ri.num_tiles_x * ri.num_tiles_y;
    for (unsigned int n = 0; n < num_threads; ++n) {
        thread_info_t thread_info;
        thread_info.id = n;
        thread_info.render_info = &ri;
        threads.emplace_back(render, std::move(thread_info), std::ref(num_tiles));
    }

    for (auto& thread : threads)
        thread.join();

    std::ofstream ofs;
    ofs.open("./output.ppm", std::ios::binary);
    ofs << "P6\n" << ri.width << " " << ri.height << "\n255\n";
    ofs.write((char*)ri.buffer, ri.width * ri.height * 3);
    ofs.close();
    free(ri.buffer);

    return EXIT_SUCCESS;
}

The goal is to write a simple example in which we split a frame into tiles (smaller chunks of say 32x32 pixels) and get the threads to render the pixels of each tile. I want to use the minimalist approach and thus avoid doing something like a task scheduler. Therefore I have chosen to calculate the number of tiles in the main program, store that in an atomic variable, and use that in the threads to provide each thread with a current tile number. Threads then decrement that number and keep doing so while that total number of tiles is greater or equal to 0.

To make the exercise more fun, I am currently assigning a random color to each tile before I implement the rest of the rendering process. I am using a random number generator from the standard lib for now. I declared the generator thread_local though to be honest unless I actually call the function render() recursively or need other functions called from within the thread function to use the generator I don't think this is necessary. As long as each thread own its own random number generator I believe this is thread safe too.

What do you think? Here is the code:

#include <thread>
#include <atomic>
#include <iostream>
#include <fstream>
#include <mutex>
#include <random>

struct render_info_t
{
    unsigned int width { 640 };
    unsigned int height { 480 };
    unsigned int tile_size { 32 };
    unsigned int num_tiles_x{}, num_tiles_y{};
    unsigned char *buffer { nullptr };
};

struct thread_info_t
{
    unsigned int id;
    const render_info_t* render_info;
};

void render(const thread_info_t& thread_info, std::atomic_int &count)
{
    int curr_tile {};
    const render_info_t* ri = thread_info.render_info;
    thread_local std::mt19937 gen(std::hash<std::jthread::id>()(std::this_thread::get_id()));
    std::uniform_real_distribution<float> dist(0.0f, 1.f);
    unsigned char *buffer = (unsigned char*)malloc(ri->tile_size * ri->tile_size * 3);
    
    while ((curr_tile = --count) >= 0) {
        unsigned char *curr_pixel = buffer;
        float r = dist(gen);
        float g = dist(gen);
        float b = dist(gen);

        unsigned int curr_tile_y = curr_tile / ri->num_tiles_x;
        unsigned int curr_tile_x = curr_tile - curr_tile_y * ri->num_tiles_x;
        unsigned int x0 = curr_tile_x * ri->tile_size;
        unsigned int x1 = std::min((curr_tile_x + 1) * ri->tile_size, ri->width);
        unsigned int y0 = curr_tile_y * ri->tile_size;
        unsigned int y1 = std::min((curr_tile_y + 1) * ri->tile_size, ri->height);
        for (unsigned int y = y0; y < y1 ; ++y) {
            for (unsigned int x = x0; x < x1; ++x, curr_pixel += 3) {
                /*
                ** TODO trace ray at pixel coordinate x, y
                */
                curr_pixel[0] = (unsigned char)(r * 255);
                curr_pixel[1] = (unsigned char)(g * 255);
                curr_pixel[2] = (unsigned char)(b * 255);
            }
        }

        // copy the tile's pixels into the image buffer
        unsigned char *row = buffer;
        unsigned char *from = ri->buffer + (y0 * ri->width + x0) * 3;
        for (unsigned int y = y0; y < y1 ; ++y, row += ri->tile_size * 3, from += ri->width * 3) { // , from += ri->width * 3
            // the data pointed by the pointer ri->buffer is not part of the struct, so it can be change
            memcpy(from, row, ri->tile_size * 3);
        }
    }
    free(buffer);
}

int main(int argc, char **argv)
{
    unsigned int num_threads = std::jthread::hardware_concurrency();
    std::cout << "Rendering with " << num_threads << " threads" << std::endl;
    std::vector<std::jthread> threads;
    render_info_t ri;
    ri.num_tiles_x = (ri.width + ri.tile_size - 1) / ri.tile_size;
    ri.num_tiles_y = (ri.height + ri.tile_size - 1) / ri.tile_size;
    ri.buffer = (unsigned char*)malloc(ri.width * ri.height * 3);
    std::atomic_int num_tiles = ri.num_tiles_x * ri.num_tiles_y;
    for (unsigned int n = 0; n < num_threads; ++n) {
        thread_info_t thread_info;
        thread_info.id = n;
        thread_info.render_info = &ri;
        threads.emplace_back(render, std::move(thread_info), std::ref(num_tiles));
    }

    for (auto& thread : threads)
        thread.join();

    std::ofstream ofs;
    ofs.open("./output.ppm", std::ios::binary);
    ofs << "P6\n" << ri.width << " " << ri.height << "\n255\n";
    ofs.write((char*)ri.buffer, ri.width * ri.height * 3);
    ofs.close();
    free(ri.buffer);

    return EXIT_SUCCESS;
}
Source Link

Simple multi-threading setup for tile rendering (C++)

I am writing a small framework that I intend to build upon in the future. I am also using this framework to teach people how to support multi-threading in 3D rendering, and thus wanted to be sure I was exposing them to code that was decently clean.

The goal is to write a simple example in which we split a frame into tile (smaller chunks of say 32x32 pixels) and get the threads to render the pixels of each tile. I want to use the minimalist approach and thus avoid doing something like a task scheduler. Therefore I have chosen to calculate the number of tiles in the main program, store that in an atomic variable, and use that in the threads to provide each thread with a current tile number. Threads then decrement that number and keep doing so while that total number of tiles is greater or equal to 0.

I have chosen to store the current values for the calculated pixels for each tile in a small buffer managed by the thread and then copy the values of the tile's pixels into the image buffer. I believe this is thread-safe but would love to get your feedback on that.

To make the exercise more fun, I am currently assigning a random color to each tile before I implement the rest of the rendering process. I am using a random number generator from the standard lib for now. I declared the generator thread_local though to be honest unless I actually call the function render() reclusively or need other functions called from within the thread function to use the generator I don't think this is necessary. As long as each thread own its own random number generator I believe this is thread safe too.

Your feedback would be greatly appreciated, where here what I am looking for is:

  1. Simplicity: easier for people to learn from
  2. Correctness: if people learn from that, it better be correct and ideally pass a C++ peer review.
  3. Efficiency: while I am not looking to save 2 or 3 cycles if you see things that could definitely impact performance please let me know. One thing I have been thinking about at this point in time is that the way I allocate memory dynamically is not caring about alignment. So that's one possible thing I could add / improve.

What do you think. Here is the code:

#include <thread>
#include <atomic>
#include <iostream>
#include <fstream>
#include <mutex>
#include <random>

struct render_info_t
{
    unsigned int width { 640 };
    unsigned int height { 480 };
    unsigned int tile_size { 32 };
    unsigned char *buffer { nullptr };
    unsigned int num_tiles_x{}, num_tiles_y{};
};

struct thread_info_t
{
    unsigned int id;
    const render_info_t* render_info;
};

void render(const thread_info_t& thread_info, std::atomic_int &count)
{
    int curr_tile {};
    const render_info_t* ri = thread_info.render_info;
    thread_local std::mt19937 gen(std::hash<std::jthread::id>()(std::this_thread::get_id()));
    std::uniform_real_distribution<float> dist(0.0f, 1.f);
    unsigned char *buffer = (unsigned char*)malloc(ri->tile_size * ri->tile_size * 3);
    
    while ((curr_tile = --count) >= 0) {
        unsigned char *curr_pixel = buffer;
        float r = dist(gen);
        float g = dist(gen);
        float b = dist(gen);

        unsigned int curr_tile_y = curr_tile / ri->num_tiles_x;
        unsigned int curr_tile_x = curr_tile - curr_tile_y * ri->num_tiles_x;
        unsigned int x0 = curr_tile_x * ri->tile_size;
        unsigned int x1 = std::min((curr_tile_x + 1) * ri->tile_size, ri->width);
        unsigned int y0 = curr_tile_y * ri->tile_size;
        unsigned int y1 = std::min((curr_tile_y + 1) * ri->tile_size, ri->height);
        for (unsigned int y = y0; y < y1 ; ++y) {
            for (unsigned int x = x0; x < x1; ++x, curr_pixel += 3) {
                /*
                ** TODO trace ray at pixel coordinate x, y
                */
                curr_pixel[0] = (unsigned char)(r * 255);
                curr_pixel[1] = (unsigned char)(g * 255);
                curr_pixel[2] = (unsigned char)(b * 255);
            }
        }

        // copy the tile's pixels into the image buffer
        unsigned char *row = buffer;
        unsigned char *from = ri->buffer + (y0 * ri->width + x0) * 3;
        for (unsigned int y = y0; y < y1 ; ++y, row += ri->tile_size * 3, from += ri->width * 3) { // , from += ri->width * 3
            // the data pointed by the pointer ri->buffer is not part of the struct, so it can be change
            memcpy(from, row, ri->tile_size * 3);
        }
    }
    free(buffer);
}

int main(int argc, char **argv)
{
    unsigned int num_threads = std::jthread::hardware_concurrency();
    std::cout << "Rendering with " << num_threads << " threads" << std::endl;
    std::vector<std::jthread> threads;
    render_info_t ri;
    ri.num_tiles_x = (ri.width + ri.tile_size - 1) / ri.tile_size;
    ri.num_tiles_y = (ri.height + ri.tile_size - 1) / ri.tile_size;
    ri.buffer = (unsigned char*)malloc(ri.width * ri.height * 3);
    std::atomic_int num_tiles = ri.num_tiles_x * ri.num_tiles_y;
    for (unsigned int n = 0; n < num_threads; ++n) {
        thread_info_t thread_info;
        thread_info.id = n;
        thread_info.render_info = &ri;
        threads.emplace_back(render, std::move(thread_info), std::ref(num_tiles));
    }

    for (auto& thread : threads)
        thread.join();

    std::ofstream ofs;
    ofs.open("./output.ppm", std::ios::binary);
    ofs << "P6\n" << ri.width << " " << ri.height << "\n255\n";
    ofs.write((char*)ri.buffer, ri.width * ri.height * 3);
    ofs.close();
    free(ri.buffer);

    return EXIT_SUCCESS;
}