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

Color eq 2 #283

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

Color eq 2 #283

wants to merge 9 commits into from

Conversation

aurelienpierre
Copy link
Collaborator

Guided filter still produces NaN (then black squares) in high ISO pictures. Can't figure out why, mitigation is to raise the black point.

@aurelienpierre aurelienpierre added this to the 1.0 milestone Dec 18, 2023
Copy link

Quality Gate Passed Quality Gate passed

The SonarCloud Quality Gate passed, but some issues were introduced.

92 New issues
0 Security Hotspots
No data about Coverage
2.4% Duplication on New Code

See analysis details on SonarCloud

@Jiyone
Copy link
Collaborator

Jiyone commented Dec 18, 2023

Thank you !

We need to remove all stdout in several openMP dt_omp_firstprivate() function first to be able to compile coloreqal.c 😁

(see https://github.com/aurelienpierreeng/ansel/actions/runs/7250867982/job/19752002681?pr=283#step:4:1292 )


// fetch the corresponding y values
const float y_prev = gamut_lut[xi];
const float y_next = gamut_lut[xii];

Choose a reason for hiding this comment

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

Hi @aurelienpierre , i stumbled across this while looking for colorbalancergb issues both with opencl and cpu code. As we later do a simple lin approx i think we can/should avoid the division by (x_next - x_prev) and simply do

  // fetch the corresponding y values
  const float y_prev = gamut_lut[xi];

  // return y_prev if we are on the same integer LUT element or do linear interpolation
  return y_prev + ((xi != xii) ? (x_test - x_prev) * (gamut_lut[xii] - y_prev) : 0.0f);

Copy link
Collaborator Author

@aurelienpierre aurelienpierre Dec 28, 2023

Choose a reason for hiding this comment

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

it's not a linear interpolation if you don't do $y_{prev} + dx_{test} * \Delta Y_{samples} / \Delta X_{samples}$. What you propose is numerically wrong.

Choose a reason for hiding this comment

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

Isn't dx here either 1 or zero due to floor/ceil above?

@jenshannoschwalm
Copy link

Removed my comments...

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.

3 participants