lighttransport / nanort

NanoRT, single header only modern ray tracing kernel.
MIT License
1.07k stars 89 forks source link

A couple minor typo corrections #51

Closed Tyler799 closed 4 years ago

Tyler799 commented 4 years ago

I noticed a handful as I was working, thought I could clean them up. There's also the fact that many non-sentences have a period after them, but that's minor and I didn't want to make a fuss of it.

Also, I noticed "Thomas-Mueller" style barycentric coordinates. Is that right? I can't find any reference to that name. https://www.sciencedirect.com/science/article/pii/S0091679X18300128 mentions a Thomas Müller-Reichert.

There's also the well-known Möller-Trumbore intersection algorithm.

Is that referencing one of those, or someone else?

As a sidenote, what style are these files supposed to follow? Allman style? K&R? It looks like a mix of both. Was wondering if there's a style specified somewhere.

I'm thinking of doing some cleanup for a few of the methods. Nothing critical or functional, mainly just making them a little more clean. std::min and std::max existed in C++03 and can help with a few of them, a few have redundant conditional checks or ones that could be re-written, etc.

Adding doxygen comment blocks to each template or method might also be a great help to users. I wouldn't mind doing that with some assistance.

syoyo commented 4 years ago

Awesome!

Thomas-Mueller" style barycentric coordinates. Is that right?

No. As you pointed out, Möller-Trumbore is the correct one.

I meant a method described in "Fast Minimum Storage RayTriangle Intersectio" http://webserver2.tecgraf.puc-rio.br/~mgattass/cg/trbRR/Fast%20MinimumStorage%20RayTriangle%20Intersection.pdf

As a sidenote, what style are these files supposed to follow

Please use clang-format(using clang 9.0 would be better) to format the source code. .clang-format is provided: https://github.com/lighttransport/nanort/blob/master/.clang-format

I'm thinking of doing some cleanup for a few of the methods. Nothing critical or functional, mainly > just making them a little more clean. std::min and std::max existed in C++03 and can help with a > few of them, a few have redundant conditional checks or ones that could be re-written, etc.

Yes, further cleanup is appreciated.

Adding doxygen comment blocks to each template or method might also be a great help to users. I wouldn't mind doing that with some assistance.

I see. Recently I've started adding doxygen comments to other projects for example https://github.com/syoyo/tacotron-tts-cpp/blob/master/src/tf_synthesizer.h#L23

I think it'd be better to write doxygen comment by myself firstly. Then you can review it.

Tyler799 commented 4 years ago

Alright. Thanks for the reply. I'll be away from my dev setup for awhile, but may have some of that by sometime Sunday in another pr.

Tyler799 commented 4 years ago

Was this going to be merged? Is there anything I needed to change?

syoyo commented 4 years ago

Was this going to be merged? Is there anything I needed to change?

Oh, sorry. I have just merged. Additional changes(e.g. apply clang-format) will be done in other commits/PRs.