Lighten does not correctly lighten #3

Closed
aidanhs opened this Issue Mar 2, 2013 · 2 comments

Projects

None yet

2 participants

@aidanhs
net.brehaut.Color({red: 70/255, green: 60/255, blue: 50/255}).toHSL().lightenByAmount(1).getLightness()

The above should return 1 (unless I've completely misunderstood colourspaces). It doesn't.

The issue appears to be that 'value' is not the same thing as 'lightness'.

    lightenByAmount: cloneOnApply(function ( val ) {
      this.value = Math.min(1, Math.max(this.value + val, 0));
    }),

    lightenByRatio: cloneOnApply(function ( val ) {
      this.value = Math.min(1, Math.max(this.value * (1 + val), 0));
    }),
@brehaut
Owner

Good catch.

The lightenBy methods predate the inclusion of the HSL colourspace (and thus the lightness property), due to lightness being an obvious name for what the operation does in HSV space, even though its not strictly correct. Now that HSL exists, it becomes semantically squiffy.

I'll move the lightenBy… methods over to the HSL model (using lightness rather than value), and replace them with new changeValueBy… variants on HSV. (suggestions for a better name would be appreciated).

@brehaut brehaut added a commit that referenced this issue Mar 4, 2013
@brehaut fixes issue #3 654d553
@brehaut
Owner

lightenBy… and darkenBy… are now both on the HSL model and behave as expected.

HSV now contains valueBy… and devalueBy… methods in their place that work on value. I'm still open to better names for these.

@brehaut brehaut closed this Mar 4, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment