meltingice / CamanJS

Javascript HTML5 (Ca)nvas (Man)ipulation
http://camanjs.com
BSD 3-Clause "New" or "Revised" License
3.56k stars 406 forks source link

Bug in Sepia filter #209

Open ddcovery opened 8 years ago

ddcovery commented 8 years ago

When calculating the rgba.g the filter uses the modified rgba.r and not the original one. When calculating the rgba.b the filter uses the modified rgba.r, rgba.g instead the original ones.

This is a possible correction:

Filter.register "sepia", (adjust = 100) ->
  adjust /= 100
  @process "sepia", (rgba) ->
    r = rgba.r * (1 - 0.607 * adjust) + rgba.g * 0.769 * adjust + rgba.b * 0.189 * adjust
    g = rgba.r * 0.349 * adjust + rgba.g * (1 - 0.314 * adjust) + rgba.b * 0.168 * adjust
    b = rgba.r * 0.272 * adjust + rgba.g * 0.534 * adjust + rgba.b * (1- 0.869 * adjust)
    rgba.r = Math.min(255, r)
    rgba.g = Math.min(255, g)
    rgba.b = Math.min(255, b)

    rbga
ddcovery commented 8 years ago

One possible improvement, is to store de color matrix as 3 vectors (matrixR, matrixG, matrixB) and avoid to recalculate the same in each iteration.

Filter.register "sepia", (adjust = 100) ->
  adjust /= 100
  matrixR = [1 - 0.607 * adjust, 0.769 * adjust, 0.189 * adjust]
  matrixG = [0.349 * adjust, 1 - 0.314 * adjust, 0.168 * adjust]
  matrixB = [0.272 * adjust, 0.534 * adjust, 1 - 0.869 * adjust]

and then

    @process "sepia", (rgba) ->
      r = rgba.r * matrixR[0] + rgba.g * matrixR[1] + rgba.b * matrixR[2];
      g = rgba.r * matrixG[0] + rgba.g * matrixG[1] + rgba.b * matrixG[2];
      b = rgba.r * matrixB[0] + rgba.g * matrixB[1] + rgba.b * matrixB[2];
      ....

and then

av01d commented 5 years ago

Which then results in the following vanilla Javascript:

  Filter.register("sepia", function(adjust) {
    if (adjust == null) {
      adjust = 100;
    }
    adjust /= 100;
    var matrixR = [1 - 0.607 * adjust, 0.769 * adjust, 0.189 * adjust],
        matrixG = [0.349 * adjust, 1 - 0.314 * adjust, 0.168 * adjust],
        matrixB = [0.272 * adjust, 0.534 * adjust, 1 - 0.869 * adjust];
    return this.process("sepia", function(rgba) {
      var r = rgba.r * matrixR[0] + rgba.g * matrixR[1] + rgba.b * matrixR[2],
          g = rgba.r * matrixG[0] + rgba.g * matrixG[1] + rgba.b * matrixG[2],
          b = rgba.r * matrixB[0] + rgba.g * matrixB[1] + rgba.b * matrixB[2];
      rgba.r = Math.min(255, r);
      rgba.g = Math.min(255, g);
      rgba.b = Math.min(255, b);
      return rgba;
    });
  });