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

Edit pass on Chapter 01 #8

Closed
wants to merge 1 commit into from
Closed

Edit pass on Chapter 01 #8

wants to merge 1 commit into from

Conversation

rupsis
Copy link
Collaborator

@rupsis rupsis commented Feb 16, 2024

All changes presented here are suggestions.

These suggestions are largely subjective, with the occasional objective suggestion (i.e spelling correction).
I'll try to annotate my changes, and provide additional editing notes.

Feel free to either open a dialogue on this PR, and I'll work with you on edits. Or use this as a reference (cherry pick suggestions) and close the PR out.

Note: This is an editing pass, and by no means a comprehensive revision.

variety of optical phenomena and can be used to render photorealistic images. It is one of the
foundational techniques in Computer Graphics used to model light transport.
_Ray Tracing_ is a rendering method in Computer Graphics that simulates the flow of light.
It can accurately recreate a variety of optical phenomena and be used to render photorealistic images.
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Slightly more concise opening

Shirley's [_Ray Tracing In One Weekend_][RTIOW] shows you how to get started with a CPU path tracer
objects based on surface properties. The algorithm is remarkably simple and relatively easy
to build a basic path tracer, which supports a small number of material and geometry types. Peter
Shirley's [_Ray Tracing In One Weekend_][RTIOW] (RTIOW) shows you how to get started with a CPU path tracer
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The acronym "RTIOW" is used without being introduced.

significant speedup by distributing the work across many processor cores. This brings us to GPUs.
Graphics -- the algorithm lends itself very well to parallelism. It is possible to achieve a
significant speedup by distributing the work across many processor cores. This brings us to
the Graphics Processing Unit (GPU).
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GPU acronym was used (albeit just slightly) before it was introduced.

over large amounts of data in parallel. This parallelism has been instrumental to achieving
realistic visuals in real-time applications like video games. GPUs were traditionally used to
accelerate _scanline rasterization_ but they have since become programmable and capable of running
a variety of parallel workloads. Notably, dedicated hardware cores to accelerate ray tracing are
becoming more common in modern GPUs.
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sentence read a little awkward.

the same material but I highly recommend completing _RTIOW_ before embarking on building
the GPU version since doing so will teach you the path tracing algorithm in a much more approachable
the GPU version. Doing so will teach you the path tracing algorithm in a much more approachable
way and it will make you appreciate both the advantages and challenges of moving to a GPU-based
architecture.
Copy link
Collaborator Author

@rupsis rupsis Feb 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A rather long run on sentence. Should we use the RTIOW acronym over full title for consistency?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good!


* For ease of Rust development I use [*anyhow*](https://docs.rs/anyhow/latest/anyhow/) and
[*bytemuck*](https://docs.rs/bytemuck/latest/bytemuck/). *anyhow* is a popular error handling
utility and integrates seamlessly. *bytemuck* provides a safe abstraction for the equivalent of
`reinterpret_cast` in C++, which normally requires [`unsafe`][rust-unsafe] Rust. I use it to
`reinterpret_cast` in C++, which normally requires [`unsafe`][rust-unsafe] Rust. It's used to
bridge CPU data types with their GPU equivalents.

* Lastly, I use [*pollster*](https://docs.rs/pollster/latest/pollster/) to execute asynchronous
wgpu API functions (which is only called from a single line).
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed some redundant "I use" verbiage, but we could go further and initially state "These are the dependencies used". Each dependency listed then can have justification for it's use.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the passive voice that you went with.

developing.
The [wgpu API documentation](https://docs.rs/wgpu/latest/wgpu/),
[WebGPU](https://www.w3.org/TR/webgpu/), and [WGSL](https://www.w3.org/TR/WGSL/) specifications are
great references to keep handy when you're developing.
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In line list probably should be a bulleted list, but 🤷


The GPU (Graphics Processing Unit) is a type of processor designed to run the same set of oparations
The GPU is a type of processor designed to run the same set of operations
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spelling -> operations

@armansito
Copy link
Collaborator

Hey @rupsis, sorry for taking a long time on getting back to you. I agree with most of your suggestions, I went ahead and applied them (with some minor tweaks) to the dev branch.

Please go ahead and close this PR. Thank you very much for the edits!

@rupsis
Copy link
Collaborator Author

rupsis commented May 28, 2024

Hey @rupsis, sorry for taking a long time on getting back to you. I agree with most of your suggestions, I went ahead and applied them (with some minor tweaks) to the dev branch.

Please go ahead and close this PR. Thank you very much for the edits!

Hey @armansito No worries at all :)

I'm happy to provide more edits in the future (provided they're useful) if you're interested.

@rupsis rupsis closed this May 28, 2024
@armansito
Copy link
Collaborator

Hey @rupsis, sorry for taking a long time on getting back to you. I agree with most of your suggestions, I went ahead and applied them (with some minor tweaks) to the dev branch.
Please go ahead and close this PR. Thank you very much for the edits!

Hey @armansito No worries at all :)

I'm happy to provide more edits in the future (provided they're useful) if you're interested.

Yes, I'd definitely appreciate your feedback on the rest of the book, especially once it's complete.

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.

None yet

2 participants