Skip to content

Fix critical logic errors in distributed path tracer - #6

Draft
vmanam0451 with Copilot wants to merge 4 commits into
ecs-cdkfrom
copilot/review-logic-errors-concurrency
Draft

Fix critical logic errors in distributed path tracer#6
vmanam0451 with Copilot wants to merge 4 commits into
ecs-cdkfrom
copilot/review-logic-errors-concurrency

Conversation

Copilot AI commented Jan 14, 2026

Copy link
Copy Markdown

Code review identified several serious bugs preventing the distributed path tracer from functioning correctly: incorrect ray routing, off-by-one resolution errors, and NDC calculation using tile dimensions instead of global image dimensions.

Ray Routing Fixes

  • ACCUMULATE stage rays dropped: map_ray_stage_to_queue doesn't handle ACCUMULATE—changed to m_batch_sender.enqueue_ray(ray, MASTER_ID)
  • Direct lighting results misrouted: Added owner check before routing (matching existing object intersection logic)
// Before: always local
map_ray_stage_to_queue(best_ray);

// After: route to owner
if (best_ray.worker_id == m_worker_info.worker_id)
    map_ray_stage_to_queue(best_ray);
else {
    best_ray.type = models::ray_type::OWN;
    m_batch_sender.enqueue_ray(best_ray, best_ray.worker_id);
}

Resolution/Bounds Fixes

  • Master resolution off-by-one: (max - min)(max - min) + 1
  • Master bounds in preprocessor: max_x: Xmax_x: X - 1 (inclusive bounds)
  • NDC using tile resolution: Workers calculated NDC from local tile size, causing all tiles to render same view. Added image_width/image_height fields to worker_info

Concurrency Fix

  • Batch sender data loss on shutdown: Removed early break in flush_loop() that skipped final flush when m_terminate set

Initialization

  • Added proper default values for cloud_ray fields (object_intersect_distance, alpha, type)
  • Fixed uvec2 declarations incorrectly using fvec2 constructor
Original prompt

Review my code.

Compile all SERIOUS logic errors.

Focus on any issues pertaining to concurrency (path-tracer-core)
Focus on incorrect aws resources setup (path-tracer-preprocessor & cdk)
Focus on incorrect usage of aws resources such as sns & sqs (path-tracer-core & path-tracer-preprocessor)

Essentially, verify that the application will generate a scene and work


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI and others added 3 commits January 14, 2026 19:18
…ssor

Fixes:
1. Master resolution off-by-one: Added +1 to resolution calculation
2. Master bounds in preprocessor: Fixed max_x/max_y to use X-1/Y-1
3. ACCUMULATE stage routing: Changed from map_ray_stage_to_queue to
   m_batch_sender.enqueue_ray for master
4. Direct lighting results routing: Added proper owner check before
   routing (similar to object intersection results)
5. Batch sender flush: Removed early break to ensure final flush
6. Global resolution for NDC: Added image_width/image_height fields
   and global_resolution member for correct ray generation
7. Ray initialization: Added proper initialization for all cloud_ray fields

Co-authored-by: vmanam0451 <25410102+vmanam0451@users.noreply.github.com>
…t fvec2)

Co-authored-by: vmanam0451 <25410102+vmanam0451@users.noreply.github.com>
Co-authored-by: vmanam0451 <25410102+vmanam0451@users.noreply.github.com>
Copilot AI changed the title [WIP] Review code for logic and AWS resource issues Fix critical logic errors in distributed path tracer Jan 14, 2026
Copilot AI requested a review from vmanam0451 January 14, 2026 19:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants