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

contrast() producing incorrect results #2743

Closed
Nettsentrisk opened this issue Dec 2, 2015 · 30 comments
Closed

contrast() producing incorrect results #2743

Nettsentrisk opened this issue Dec 2, 2015 · 30 comments

Comments

@Nettsentrisk
Copy link

The contrast function is supposed to "Choose which of two colors provides the greatest contrast with another."

Examples here show that it's not producing the correct results: http://codepen.io/anon/pen/eJORae

The light and dark colors default to white and black respectively.

contrast(#F1560C) gives #FFF, which is a contrast ratio of 3.46, yet #000 has a contrast ratio of 6.08. Why is it picking #FFF when #000 gives a greater contrast?

contrast(#F08202) produces the same incorrect result.

The function is supposed to help "ensuring that a color is readable against a background, which is also useful for accessibility compliance".

Choosing a color that has a 3.46 contrast ratio, below the WCAG AA requirement of 4.5, instead of a color that gives a contrast ratio of 6.08, is not a correct result.

@Nettsentrisk
Copy link
Author

SCSS produces the correct result: http://codepen.io/anon/pen/dGbzNx

@Nettsentrisk
Copy link
Author

Compass (SCSS/SASS) has abandoned the threshold variable; they also actually do the comparison between the contrast ratios against black and white, which seems to be missing in LESS: https://github.com/Compass/compass/blob/d5df161d0df7edc686e576b22412f437dd0590cc/core/stylesheets/compass/utilities/color/_contrast.scss

$color-brightness: brightness($color);
    $dark-text-brightness: brightness($dark);
    $light-text-brightness: brightness($light);

    @return if(abs($color-brightness - $light-text-brightness) > abs($color-brightness - $dark-text-brightness), $light, $dark);

The LESS code now only compares the input color to the default value of threshold, the unexplained value of 0.43. There's no comparison going on in the code that can actually determine "which of the two colors provides the greatest contrast with another".

@Synchro
Copy link
Member

Synchro commented Dec 2, 2015

I'll look at this later, but I can tell you that both the threshold param and its default value was taken directly from SCSS and retained for compatibility with it. I don't know why they used it in the first place.

@Nettsentrisk
Copy link
Author

Compass might not be solving this completely correctly, either.

W3C describes how to determine contrast ratios here, which involves more than just comparing the brightness/luma of the colors:

contrast ratio
(L1 + 0.05) / (L2 + 0.05), where
L1 is the relative luminance of the lighter of the colors, and
L2 is the relative luminance of the darker of the colors.

@seven-phases-max
Copy link
Member

In other words: we need a new contrast ("contrast2") function that simply returns a more contrast color (compared to a first arg) from a list of the remaining args. Where "contrast" is determined by some "fair" logic (W3C, WCAG or so).
Current contrast function (initially copied from SASS but then came through a series of changes that shifted its meaning) has very few to do with "contrast" and has quite overengineered and flawed logic...
Notice (for example):

contrast(red, blue, red); // -> red

that does not make any sense :)

Synchro added a commit to Synchro/less.js that referenced this issue Dec 2, 2015
@Synchro
Copy link
Member

Synchro commented Dec 2, 2015

I've just pushed a new contrast2 function in my fork that works as @seven-phases-max describes. What do you think?

One thing I can't work out is why the contrast function is like it is (I wrote it originally) - there have been numerous changes to it, but I also recall that we switched to using an sRGB gamma-corrected luma calculation via someone else's PR, however, I see that that's not there any more!

This answer has a really nice visual comparison of contrast & luma calculations, which includes my PHP implementation in the same question, so I'm at a loss as to why we are not using that correct algorithm, and for some reason have reverted to the same problematic one as SCSS (which runs into trouble with dark colours).

@Nettsentrisk
Copy link
Author

I think you guys have the correct luma formula; W3C has that defined as you have implemented.

Why not just redo the current contrast function to actually do what the documentation already says it's supposed to do? :)

I don't think you'll introduce a "breaking change" by doing so, because you'll just be fixing it to do what it's always supposed to have been doing. It's just that no one has noticed because the current algorithm has gotten lucky. I think the 0.43 threshold thing has ensured, by luck, that it's mostly given the correct result.

Colors such as bright red are pesky when it comes to contrast, so it's not until you get into colors like that where the results start being wrong. People probably assumed it was doing it correctly by not double-checking its results.

So I'd say, instead of checking against this apparently magic value of 0.43, just replace that check with the one that SCSS/Compass has, where you actually compare the input color's contrast with the specified light and dark color, and then pick the one that gives the most contrast, per the documentation's explanation.

Introducing "contrast2" is definitely just going to make this more confusing :)

@matthew-dean
Copy link
Member

Why not just redo the current contrast function to actually do what the documentation already says it's supposed to do? :)

I don't think you'll introduce a "breaking change" by doing so, because you'll just be fixing it to do what it's always supposed to have been doing. It's just that no one has noticed because the current algorithm has gotten lucky. I think the 0.43 threshold thing has ensured, by luck, that it's mostly given the correct result.

My thoughts exactly. Although... I think the intended purpose of contrast in the past was that the first color was evaluated for "light v. dark", and simply picked the corresponding color option based on that threshold, rather than actually comparing the "light" and "dark" values (which would need to be named) to the input color in order to select greatest contrast. (I could be wrong though.) However, I agree that changing this function in the proposed way would still follow the intention of the style author, and better represent that intention, even if the code logic is very different.

@matthew-dean
Copy link
Member

@Synchro You already submitted a "contrast2" function? Ugh, please no. No one will understand the difference between the two contrast functions. Let's fix the existing function. I doubt many people are customizing the threshold value, and we could a) silently ignore it rather than throw an error, or b) switch to legacy logic if they include a custom threshold.

@Synchro
Copy link
Member

Synchro commented Dec 2, 2015

@matthew-dean I just implemented Max's suggestion to give it a try.

The original isn't just comparing the first colour with the threshold because the light/dark colours are compared to each other first and that determines which is chosen. It is confusing though. I'll improve the current function to use the proper ratio calc and skip the threshold value.

@matthew-dean
Copy link
Member

@Synchro Ah, good info. I've used contrast a bunch but never knew that. But in my cases the light and dark values were usually spaced pretty far apart.

Just curious: with the new algorithm: is there a case for customization for the logic? Or does threshold basically make no sense.

@Synchro
Copy link
Member

Synchro commented Dec 2, 2015

I think the threshold it is there to allow you to bias things one way or another - for example if you have a dark-on-light theme you might want to make the switch point biased in favour of darker foreground colours, or simply to give you control over whether 50% grey maps to black or white. I've no idea why SASS used the 0.43 default.

There is an underlying thing too - our luma calc takes alpha into account, which isn't in any way standard, but it means you might get slightly more sane contrast values with partially transparent colours.

@matthew-dean
Copy link
Member

our luma calc takes alpha into account, which isn't in any way standard, but it means you might get slightly more sane contrast values with partially transparent colours.

How so? If that color is laid upon a black background or white background, then that color will be added to a translucent color. So I don't see how alpha is relevant or deterministic unless you input the color it's overlaying.

@seven-phases-max
Copy link
Member

Well, we need a new "contrast2" for backward-compatibility - contrast is used too wide so we just can't silently change it. (And any confusion would be a subject for the docs to mark the legacy one as provided only for backward compatibility).


Btw., (thing to discuss) regardless of above, I wonder if the WCAG algo mentioned above is fair enough to be named "contrast". Since it takes only luminance into account (and totally ignores hue and so) it will treat two slightly different shades of red as more contrast to each other if compared to red and blue shades having equal luminance (while the latter are definitely more "contrast" by all means).

@Synchro
Copy link
Member

Synchro commented Dec 3, 2015

@matthew-dean That was my point, but without it, rgba(255,0,0,0.1) and rgba(255,0,0,1) would be treated the same, which seems less helpful than not doing anything.

@Synchro
Copy link
Member

Synchro commented Dec 3, 2015

The WCAG calc does take hue into account since it's used to calculate the luma in the first place, and luma is entirely a perceptual value, not just statistical. I think you'd find it very hard to find an example like you describe.

As for ignoring threshold, that's what SASS did, and I don't see much of a problem since it's effectively buggy behaviour anyway.

@Synchro
Copy link
Member

Synchro commented Dec 3, 2015

BTW I realised I was looking at an old implementation of our luma calc. The current one does use an sRGB gamma-corrected calc.

@matthew-dean
Copy link
Member

Well, we need a new "contrast2" for backward-compatibility

Pleeeeease no. Less is awash with color functions enough without starting to duplicate names by adding a "2" on the end. I would rather we make a breaking 3.0 change and switch behavior of contrast than duplicate names, especially since both functions serve the same purpose, just employing slightly different methods. Please don't add to Less's function list unnecessarily. There are several ways to resolve this issue.

Synchro added a commit to Synchro/less.js that referenced this issue Dec 3, 2015
@Synchro
Copy link
Member

Synchro commented Dec 3, 2015

I've amended the existing contrast function so that it does a straightforward contrast ratio comparison and ignores threshold. I renamed light and dark to color1 and color2 to reflect that there isn't really any such distinction. The only changes in the test config are those relating to altering the threshold param significantly, which would only be applicable to those trying to make things deliberately hard to read!

The contrast function calculation is now essentially identical to the contrast2 implementation, they only differ in signature, so the new one can be scrapped.

@matthew-dean
Copy link
Member

This seems reasonable to me.

Synchro added a commit to Synchro/less.js that referenced this issue Dec 3, 2015
@Nettsentrisk
Copy link
Author

Threshold doesn't really make sense when the aim of the function is to choose between two colors in which gives more contrast to the input color.

The use case is very clear: I want to know which color will give me a WCAG 2.0 AA compatible contrast to the color I specify. Threshold has no place in such a use case.

What you could do is allow for passing in a list of colors to contrast with the first color, and choose the one that gives the best contrast from all of them.

contrast(inputColor, [color1, [color 2, [color3, etc.]]])

If any of the values after inputColor are a percentage, ignore it.

Default can be the same as today, contrast(inputColor, #000, #FFF).

There's no need to specify a "dark" or "light" parameter here, as you're choosing the one that gives the best contrast to inputColor regardless.

@Nettsentrisk
Copy link
Author

An idea for a new function that provides a different use case is one that returns the color that gives the minimum or maximum contrast closest to a color you specify.

Let's say I have a green background and would like to have a white text. However, the contrast is not >= 4.5 so it is not according to WCAG 2.0 AA requirements. I want a function to give me the closest green color to what I already have, that satisfies the 4.5 contrast ratio.

Basically, a function that provides the use case that this web page does, would be a welcome addition: http://contrast-finder.tanaguru.com/

Not sure what you would call such a function, but with accessibility becoming more important, it's definitely a good use case.

@Synchro
Copy link
Member

Synchro commented Dec 3, 2015

@Nettsentrisk That's exactly what my contrast2 function did, and removing the dark/light naming is what I've done in the amended contrast function. Neither function will guarantee AA compliance because it will only return one of the colours you supply, and it's possible that all of them are below acceptability, so it can only return a best-effort. Also I really don't see any great advantage in accepting more than 2 choices.

Generating a colour may not be possible at all - for example if your colour is a pure blue, it's probably impossible to find a colour with sufficient contrast without altering the hue.

@Nettsentrisk
Copy link
Author

Well there's two different use cases here:

  1. I have a set color, and I want to know what color, from a selection (2+) I specify, gives me the best contrast to it; without specifying more than input color, the default for the 2 colors is black and white (which will be more likely to guarantee an acceptable contrast according to WCAG AA).
  2. I have a set color, and a color I wish to use with it, but need to ascertain the closest color to it that has an acceptable contrast level. 2 or 3 parameters, input color, contrast color (and possibly contrast level that is acceptable to me, default is 4.5).

These are not the same, as # 2 you've got a much more complex algorithm to find a color as close as possible to the color specified, whereas # 1 is simply testing the contrast between the input color and the colors specified to find the one that gives the best contrast.

2 is what http://contrast-finder.tanaguru.com/ does when you ask it to find the closest possible color that is in line with the contrast level specified. As you point out, you're not guaranteed a result that is anywhere actually close to your wish in this instance, but it will nonetheless have to produce a result that still satisfies the contrast requirement.

1 you the author are given more responsibility to control whether the colors specified will guarantee a result that is according to accessibility requirements. If you allow for any number of contrast colors (not just two), you give the author more possibilities to succeed, by letting them specify a larger number of possible colors to ensure an acceptable result.

@Synchro
Copy link
Member

Synchro commented Dec 7, 2015

Sure, but 2 is an entirely different proposition to 1, and really not within the remit of this function. The main reason I wrote this function in the first place was in a scenario where a theme is switchable between broadly dark-on-light and light-on-dark palettes, but the colours used are mostly unchanged. This works really well with things like bootstrap, which is where all this started. At the time Less had very little support for conditions, and this function was a sneaky way of slipping in conditional processing without having to use cumbersome, repetetive guards. You could easily add a 'findContrasting' function that generates a colour for you like that site does, but that is entirely orthogonal to what this function does.

@Nettsentrisk
Copy link
Author

Number 2 proposed above was just an idea for a new function for a different use case, number 1 should take over as the way contrast() works :)

@seven-phases-max
Copy link
Member

Well, we need a new "contrast2" for backward-compatibility

Pleeeeease no.

:) I only suggested conrast2 in case if it's important we could implement and use that contrast2 right now (and not waiting for v3 or v4 and/or not bother with backward-compatibility equilibriums).

@matthew-dean
Copy link
Member

matthew-dean commented Jun 17, 2016

To satisfy anyone's curiosity about the "magic value" 0.43, I found this today when reviewing related contrast() issues (emphasis mine):

Gamma is literally the denominator e in the exponent fraction 1/e that defines a nonlinear response compression; to find e, take the reciprocal of the gamma. A gamma of 1.0 indicates a 45° slope in the semilog plot. A higher gamma defines greater contrast, but across a more limited span of luminance. A gamma value of about 2.3 _(1/e = 0.43)_ is typical of computer monitors and the human eye.

From: http://www.handprint.com/HP/WCL/color4.html

I don't have a PhD in color theory, but I think that's trying to say that contrast is non-linear (for both monitors and human perception), therefore the contrast point "midway" between a dark value and light value (relative to another color) is not necessarily at 0.5 as one might expect.

@Synchro
Copy link
Member

Synchro commented Jun 18, 2016

Ok, so the 0.43 is a fudge factor when using values that are not gamma-corrected - but the new implementation does use gamma correction, so it doesn't apply.

@Nettsentrisk
Copy link
Author

Involved with a project using Less again now, and I was blissfully unaware that this whole WCAG-compliant contrast function solution had been reverted in v. 3.0. So Less doesn't have a contrast function that is reliable for setting WCAG-compliant contrasting colors anymore.

Thus I don't quite understand why it says this in the documentation:

This function works the same way as the contrast function in Compass for SASS. In accordance with WCAG 2.0, colors are compared using their gamma-corrected luma value, not their lightness.

This is misleading in that it gives the impression that the function provides WCAG compliant results, but that of course is not true, which this whole discussion thread documents and was the reason for the change previous to v. 3.0.

A new color function that does what this issue solved should be created to fulfill this, or the Less project should be bold enough to stand by the decision to make the contrast function WCAG-compliant.

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

No branches or pull requests

5 participants
@Synchro @matthew-dean @Nettsentrisk @seven-phases-max and others