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

(bug) The mix function has a fatally wrong algorithm #654

Open
debiru opened this issue Aug 4, 2024 · 0 comments
Open

(bug) The mix function has a fatally wrong algorithm #654

debiru opened this issue Aug 4, 2024 · 0 comments

Comments

@debiru
Copy link

debiru commented Aug 4, 2024

  • polished version: v4.3.1
  • JSS-in_CSS library and version: unknown
  • Any relevant JS library and version: unknown

What You Are Seeing

I was looking into the implementation of the mix colors function.

Among them I found polished and dart-sass implementations.

However, upon closer inspection, I found a bug in the polished implementation.

dart-sass:

  var normalizedWeight = weightScale * 2 - 1;
  var alphaDistance = color1.alpha - color2.alpha;

  var combinedWeight1 = normalizedWeight * alphaDistance == -1
      ? normalizedWeight
      : (normalizedWeight + alphaDistance) /
          (1 + normalizedWeight * alphaDistance);
  var weight1 = (combinedWeight1 + 1) / 2;
  var weight2 = 1 - weight1;

polished:

  // The formula is copied from the original Sass implementation:
  // http://sass-lang.com/documentation/Sass/Script/Functions.html#mix-instance_method
  const alphaDelta = color1.alpha - color2.alpha
  const x = parseFloat(weight) * 2 - 1
  const y = x * alphaDelta === -1 ? x : x + alphaDelta
  const z = 1 + x * alphaDelta
  const weight1 = (y / z + 1) / 2.0
  const weight2 = 1 - weight1

Converting the dart-sass implementation to polished variable names looks like this:

  var alphaDelta = color1.alpha - color2.alpha;
  var x = weightScale * 2 - 1;

  var z = 1 + x * alphaDelta;
  var y = x * alphaDelta == -1 ? x : (x + alphaDelta) / z;
  var weight1 = (y + 1) / 2;
  var weight2 = 1 - weight1;

What You Expected To See

The code of polished is wrong:

  const y = x * alphaDelta === -1 ? x : x + alphaDelta
  const z = 1 + x * alphaDelta
  const weight1 = (y / z + 1) / 2.0

Here's the correct code:

  const z = 1 + x * alphaDelta
  const y = x * alphaDelta === -1 ? x : (x + alphaDelta) / z
  const weight1 = (y + 1) / 2.0

Do not divide by z when x * alphaDelta === -1.

Notes

Furthermore, there is also the following problem. #613

dart-sass:

  return SassColor.rgb(
      fuzzyRound(color1.red * weight1 + color2.red * weight2),
      fuzzyRound(color1.green * weight1 + color2.green * weight2),
      fuzzyRound(color1.blue * weight1 + color2.blue * weight2),
      color1.alpha * weightScale + color2.alpha * (1 - weightScale));
}

polished:

  const mixedColor = {
    red: Math.floor(color1.red * weight1 + color2.red * weight2),
    green: Math.floor(color1.green * weight1 + color2.green * weight2),
    blue: Math.floor(color1.blue * weight1 + color2.blue * weight2),
    alpha:
      color1.alpha * parseFloat(weight) + color2.alpha * (1 - parseFloat(weight)),
  }

Math.round should be used instead of Math.floor.

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

1 participant