bug#51556: 29.0.50; Poor contrast of Customize SVG icons with dark backgrounds

bug#51556: 29.0.50; Poor contrast of Customize SVG icons with dark backgrounds

[Kévin Le Gouguec]

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

On darker themes (e.g. modus-vivendi), the Customize SVG icons
(etc/images/right.svg and etc/images/down.svg) are not very visible, due
to the poor contrast between their fill color (#2e3436) and dark
backgrounds[1].

Empirically, AFAICT, the "nuclear" solution of removing "fill:#2e3436"
from the SVG files causes the icons to render with the foreground color
of the default face.  That solves the contrast problem, but maybe a more
subtle solution could be devised?

Once the "fill:…" bit is removed, it becomes possible to change the SVG
image's color, either when calling create-image[2], or by applying
faces[3].

A quick dive into widget-create-child-and-convert (down to
widget-image-insert) suggests that there's no way to tell widget.el to
slap a face onto an image; perhaps a new keyword could be added to that
effect?

Then we could add a face that (1) could preserve the original color
(#2e3436) for light backgrounds (2) be set to something more reasonable
for dark backgrounds (3) be themed.

I wouldn't mind working on that (time permitting); I'd like to collect
some feedback first to make sure what I suggest is the right approach;
maybe there are better solutions I haven't considered.

Thanks for your time.


[1] https://webaim.org/resources/contrastchecker/?fcolor=2E3436&bcolor=000000

[2] E.g. after saving the attached rightnofill.svg, evaluate
    (insert-image (create-image "rightnofill.svg"
                                nil nil :foreground "orange"))

[3] E.g. after saving the attached rightnofill.svg, evaluate
    (insert-image (create-image "rightnofill.svg"))
    (add-text-properties (point-at-bol) (point-at-eol) '(font-lock-face success))


In GNU Emacs 29.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.30, cairo version 1.16.0)
 of 2021-10-30 built on amdahl30
Repository revision: c30f95078c0735447c0bf293f2e6f573bc7057a3
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12013000
System Description: openSUSE Tumbleweed

Configured using:
 'configure --with-cairo --with-gconf'

Configured features:
ACL CAIRO DBUS FREETYPE GCONF GIF GLIB GMP GNUTLS GPM GSETTINGS HARFBUZZ
JPEG JSON LCMS2 LIBOTF LIBSELINUX LIBSYSTEMD LIBXML2 M17N_FLT MODULES
NOTIFY INOTIFY PDUMPER PNG RSVG SECCOMP SOUND THREADS TIFF
TOOLKIT_SCROLL_BARS X11 XDBE XIM XPM GTK3 ZLIB

Important settings:
  value of $LANG: en_US.UTF-8
  value of $XMODIFIERS: @im=local
  locale-coding-system: utf-8-unix


[-- Attachment #2: rightnofill.svg --]
[-- Type: image/svg+xml, Size: 4878 bytes --]

<?xml version='1.0' encoding='UTF-8' standalone='no'?>
<svg xmlns:cc='http://creativecommons.org/ns#' xmlns:dc='http://purl.org/dc/elements/1.1/' sodipodi:docname='pan-end-symbolic.svg' inkscape:export-filename='/home/sam/source-symbolic.png' inkscape:export-xdpi='270' inkscape:export-ydpi='270' height='16' id='svg7384' xmlns:inkscape='http://www.inkscape.org/namespaces/inkscape' xmlns:rdf='http://www.w3.org/1999/02/22-rdf-syntax-ns#' xmlns:sodipodi='http://sodipodi.sourceforge.net/DTD/sodipodi-0.dtd' style='enable-background:new' xmlns:svg='http://www.w3.org/2000/svg' version='1.1' inkscape:version='1.0 (4035a4fb49, 2020-05-01)' width='16' xmlns='http://www.w3.org/2000/svg'>
  <sodipodi:namedview inkscape:bbox-nodes='true' inkscape:bbox-paths='false' bordercolor='#000000' borderlayer='false' borderopacity='0.50196078' inkscape:current-layer='layer10' inkscape:cx='31.147668' inkscape:cy='7.96251' inkscape:document-rotation='0' gridtolerance='10' inkscape:guide-bbox='true' guidetolerance='10' id='namedview88' inkscape:measure-end='0,0' inkscape:measure-start='0,0' inkscape:object-nodes='true' inkscape:object-paths='true' objecttolerance='10' pagecolor='#e2e2e2' inkscape:pageopacity='0' inkscape:pageshadow='2' showborder='false' showgrid='true' showguides='false' inkscape:showpageshadow='false' inkscape:snap-bbox='true' inkscape:snap-bbox-edge-midpoints='false' inkscape:snap-bbox-midpoints='true' inkscape:snap-center='false' inkscape:snap-global='true' inkscape:snap-grids='true' inkscape:snap-intersection-paths='false' inkscape:snap-midpoints='true' inkscape:snap-nodes='true' inkscape:snap-object-midpoints='true' inkscape:snap-others='true' inkscape:snap-page='false' inkscape:snap-smooth-nodes='true' inkscape:snap-to-guides='true' inkscape:window-height='1205' inkscape:window-maximized='0' inkscape:window-width='1553' inkscape:window-x='26' inkscape:window-y='23' inkscape:zoom='1'>
    <inkscape:grid color='#000000' dotted='false' empcolor='#0800ff' empopacity='0.4627451' empspacing='4' enabled='true' id='grid4866' opacity='0.16470588' originx='-112.00585' originy='-951.99999' snapvisiblegridlinesonly='true' spacingx='1' spacingy='1' type='xygrid' visible='true'/>
    <inkscape:grid dotted='true' empcolor='#3f3fff' empopacity='0' empspacing='4' id='grid3540' originx='-112.00585' originy='-951.99999' spacingx='0.5' spacingy='0.5' type='xygrid'/>
  </sodipodi:namedview>
  <metadata id='metadata90'>
    <rdf:RDF>
      <cc:Work rdf:about=''>
        <dc:format>image/svg+xml</dc:format>
        <dc:type rdf:resource='http://purl.org/dc/dcmitype/StillImage'/>
        <dc:title>Gnome Symbolic Icons</dc:title>
        <cc:license rdf:resource='http://creativecommons.org/licenses/by-sa/4.0/'/>
      </cc:Work>
      <cc:License rdf:about='http://creativecommons.org/licenses/by-sa/4.0/'>
        <cc:permits rdf:resource='http://creativecommons.org/ns#Reproduction'/>
        <cc:permits rdf:resource='http://creativecommons.org/ns#Distribution'/>
        <cc:requires rdf:resource='http://creativecommons.org/ns#Notice'/>
        <cc:requires rdf:resource='http://creativecommons.org/ns#Attribution'/>
        <cc:permits rdf:resource='http://creativecommons.org/ns#DerivativeWorks'/>
        <cc:requires rdf:resource='http://creativecommons.org/ns#ShareAlike'/>
      </cc:License>
    </rdf:RDF>
  </metadata>
  <title id='title8473'>Gnome Symbolic Icons</title>
  <defs id='defs7386'/>
  <g inkscape:groupmode='layer' id='layer10' inkscape:label='ui' transform='translate(-112.00585,-951.99999)'>
    <path inkscape:connector-curvature='0' d='m 117,966 6.00585,-6.00001 L 117,954 Z' id='path6412' sodipodi:nodetypes='cccc' style='fill-opacity:1;stroke:none'/>
  </g>
  <g inkscape:groupmode='layer' id='layer1' inkscape:label='status' transform='translate(-112.00585,-887.99999)'/>
  <g inkscape:groupmode='layer' id='layer11' inkscape:label='legacy' transform='translate(-112.00585,-951.99999)'/>
  <g inkscape:groupmode='layer' id='layer7' inkscape:label='places' transform='translate(-112.00585,-887.99999)'/>
  <g inkscape:groupmode='layer' id='layer6' inkscape:label='mimetypes' transform='translate(-112.00585,-887.99999)'/>
  <g inkscape:groupmode='layer' id='layer5' inkscape:label='emotes' transform='translate(-112.00585,-887.99999)'/>
  <g inkscape:groupmode='layer' id='layer9' inkscape:label='emblems' transform='translate(-112.00585,-887.99999)'/>
  <g inkscape:groupmode='layer' id='layer2' inkscape:label='devices' transform='translate(-112.00585,-887.99999)'/>
  <g inkscape:groupmode='layer' id='layer8' inkscape:label='categories' transform='translate(-112.00585,-887.99999)'/>
  <g inkscape:groupmode='layer' id='layer3' inkscape:label='apps' transform='translate(-112.00585,-887.99999)'/>
  <g inkscape:groupmode='layer' id='layer4' inkscape:label='actions' transform='translate(-112.00585,-887.99999)'/>
</svg>

bug#51556: 29.0.50; Poor contrast of Customize SVG icons with dark backgrounds

[Eli Zaretskii]

> From: Kévin Le Gouguec <kevin.legouguec@gmail.com>
> Date: Mon, 01 Nov 2021 18:56:13 +0100
> 
> On darker themes (e.g. modus-vivendi), the Customize SVG icons
> (etc/images/right.svg and etc/images/down.svg) are not very visible, due
> to the poor contrast between their fill color (#2e3436) and dark
> backgrounds[1].

That's for the themes to resolve, isn't it?  They could supply their
own icons, because only they know which colors will go well with the
theme's colors.

So I don't think we should try solving this in general.

bug#51556: 29.0.50; Poor contrast of Customize SVG icons with dark backgrounds

[Kévin Le Gouguec]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Kévin Le Gouguec <kevin.legouguec@gmail.com>
>> Date: Mon, 01 Nov 2021 18:56:13 +0100
>> 
>> On darker themes (e.g. modus-vivendi), the Customize SVG icons
>> (etc/images/right.svg and etc/images/down.svg) are not very visible, due
>> to the poor contrast between their fill color (#2e3436) and dark
>> backgrounds[1].
>
> That's for the themes to resolve, isn't it?  They could supply their
> own icons, because only they know which colors will go well with the
> theme's colors.

That's sort of what I suggest?  In the rest of my message, I propose
allowing widgets to apply faces to their icons (:on-glyph and
:off-glyph), so that themes can customize these faces.  This should work
for all SVG images that do not define a fill color.

Or is there a way for themes to define sets of icons?  "(elisp) Custom
Themes" describes custom-theme-set-variables and custom-theme-set-faces,
but I couldn't find anything related to icons.

Or are you suggesting adding the ability for themes to include sets of
icons?  That seems a bit heavy-handed to me; I don't think a lot of
theme authors will want to bother with SVG editing.  For the purposes of
this bug, merely letting themes customize the icon color (through faces)
ought to be enough?

I'm not sure which path forward you prefer (if any).  Sorry for being
slow.

bug#51556: 29.0.50; Poor contrast of Customize SVG icons with dark backgrounds

[Jim Porter]

On 11/1/2021 12:14 PM, Eli Zaretskii wrote:
>> From: Kévin Le Gouguec <kevin.legouguec@gmail.com>
>> Date: Mon, 01 Nov 2021 18:56:13 +0100
>>
>> On darker themes (e.g. modus-vivendi), the Customize SVG icons
>> (etc/images/right.svg and etc/images/down.svg) are not very visible, due
>> to the poor contrast between their fill color (#2e3436) and dark
>> backgrounds[1].
> 
> That's for the themes to resolve, isn't it?  They could supply their
> own icons, because only they know which colors will go well with the
> theme's colors.

The method suggested in the original report (see below) would allow for 
themes to do this by customizing the appropriate face, wouldn't it? That 
seems like a simpler way for themes to apply the appropriate colors.

As someone developing a dark Emacs theme, I think this would be a 
welcome improvement.

On 11/1/2021 10:56 AM, Kévin Le Gouguec wrote:
 > A quick dive into widget-create-child-and-convert (down to
 > widget-image-insert) suggests that there's no way to tell widget.el to
 > slap a face onto an image; perhaps a new keyword could be added to that
 > effect?
 >
 > Then we could add a face that (1) could preserve the original color
 > (#2e3436) for light backgrounds (2) be set to something more reasonable
 > for dark backgrounds (3) be themed.

bug#51556: 29.0.50; Poor contrast of Customize SVG icons with dark backgrounds

[Eli Zaretskii]

> From: Kévin Le Gouguec <kevin.legouguec@gmail.com>
> Cc: 51556@debbugs.gnu.org
> Date: Mon, 01 Nov 2021 21:10:13 +0100
> 
> Or is there a way for themes to define sets of icons?

In some cases, yes.  For example, on the tool bar and the tab bar.

> "(elisp) Custom
> Themes" describes custom-theme-set-variables and custom-theme-set-faces,
> but I couldn't find anything related to icons.
> 
> Or are you suggesting adding the ability for themes to include sets of
> icons?

If that's what it will take, yes.

> That seems a bit heavy-handed to me; I don't think a lot of
> theme authors will want to bother with SVG editing.

The default will be to use the existing icons, as we did before.

> For the purposes of this bug, merely letting themes customize the
> icon color (through faces) ought to be enough?

That will only affect the background, which might not be enough.

bug#51556: 29.0.50; Poor contrast of Customize SVG icons with dark backgrounds

[Eli Zaretskii]

> Cc: 51556@debbugs.gnu.org
> From: Jim Porter <jporterbugs@gmail.com>
> Date: Mon, 1 Nov 2021 13:49:40 -0700
> 
> >> On darker themes (e.g. modus-vivendi), the Customize SVG icons
> >> (etc/images/right.svg and etc/images/down.svg) are not very visible, due
> >> to the poor contrast between their fill color (#2e3436) and dark
> >> backgrounds[1].
> > 
> > That's for the themes to resolve, isn't it?  They could supply their
> > own icons, because only they know which colors will go well with the
> > theme's colors.
> 
> The method suggested in the original report (see below) would allow for 
> themes to do this by customizing the appropriate face, wouldn't it? That 
> seems like a simpler way for themes to apply the appropriate colors.

AFAIK, only the background color of the face will affect the display,
and that is not enough.  Since a theme determines the looks, it should
also control the icons where that matters.

bug#51556: 29.0.50; Poor contrast of Customize SVG icons with dark backgrounds

[Stefan Kangas]

Eli Zaretskii <eliz@gnu.org> writes:

> AFAIK, only the background color of the face will affect the display,
> and that is not enough.  Since a theme determines the looks, it should
> also control the icons where that matters.

As I have recently announced on emacs-devel, I am working on developing
a library supporting icon sets.

I think it is better to think of icons as orthogonal to the colors: this
allows choosing any combination of icons and colors.  For example, I
might want to use the icons from some free set X, but I want to use it
with all color themes.  This will also allow color theme developers to
simply say "this or that icon, whatever it is, should have this color",
without having to provide a large amount of icons to get it right.

Thinking about how to fix this in emacs-28, we discussed adding a face
and just removing the foreground color from the svg.

If we were add a face here, I think the more general customization that
I hope we will introduce in Emacs 29.1 (read: if we accept my new
library) would render that face ineffective.  So it might not be worth
it, I don't know.  Perhaps something like that could wait for 29.

The simplest solution for emacs-28 seems to simply remove the foreground
color of that glyph, which IIUC (but I didn't test) should mean it will
just use the foreground color of the current face in the customize
buffer.  Which should look okay in most cases, I think.

bug#51556: 29.0.50; Poor contrast of Customize SVG icons with dark backgrounds

[Eli Zaretskii]

> From: Stefan Kangas <stefan@marxist.se>
> Date: Tue, 2 Nov 2021 07:41:43 -0700
> Cc: Jim Porter <jporterbugs@gmail.com>, 51556@debbugs.gnu.org, kevin.legouguec@gmail.com
> 
> Thinking about how to fix this in emacs-28, we discussed adding a face
> and just removing the foreground color from the svg.

Foreground or background?

AFAIR, we already are able to display SVG with non-default background.
As for foreground: what does it mean to remove the foreground from an
image?  Aren't the foreground colors the most important part of an
image?

bug#51556: 29.0.50; Poor contrast of Customize SVG icons with dark backgrounds

[Stefan Kangas]

Eli Zaretskii <eliz@gnu.org> writes:

>> Thinking about how to fix this in emacs-28, we discussed adding a face
>> and just removing the foreground color from the svg.
>
> Foreground or background?

Foreground.

> AFAIR, we already are able to display SVG with non-default background.
> As for foreground: what does it mean to remove the foreground from an
> image?  Aren't the foreground colors the most important part of an
> image?

It used to be the case that icons normally had 3D appearance and so on,
in which case we obviously couldn't change the colors easily.  Such
icons were normally distributed in some bitmap format.

These days, icons are treated more like glyphs, and the intention is
that they will have colors suitable for the context they are used in.
Such icons are normally distributed as SVG or indeed as TTF files.

See here for some examples:

    https://google.github.io/material-design-icons/#coloring

bug#51556: 29.0.50; Poor contrast of Customize SVG icons with dark backgrounds

[Stefan Kangas]

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

Stefan Kangas <stefan@marxist.se> writes:

>> As for foreground: what does it mean to remove the foreground from an
>> image?

(I realize that I didn't answer this part in my previous reply.)  SVG is
a special case, where we just use the foreground of the current face.

In this case, I think the fix is as simple as the attached patch.  I've
tested this with several themes, and it looks very good to my eyes.

I suggest that we install this fix on emacs-28.

[-- Attachment #2: 0001-Use-current-face-foreground-for-SVG-icons-in-customi.patch --]
[-- Type: text/x-diff, Size: 4310 bytes --]

From b6682bb4f3d7992f0594b848605937f358f93a33 Mon Sep 17 00:00:00 2001
From: Stefan Kangas <stefan@marxist.se>
Date: Tue, 2 Nov 2021 16:21:19 +0100
Subject: [PATCH] Use current face foreground for SVG icons in customize

* etc/images/down.svg:
* etc/images/left.svg:
* etc/images/right.svg:
* etc/images/up.svg: Don't define foreground; this means they will use
the foreground of the currently defined face instead.  (Bug#51556)
---
 etc/images/down.svg  | 2 +-
 etc/images/left.svg  | 2 +-
 etc/images/right.svg | 2 +-
 etc/images/up.svg    | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/etc/images/down.svg b/etc/images/down.svg
index e2760427d7..707cd23ea4 100644
--- a/etc/images/down.svg
+++ b/etc/images/down.svg
@@ -25,7 +25,7 @@
   <title id='title8473'>Gnome Symbolic Icons</title>
   <defs id='defs7386'/>
   <g inkscape:groupmode='layer' id='layer10' inkscape:label='ui' transform='translate(-152.00586,-952)'>
-    <path inkscape:connector-curvature='0' d='m 166,957 -5.99414,5.99999 L 154,957 Z' id='path6424' sodipodi:nodetypes='cccc' style='fill:#2e3436;fill-opacity:1;stroke:none'/>
+    <path inkscape:connector-curvature='0' d='m 166,957 -5.99414,5.99999 L 154,957 Z' id='path6424' sodipodi:nodetypes='cccc' style='fill-opacity:1;stroke:none'/>
   </g>
   <g inkscape:groupmode='layer' id='layer1' inkscape:label='status' transform='translate(-152.00586,-888)'/>
   <g inkscape:groupmode='layer' id='layer11' inkscape:label='legacy' transform='translate(-152.00586,-952)'/>
diff --git a/etc/images/left.svg b/etc/images/left.svg
index d6429bc410..893515d2df 100644
--- a/etc/images/left.svg
+++ b/etc/images/left.svg
@@ -25,7 +25,7 @@
   <title id='title8473'>Gnome Symbolic Icons</title>
   <defs id='defs7386'/>
   <g inkscape:groupmode='layer' id='layer10' inkscape:label='ui' transform='translate(-92.005848,-951.99999)'>
-    <path inkscape:connector-curvature='0' d='M 103,966 97.00585,959.99999 103,954 Z' id='path6400' sodipodi:nodetypes='cccc' style='fill:#2e3436;fill-opacity:1;stroke:none'/>
+    <path inkscape:connector-curvature='0' d='M 103,966 97.00585,959.99999 103,954 Z' id='path6400' sodipodi:nodetypes='cccc' style='fill-opacity:1;stroke:none'/>
   </g>
   <g inkscape:groupmode='layer' id='layer1' inkscape:label='status' transform='translate(-92.005848,-887.99999)'/>
   <g inkscape:groupmode='layer' id='layer11' inkscape:label='legacy' transform='translate(-92.005848,-951.99999)'/>
diff --git a/etc/images/right.svg b/etc/images/right.svg
index d58cd36435..6c7d715939 100644
--- a/etc/images/right.svg
+++ b/etc/images/right.svg
@@ -25,7 +25,7 @@
   <title id='title8473'>Gnome Symbolic Icons</title>
   <defs id='defs7386'/>
   <g inkscape:groupmode='layer' id='layer10' inkscape:label='ui' transform='translate(-112.00585,-951.99999)'>
-    <path inkscape:connector-curvature='0' d='m 117,966 6.00585,-6.00001 L 117,954 Z' id='path6412' sodipodi:nodetypes='cccc' style='fill:#2e3436;fill-opacity:1;stroke:none'/>
+    <path inkscape:connector-curvature='0' d='m 117,966 6.00585,-6.00001 L 117,954 Z' id='path6412' sodipodi:nodetypes='cccc' style='fill-opacity:1;stroke:none'/>
   </g>
   <g inkscape:groupmode='layer' id='layer1' inkscape:label='status' transform='translate(-112.00585,-887.99999)'/>
   <g inkscape:groupmode='layer' id='layer11' inkscape:label='legacy' transform='translate(-112.00585,-951.99999)'/>
diff --git a/etc/images/up.svg b/etc/images/up.svg
index 9e1a245be7..e358c29912 100644
--- a/etc/images/up.svg
+++ b/etc/images/up.svg
@@ -25,7 +25,7 @@
   <title id='title8473'>Gnome Symbolic Icons</title>
   <defs id='defs7386'/>
   <g inkscape:groupmode='layer' id='layer10' inkscape:label='ui' transform='translate(-132.00585,-952)'>
-    <path inkscape:connector-curvature='0' d='M 146,963 140.00585,956.99999 134,963 Z' id='path6418' sodipodi:nodetypes='cccc' style='fill:#2e3436;fill-opacity:1;stroke:none'/>
+    <path inkscape:connector-curvature='0' d='M 146,963 140.00585,956.99999 134,963 Z' id='path6418' sodipodi:nodetypes='cccc' style='fill-opacity:1;stroke:none'/>
   </g>
   <g inkscape:groupmode='layer' id='layer1' inkscape:label='status' transform='translate(-132.00585,-888)'/>
   <g inkscape:groupmode='layer' id='layer11' inkscape:label='legacy' transform='translate(-132.00585,-952)'/>
-- 
2.30.2

bug#51556: 29.0.50; Poor contrast of Customize SVG icons with dark backgrounds

[Eli Zaretskii]

> From: Stefan Kangas <stefan@marxist.se>
> Date: Tue, 2 Nov 2021 08:26:12 -0700
> Cc: jporterbugs@gmail.com, 51556@debbugs.gnu.org, kevin.legouguec@gmail.com
> 
> >> As for foreground: what does it mean to remove the foreground from an
> >> image?
> 
> (I realize that I didn't answer this part in my previous reply.)  SVG is
> a special case, where we just use the foreground of the current face.

I don't think I understand what that means.  We disregard the colors
specified by the SVG image?

> I suggest that we install this fix on emacs-28.

Sorry, at this stage I'd prefer not to install anything on the release
branch that isn't a bugfix.  Or else take it up with Lars, and ask him
to forget about releasing Emacs 28.1 soon, because this way it will
take a while ;-)

bug#51556: 29.0.50; Poor contrast of Customize SVG icons with dark backgrounds

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> Sorry, at this stage I'd prefer not to install anything on the release
> branch that isn't a bugfix.

I agree.  The chevrons in the Customize buffers aren't optimal on a dark
background, but it's not worth changing at this point in emacs-28.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#51556: 29.0.50; Poor contrast of Customize SVG icons with dark backgrounds

[Stefan Kangas]

Eli Zaretskii <eliz@gnu.org> writes:

> I don't think I understand what that means.  We disregard the colors
> specified by the SVG image?

No, we do respect colors specified by the SVG image.  That's why we see
the current behavior.

However, if no foreground color is specified in the SVG file, we use the
foreground color of the current face instead.  That's why the fix I
propose work.

> Sorry, at this stage I'd prefer not to install anything on the release
> branch that isn't a bugfix.  Or else take it up with Lars, and ask him
> to forget about releasing Emacs 28.1 soon, because this way it will
> take a while ;-)

I consider this a bug fix, but admittedly only a minor one.  However, I
also note that the visibility of those symbols is worse with a dark
theme in Emacs 28 than in Emacs 27, so we might consider it a
regression.

That said, I of course understand that some kind of line have to be
drawn, and I'm personally fine with installing this only on master.  If
people feel that it is important, perhaps we could consider backporting
the fix to Emacs 28.2 later (read: please speak up if you think so).

bug#51556: 29.0.50; Poor contrast of Customize SVG icons with dark backgrounds

[Eli Zaretskii]

> From: Stefan Kangas <stefan@marxist.se>
> Date: Tue, 2 Nov 2021 10:01:56 -0700
> Cc: jporterbugs@gmail.com, 51556@debbugs.gnu.org, kevin.legouguec@gmail.com
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > I don't think I understand what that means.  We disregard the colors
> > specified by the SVG image?
> 
> No, we do respect colors specified by the SVG image.  That's why we see
> the current behavior.
> 
> However, if no foreground color is specified in the SVG file, we use the
> foreground color of the current face instead.  That's why the fix I
> propose work.

What is the difference between "colors specified by the SVG image"
(which you say we respect), and "foreground color is specified in the
SVG file" (which you want us to disregard)?

> > Sorry, at this stage I'd prefer not to install anything on the release
> > branch that isn't a bugfix.  Or else take it up with Lars, and ask him
> > to forget about releasing Emacs 28.1 soon, because this way it will
> > take a while ;-)
> 
> I consider this a bug fix, but admittedly only a minor one.  However, I
> also note that the visibility of those symbols is worse with a dark
> theme in Emacs 28 than in Emacs 27, so we might consider it a
> regression.

Let's not exaggerate, okay?

bug#51556: 29.0.50; Poor contrast of Customize SVG icons with dark backgrounds

[Stefan Kangas]

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

Eli Zaretskii <eliz@gnu.org> writes:

>> No, we do respect colors specified by the SVG image.  That's why we see
>> the current behavior.
>>
>> However, if no foreground color is specified in the SVG file, we use the
>> foreground color of the current face instead.  That's why the fix I
>> propose work.
>
> What is the difference between "colors specified by the SVG image"
> (which you say we respect), and "foreground color is specified in the
> SVG file" (which you want us to disregard)?

You missed quoting "no" in "no foreground color is specified in the SVG
file".  So the difference is that in one case the SVG files specifies
the foreground color, in the other case it does not.

Also, note that I'm not proposing any change.  I'm only describing the
current behavior.

>> I consider this a bug fix, but admittedly only a minor one.  However, I
>> also note that the visibility of those symbols is worse with a dark
>> theme in Emacs 28 than in Emacs 27, so we might consider it a
>> regression.
>
> Let's not exaggerate, okay?

I fail to see any exaggeration.

I've attached two screenshots of the customize buffer using the
"tsdh-dark" theme, one in Emacs 27 and the other one in Emacs 28.

[-- Attachment #2: emacs27.png --]
[-- Type: image/png, Size: 5913 bytes --]

[-- Attachment #3: emacs28.png --]
[-- Type: image/png, Size: 5885 bytes --]

bug#51556: 29.0.50; Poor contrast of Customize SVG icons with dark backgrounds

[Eli Zaretskii]

> From: Stefan Kangas <stefan@marxist.se>
> Date: Tue, 2 Nov 2021 10:44:23 -0700
> Cc: jporterbugs@gmail.com, 51556@debbugs.gnu.org, kevin.legouguec@gmail.com
> 
> > What is the difference between "colors specified by the SVG image"
> > (which you say we respect), and "foreground color is specified in the
> > SVG file" (which you want us to disregard)?
> 
> You missed quoting "no" in "no foreground color is specified in the SVG
> file".  So the difference is that in one case the SVG files specifies
> the foreground color, in the other case it does not.

I didn't miss anything, no.

If the solution you propose only works for SVG that specify no
foreground, then it won't be able to solve all of the situations where
a different theme makes an icon barely visible or unpleasant to the
eye.  Which is why I think a better solution would be to allow themes
to specify different icons where necessary.

> >> I consider this a bug fix, but admittedly only a minor one.  However, I
> >> also note that the visibility of those symbols is worse with a dark
> >> theme in Emacs 28 than in Emacs 27, so we might consider it a
> >> regression.
> >
> > Let's not exaggerate, okay?
> 
> I fail to see any exaggeration.
> 
> I've attached two screenshots of the customize buffer using the
> "tsdh-dark" theme, one in Emacs 27 and the other one in Emacs 28.

If we want to fix this, we can go back to down.xpm etc. on the release
branch, and discuss how to solve this on master.

bug#51556: 29.0.50; Poor contrast of Customize SVG icons with dark backgrounds

[Stefan Kangas]

Eli Zaretskii <eliz@gnu.org> writes:

> If the solution you propose only works for SVG that specify no
> foreground, then it won't be able to solve all of the situations where
> a different theme makes an icon barely visible or unpleasant to the
> eye.

It is certainly the correct solution for all the sets of scalable icons
that I have reviewed.  Which SVG icons do you have in mind?  Could you
point me to them?

For the icons I know of, the best solution if you need to change the
color of this or that icon, is to either change the active defface to
use the correct color, or to introduce a new defface.  This is, not by
accident, the chosen solution also for icons on the web.

> Which is why I think a better solution would be to allow themes
> to specify different icons where necessary.

Color themes should not *need* to provide their own icons when all they
want is to change the color of an icon.  That puts an unnecessary and
completely avoidable burden on theme developers, for starters because
they would need to learn how to edit SVG files (which is not trivial).
It is also less flexible, as how icons look is orthogonal to what color
they have.

If you propose that this should exist as an option, that is fine by me,
but I don't see any need for it.

I don't think we should impose an inflexible solution when I have
already indicated in several messages that I'm working on a better one.
The solution you propose goes in the completely opposite direction of
that work, so I hope that we won't do that.

Furthermore, the patch I have already posted here is sufficient for the
purposes of this bug report.  I suggest we install it.

bug#51556: 29.0.50; Poor contrast of Customize SVG icons with dark backgrounds

[Jim Porter]

On 11/2/2021 11:01 AM, Eli Zaretskii wrote:
> If the solution you propose only works for SVG that specify no
> foreground, then it won't be able to solve all of the situations where
> a different theme makes an icon barely visible or unpleasant to the
> eye.  Which is why I think a better solution would be to allow themes
> to specify different icons where necessary.

I think it might help to break this problem down into parts. There are a 
few cases I can think of where the question of how to color an SVG might 
come up:

1) Single-color SVGs

Some SVGs are just a single-color shape, like the Customize arrows. 
There's no *particular* color that's right for these, aside from "what 
looks good". In some ways, these SVGs are like an ordinary textual 
glyph, except that we've defined our own fancy shape to use. In this 
case, I think it makes sense to be able to specify what color to use for 
the SVG independently of the shape so that themes can adjust it as-needed.

This case is the one initially described in this bug.

2) Multi-color SVGs

Some SVGs have many colors. There's a particular set of colors that 
makes sense for this SVG, as defined in the file by the author. It would 
be difficult to programmatically alter the colors of such an SVG and 
maintain a good result, so we probably shouldn't try. This is a case 
where it may be nice to let themes specify a completely-different SVG to 
use.

I'd consider this lower priority, since I'm not aware of a specific SVG 
in this category that's causing visual issues with themes.

3) Dynamically-colored SVGs

This could be considered a variant of (1). Sometimes, we might want to 
define a particular shape, but give it different colors in different 
contexts. For example, if we wanted to use SVGs to display something 
similar to the circles in the fringe that mark GDB breakpoints, we could 
have a single breakpoint.svg file that we color bright red or gray 
depending on the breakpoint's state.

This is another case where it would be convenient to be able to set an 
SVG's color independently of its shape.

4) Third-party packages

Some packages might want to provide SVG icons to use in one way or 
another, along the same lines as the Customize arrows (i.e. a 
single-color SVG). If you're the author of a little-known package, you 
can't really expect theme authors to provide customized icons for your 
package that fit with the theme. However, themes *do* specify at least a 
handful of faces. As a package author, it would be convenient to be able 
to say "render this icon using the foreground color of the `error' 
face", for example. Then your package looks good with most themes 
without requiring anyone to provide custom SVGs.

bug#51556: 29.0.50; Poor contrast of Customize SVG icons with dark backgrounds

[Eli Zaretskii]

> From: Stefan Kangas <stefan@marxist.se>
> Date: Tue, 2 Nov 2021 11:43:08 -0700
> Cc: jporterbugs@gmail.com, 51556@debbugs.gnu.org, kevin.legouguec@gmail.com
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > If the solution you propose only works for SVG that specify no
> > foreground, then it won't be able to solve all of the situations where
> > a different theme makes an icon barely visible or unpleasant to the
> > eye.
> 
> It is certainly the correct solution for all the sets of scalable icons
> that I have reviewed.  Which SVG icons do you have in mind?  Could you
> point me to them?

Try splash.svg, as a trivial example (and forget that it's too large
for an icon, this is just an example).

> For the icons I know of, the best solution if you need to change the
> color of this or that icon, is to either change the active defface to
> use the correct color, or to introduce a new defface.  This is, not by
> accident, the chosen solution also for icons on the web.

We have a disconnect here, because I don't follow.  Are you talking
only about SVG that use only the gray color for its lines?

> > Which is why I think a better solution would be to allow themes
> > to specify different icons where necessary.
> 
> Color themes should not *need* to provide their own icons when all they
> want is to change the color of an icon.

I said "should allow", and you say "need".  We are mis-communicating.
And that's in addition to the color business, where we also have some
sort of disconnect.

> Furthermore, the patch I have already posted here is sufficient for the
> purposes of this bug report.  I suggest we install it.

On master?  I don't object.

bug#51556: 29.0.50; Poor contrast of Customize SVG icons with dark backgrounds

[Stefan Kangas]

Jim Porter <jporterbugs@gmail.com> writes:

> I think it might help to break this problem down into parts. There are a
> few cases I can think of where the question of how to color an SVG might
> come up:

Thanks, this summary is useful.  I agree with all your points, but I
have some comments.

> 3) Dynamically-colored SVGs
>
> This could be considered a variant of (1). Sometimes, we might want to
> define a particular shape, but give it different colors in different
> contexts. For example, if we wanted to use SVGs to display something
> similar to the circles in the fringe that mark GDB breakpoints, we could
> have a single breakpoint.svg file that we color bright red or gray
> depending on the breakpoint's state.

I don't think this is fundamentally different from (1).  The shape is
independent from the color in both cases.

> If you're the author of a little-known package, you can't really
> expect theme authors to provide customized icons for your package that
> fit with the theme. However, themes *do* specify at least a handful of
> faces. As a package author, it would be convenient to be able to say
> "render this icon using the foreground color of the `error' face", for
> example. Then your package looks good with most themes without
> requiring anyone to provide custom SVGs.

Exactly the point.  This goes for built-in packages as well.

bug#51556: 29.0.50; Poor contrast of Customize SVG icons with dark backgrounds

[Eli Zaretskii]

> From: Stefan Kangas <stefan@marxist.se>
> Date: Tue, 2 Nov 2021 11:58:17 -0700
> Cc: 51556@debbugs.gnu.org, kevin.legouguec@gmail.com
> 
> > If you're the author of a little-known package, you can't really
> > expect theme authors to provide customized icons for your package that
> > fit with the theme. However, themes *do* specify at least a handful of
> > faces. As a package author, it would be convenient to be able to say
> > "render this icon using the foreground color of the `error' face", for
> > example. Then your package looks good with most themes without
> > requiring anyone to provide custom SVGs.
> 
> Exactly the point.  This goes for built-in packages as well.

How come built-in packages qualify as "little-known"?

For really little-known packages, my suggestion would be to use icons
whose visibility should be good with any theme.  Or they could not use
icons at all.

bug#51556: 29.0.50; Poor contrast of Customize SVG icons with dark backgrounds

[Stefan Kangas]

Eli Zaretskii <eliz@gnu.org> writes:

>> It is certainly the correct solution for all the sets of scalable icons
>> that I have reviewed.  Which SVG icons do you have in mind?  Could you
>> point me to them?
>
> Try splash.svg, as a trivial example (and forget that it's too large
> for an icon, this is just an example).

That's correct, this approach won't work for multi-color SVG's.  But see
Jim Porter's reply.

>> For the icons I know of, the best solution if you need to change the
>> color of this or that icon, is to either change the active defface to
>> use the correct color, or to introduce a new defface.  This is, not by
>> accident, the chosen solution also for icons on the web.
>
> We have a disconnect here, because I don't follow.  Are you talking
> only about SVG that use only the gray color for its lines?

I'm talking about what Jim Porter calls "Single-color SVGs".  This case
is applicable to all sets of free and scalable icons that I have
reviewed.  This includes the most popular and extensive icon sets, such
as fontawesome, octicons and material icons.

In all these cases, the SVG files may or may not come with a foreground
color, but that is an implementation accident: they are mainly intended
for use in a web browser, where the web browser will override the
foreground color with the foreground color specified by CSS.

In Emacs, we don't let faces override a foreground color specified in an
SVG file (that won't work in general, as is clear from the "splash.svg"
example), so if we want this functionality, we must take care to remove
the foreground color in the SVG file.

> I said "should allow", and you say "need".

Aha.  Thanks for clarifying.  Then your position does not seem to be all
that different from mine.  Perhaps it is only a difference in
priorities, or perhaps there will exist no difference in practice.

>> Furthermore, the patch I have already posted here is sufficient for the
>> purposes of this bug report.  I suggest we install it.
>
> On master?  I don't object.

Thank you, installed as commit 11702a6dd7.

bug#51556: 29.0.50; Poor contrast of Customize SVG icons with dark backgrounds

[Stefan Kangas]

Eli Zaretskii <eliz@gnu.org> writes:

>> > If you're the author of a little-known package, you can't really
>> > expect theme authors to provide customized icons for your package that
>> > fit with the theme. However, themes *do* specify at least a handful of
>> > faces. As a package author, it would be convenient to be able to say
>> > "render this icon using the foreground color of the `error' face", for
>> > example. Then your package looks good with most themes without
>> > requiring anyone to provide custom SVGs.
>>
>> Exactly the point.  This goes for built-in packages as well.
>
> How come built-in packages qualify as "little-known"?

They don't.  I'm saying that the above points apply whether or not a
package is "little-known", "well-known" or even "built-in".

bug#51556: 29.0.50; Poor contrast of Customize SVG icons with dark backgrounds

[Alan Third]

On Tue, Nov 02, 2021 at 11:43:58AM -0700, Jim Porter wrote:
> 
> 1) Single-color SVGs
> 
> Some SVGs are just a single-color shape, like the Customize arrows. There's
> no *particular* color that's right for these, aside from "what looks good".
> In some ways, these SVGs are like an ordinary textual glyph, except that
> we've defined our own fancy shape to use. In this case, I think it makes
> sense to be able to specify what color to use for the SVG independently of
> the shape so that themes can adjust it as-needed.
> 
> This case is the one initially described in this bug.

This is the case we looked at when we originally replaced the
customize widgets with SVGs. At the time we very intentionally removed
the colour and size information from the SVG files (checkbox.svg, etc)
so that they would match the colours and font size of the surrounding
text.

I guess, unfortunately, these arrows were missed.

-- 
Alan Third

bug#51556: 29.0.50; Poor contrast of Customize SVG icons with dark backgrounds

[Kévin Le Gouguec]

Alan Third <alan@idiocy.org> writes:

> This is the case we looked at when we originally replaced the
> customize widgets with SVGs. At the time we very intentionally removed
> the colour and size information from the SVG files (checkbox.svg, etc)
> so that they would match the colours and font size of the surrounding
> text.
>
> I guess, unfortunately, these arrows were missed.

Ah, that makes sense; sorry I didn't think of looking at the history of
the other icons, I would probably have made a simpler report focusing on
the solution that Stefan pushed today, instead of babbling about adding
features to the widget code.

FWIW, I think your comment reinforces the idea that Stefan's fix belongs
on the release branch, but I understand that we all want 28.1 out the
door already, so 🤷

Thank you all for your help.

bug#51556: 29.0.50; Poor contrast of Customize SVG icons with dark backgrounds

[Eli Zaretskii]

> Date: Tue, 2 Nov 2021 20:19:52 +0000
> From: Alan Third <alan@idiocy.org>
> Cc: Eli Zaretskii <eliz@gnu.org>, Stefan Kangas <stefan@marxist.se>,
> 	51556@debbugs.gnu.org, kevin.legouguec@gmail.com
> 
> On Tue, Nov 02, 2021 at 11:43:58AM -0700, Jim Porter wrote:
> > 
> > 1) Single-color SVGs
> > 
> > Some SVGs are just a single-color shape, like the Customize arrows. There's
> > no *particular* color that's right for these, aside from "what looks good".
> > In some ways, these SVGs are like an ordinary textual glyph, except that
> > we've defined our own fancy shape to use. In this case, I think it makes
> > sense to be able to specify what color to use for the SVG independently of
> > the shape so that themes can adjust it as-needed.
> > 
> > This case is the one initially described in this bug.
> 
> This is the case we looked at when we originally replaced the
> customize widgets with SVGs. At the time we very intentionally removed
> the colour and size information from the SVG files (checkbox.svg, etc)
> so that they would match the colours and font size of the surrounding
> text.
> 
> I guess, unfortunately, these arrows were missed.

Thanks.  Since these SVG images were missed back then, I've changed my
mind about the safety of the change, and cherry-picked the fix to the
emacs-28 branch.