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 tint feature not working as expected #1235

Closed
wezside opened this issue May 21, 2018 · 11 comments
Closed

Color tint feature not working as expected #1235

wezside opened this issue May 21, 2018 · 11 comments
Labels
Milestone

Comments

@wezside
Copy link

wezside commented May 21, 2018

If I use the following code I would expect three tinted images, a red tint, green tint, blue tint. Here is what the code produces:

var c = color({r: 255, g: 0, b: 0});
//var c = color({r: 0, g: 255, b: 0});
//var c = color({r: 0, g: 0, b: 255});
var tmp = __dirname + '/../public/upload/tmp/monalisa.jpg';
sharp(tmp)
 	.tint(c)
	.toFile(__dirname + '/../public/upload/tmp/output.jpg')
	.then(function(data)
	{

})
.catch(function(err) { console.log(err); });

red
green
blue

@wezside
Copy link
Author

wezside commented May 21, 2018

Not sure if this will help. But by adding linear() the code tints the image as expected:
[Edit] This is incorrect please read the next comment

var c = color({r: 255, g: 0, b: 0});
var tmp = __dirname + '/../public/upload/tmp/monalisa.jpg';
sharp(tmp)
 	.linear(1.0, 1.0)
 	.tint(c)
	.toFile(__dirname + '/../public/upload/tmp/output.jpg')
	.then(function(data)
	{

})
.catch(function(err) { console.log(err); });

output

@wezside
Copy link
Author

wezside commented May 21, 2018

So after some more digging into the source it would appear that the issue is related to the LAB colour space usage. sharp uses the color module to retrieve the darkness this.options.tintA = color.a(). This produces a lab colour which can be positive or negative for lightness/darkness, difference in red and green and difference in yellow and blue [source]. It appears that Vips colourspace VIPS_INTERPRETATION_LAB is not able to interpret these negative values (maybe it requires this normalised?) but in the interim for me to get the tint to work I used the LCH colourspace. The module colour unfortunately doesn't expose these conversions from the underlying module color-convert so this is a workaround at best.

In colour.js

const convert = require('color-convert');
function tint (rgb) {
	const colour = color(rgb);
	const lch = convert.rgb.lch(rgb.red(), rgb.green(), rgb.blue()); 
	this.options.tintA = lch[1];
	this.options.tintB = lch[2];
	return this;
}

In operations.cc change the luminance colour space to LCH:

VImage luminance = image.colourspace(VIPS_INTERPRETATION_LCH)[0];

Rebuild sharp:

npm rebuild --build-from-source sharp

Result for colour {r: 0, g: 246, b: 255} (Cyan)

output

@lovell
Copy link
Owner

lovell commented May 21, 2018

Thank you for reporting this @wezside and great detective work on what's causing it.

@jcupitt What are the valid ranges for the AB chroma values of LAB in libvips? The docs currently say "bands have the obvious range" and I'm sure you've probably told me before but I can't remember and can't find them mentioned in previous libvips issues.

@lovell lovell added the triage label May 21, 2018
@wezside
Copy link
Author

wezside commented May 21, 2018

No problem. Great library. It's worth pointing out that my "fix" still has some issues in outer ranges (like 350 degree hue). I had one tint colour lch [ 20, 31, 350 ] which doesn't implement correctly. Not sure if these are precision errors or calculation errors in the colour space conversions.

@jcupitt
Copy link
Contributor

jcupitt commented May 22, 2018

Hello, float LAB is 0-100 for L and +/- 128 for ab. I guess tint() colourizes a monochrome image? I usually do this by making an identity LUT and remapping white to the target RGB. If you do the remapping in some other colourspace, you can get different sorts of interpolation.

I'll try a small example.

@jcupitt
Copy link
Contributor

jcupitt commented May 22, 2018

Here's a simple tint in Ruby:

#!/usr/bin/env ruby

require 'vips'

colour = [255, 0, 0]

image = Vips::Image.new_from_file ARGV[0]
image = image.colourspace "b-w"

lut = Vips::Image.identity
lut = (lut / 256) * colour
lut = lut.cast "uchar"

image = image.maplut lut

image.write_to_file ARGV[1]

That'll do an 8-bit image, you'd need to change the 256 and the identity for a 16-bit one.

@jcupitt
Copy link
Contributor

jcupitt commented May 22, 2018

I see sharp is going to LAB, keeping L and setting a constant value for ab. I remember experimenting with this: it doesn't work that well, unfortunately, since people expect saturation to scale with luminance, but constant ab will give you saturated dark colours and washed out bright ones (relatively).

Here's another tint. This one makes a gradient between any two colours in CIELAB and maps black-white in the original to that.

#!/usr/bin/env ruby

require 'vips'

# make a lut which is a smooth gradient from start colour to stop colour, with
# start and stop in CIELAB
def gradient(start, stop)
    lut = Vips::Image.identity / 255
    lut = lut * stop + (lut * -1 + 1) * start
    lut.colourspace(:srgb, source_space: :lab)
end

# various colours as CIELAB triples
black = [0, 0, 0]
red = [53, 80, 67]
green = [88, -86, 83]
blue = [32, 79, -108]
white = [100, 0, 0]

image = Vips::Image.new_from_file ARGV[0]
image = image.colourspace("b-w").maplut(gradient black, red)
image.write_to_file ARGV[1]

Sample output:

x

@jcupitt
Copy link
Contributor

jcupitt commented May 22, 2018

(the odd lut * stop + (lut * -1 + 1) * start is necessary in Ruby since it doesn't allow constant - image. In C++ you could just write lut * stop + (1 - lut) * start)

@lovell
Copy link
Owner

lovell commented May 22, 2018

Brilliant, thanks @jcupitt, I'll try your first example as that looks perfectly suitable and should be a bit simpler/faster than a (L)AB conversion.

@lovell lovell added bug and removed triage labels May 22, 2018
@lovell lovell added this to the v0.20.3 milestone May 22, 2018
lovell added a commit that referenced this issue May 23, 2018
Add test cases for more tint colours and input interpretations
@lovell
Copy link
Owner

lovell commented May 23, 2018

On closer inspection it turns out there were a couple of bugs in the current implementation where negative AB values would be ignored and LAB interpretation was sometimes lost. Commit 54a71fc addresses these and adds new test cases that would have caught this problem.

This fix will be in v0.20.3 - thanks again for reporting @wezside and thanks as always for your help @jcupitt.

@lovell
Copy link
Owner

lovell commented May 29, 2018

v0.20.3 now available.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants