Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Project 3: Utkarsh Dwivedi #23

Open
wants to merge 49 commits into
base: main
Choose a base branch
from

Conversation

utkarshdwivedi3997
Copy link

@utkarshdwivedi3997 utkarshdwivedi3997 commented Oct 10, 2023

Using 3 late days

Repo link

Features Implemented:

Core Features

  • Material shading (diffuse, perfectly specular, imperfect specular)
  • Toggleable path termination with stream compaction
  • Toggleable material sorting
  • Toggleable caching of first bounces

Extra Features

  • Anti-aliasing with subpixel jittering

  • Depth of Field with thin-lens camera

  • Refraction

  • Fresnel effect

  • Glass / Plastic materials (mix of refraction, reflection and lambertian diffuse shading)

  • GLTF Mesh loading (handles both non-interleaved and interleaved buffers)

  • Russian Roulette

  • Bounding Volume Hierarchy with longest axis splitting

  • "Final rays post processing?": Gamma correction and HDR conversion

Modifications to CMakeLists:

Added this to the headers section:

  • external/include/tiny_gltf.h

tinygltf has been included in the project

Adding feedback on 10/20/23

Overall this project was extremely disappointing and did not feel rewarding to me. As someone who has taken CIS 561 (which I presume is a good chunk of people who take this course), I don't feel like I learned much through this assignment. My assumption going into it was that I would learn many CUDA specific optimisation tricks while working on the assignment, but by the end of working on it I only found that my time was spent simply on implementing a path tracer's basic visual features and adding more features to make a barely functioning path tracer.

I have many things to say about this project and I hope it is taken into account and the project is updated for future students:

  1. Instead of making people write a path-tracer, I'd have liked to be given a path-tracer that was already implemented, and then be asked to find where and how it was unoptimized, and then directly use everything we have learned in class to optimise the path tracer. The project is a little too open-ended, and I did not realise I should have simply focused on optimisations until I was done with the project. There also is not enough time to both implement a good looking path-tracer and perform optimisations on top of it.
  2. The code-base is severely outdated and tunnels people into implementing the path tracer in very specific functions and code styles. While it is allowed to completely rewrite the whole path-tracer, that is not realistic for a two week long project.
  3. There are many instances in the starter base code where a single line of code does an obscene number of computations. It makes it extremely unreadable. The point of starter code is that it should be easy to grasp: as a student I should not need to spend more time than is reasonable to simply understand what the base code is doing. That is time wasted that I could have spent implementing features. A clear example of this is in the generateRayFromCamera() kernel:
		segment.ray.direction = glm::normalize(cam.view
			- cam.right * cam.pixelLength.x * ((float)x - (float)cam.resolution.x * 0.5f)
			- cam.up * cam.pixelLength.y * ((float)y - (float)cam.resolution.y * 0.5f)
		);
  1. The starter base code seems to follow no code/naming conventions. Variables are sometimes named in camelCase, sometimes in snake_case, and sometimes alllowercase. A student shouldn't have to write this as feedback.
  2. The cosine weighted hemisphere distribution code uses some 1/sqrt(3) math that is not even explained. We're simply pointed to whoever Peter Kutz is, without any links to an article or video describing what that method is doing.
  3. I genuinely do not understand the reason for making the first bounce cache be a requirement for this assignment. I don't see a real world scenario in which this is useful. The moment any randomness is added (like AA or DOF), this becomes a useless optimisation that is a waste of time that could have been spent in a better use case.
  4. Recommending using GLTF instead of OBJ for loading meshes does not make sense for this project. The purpose of GLTF (at least from what I understood based on the very limited time I got to learn it during this project), is that it makes buffering data into the GPU easy. It stores the buffers directly so that all we have to do is pass it to open GL or Vulkan. Using it in a CUDA path tracer makes no sense because first those buffers have to be parsed in a way that makes sense to CUDA. After I had already spent a lot of time learning and implementing GLTF I realised my time could have been better spent making a more physically accurate path tracer or learning more about CUDA optimisations.
  5. This course is generally handled with a lack of importance given to CPU side optimisations, but that is not how things really work. I learned more about optimising graphics through optimising my game than this assignment. Optimisation is less about any one specific method, more about profiling and then finding the bottleneck and then optimising that bottleneck. Many times that reveals more bottlenecks that were the real cause and were simply shielded by the bottleneck that was just fixed. Many times aggressive optimisation from the beginning is a really bad idea and a simple waste of time. We should get this experience from an assignment in this class, not a project about implementing a path tracer. We have a whole class dedicated to that.
  6. There's a general lack of guidance in this project that would enable us to make better choices about what features we want to implement.

…trically interpolated normals when vertex normals are available, otherwise flat shaded normals
…t case), added russian roulette but there's no performance benefit
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.

1 participant