Code refactor: make basic_viewport and aa_viewport no longer a child class. Move data into classes.

Code refactor: make threading.h more specific, reducing redundant data copies in memory.
Code refactor: make camera parameters more clear and well-defined in viewport.h, ready to extend camera flexibility.

4 files changed, 143 insertions, 106 deletions

diff --git a/aa.h b/aa.h
index 988a2f7..3a4def4 100644
--- a/aa.h
+++ b/aa.h
@@ -14,64 +14,71 @@
 #include <random>
 
 // Antialiasing viewport
-template<typename T>
-class aa_viewport : public viewport<T> {
+// U: color depth, V: pos
+template<typename U, typename V>
+class aa_viewport {
+    vec3<V> cxyz;
+    vec3<V> screen_center;
+    uint32_t image_width; // how many pixels every row has
+    uint32_t image_height; // how many pixels every column has
+    V screen_hw; // determined if screen_height is known
+    V screen_hh; // determined if screen_width is known
+    hitlist &world;
     unsigned samples;
-    std::vector<basic_viewport<T>> *subviews;
     int threads;
 
 public:
-    aa_viewport(double width, double height, vec3d viewport_center, unsigned samples, int threads = -1)
-            : samples(samples), threads{(threads > 0) ? threads : (int)std::thread::hardware_concurrency()} {
-        assert(samples >= 1);
-        subviews = new std::vector<basic_viewport<T>>{samples, {width, height, viewport_center}};
-    }
 
-    ~aa_viewport() {
-        delete subviews;
+    aa_viewport(const vec3<V> &cxyz,
+                const vec3<V> &screen_center,
+                uint16_t image_width,
+                uint16_t image_height,
+                V screen_hw,
+                V screen_hh,
+                hitlist &world,
+                unsigned samples,
+                int threads = -1) :
+            cxyz(cxyz),
+            screen_center(screen_center),
+            image_width(image_width),
+            image_height(image_height),
+            screen_hw(screen_hw),
+            screen_hh(screen_hh),
+            world(world),
+            samples(samples),
+            threads((threads > 0) ? threads : (int) std::thread::hardware_concurrency()) {
+        assert(samples >= 1);
     }
 
-    virtual bitmap<T> render(const hitlist &world, vec3d viewpoint, uint16_t image_width, uint16_t image_height) {
+    bitmap<U> render() {
         static constexpr auto seed = 123456789012345678ULL;
-        const unsigned thread_count = std::min((unsigned)threads, samples);
+        const unsigned thread_count = std::min((unsigned) threads, samples);
         std::cerr << "Preparing tasks..." << std::endl;
-        std::vector<bitmap<T>> images{samples, {0, 0}};
+        std::vector<bitmap<U>> images{samples, {0, 0}};
         std::mt19937_64 seedgen{seed}; // generates seeds for workers
 
-        const struct s_render_shared {
-            std::vector<basic_viewport<T>> &subs;
-            vec3d viewpoint;
-            uint16_t image_width;
-            uint16_t image_height;
-            const hitlist &world;
-            std::vector<bitmap<T>> &images;
-        } s_{.subs=*subviews, .viewpoint = viewpoint,
-                .image_width=image_width, .image_height=image_height,
-                .world=world, .images=images
-        };
-
         struct s_render_task {
-            uint32_t task_id;
-            uint64_t seed;
+            uint64_t bias_seed;
             uint64_t diffuse_seed;
-            const s_render_shared &shared;
         };
 
-        thread_pool<s_render_task> pool{thread_count};
+        thread_pool<s_render_task, typeof(*this), typeof(images)> pool{thread_count, *this, images};
         timer tim{true};
 
         std::cerr << "Seeding tasks..." << std::endl;
         tim.start_measure();
         for (typeof(samples) i = 0; i < samples; ++i) {
-            pool.submit_task([](s_render_task &task) {
-                bias_ctx bc{task.seed};
-                auto image = task.shared.subs[task.task_id].render(
-                        task.shared.world, task.shared.viewpoint,
-                        task.shared.image_width, task.shared.image_height,
-                        bc, task.diffuse_seed);
-                task.shared.images[task.task_id] = image;
+            pool.submit_task([](size_t tid, s_render_task &task, const aa_viewport<U, V> &ctx, typeof(images) &images) {
+                basic_viewport<U, V> vp{
+                        ctx.cxyz, ctx.screen_center,
+                        ctx.image_width, ctx.image_height,
+                        ctx.screen_hw, ctx.screen_hh,
+                        ctx.world
+                };
+                bias_ctx bc{task.bias_seed};
+                images[tid] = vp.render(task.diffuse_seed, bc);
             }, s_render_task{
-                    .task_id = i, .seed=seedgen(), .diffuse_seed=seedgen(), .shared=s_
+                    .bias_seed=seedgen(), .diffuse_seed=seedgen()
             });
         }
         tim.stop_measure();
@@ -84,12 +91,10 @@ public:
         tim.stop_measure();
         std::cerr << "Finish rendering sub-pixels. Speed: " << 1.0 * tim.duration().count() / samples << "sec/image, "
                   << 1.0 * samples * image_width * image_height / tim.duration().count() << " pixel/sec" << std::endl;
-        return bitmap<T>::average(images);
+        return bitmap<U>::average(images);
     }
 
 };
 
-using aa_viewport8b = aa_viewport<uint8_t>;
-
 
 #endif //RT_AA_H
diff --git a/main_simple_scanner.cpp b/main_simple_scanner.cpp
index 6e79293..e3aedc6 100644
--- a/main_simple_scanner.cpp
+++ b/main_simple_scanner.cpp
@@ -23,8 +23,10 @@
 //#define SCENE_REFLECT
 #define SCENE_DIALECT
 
-// T: intermediate color depth
-template<typename T>
+static constexpr uint64_t default_diffuse_seed = 123456789012345678ULL;
+
+// T: color depth, V: pos
+template<typename T, typename V>
 void generate_image(uint16_t image_width, uint16_t image_height, double viewport_width, double focal_length,
                     double sphere_z, double sphere_r, unsigned samples, const std::string &caption = "",
                     unsigned caption_scale = 1) {
@@ -35,13 +37,30 @@ void generate_image(uint16_t image_width, uint16_t image_height, double viewport
     }
     std::cerr << "Initializing context..." << std::endl;
     double r = 1.0 * image_width / image_height;
-    viewport<T> *vp;
-    if (samples == 1) {
-        vp = new basic_viewport<T>{viewport_width, viewport_width / r, vec3d{0, 0, -focal_length}};
-    } else {
-        vp = new aa_viewport<T>{viewport_width, viewport_width / r, vec3d{0, 0, -focal_length}, samples};
-    }
     hitlist world;
+
+    ////////////////
+    // noaa rendering
+    bias_ctx no_bias{};
+    basic_viewport<T, V> vp_noaa{
+            vec3<V>::zero(), // camera position as the coordinate origin
+            vec3d{0, 0, -focal_length},
+            image_width, image_height,
+            viewport_width / 2.0, ((double) image_height / image_width) * viewport_width / 2.0,
+            world
+    };
+    ////////////////
+
+    ////////////////
+    // aa rendering
+    aa_viewport<T, V> vp_aa{
+            vec3<V>::zero(), // camera position as the coordinate origin
+            vec3d{0, 0, -focal_length},
+            image_width, image_height,
+            viewport_width / 2.0, ((double) image_height / image_width) * viewport_width / 2.0,
+            world, samples
+    };
+    ////////////////
 #ifdef SCENE_DIFFUSE
     material_diffuse_lambertian materi{0.5};
     world.add_object(std::make_shared<sphere>(
@@ -76,8 +95,7 @@ void generate_image(uint16_t image_width, uint16_t image_height, double viewport
     timer tm;
     std::cerr << "Rendering..." << std::endl;
     tm.start_measure();
-    auto image = vp->render(world, vec3d::zero(),
-                            image_width, image_height); // camera position as the coordinate origin
+    auto image = ((samples == 1) ? vp_noaa.render(default_diffuse_seed, no_bias) : vp_aa.render());
     tm.stop_measure();
     std::cerr << "Applying gamma2..." << std::endl;
     tm.start_measure();
@@ -94,7 +112,6 @@ void generate_image(uint16_t image_width, uint16_t image_height, double viewport
     } else {
         std::cerr << "NOPRINT is defined. PPM Image won't be printed." << std::endl;
     }
-    delete vp;
 }
 
 int main(int argc, char **argv) {
@@ -115,9 +132,9 @@ int main(int argc, char **argv) {
         cap = std::string{argv[8]};
     }
     const auto image_width = std::stoul(iw);
-    generate_image<uint16_t>(image_width, std::stoul(ih),
-                             std::stod(vw), std::stod(fl),
-                             std::stod(sz), std::stod(sr),
-                             std::stoul(sp), cap,
-                             std::max((int) (1.0 * image_width * 0.010 / 8), 1));
+    generate_image<uint16_t, double>(image_width, std::stoul(ih),
+                                     std::stod(vw), std::stod(fl),
+                                     std::stod(sz), std::stod(sr),
+                                     std::stoul(sp), cap,
+                                     std::max((int) (1.0 * image_width * 0.010 / 8), 1));
 }
\ No newline at end of file
diff --git a/threading.h b/threading.h
index 73a1741..6d79494 100644
--- a/threading.h
+++ b/threading.h
@@ -18,31 +18,36 @@
 // Tasks should be added into the queue before starting.
 // Once the task queue is empty, threads quit.
 
+template<typename T_Args, typename T_ImmuCtx, typename T_MutCtx>
+using task_func_t = void (*)(size_t, T_Args &, const T_ImmuCtx &, T_MutCtx &);
+
 // internal usage
-template<typename T>
+template<typename T, typename U, typename V>
 struct s_task {
-    void (*f)(T &);
-
+    task_func_t<T, U, V> f;
     T arg;
 };
 
-template<typename T>
+template<typename T, typename U, typename V>
 class thread_pool {
 
     unsigned thread_count;
     std::vector<std::thread> workers;
     std::atomic<size_t> counter{0}; // index to the first available task in queue
-    std::vector<s_task<T>> tasks;
+    std::vector<s_task<T, U, V>> tasks;
+    const U &shared_ctx; // reference to immutable shared context
+    V &mut_shared_ctx; // mutable shared context
 
     void worker_main();
 
 public:
-    explicit thread_pool(unsigned thread_count) : thread_count{thread_count} {
+    explicit thread_pool(unsigned thread_count, const U &shared_ctx, V &mut_shared_ctx) :
+            thread_count{thread_count}, shared_ctx{shared_ctx}, mut_shared_ctx{mut_shared_ctx} {
         std::cerr << "Using " << (counter.is_lock_free() ? "lock-free" : "locking") << " dispatcher." << std::endl;
     }
 
     // Thread unsafe!
-    void submit_task(void (*f)(T &), T &&t);
+    void submit_task(task_func_t<T, U, V> f, T &&t);
 
     void start();
 
@@ -50,36 +55,36 @@ public:
     void wait();
 };
 
-template<typename T>
-void thread_pool<T>::start() {
+template<typename T, typename U, typename V>
+void thread_pool<T, U, V>::start() {
     if (workers.empty()) {
         for (typeof(thread_count) i = 0; i < thread_count; ++i) {
-            workers.emplace_back(std::thread{&thread_pool<T>::worker_main, this});
+            workers.emplace_back(std::thread{&thread_pool<T, U, V>::worker_main, this});
         }
     } else {
         // TODO
     }
 }
 
-template<typename T>
-void thread_pool<T>::worker_main() {
+template<typename T, typename U, typename V>
+void thread_pool<T, U, V>::worker_main() {
     const auto max_cnt = tasks.size();
     while (true) {
         const auto i = counter.fetch_add(1, std::memory_order_relaxed); // we only need atomicity
         if (i >= max_cnt) break; // all tasks are done
         auto &task = tasks[i];
-        task.f(task.arg);
+        task.f(i, task.arg, shared_ctx, mut_shared_ctx);
     }
 }
 
 // Do not submit after starting.
-template<typename T>
-void thread_pool<T>::submit_task(void (*f)(T &), T &&t) {
-    tasks.push_back(s_task<T>{.f=f, .arg=std::move(t)});
+template<typename T, typename U, typename V>
+void thread_pool<T, U, V>::submit_task(task_func_t<T, U, V> f, T &&t) {
+    tasks.push_back(s_task<T, U, V>{.f=f, .arg=std::move(t)});
 }
 
-template<typename T>
-void thread_pool<T>::wait() {
+template<typename T, typename U, typename V>
+void thread_pool<T, U, V>::wait() {
     for (auto &th: workers) {
         th.join();
     }
diff --git a/viewport.h b/viewport.h
index 2924582..02c1466 100644
--- a/viewport.h
+++ b/viewport.h
@@ -18,6 +18,7 @@
 #include <iostream>
 #include <cstdint>
 #include <random>
+#include <cmath>
 
 // bias context, used for placing sub-pixels
 class bias_ctx {
@@ -41,32 +42,40 @@ public:
     }
 };
 
-template<typename T>
-class viewport {
-public:
-    virtual bitmap<T> render(const hitlist &world, vec3d viewpoint, uint16_t image_width, uint16_t image_height) = 0;
-
-    virtual ~viewport() = default;
-};
-
+// TODO rename to camera
 // Single sampled viewport which supports bias sampling
-template<typename T>
-class basic_viewport : public viewport<T> {
-    const double half_width, half_height; // viewport size
-    const vec3d center; // coordinate of the viewport center point
-
+// U: color depth, V: pos
+template<typename U, typename V>
+class basic_viewport {
+    vec3<V> cxyz; // coordinate of the focus point
+    vec3<V> screen_center;
+//    double pitch; // TODO implement
+//    double yaw; // TODO implement
+    uint32_t image_width; // how many pixels every row has
+    uint32_t image_height; // how many pixels every column has
+    V screen_hw; // determined if screen_height is known
+    V screen_hh; // determined if screen_width is known
+//    double fov_h; // horizontal FOV, determined if screen_width or screen_height is known
+//    double focus_length; // distance between the focus point and the image screen
+    hitlist &world;
 public:
-    basic_viewport() = delete;
 
-    basic_viewport(double width, double height, vec3d viewport_center) :
-            half_width(width / 2.0), half_height(height / 2.0), center(viewport_center) {}
+    basic_viewport(const vec3<V> &cxyz, const vec3<V> &screen_center,
+                   uint32_t image_width, uint32_t image_height,
+                   double fov_h, hitlist &world) :
+            cxyz{cxyz}, screen_center{screen_center}, image_width{image_width}, image_height{image_height},
+            screen_hw{(cxyz - screen_center).norm() * tan((double) fov_h / 2.0)},
+            screen_hh{screen_hw * ((double) image_height / image_width)},
+            world{world} {}
 
-    virtual bitmap<T>
-    render(const hitlist &world, vec3d viewpoint, uint16_t image_width, uint16_t image_height) override {
-        bias_ctx bc{};
-        static constexpr uint64_t default_diffuse_seed = 123456789012345678ULL;
-        return render(world, viewpoint, image_width, image_height, bc, default_diffuse_seed);
-    }
+    basic_viewport(const vec3<V> &cxyz, const vec3<V> &screen_center,
+                   uint32_t image_width, uint32_t image_height,
+                   double screen_hw, double screen_hh,
+                   hitlist &world) :
+            cxyz{cxyz}, screen_center{screen_center}, image_width{image_width}, image_height{image_height},
+            screen_hw{screen_hw},
+            screen_hh{screen_hh},
+            world{world} {}
 
     /**
      * Generate the image seen on given viewpoint.
@@ -74,13 +83,14 @@ public:
      * @param by bias on y axis (0.0 <= by < 1.0)
      * @return
      */
-    virtual bitmap<T> render(const hitlist &world, vec3d viewpoint,
-                             uint16_t image_width, uint16_t image_height,
-                             bias_ctx &bias, uint64_t diffuse_seed) const {
-        bitmap<T> image{image_width, image_height};
+    bitmap<U> render(uint64_t diffuse_seed, bias_ctx &bias
+            /* by putting thread-specific parameters in call argument list, make users convenient*/) const {
+        // The implementation keep all mutable state in local stack,
+        // keeping the class immutable and thread-safe.
+        bitmap<U> image{image_width, image_height};
         random_uv_gen_3d ruvg{diffuse_seed};
-        double bx, by;
-        const auto r = center - viewpoint;
+        V bx, by;
+        const auto r = screen_center - cxyz;
         const int img_hw = image_width / 2, img_hh = image_height / 2;
         // iterate over every pixel on the image
         for (int j = -img_hh + 1; j <= img_hh; ++j) { // axis y, transformation is needed
@@ -91,13 +101,13 @@ public:
                 assert(bx < 1.0);
                 assert(by < 1.0);
                 const vec3d off{
-                        .x=(1.0 * i + bx) / img_hw * half_width,
-                        .y=(1.0 * j + by) / img_hh * half_height,
+                        .x=(1.0 * i + bx) / img_hw * screen_hw,
+                        .y=(1.0 * j + by) / img_hh * screen_hh,
                         .z=0.0
                 }; // offset on screen plane
                 const auto dir = r + off; // direction vector from camera to current pixel on screen
-                ray3d ray{viewpoint, dir}; // from camera to pixel (on the viewport)
-                const auto pixel = world.color<T>(ray, ruvg);
+                ray3d ray{cxyz, dir}; // from camera to pixel (on the viewport)
+                const auto pixel = world.color<U>(ray, ruvg);
                 const auto x_ = i + img_hw, y_ = -j + img_hh;
                 image.set(x_, y_, pixel);