Re: CSS contrast (#30295) (was Re: Heads-up: Emacs 26.1 RC1)

Re: CSS contrast (#30295) (was Re: Heads-up: Emacs 26.1 RC1)

[Richard Copley]

[-- Attachment #1: Type: text/plain, Size: 1521 bytes --]

On 20 March 2018 at 07:08, Eli Zaretskii <eliz@gnu.org> wrote:

> > From: Richard Copley <rcopley@gmail.com>
> > Date: Mon, 19 Mar 2018 21:33:12 +0000
> > Cc: John Wiegley <johnw@gnu.org>, Emacs Development <emacs-devel@gnu.org>,
> Tom Tromey <tom@tromey.com>
> >
> > https://debbugs.gnu.org/cgi/bugreport.cgi?bug=30295#11
> >
> > Please try this in CSS mode:
> >
> > div: {
> >   #87e087
> > }
> >
> > I hope you'll agree it's bad
>
> Actually, doesn't look too bad here.
>




> > It's also easy and safe to fix.
>
> I don't see any proposed patches in the bug report.  What did I miss?
>

VVVVVVVVVVV


> > As ever I'm reluctant to provide a patch (though I do have one and I'll
> certainly be using it), because I haven't
> > assigned copyright, but I did make concrete suggestions for a fix.
>
> If someone can present a patch, I promise to review it.
>
> Thanks.
>

I can implement both suggestions (only one would be needed); they will
be one- or two-line changes which hopefully qualify as "tiny" for
contribution policy purposes.

Years ago I had a few lines of code accepted into the build system:

commit 6c0a9ed1b100c4dfbac327e6405396f1586267eb
Author: Richard Copley <rcopley@gmail.com>
Date:   Fri Jun 7 11:08:29 2013 +0300
Fix bug #14513 with --enable-locallisppath not working on MS-Windows.

Back then you (Eli) told me my limit was "all but exhausted" IIRC, but
that code has been improved or superseded and only a vestige of it
remains in Emacs.

Would that be OK?
Sincerely, apologies for the hassle.

[-- Attachment #2: Type: text/html, Size: 2852 bytes --]

Re: CSS contrast (#30295) (was Re: Heads-up: Emacs 26.1 RC1)

[Eli Zaretskii]

> From: Richard Copley <rcopley@gmail.com>
> Date: Tue, 20 Mar 2018 19:02:27 +0000
> Cc: John Wiegley <johnw@gnu.org>, Tom Tromey <tom@tromey.com>,
> 	Emacs Development <emacs-devel@gnu.org>
> 
> I can implement both suggestions (only one would be needed); they will
> be one- or two-line changes which hopefully qualify as "tiny" for
> contribution policy purposes.

One- or two-line changes should be okay.

Thanks.

Re: CSS contrast (#30295) (was Re: Heads-up: Emacs 26.1 RC1)

[Richard Copley]

[-- Attachment #1.1: Type: text/plain, Size: 483 bytes --]

On 20 March 2018 at 19:17, Eli Zaretskii <eliz@gnu.org> wrote:

> > From: Richard Copley <rcopley@gmail.com>
> > Date: Tue, 20 Mar 2018 19:02:27 +0000
> > Cc: John Wiegley <johnw@gnu.org>, Tom Tromey <tom@tromey.com>,
> >       Emacs Development <emacs-devel@gnu.org>
>
> One- or two-line changes should be okay.
>

Thanks.
Patch 1 is a small fix for the current formula using color-distance.
Patch 2 uses luminance as in Tom's original patch.
I think patch 2 gives better contrast.

[-- Attachment #1.2: Type: text/html, Size: 1108 bytes --]

[-- Attachment #2: css-contrast-1.patch --]
[-- Type: application/octet-stream, Size: 521 bytes --]

diff --git a/lisp/textmodes/css-mode.el b/lisp/textmodes/css-mode.el
index 727bc18ebb..f802396ae6 100644
--- a/lisp/textmodes/css-mode.el
+++ b/lisp/textmodes/css-mode.el
@@ -1096,7 +1096,7 @@ css--contrasty-color
 color, the returned color can be used as the foreground and still
 be readable."
   ;; See bug#25525 for a discussion of this.
-  (if (> (color-distance name "black") 292485)
+  (if (> (color-distance name "black") (color-distance name "white"))
       "black" "white"))
 
 (defcustom css-fontify-colors t

[-- Attachment #3: css-contrast-2.patch --]
[-- Type: application/octet-stream, Size: 707 bytes --]

diff --git a/lisp/textmodes/css-mode.el b/lisp/textmodes/css-mode.el
index 727bc18ebb..8b5f8d5dc3 100644
--- a/lisp/textmodes/css-mode.el
+++ b/lisp/textmodes/css-mode.el
@@ -1096,8 +1096,11 @@ css--contrasty-color
 color, the returned color can be used as the foreground and still
 be readable."
   ;; See bug#25525 for a discussion of this.
-  (if (> (color-distance name "black") 292485)
-      "black" "white"))
+  (let* ((rgb (color-name-to-rgb name))
+         (hsl (apply #'color-rgb-to-hsl rgb))
+         (luminance (nth 2 hsl)))
+    (if (> luminance 0.5)
+        "black" "white")))
 
 (defcustom css-fontify-colors t
   "Whether CSS colors should be fontified using the color as the background.

Re: CSS contrast (#30295) (was Re: Heads-up: Emacs 26.1 RC1)

[Yuri Khan]

On Wed, Mar 21, 2018 at 2:56 AM, Richard Copley <rcopley@gmail.com> wrote:

> Patch 1 is a small fix for the current formula using color-distance.
> Patch 2 uses luminance as in Tom's original patch.
> I think patch 2 gives better contrast.

Why not use the WCAG contrast ratio formula?

https://www.w3.org/TR/WCAG20/#visual-audio-contrast

(Except that, if implemented literally, it will probably not fit the
trivial change threshold.)

Re: CSS contrast (#30295) (was Re: Heads-up: Emacs 26.1 RC1)

[Eli Zaretskii]

> From: Richard Copley <rcopley@gmail.com>
> Date: Tue, 20 Mar 2018 19:56:12 +0000
> Cc: John Wiegley <johnw@gnu.org>, Tom Tromey <tom@tromey.com>, 
> 	Emacs Development <emacs-devel@gnu.org>
> 
> Patch 1 is a small fix for the current formula using color-distance.
> Patch 2 uses luminance as in Tom's original patch.
> I think patch 2 gives better contrast.

Thanks, but I think this is a step backward, as it more or less goes
back to the code before the discussion of bug#25525.

How about making the threshold a customizable value instead, with the
current hard-coded value the default?  That'd be compatible, and
should allow you to get the contrast of your liking.

Re: CSS contrast (#30295) (was Re: Heads-up: Emacs 26.1 RC1)

[Eli Zaretskii]

> From: Yuri Khan <yuri.v.khan@gmail.com>
> Date: Wed, 21 Mar 2018 12:52:15 +0700
> Cc: Eli Zaretskii <eliz@gnu.org>, John Wiegley <johnw@gnu.org>,
> 	Tom Tromey <tom@tromey.com>, Emacs Development <emacs-devel@gnu.org>
> 
> Why not use the WCAG contrast ratio formula?
> 
> https://www.w3.org/TR/WCAG20/#visual-audio-contrast
> 
> (Except that, if implemented literally, it will probably not fit the
> trivial change threshold.)

I have nothing against implementing the WCAG contrast criterion, but
that would certainly need to be done on master, not on the release
branch.  This discussion was about what to do for Emacs 26.1.

Re: CSS contrast (#30295) (was Re: Heads-up: Emacs 26.1 RC1)

[Richard Copley]

[-- Attachment #1: Type: text/plain, Size: 1106 bytes --]

On 21 March 2018 at 07:38, Eli Zaretskii <eliz@gnu.org> wrote:

> > From: Richard Copley <rcopley@gmail.com>
> > Date: Tue, 20 Mar 2018 19:56:12 +0000
> > Cc: John Wiegley <johnw@gnu.org>, Tom Tromey <tom@tromey.com>,
> >       Emacs Development <emacs-devel@gnu.org>
> >
> > Patch 1 is a small fix for the current formula using color-distance.
> > Patch 2 uses luminance as in Tom's original patch.
> > I think patch 2 gives better contrast.
>
> Thanks, but I think this is a step backward, as it more or less goes
> back to the code before the discussion of bug#25525.
>

Patch 2 does, yes. It's good to step backwards, because (the effect of) the
original code was OK, whereas the current code leads to unreadable text.

How about making the threshold a customizable value instead, with the
> current hard-coded value the default?  That'd be compatible, and
> should allow you to get the contrast of your liking.
>

It sounds like you want to insist that white-on-pale-green is somehow
better than black-on-pale-green, or that there's some other consideration
that's more important.
I don't understand.

[-- Attachment #2: Type: text/html, Size: 1950 bytes --]

Re: CSS contrast (#30295) (was Re: Heads-up: Emacs 26.1 RC1)

[Richard Copley]

[-- Attachment #1: Type: text/plain, Size: 1984 bytes --]

On 21 March 2018 at 09:59, Richard Copley <rcopley@gmail.com> wrote:

> On 21 March 2018 at 07:38, Eli Zaretskii <eliz@gnu.org> wrote:
>
>> > From: Richard Copley <rcopley@gmail.com>
>> > Date: Tue, 20 Mar 2018 19:56:12 +0000
>> > Cc: John Wiegley <johnw@gnu.org>, Tom Tromey <tom@tromey.com>,
>> >       Emacs Development <emacs-devel@gnu.org>
>> >
>> > Patch 1 is a small fix for the current formula using color-distance.
>> > Patch 2 uses luminance as in Tom's original patch.
>> > I think patch 2 gives better contrast.
>>
>> Thanks, but I think this is a step backward, as it more or less goes
>> back to the code before the discussion of bug#25525.
>>
>
> Patch 2 does, yes. It's good to step backwards, because (the effect of)
> the original code was OK, whereas the current code leads to unreadable text.
>
> How about making the threshold a customizable value instead, with the
>> current hard-coded value the default?  That'd be compatible, and
>> should allow you to get the contrast of your liking.
>>
>
> It sounds like you want to insist that white-on-pale-green is somehow
> better than black-on-pale-green, or that there's some other consideration
> that's more important.
> I don't understand.
>

To put it another way:
Your argument seemed to be that Tom's original criterion (use white text on
backgrounds with luminance <50%) was invented by Tom.
Is the current criterion (use white text on backgrounds inside a sphere of
a given radius centred on black, in the color-distance metric space) a
standard way of doing things?

I'm not saying that the luminance-less-than-50% calculation is a standard,
either, but I've seen it used before, which is more than I can say for the
color-inside-a-sphere calculation.
I'm also not saying that color-distance is any more or less "invented" than
luminance.

I do believe that color-distance is less relevant here than luminance is.
I also believe that black text is more readable than white text, on a pale
green background.

[-- Attachment #2: Type: text/html, Size: 3503 bytes --]

Re: CSS contrast (#30295) (was Re: Heads-up: Emacs 26.1 RC1)

[Eli Zaretskii]

> From: Richard Copley <rcopley@gmail.com>
> Date: Wed, 21 Mar 2018 09:59:37 +0000
> Cc: John Wiegley <johnw@gnu.org>, Tom Tromey <tom@tromey.com>, 
> 	Emacs Development <emacs-devel@gnu.org>
> 
>  Thanks, but I think this is a step backward, as it more or less goes
>  back to the code before the discussion of bug#25525.
> 
> Patch 2 does, yes. It's good to step backwards, because (the effect of) the original code was OK, whereas
> the current code leads to unreadable text.

The "unreadable" part seems to be in the eyes of the beholder.  As I
said, it's readable for me.  Color perception differs somewhat between
people, so I'm not surprised.

>  How about making the threshold a customizable value instead, with the
>  current hard-coded value the default?  That'd be compatible, and
>  should allow you to get the contrast of your liking.
> 
> It sounds like you want to insist that white-on-pale-green is somehow better than black-on-pale-green, or that
> there's some other consideration that's more important.

That's just one example.  I'm sure there are examples with other color
pairs that point to the opposite direction.

More generally, I want to insist on making only safe changes at this
late stage in the pretest.  The current code is in Emacs for more than
a year, so suddenly discarding behavior that was tested since then and
going to a behavior that was not tested at all doesn't make sense.
Therefore, I proposed a change that will allow people who don't like
this behavior to adapt it to their color perception, without changing
the tested default.  Such a change looks safe enough to me to install
it on the release branch, even though we are long past the time for
any significant changes that don't fix grave bugs.  By contrast, your
suggestion changes user-visible behavior in incompatible ways, which
is definitely not safe at this time.

Re: CSS contrast (#30295) (was Re: Heads-up: Emacs 26.1 RC1)

[Eli Zaretskii]

> From: Richard Copley <rcopley@gmail.com>
> Date: Wed, 21 Mar 2018 11:34:47 +0000
> Cc: John Wiegley <johnw@gnu.org>, Tom Tromey <tom@tromey.com>, 
> 	Emacs Development <emacs-devel@gnu.org>
> 
> Is the current criterion (use white text on backgrounds inside a sphere of a given radius centred on black, in
> the color-distance metric space) a standard way of doing things?

We do something very similar in frame-set-background-mode and in
tty-color-approximate.  Also note that color-distance is based on a
metric that we didn't invent.  Not sure whether it makes that
"standard".

> I'm not saying that the luminance-less-than-50% calculation is a standard, either, but I've seen it used before,
> which is more than I can say for the color-inside-a-sphere calculation.
> I'm also not saying that color-distance is any more or less "invented" than luminance.

I'm not dismissing these arguments, I'm just saying that it's too late
to raise them for what will become Emacs 26.1.  We can make those
changes on master, if people agree with your reasoning, or we could
use the contrast criterion there, or something else.  But for the
release branch, the only change I can think of which will allow you to
get what you want in a safe way is the one I proposed: introduce a
customizable threshold.

Re: CSS contrast (#30295) (was Re: Heads-up: Emacs 26.1 RC1)

[Richard Copley]

[-- Attachment #1: Type: text/plain, Size: 1998 bytes --]

On 21 March 2018 at 12:47, Eli Zaretskii <eliz@gnu.org> wrote:

> > From: Richard Copley <rcopley@gmail.com>
> > Date: Wed, 21 Mar 2018 11:34:47 +0000
> > Cc: John Wiegley <johnw@gnu.org>, Tom Tromey <tom@tromey.com>,
> >       Emacs Development <emacs-devel@gnu.org>
> >
> > Is the current criterion (use white text on backgrounds inside a sphere
> of a given radius centred on black, in
> > the color-distance metric space) a standard way of doing things?
>
> We do something very similar in frame-set-background-mode and in
> tty-color-approximate.


I don't see it. You're talking about this?

          ((>= (apply '+ (color-values bg-color frame))
               ;; Just looking at the screen, colors whose
               ;; values add up to .6 of the white total
               ;; still look dark to me.
               (* (apply '+ (color-values "white" frame)) .6))

That is a luminance comparison against 60%, except that instead of
computing luminance in any of the usual ways, it simply adds up the R, G
and B components.

Also note that color-distance is based on a
> metric that we didn't invent.  Not sure whether it makes that
> "standard".
>

It's this particular way of using color-distance that we appear to have
invented. If we didn't then we took it from somewhere else, and someone
(Tom?) should know where.

And we didn't invent luminance!

> I'm not dismissing these arguments, I'm just saying that it's too late

> to raise them for what will become Emacs 26.1.  We can make those
> changes on master, if people agree with your reasoning, or we could
> use the contrast criterion there, or something else.


Fair enough.


>  But for the
> release branch, the only change I can think of which will allow you to
> get what you want in a safe way is the one I proposed: introduce a
> customizable threshold.
>

I don't believe the concept of a sphere centred on black is a useful one in
this context, or that allowing the user to vary its radius makes it any
better.

Thanks.

[-- Attachment #2: Type: text/html, Size: 3444 bytes --]

Re: CSS contrast (#30295) (was Re: Heads-up: Emacs 26.1 RC1)

[Eli Zaretskii]

> From: Richard Copley <rcopley@gmail.com>
> Date: Wed, 21 Mar 2018 13:22:37 +0000
> Cc: John Wiegley <johnw@gnu.org>, Tom Tromey <tom@tromey.com>, 
> 	Emacs Development <emacs-devel@gnu.org>
> 
>  We do something very similar in frame-set-background-mode and in
>  tty-color-approximate.
> 
> I don't see it. You're talking about this?
> 
>           ((>= (apply '+ (color-values bg-color frame))
>                ;; Just looking at the screen, colors whose
>                ;; values add up to .6 of the white total
>                ;; still look dark to me.
>                (* (apply '+ (color-values "white" frame)) .6))

That's the first example, yes.

> It's this particular way of using color-distance that we appear to
> have invented.

Not according to the article from which the implementation of
color-distance was taken.

> If we didn't then we took it from
> somewhere else, and someone (Tom?) should know where.

The comments in the sources of color-distance tell where we took it
from.

> I don't believe the concept of a sphere centred on black is a useful one in this context, or that allowing the
> user to vary its radius makes it any better.

I suggest to read the original article mentioned above.

Re: CSS contrast (#30295) (was Re: Heads-up: Emacs 26.1 RC1)

[Richard Copley]

[-- Attachment #1: Type: text/plain, Size: 1742 bytes --]

On 21 March 2018 at 13:32, Eli Zaretskii <eliz@gnu.org> wrote:

> > From: Richard Copley <rcopley@gmail.com>
> > Date: Wed, 21 Mar 2018 13:22:37 +0000
> > Cc: John Wiegley <johnw@gnu.org>, Tom Tromey <tom@tromey.com>,
> >       Emacs Development <emacs-devel@gnu.org>
> >
> >  We do something very similar in frame-set-background-mode and in
> >  tty-color-approximate.
> >
> > I don't see it. You're talking about this?
> >
> >           ((>= (apply '+ (color-values bg-color frame))
> >                ;; Just looking at the screen, colors whose
> >                ;; values add up to .6 of the white total
> >                ;; still look dark to me.
> >                (* (apply '+ (color-values "white" frame)) .6))
>
> That's the first example, yes.
>
> > It's this particular way of using color-distance that we appear to
> > have invented.
>
> Not according to the article from which the implementation of
> color-distance was taken.


???

> If we didn't then we took it from
> > somewhere else, and someone (Tom?) should know where.
>
> The comments in the sources of color-distance tell where we took it
> from.
>

The comments, and the article, say where the colour distance formula itself
comes from.
The article doesn't support our usage of the formula at all.

It says:
>> This distance will only be used in comparisons, to verify whether one
colour, *A*, is closer to colour *B* or to colour *C*.
That's what my patch 1 does. It's nothing like what the current code does.

> I don't believe the concept of a sphere centred on black is a useful one
> in this context, or that allowing the
> > user to vary its radius makes it any better.
>
> I suggest to read the original article mentioned above.
>

I suggest you re-read it.

[-- Attachment #2: Type: text/html, Size: 3033 bytes --]

Re: CSS contrast (#30295) (was Re: Heads-up: Emacs 26.1 RC1)

[Eli Zaretskii]

> From: Richard Copley <rcopley@gmail.com>
> Date: Wed, 21 Mar 2018 14:24:50 +0000
> Cc: John Wiegley <johnw@gnu.org>, Tom Tromey <tom@tromey.com>,
> 	Emacs Development <emacs-devel@gnu.org>
> 
> I suggest you re-read it.

OK, I will.  But I no longer enjoy this conversation, so let's leave
it at that, okay?

Re: CSS contrast (#30295) (was Re: Heads-up: Emacs 26.1 RC1)

[Richard Copley]

[-- Attachment #1: Type: text/plain, Size: 473 bytes --]

On 21 March 2018 at 15:36, Eli Zaretskii <eliz@gnu.org> wrote:

> > From: Richard Copley <rcopley@gmail.com>
> > Date: Wed, 21 Mar 2018 14:24:50 +0000
> > Cc: John Wiegley <johnw@gnu.org>, Tom Tromey <tom@tromey.com>,
> >       Emacs Development <emacs-devel@gnu.org>
> >
> > I suggest you re-read it.
>
> OK, I will.  But I no longer enjoy this conversation, so let's leave
> it at that, okay?
>

I don't understand why you're upset, but I apologise. Yes, let's leave it.

[-- Attachment #2: Type: text/html, Size: 1079 bytes --]