protesilaos / modus-themes

Highly accessible themes for GNU Emacs, conforming with the highest standard for colour contrast between background and foreground values (WCAG AAA).
https://protesilaos.com/emacs/modus-themes
GNU General Public License v3.0
553 stars 30 forks source link

[Windows] odd background color of icons in company candidates menu #61

Open grolongo opened 1 year ago

grolongo commented 1 year ago

Hi Prot!
Since I updated modus-theme from v3 to v4, there is an odd background being applied on icons in the company candidates popup.
To be more clear here are two screenshots using modus-operandi and the new modus-operandi-tinted (which I love btw):

modus-operandi v4 operandi-v4

modus-operandi-tinted v4 operandi-inted-v4

Curiously, I'm using the same init.el across my machines and the problem only appears on Windows (using Emacs 28.2), no problem on macOS or Linux. I don't think you have a Windows machine to test things out so feel free to ask me whatever info you need.

Have a good evening!

protesilaos commented 1 year ago

Hello @grolongo! Indeed, I do not have a Windows machine, but I will try to help you.

Do you know which package produces those icons? Are they "icon files" or "font files" (like FontAwesome)? I am asking because this may not be a theme issue but something related to font rendering.

Is this problem limited to the Modus themes?

grolongo commented 1 year ago

Thanks for the reply,

Is this problem limited to the Modus themes?

So I was indeed mistaken since I can reproduce the error using other built-in themes, so modus-themes is not the problem.
I observe the same behavior using the default theme, leuven and adwaita. It is looking fine though when using wombat, tango-dark or wheatgrass... this is strange.

Do you know which package produces those icons? Are they "icon files" or "font files" (like FontAwesome)?

These are transparent svg icon files automatically pulled by Company, you can find them there: https://github.com/company-mode/company-mode/tree/master/icons

Feel free to close the issue since it looks like it is out of scope of modus-themes unless there is something I can tweak? I'll keep investigate.

protesilaos commented 1 year ago

Let's keep this open, as other users may also encounter the problem. If you find a solution, please post it here. We can close the issue afterwards.

grolongo commented 1 year ago

Hi again Prot!

So I know this issue affects other built-in themes and is not related solely to modus-themes, but I did some testing and maybe you can help me pinpoint the problem further.

Using Emacs 28.2 from the official website, this is the bare bone init.el I'm using (no early-init.el involved):

;; package.el and MELPA
(require 'package)
(add-to-list 'package-archives '("melpa" . "https://melpa.org/packages/") t)

;; Make sure `use-package’ is available.
(unless (package-installed-p 'use-package)
  (package-refresh-contents)
  (package-install 'use-package))

;; Configure `use-package' prior to loading it.
(eval-and-compile
  (setq use-package-always-demand t))

(eval-when-compile
  (require 'use-package))

(use-package company
  :ensure t
  :config (global-company-mode 1))

(use-package modus-themes
  :pin gnu
  :ensure t)

By using :pin gnu I'm getting modus-themes from Elpa (v3.0), and the correct behavior is shown:

modus-v3

Now if I remove :pin gnu I'm getting the latest modus-themes from Melpa (v4.0+), it produces the background issue:

modus-v4

Do you have an idea of what might have changed?

protesilaos commented 1 year ago

How do you enable the icons? Enabling company-mode does not do it for me. I will try to find what is happening.

protesilaos commented 1 year ago

By the way, the background of the popup should be lighter. I corrected it just now.

grolongo commented 1 year ago

How do you enable the icons? Enabling company-mode does not do it for me. I will try to find what is happening.

They are enabled by default for me, by just using the config above.
If that doesn't work for you you can tweak the company-format-margin-function variable.

By the way, the background of the popup should be lighter. I corrected it just now.

I will be testing asap and report back.

protesilaos commented 1 year ago

They are enabled by default for me, by just using the config above.

I switched to the MELPA package and got it to work. Thanks!

I checked Company's code but could not find anything that is obviously responsible for this discrepancy. On Arch Linux I get the expected results:

2023-01-12_12:22:30_1273x1054

2023-01-12_12:22:20_1273x1054

I checked again the Modus 3 faces for Company. There are two main differences:

  1. I removed some obsolete faces.
  2. I removed support for company-posframe.

The other differences are minor tweaks to the applicable colours. This is what I see in Ediff:

2023-01-12_12:28:49_2199x1054

grolongo commented 1 year ago

On Arch Linux I get the expected results

This is what is driving me nuts haha. I have normal behavior on my Linux and macOS machine but only on Windows it is doing this even though it's the same config. This is so confusing.

I changed the line manually bg-inactive to bg-dim as in your latest commit 5ce2bd34 but no luck:

modus-patched

At this point it might be a deeper bug into libsvg on Windows and I might report it as @dgutov suggested.

protesilaos commented 1 year ago

At this point it might be a deeper bug into libsvg on Windows

It could be. It still is confusing why Modus 3 works but 4 doesn't. It must mean that a change to another face is impacting this. But which one?

Using the inspector of Firefox, I get that the background colour of those icons is #ffdec0. I cannot find this in the Modus' repo. Same for the #cfd5df that appears in your "tinted" screenshot.

grolongo commented 1 year ago

Now I'm not sure if the jpeg compression of the screenshot alters it or if a bug is replacing those values.

protesilaos commented 1 year ago

Yes, that's the other problem we have. I searched the M-x list-faces-display in hope of finding something but could not narrow it down.

dgutov commented 1 year ago

Now I'm not sure if the jpeg compression of the screenshot alters it or if a bug is replacing those values

You could save the screenshot as png.

dgutov commented 1 year ago

The behavior on Windows differing from Linux and Mac makes it almost certainly a Windows bug.

It would be nice to find a quick workaround, but we hadn't managed so far. The rest of the investigation should probably consist of gathering better data for the bug report. But we probably have enough of it already -- even the quick snippet to reproduce the problem.

grolongo commented 1 year ago

You could save the screenshot as png.

My bad, the screenshot is already in png, not jpg.

The rest of the investigation should probably consist of gathering better data for the bug report.

Before going any further, it would be "nice" to find at least one other person using Emacs on Windows reproducing the same bug with the minimal init.el, to make sure it's not just my system that is doing this for some reason.

I haven't seen anybody else reporting a similar issue or jumping into this bug report. Maybe that's too trivial for them to even care which is fair, or maybe I'm just the only one.

dgutov commented 1 year ago

The fact of the matter is, not everybody who encounters a problem, reports it. Alas.