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 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.

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()` 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.

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 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;
	}