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

Replace vec methods with operator overloads #230

Open
no-lex opened this issue Oct 28, 2021 · 0 comments
Open

Replace vec methods with operator overloads #230

no-lex opened this issue Oct 28, 2021 · 0 comments
Labels
inconsistency Issues that break convention rather than being faulty implementations misimplementation Circumlocutious or redundant implementation

Comments

@no-lex
Copy link
Member

no-lex commented Oct 28, 2021

The current class vec, while useful, requires the user to use an unusual means of calculating operations: named methods corresponding to the desired operation. However, C++ gives us operator overloads, and that is the way these types of operations should be conducted. Operator overloads don't require the developer to memorize what the method names are: you just have to call on the well known operators themselves (* + - / += -= > < etc.).

Some things to be aware of when rewriting methods:

From rendersky:

   vec lambda(680e-9f, 550e-9f, 450e-9f),
       betar = vec(lambda).square().square().recip().mul(1.86e-31f / atmodensity),
       betam = vec(lambda).recip().mul(2*M_PI).square().mul(atmohazefade.tocolor().mul(atmohazefadescale)).mul(1.36e-19f * std::max(atmohaze, 1e-3f)),
       betarm = vec(betar).div(1+atmoclarity).add(betam);

We see spurious vec() constructor calls here because vec has no equivalent of the + operator, only the += operator. These vec() calls construct a new object to += on so that the original is not modified. Of course, these should be replaced by + when encountered.

In general, the operator methods already defined are of the += type: they modify the object in place. This includes some operators such as ** which don't even have such operation/assignment equivalents.

The mul(vec o) operator is not the dot operator: mul returns the element-wise multiplication of this with o.

@no-lex no-lex added inconsistency Issues that break convention rather than being faulty implementations misimplementation Circumlocutious or redundant implementation labels Oct 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
inconsistency Issues that break convention rather than being faulty implementations misimplementation Circumlocutious or redundant implementation
Projects
None yet
Development

No branches or pull requests

1 participant