Let luma follow spec

[roelvanduijnhoven]

The luma function found in LESS is not implemented as defined in the specification (http://www.w3.org/TR/2008/REC-WCAG20-20081211/#relativeluminancedef).

[seven-phases-max]

Well, there's difference between luma and luminance so technically there's no reason for the Less luma to follow that particular spec. (See the luma in the docs).

In general I'm somewhat neutral about this change but... if the function uses luminance algorithm shouldn't we rename it then? (Or maybe even to have both?)

[Synchro]

The diference between the two implementations is that the one @roelvanduijnhoven suggests includes gamma correction and uses the sRGB space - the coefficients are the same, and the resuts will not be that different. Strictly speaking, luminance is the linear photometric mapping (what we have now), luma is the gamma-corrected one, though they are often misnamed.

This calculation also appears inline in the contrast function as I couldn't work out how to call this one from there, so it either needs copying there, or that figured out!

I'm +1 for using this version, though the test cases need updating too.

[roelvanduijnhoven]

@marcus luma is simply a function on the color object. Thus one can access
it via color.luma().

I'll update tests.
Op 20 feb. 2014 17:25 schreef "Marcus Bointon" [email protected]:

The diference between the two implementations is that the one
@roelvanduijnhoven https://github.com/roelvanduijnhoven suggests
includes gamma correction and uses the sRGB space - the coefficients are
the same, and the resuts will not be that different. Strictly speaking,
luminance is the linear photometric mapping (what we have now), luma is
the gamma-corrected one, though they are often misnamed.

This calculation also appears inline in the contrast function as I
couldn't work out how to call this one from there, so it either needs
copying there, or that figured out!

I'm +1 for using this version, though the test cases need updating too.

Reply to this email directly or view it on GitHubhttps://github.com//pull/1890#issuecomment-35639431
.

[seven-phases-max]

the coefficients are the same, and the resuts will not be that different.

The gamma correction is quite noticable:

so the result may be quite different too, e.g. for rgb(102, 204, 102) current result is 0.686 but new will be 0.470 so I worry about correct name a bit. But I did more search and you're right: what we have now is the luminance actually and the luma should use gamma-correction. So I'm for it (if we're not too strict about backward-compatibility in this case :).

[roelvanduijnhoven]

Tests are updated.

[roelvanduijnhoven]

contrast already uses luma correctly @Synchro.

[lukeapage]

It looks like this should be merged into 2.0.0 since it is a breaking change.

Any objections to that plan?

[Synchro]

Strictly speaking it's a bug fix rather than a breaking change, and changing the calculation won't break anyones Less files. It's also a reasonably subtle change, and more likely to match what was actually expected. You could push it to 2.0, but I don't see too much harm in doing it now.

[lukeapage]

Okay.. should contrast use luma or luminance? The thing with contrast is it could produce some quite big changes...
e.g. see the test changes, (which btw someone will need to adjust so we still get test coverage of both sides of contrast)

[lukeapage]

p.s. I'm just about ready to release 1.7.0 apart from this and 2.0.0 shouldn't be more than a couple of months away. I don't think it would be good to go into a patch release 1.7.x so... would be good to make a decision soon ;)

[Synchro]

The W3C spec says to use the gamma-corrected calculation so that's the right thing to do really. If we are going to roll this into 1.7, I think we should leave the current calc in there but rename it luminance so it's available for workarounds for anyone that has a problem with this change, then use @roelvanduijnhoven's implementation as the luma function. It will need docs changes much like the tests too.

[lukeapage]

@Synchro sounds like a plan, but which should contrast use?

[Synchro]

Contrast should remain using luma (i.e. no code change needed there), though it will be using the new version. The change will be less obvious in the contrast function as it doesn't affect the colours directly, but it may choose a different one in some edge cases.
Thinking about it, it may be slightly difficult to work around that to use the new/old luminance function - I originally wrote contrast because Less lacked an 'if' statement, and so the condition is built into the function instead. At the time, guards were difficult/cumbersome to use for it - has that changed sufficiently to enable them to be used as a workaround?

[lukeapage]

See #1894 - it seems there is still a demand to have an inline conditional like this.

I'm more concerned about contrast being a breaking change than luma so for that reason I'm leaning towards making the change but leaving contrast using luminence so it is unchanged and then changing contrast in 2.0

[Synchro]

Here is an example showing the difference between the two calculations when used in a contrast function. The left hand table is the current luminance calculation, the right hand is the new luma calculation. With the stock 43% threshold, luma is generally biased towards lighter results, I think it's hard to call which looks better - there are cases either way: compare #6666ff and #cc99cc.
You can vary the threshold value on that page by adding a t param, like this.

Just to complicate things further 😇, I ran into another discussion of these calculations, suggesting that because the input colours are already gamma-corrected, you should un-correct them before calculating luma.

[lukeapage]

okay then I'm taking this into 1.7 (scream now or hold your peace!) and adding a luminence function for backwards compatability and fixing the tests so that contrast-low-threshold still comes out with the same colour (changing threshold from 10% to 9% seems to do it)

[roelvanduijnhoven]

Allright. I'm all for it.

I'll finish of my PR using input given in this thread.
Op 27 feb. 2014 07:23 schreef "Luke Page" [email protected]:

okay then I'm taking this into 1.7 (scream now or hold your peace!) and
adding a luminence function for backwards compatability and fixing the
tests so that contrast-low-threshold still comes out with the same colour
(changing threshold from 10% to 9% seems to do it)

Reply to this email directly or view it on GitHubhttps://github.com//pull/1890#issuecomment-36214288
.

[roelvanduijnhoven]

PR is updated. I chose to implement luminance as a function instead of adding it as a method on the Color prototype.

[lukeapage]

Cool perfect. I was part of the way through making the changes myself, but I will just take yours.