[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#41544: 26.3; Possible incorrect results from color-distance
From: |
Eli Zaretskii |
Subject: |
bug#41544: 26.3; Possible incorrect results from color-distance |
Date: |
Sat, 06 Jun 2020 21:27:42 +0300 |
> From: Mattias Engdegård <mattiase@acm.org>
> Date: Sat, 6 Jun 2020 18:54:41 +0200
> Cc: 41544@debbugs.gnu.org
>
> Since we are making little progress, let's leave color-name-to-rgb unchanged
> for the moment. We can both change our minds later. It's not strictly
> required for the introduction and use of color-dark-p; patch updated.
What practical problem is being solved here? (Please don't say "this
was done in different ways", that's not a practical problem from my
POV.)
> * lisp/facemenu.el (list-colors-print): Use readable-foreground-color.
I don't mind installing this part, but have the rationale should be
spelled out in the log message.
> --- a/lisp/faces.el
> +++ b/lisp/faces.el
> @@ -1786,15 +1786,24 @@ defined-colors-with-face-attributes
>
> (defun readable-foreground-color (color)
> "Return a readable foreground color for background COLOR."
> - (let* ((rgb (color-values color))
> - (max (apply #'max rgb))
> - (black (car (color-values "black")))
> - (white (car (color-values "white"))))
> - ;; Select black or white depending on which one is less similar to
> - ;; the brightest component.
> - (if (> (abs (- max black)) (abs (- max white)))
> - "black"
> - "white")))
What was wrong with the original code? If it produced sub-optimal
results, please show an example of that.
> +(defun color-dark-p (rgb)
> + "Whether RGB is more readable against white than black.
> +RGB is a 3-element list (R G B), each component in the range [0,1]."
> + (let* ((sr (nth 0 rgb))
> + (sg (nth 1 rgb))
> + (sb (nth 2 rgb))
> + ;; Use the power 2.2 as an approximation to sRGB gamma;
> + ;; it should be good enough for the purpose of this function.
> + (r (expt sr 2.2))
> + (g (expt sg 2.2))
> + (b (expt sb 2.2)))
> + (unless (<= 0 (min r g b) (max r g b) 1)
> + (error "RGB components %S not in [0,1]" rgb))
> + ;; The cut-off value was determined experimentally; see bug#41544.
> + (< (+ (* r 0.299) (* g 0.587) (* b 0.114))
> + (eval-when-compile (expt 0.6 2.2)))))
This code's algorithm and rationale should be explained in the
comments before we can discuss whether it's an improvement and why.
> diff --git a/lisp/frame.el b/lisp/frame.el
> index 6c2f774709..253528da75 100644
> --- a/lisp/frame.el
> +++ b/lisp/frame.el
> @@ -1156,6 +1156,13 @@ frame-background-mode
This is a non-starter, sorry. I'm not interested in changing what is
considered dark and light background of a frame.
> diff --git a/lisp/term/rxvt.el b/lisp/term/rxvt.el
> index 31e3d6ede4..5dc754c8e0 100644
> --- a/lisp/term/rxvt.el
> +++ b/lisp/term/rxvt.el
> @@ -206,13 +206,11 @@ rxvt-set-background-mode
Likewise here.
> diff --git a/lisp/term/w32console.el b/lisp/term/w32console.el
> index 36e9d896c7..0e9d7c8b5c 100644
> --- a/lisp/term/w32console.el
> +++ b/lisp/term/w32console.el
> @@ -86,9 +86,9 @@ terminal-init-w32console
> (setq r (nth 2 descr)
> g (nth 3 descr)
> b (nth 4 descr))
> - (if (< (+ r g b) (* .6 (+ 65535 65535 65535)))
> - (setq bg-mode 'dark)
> - (setq bg-mode 'light))
> + (setq bg-mode (if (color-dark-p
> + (list (/ r 65535.0) (/ g 65535.0) (/ b 65535.0)))
> + 'dark 'light))
> (set-terminal-parameter nil 'background-mode bg-mode))
And here.
> diff --git a/lisp/term/xterm.el b/lisp/term/xterm.el
> index 1a727e3933..bf9bcae526 100644
> --- a/lisp/term/xterm.el
> +++ b/lisp/term/xterm.el
> @@ -1120,9 +1120,8 @@ xterm-register-default-colors
> (clear-face-cache)))
>
> (defun xterm-maybe-set-dark-background-mode (redc greenc bluec)
> - ;; Use the heuristic in `frame-set-background-mode' to decide if a
> - ;; frame is dark.
> - (when (< (+ redc greenc bluec) (* .6 (+ 65535 65535 65535)))
> + (when (color-dark-p (mapcar (lambda (c) (/ c 65535.0))
> + (list redc greenc bluec)))
> (set-terminal-parameter nil 'background-mode 'dark)
And here.
> -(defun css--contrasty-color (name)
> - "Return a color that contrasts with NAME.
> -NAME is of any form accepted by `color-distance'.
> -The returned color will be usable by Emacs and will contrast
> -with NAME; in particular so that if NAME is used as a background
> -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"))
> -
> (defcustom css-fontify-colors t
> "Whether CSS colors should be fontified using the color as the background.
> When non-`nil', a text representing CSS color will be fontified
> @@ -1199,7 +1188,8 @@ css--fontify-region
> (add-text-properties
> start (point)
> (list 'face (list :background color
> - :foreground (css--contrasty-color color)
> + :foreground (readable-foreground-color
> + color)
> :box '(:line-width -1))))))))))))
> extended-region))
Here, once again I will ask what practical problem is being fixed.
Thanks.
- bug#41544: 26.3; Possible incorrect results from color-distance, (continued)
- bug#41544: 26.3; Possible incorrect results from color-distance, Eli Zaretskii, 2020/06/06
- bug#41544: 26.3; Possible incorrect results from color-distance, Mattias Engdegård, 2020/06/06
- bug#41544: 26.3; Possible incorrect results from color-distance, Eli Zaretskii, 2020/06/06
- bug#41544: 26.3; Possible incorrect results from color-distance, Mattias Engdegård, 2020/06/06
- bug#41544: 26.3; Possible incorrect results from color-distance, Drew Adams, 2020/06/06
- bug#41544: 26.3; Possible incorrect results from color-distance, Mattias Engdegård, 2020/06/07
- bug#41544: 26.3; Possible incorrect results from color-distance, Eli Zaretskii, 2020/06/07
- bug#41544: 26.3; Possible incorrect results from color-distance, Drew Adams, 2020/06/07
- bug#41544: 26.3; Possible incorrect results from color-distance, Mattias Engdegård, 2020/06/09
- bug#41544: 26.3; Possible incorrect results from color-distance, Drew Adams, 2020/06/07
- bug#41544: 26.3; Possible incorrect results from color-distance,
Eli Zaretskii <=
- bug#41544: 26.3; Possible incorrect results from color-distance, Simen Heggestøyl, 2020/06/07
- bug#41544: 26.3; Possible incorrect results from color-distance, Mattias Engdegård, 2020/06/07
- bug#41544: 26.3; Possible incorrect results from color-distance, Simen Heggestøyl, 2020/06/07
- bug#41544: 26.3; Possible incorrect results from color-distance, Mattias Engdegård, 2020/06/07
- bug#41544: 26.3; Possible incorrect results from color-distance, Simen Heggestøyl, 2020/06/08
- bug#41544: 26.3; Possible incorrect results from color-distance, Eli Zaretskii, 2020/06/07
- bug#41544: 26.3; Possible incorrect results from color-distance, Drew Adams, 2020/06/07
- bug#41544: 26.3; Possible incorrect results from color-distance, Simen Heggestøyl, 2020/06/07
- bug#41544: 26.3; Possible incorrect results from color-distance, Mattias Engdegård, 2020/06/08
- bug#41544: 26.3; Possible incorrect results from color-distance, Drew Adams, 2020/06/08