Modularize photon mapping system for better separation of concerns and testability

Makefile

@@ -5,11 +5,11 @@ CXX = g++
 CXXFLAGS =  -O2 -std=c++20 -Wall -Wextra -Iinclude
 
 # Core library sources (exclude CLI programs and main)
-LIB_SRCS = $(filter-out src/tonemap_cli.cpp src/pathtracer_cli.cpp src/main.cpp, $(wildcard src/*.cpp))
+LIB_SRCS = $(filter-out src/tonemap_cli.cpp src/pathtracer_cli.cpp src/main.cpp, $(wildcard src/*.cpp)) $(wildcard src/photon/*.cpp)
 LIB_OBJS = $(LIB_SRCS:src/%.cpp=build/%.o)
 
 # Individual test executables
-TEST_EXECUTABLES = build/test_p2 build/test_p3 build/test_bmp build/main build/test_geometry build/test_intersect build/test_parallel build/test_yaml
+TEST_EXECUTABLES = build/test_p2 build/test_p3 build/test_bmp build/main build/test_geometry build/test_intersect build/test_parallel build/test_yaml build/test_photon_mapping build/test_photon_render
 
 # CLI executables
 TONEMAP_SRCS = $(LIB_SRCS) src/tonemap_cli.cpp
@@ -50,6 +50,12 @@ build/test_parallel: test/test_parallel.cpp $(LIB_OBJS)
 build/test_yaml: test/test_yaml.cpp $(LIB_OBJS)
 	$(CXX) $(CXXFLAGS) -o $@ $^
 
+build/test_photon_mapping: test/test_photon_mapping.cpp $(LIB_OBJS)
+	$(CXX) $(CXXFLAGS) -o $@ $^
+
+build/test_photon_render: test/test_photon_render.cpp $(LIB_OBJS)
+	$(CXX) $(CXXFLAGS) -o $@ $^
+
 # Build the unified test executable
 $(TEST_EXEC): $(TEST_OBJS)
 	$(CXX) $(CXXFLAGS) -o $@ $^
@@ -63,11 +69,12 @@ $(PATHTRACER_EXEC): $(PATHTRACER_OBJS)
 
 # Build object files
 build/%.o: src/%.cpp
+	@mkdir -p $(dir $@)
 	$(CXX) $(CXXFLAGS) -c -o $@ $<
 
 # Clean up
 clean:
-	rm -f build/*.o $(TEST_EXECUTABLES) $(CLI_EXECS)
+	rm -f build/*.o build/photon/*.o $(TEST_EXECUTABLES) $(CLI_EXECS)
 
 # Run all tests
 test: $(TEST_EXECUTABLES)

docs/photon_mapping_modularization.md

@@ -0,0 +1,166 @@
+# Photon Mapping Modularization
+
+This document describes the modularization of the photon mapping system in the graphics renderer.
+
+## Problem Statement
+
+The original photon mapping implementation had several issues:
+1. **Tight coupling**: Photon mapping code was tightly integrated into the Scene class
+2. **Mixed responsibilities**: Scene class handled both scene management AND photon mapping
+3. **No clear separation**: Photon generation, storage, and querying were all mixed together
+4. **Hard to test**: Photon mapping functionality was embedded in Scene, making it hard to test independently
+5. **No abstraction**: No clear interface for photon mapping operations
+
+## New Modular Architecture
+
+The new system separates photon mapping into three distinct modules:
+
+### 1. PhotonMap (`include/photon/photon_map.hpp`)
+- **Responsibility**: Photon storage and spatial queries
+- **Key Features**:
+  - Stores photons using a KD-tree for efficient spatial queries
+  - Provides methods to build photon maps from photon lists
+  - Supports nearest neighbor queries with radius constraints
+  - Thread-safe and memory efficient
+
+### 2. PhotonMapper (`include/photon/photon_mapper.hpp`)
+- **Responsibility**: Photon generation and ray tracing
+- **Key Features**:
+  - Generates photons from light sources in a scene
+  - Traces photons through the scene using Monte Carlo methods
+  - Separates regular photons from caustic photons
+  - Handles photon flux calculation and Russian roulette termination
+
+### 3. PhotonMappingRenderer (`include/photon/photon_mapping_renderer.hpp`)
+- **Responsibility**: Radiance estimation using photon maps
+- **Key Features**:
+  - Performs photon density estimation for diffuse surfaces
+  - Handles specular and transmission bounces
+  - Integrates with existing kernel system for smoothing
+  - Supports both regular and caustic photon contributions
+
+## Directory Structure
+
+```
+include/photon/
+├── photon_map.hpp              # PhotonMap class
+├── photon_mapper.hpp           # PhotonMapper class  
+├── photon_mapping_renderer.hpp # PhotonMappingRenderer class
+└── photon_mapping.hpp          # Main include file
+
+src/photon/
+├── photon_map.cpp              # PhotonMap implementation
+├── photon_mapper.cpp           # PhotonMapper implementation
+└── photon_mapping_renderer.cpp # PhotonMappingRenderer implementation
+```
+
+## Key Classes
+
+### Photon Class
+```cpp
+class Photon {
+public:
+    Point position;      // Where the photon is stored
+    Direction incidentDir; // Direction the photon came from
+    RGB flux;           // Photon energy/flux
+
+    Photon(Point p, Direction d, RGB f);
+    double getPosition(std::size_t i) const; // For KD-tree indexing
+};
+```
+
+### PhotonMap Class
+```cpp
+class PhotonMap {
+public:
+    void build(const std::list<Photon>& photons);
+    std::vector<Photon> findNearestPhotons(const Point& queryPoint, 
+                                           unsigned long maxPhotons,
+                                           float maxRadius) const;
+    bool isBuilt() const;
+    size_t size() const;
+    void clear();
+};
+```
+
+### PhotonMapper Class
+```cpp
+class PhotonMapper {
+public:
+    void generatePhotonMaps(const Scene& scene, int numPhotons, unsigned maxBounces);
+    const PhotonMap& getRegularPhotonMap() const;
+    const PhotonMap& getCausticPhotonMap() const;
+    bool hasPhotonMaps() const;
+    void clear();
+};
+```
+
+## Usage Example
+
+```cpp
+// Create photon mapper
+auto photonMapper = std::make_shared<photon::PhotonMapper>();
+
+// Generate photon maps
+photonMapper->generatePhotonMaps(scene, 10000, 5);
+
+// Create renderer
+auto renderer = std::make_shared<photon::PhotonMappingRenderer>();
+renderer->setPhotonMapper(photonMapper);
+
+// Use in rendering
+RGB color = renderer->renderPixel(viewDirection, intersection, scene, config, kernel, maxBounces);
+```
+
+## Benefits
+
+1. **Separation of Concerns**: Each class has a single, well-defined responsibility
+2. **Testability**: Each component can be tested independently
+3. **Reusability**: PhotonMapper can be used with different rendering strategies
+4. **Maintainability**: Easier to understand and modify photon mapping code
+5. **Extensibility**: Easy to add new photon mapping algorithms or optimizations
+6. **Performance**: Specialized classes can be optimized for their specific tasks
+
+## Backward Compatibility
+
+The original Scene class methods (`generarMapaFotones`, `ecuacionRenderFotones`) are still available but marked as legacy. The new PhotonMappingStrategy automatically uses the new modular system.
+
+## Testing
+
+New tests have been added:
+- `test/test_photon_mapping.cpp` - Tests the photon mapping module functionality
+- `test/test_photon_render.cpp` - Tests actual rendering with photon mapping
+
+Run tests with:
+```bash
+make build/test_photon_mapping && ./build/test_photon_mapping
+make build/test_photon_render && ./build/test_photon_render
+```
+
+## Migration Guide
+
+For users who want to use the new photon mapping system directly:
+
+1. Include the photon mapping header:
+```cpp
+#include "photon/photon_mapping.hpp"
+```
+
+2. Create and configure photon mapper:
+```cpp
+auto photonMapper = std::make_shared<photon::PhotonMapper>();
+photonMapper->generatePhotonMaps(scene, numPhotons, maxBounces);
+```
+
+3. Create renderer:
+```cpp
+auto renderer = std::make_shared<photon::PhotonMappingRenderer>();
+renderer->setPhotonMapper(photonMapper);
+```
+
+4. Use in rendering loop:
+```cpp
+RGB color = renderer->renderPixel(viewDir, intersection, scene, config, kernel, bounces);
+```
+
+The PhotonMappingStrategy automatically uses this new system, so existing code using `RenderingAlgorithm::PHOTON_MAPPING` will automatically benefit from the improvements.

include/photon/photon_map.hpp

@@ -0,0 +1,66 @@
+#pragma once
+
+#include <list>
+#include <vector>
+#include "geometry.hpp"
+#include "RGB.hpp"
+#include "kdtree.h"
+
+namespace photon {
+
+class Photon {
+public:
+    Point position;
+    Direction incidentDir;
+    RGB flux;
+
+    Photon(Point p, Direction d, RGB f) : position(p), incidentDir(d), flux(f) {}
+    ~Photon() {}
+
+    double getPosition(std::size_t i) const {
+        switch(i) {
+            case 0: return position.x;
+            case 1: return position.y;
+            case 2: return position.z;
+            default: throw std::out_of_range("Index out of range for Point");
+        }
+    }
+};
+
+struct PhotonPositionAccessor {
+    float operator()(const Photon& p, std::size_t i) const {
+        return p.getPosition(i);
+    }
+};
+
+using PhotonKDTree = nn::KDTree<Photon, 3, PhotonPositionAccessor>;
+
+class PhotonMap {
+public:
+    PhotonMap() = default;
+    ~PhotonMap() = default;
+
+    // Build the photon map from a list of photons
+    void build(const std::list<Photon>& photons);
+
+    // Query nearest photons within a radius
+    std::vector<Photon> findNearestPhotons(const Point& queryPoint, 
+                                           unsigned long maxPhotons,
+                                           float maxRadius) const;
+
+    // Check if the photon map is built
+    bool isBuilt() const { return built_; }
+
+    // Get the number of photons in the map
+    size_t size() const { return photons_.size(); }
+
+    // Clear the photon map
+    void clear();
+
+private:
+    std::list<Photon> photons_;
+    PhotonKDTree kdTree_;
+    bool built_ = false;
+};
+
+} // namespace photon

include/photon/photon_mapper.hpp

@@ -0,0 +1,64 @@
+#pragma once
+
+#include <memory>
+#include <list>
+#include "photon_map.hpp"
+#include "geometry.hpp"
+#include "RGB.hpp"
+
+// Forward declarations
+class Scene;
+class PointLight;
+class RenderConfig;
+class Ray;
+class Intersection;
+
+namespace photon {
+
+class PhotonMapper {
+public:
+    PhotonMapper() = default;
+    ~PhotonMapper() = default;
+
+    // Generate photon maps for a scene
+    void generatePhotonMaps(const Scene& scene, 
+                           int numPhotons, 
+                           unsigned maxBounces);
+
+    // Get the regular photon map
+    const PhotonMap& getRegularPhotonMap() const { return regularPhotonMap_; }
+
+    // Get the caustic photon map
+    const PhotonMap& getCausticPhotonMap() const { return causticPhotonMap_; }
+
+    // Check if photon maps are built
+    bool hasPhotonMaps() const { return mapsBuilt_; }
+
+    // Clear all photon maps
+    void clear();
+
+private:
+    // Generate photons from a single light source
+    void generatePhotonsFromLight(const PointLight& light,
+                                  int numPhotons,
+                                  double totalEmission,
+                                  unsigned maxBounces,
+                                  const Scene& scene,
+                                  std::list<Photon>& regularPhotons,
+                                  std::list<Photon>& causticPhotons);
+
+    // Trace a photon through the scene
+    void tracePhoton(const Ray& ray,
+                     const RGB& flux,
+                     const Scene& scene,
+                     std::list<Photon>& regularPhotons,
+                     std::list<Photon>& causticPhotons,
+                     bool isCaustic,
+                     unsigned maxBounces) const;
+
+    PhotonMap regularPhotonMap_;
+    PhotonMap causticPhotonMap_;
+    bool mapsBuilt_ = false;
+};
+
+} // namespace photon

include/photon/photon_mapping.hpp

@@ -0,0 +1,16 @@
+#pragma once
+
+// Main header for the photon mapping module
+// This provides a convenient way to include all photon mapping components
+
+#include "photon_map.hpp"
+#include "photon_mapper.hpp"
+#include "photon_mapping_renderer.hpp"
+
+namespace photon {
+    // Re-export main classes for convenience
+    using Photon = photon::Photon;
+    using PhotonMap = photon::PhotonMap;
+    using PhotonMapper = photon::PhotonMapper;
+    using PhotonMappingRenderer = photon::PhotonMappingRenderer;
+}