disintegration / imaging

Imaging is a simple image processing package for Go
MIT License
5.23k stars 435 forks source link

AdjustSaturation #82

Closed j-delaney closed 5 years ago

j-delaney commented 5 years ago

For https://github.com/disintegration/imaging/issues/79. Adds an AdjustSaturation function.

For converting between RGB and HSL I just used the functions in https://github.com/disintegration/gift (which is MIT licensed and also by the creator of this repo as well so I don't imagine that will be a problem). I copied&pasted rather than including as an import because gift uses a different format for colors and would require a major refactor of this repo.

Example imagingbranchescomparison

Points of Discussion

  1. The step function is a percentage function where the percentage p that the caller supplies what percent of the way to min/max saturation the function should bring the pixel. For example, if a pixel has a saturation of 0.25 and the user supplies p=50 then the new saturation will be 0.625 because that is 50% of the way between 0.25 and 1. Is this an acceptable approach? Alternatives would be a delta (e.g. new_saturation = old_saturation + delta) or factor (e.g. new_saturation = old_saturation * factor)
  2. The saturation approach I'm using suffers from RGB clipping issues when the source image has a lot of things with high saturation already. For example: saturationclippingcmp

I've found some articles[0][1] with proposals on how to fix this sort of issue. Think it's worth solving? I'm happy to take a shot at it, but I didn't know if you feel this library should be that opinionated or not

[0] https://www.pocketmagic.net/enhance-saturation-in-images-programatically/ [1] http://www.glennchan.info/Proofs/filter/filter_comparison.html

coveralls commented 5 years ago

Coverage Status

Coverage decreased (-0.3%) to 99.662% when pulling eca413ec1417dcda4151ca763131c14799964899 on j-delaney:saturation into 9458da53d1e65e098d48467a4317c403327e4424 on disintegration:master.

coveralls commented 5 years ago

Coverage Status

Coverage decreased (-0.3%) to 99.662% when pulling eca413ec1417dcda4151ca763131c14799964899 on j-delaney:saturation into 9458da53d1e65e098d48467a4317c403327e4424 on disintegration:master.

coveralls commented 5 years ago

Coverage Status

Coverage decreased (-0.3%) to 99.662% when pulling eca413ec1417dcda4151ca763131c14799964899 on j-delaney:saturation into 9458da53d1e65e098d48467a4317c403327e4424 on disintegration:master.

coveralls commented 5 years ago

Coverage Status

Coverage decreased (-0.3%) to 99.662% when pulling eca413ec1417dcda4151ca763131c14799964899 on j-delaney:saturation into 9458da53d1e65e098d48467a4317c403327e4424 on disintegration:master.

disintegration commented 5 years ago

I think a simple "factor" algorithm gives the best results. E.g. +100% doubles the saturation value of each pixel. GIMP and Krita image editors use this approach in their hue/saturation adjustment tools.

disintegration commented 5 years ago

@j-delaney Just a friendly reminder about this pull request.

disintegration commented 5 years ago

@j-delaney, I've merged the changes as is and fixed the issues I mentioned earlier. Thank you for contribution!