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

Code maintenance #20

Open
KevinJW opened this issue Feb 8, 2023 · 4 comments
Open

Code maintenance #20

KevinJW opened this issue Feb 8, 2023 · 4 comments

Comments

@KevinJW
Copy link
Collaborator

KevinJW commented Feb 8, 2023

We should perform code maintenance to ensure the latest builds from 08 Feb are merged into a single code base again and to tag a new version once complete.

We should also look to further reduce some of the variations in algorithms where possible,

@nick-shaw
Copy link
Collaborator

If (as Luke has recommended) we always use the "discount illuminant" option, some of the code can be simplified, as "discount illuminant" uses a value of 1.0 for D which means a few things cancel out or have no effect in some subsequent equations.

We obviously need to decide when we commit to that.

@priikone
Copy link

We can go much further. AFAICS, we could pre-compute the whitepoint achromatic response and some other stuff related to whitepoint. Same applies to other things like the viewing conditions dependent parameters which should reduce to constants as well.

But all this optimization can be done later.

@nick-shaw
Copy link
Collaborator

nick-shaw commented Feb 28, 2023

There is a lot of multiplying and dividing by 100 going on. I suspect a lot of this can be optimised further down the line. The value of 100 is also stored as a lot of variables (L_A, referenceLuminance, mmScaleFactor and daniele_n_r) which I think all effectively represent the same thing.

The Blink code even contains a to_domain_100 function which is a pass-through function.

@nick-shaw
Copy link
Collaborator

Again, optimisations for later. But just noting for now.

A number of functions can have parameter pre-calculations "flattened" once they no longer need to be live parameters. An example is the constants in the Daniele Evo curve. While we should certainly document how the constants used in the final equation are calculated, it is not necessarily necessary to keep those calculations in the code. Particularly for implementations like DCTL with no init() function, meaning that the calculations may happen repeatedly. I don't know how much of this gets optimised out by a compiler when they are declared as const.

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

No branches or pull requests

3 participants