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

Inherit hue or saturation when interpolating to or from a monochrome color. #833

Closed
wants to merge 2 commits into from

Conversation

gunn
Copy link

@gunn gunn commented Oct 1, 2012

Currently if you use d3.interpolateHslor d3.interpolateHcl to blend a colour with black, you get red shades introduced. If you do the same with white, you get green shades.

For example, blue + black equals purple:

d3.interpolateHsl("#00f", "#000")(0.5)
//-> "#602060"

I would expect to see only shades of blue, from bright to dark.
Tweaking the input slightly gives us the expected result and shows where we're going wrong:

d3.interpolateHsl("#00f", "#001")(0.5)
//-> "#000088"

I think the hue value for black and white should be ignored for interpolation. This pull request fixes it for HSL and HCL and adds tests for RGB.

@mbostock
Copy link
Member

mbostock commented Oct 8, 2012

FWIW, you can avoid this problem by interpolating HSL colors rather than converting RGB to HSL. For example:

d3.interpolateHsl(d3.hsl(0, 1, .5), d3.hsl(0, 1, 0))(.5) // "#800000"
d3.interpolateHsl("hsl(0,100%,50%)", "hsl(0,100%,0%)")(.5) // "#800000"

However, since not all browsers support HSL colors, D3 converts to RGB when setting styles and attributes. (Also, browsers may munge attributes, thereby converting HSL to RGB.) So this technique requires using an attrTween or styleTween so that you can specify an explicit starting value for the transition, rather than using the computed current value from the element.

@gunn
Copy link
Author

gunn commented Oct 8, 2012

I'm not sure if I've got it right, to me this seems to still have the same problem:

d3.interpolateHsl(d3.hsl(240, 1, 0.5), d3.hsl(0, 0, 0))(.5) // "#602060"
d3.interpolateHsl("hsl(240, 100%, 50%)", "hsl(0, 0%, 0%)")(.5) // "#602060"

I guess we can fix it by going to the odd step of giving black a hue and saturation, but that's basically what I think we should be doing in the interpolate methods.

var black = d3.hsl(240, 1, 0)
d3.interpolateHsl(d3.hsl(240, 1, 0.5), black)(.5) // "#000080"

@mbostock
Copy link
Member

mbostock commented Oct 8, 2012

You typed in a hue of zero, so it interpolated the hue from 240º to 0º. Try this:

d3.interpolateHsl(d3.hsl(240, 1, 0.5), d3.hsl(240, 1, 0))(.5) // "#000080"

The point being, if you specify HSL colors for the start and end, you can control the interpolation of the hue rather than it always being zero.

@gunn
Copy link
Author

gunn commented Oct 8, 2012

That's exactly what my second example is, and what I think the interpolate methods should be doing internally.
The app I'm working on at the moment has users providing RGB values to tween between, so it's not a matter of just entering the colour differently. I'm sure other people are doing similar things.

@mbostock
Copy link
Member

mbostock commented Oct 8, 2012

In general, I like having control over the behavior. With the current API you get a default behavior (H=0º, S=0) by specifying RGB colors, or you can control the behavior by specifying HSL colors. If we change the interpolator to inherit hue and saturation from the other color as appropriate, then we also remove control because the interpolator cannot distinguish between when H or S is undefined (coming from an RGB color) and when H or S is zero.

I suppose we could change d3.hsl so that h and s are undefined when you converted from black RGB. But I suspect that might break other things.

@gunn
Copy link
Author

gunn commented Oct 10, 2012

Well it would be a shame to lose that control. I do think that the current behaviour will seem broken to most users who expect the most direct transition between two values though.

It is possible to switch to HSL (or whatever colour space) and manipulate the values before creating an interpolator - e.g. for HCL:

var a = d3.hcl(a);
var b = d3.hcl(b);

if (a.c<0.1) a.h = b.h;
else if (b.c<0.1) b.h = a.h;

d3.interpolateHcl(a, b)(0.5)

Or something more complicated for HSL, but I think that's a lot to ask a casual user to figure out.

What about an optional third argument for enabling this behaviour, treating greys as hueless by default?

d3.interpolateHsl("#00f", "#000")(0.5) // "#000080"
d3.interpolateHsl("#00f", "#000", true)(0.5) // "#602060"

@mbostock
Copy link
Member

Specifying an additional argument to the interpolator isn't very useful; you don't have that option when you use transition.attr or transition.style, for example. Also, it's not self-describing, which means you have to go search the documentation to figure out what the argument true indicates.

I think changing d3.interpolateHsl to not interpolate H or S if undefined would work. Something like:

d3.interpolateHsl = function(a, b) {
  a = d3.hsl(a);
  b = d3.hsl(b);
  var h0 = isNaN(a.h) ? b.h : a.h,
      s0 = isNaN(a.s) ? b.s : a.s,
      l0 = a.l,
      h1 = isNaN(b.h) ? 0 : b.h - h0,
      s1 = isNaN(b.s) ? 0 : b.s - s0,
      l1 = b.l - l0;
  if (h1 > 180) h1 -= 360; else if (h1 < -180) h1 += 360; // shortest path
  return function(t) {
    return d3_hsl_rgb(h0 + h1 * t, s0 + s1 * t, l0 + l1 * t) + "";
  };
};

Then you could say:

d3.interpolateHsl(d3.hsl(240, 1, 0.5), d3.hsl(undefined, undefined, 0))(1)

You could take this one step further and also change d3_rgb_hsl to leave H and S undefined when converting from a monochrome color, simply by deleting two lines:

function d3_rgb_hsl(r, g, b) {
  var min = Math.min(r /= 255, g /= 255, b /= 255),
      max = Math.max(r, g, b),
      d = max - min,
      h,
      s,
      l = (max + min) / 2;
  if (d) {
    s = l < .5 ? d / (max + min) : d / (2 - max - min);
    if (r == max) h = (g - b) / d + (g < b ? 6 : 0);
    else if (g == max) h = (b - r) / d + 2;
    else h = (r - g) / d + 4;
    h *= 60;
  }
  return d3_hsl(h, s, l);
}

I think you'd also need to fix d3_hsl_rgb to handle H and S being undefined, so that when you convert back to RGB you get a valid gray of the appropriate lightness. I think if (isNaN(s)) s = 0 would be sufficient to fix that.

@gunn
Copy link
Author

gunn commented Oct 10, 2012

I don't particularly like the idea of having an additional mode either. If we can make this approach work, it's far superior. Also it seems to me to be be more semantically correct to say that #444 has no hue than to say that it's red.

Shall I try to implement this?

@gunn
Copy link
Author

gunn commented Nov 6, 2012

bump

@mbostock
Copy link
Member

Sorry, haven't had time to address this issue lately. I assume this change would also need to affect HCL colors for consistency. I'm not sure yet about Lab*.

mbostock added a commit that referenced this pull request Apr 19, 2013
In HSL space, grayscale colors can now have undefined hue rather than assuming a
hue of 0°; likewise black and white can have undefined saturation rather than
assuming 0%. In HCL space, black can now have undefined hue and chroma. (For
non-black grayscale colors, including white, hue and chroma are implied by the
D65 standard referent.)

When interpolating between colors with undefined hue, saturation or chroma, the
defined value is used when available. For example, when interpolating from black
to blue in HCL space, the intermediate colors are now dark blues (#241178)
rather than dark purples (#600054).

Fixes #833.
@mbostock
Copy link
Member

Superseded by #1205.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn’t working
Development

Successfully merging this pull request may close these issues.

None yet

2 participants