`enable-theme` in tty client messes up gui clients

45mg commented 8 months ago

I confirm that...

[X] I have searched the issue tracker, documentation, FAQ, Discourse, and Google, in case this issue has already been reported/resolved.
[X] I have read "How to Debug Issues", and will use it to provide as much information about this issue as possible.
[X] The issue can be reproduced on the latest available commit of Doom.
[X] The issue can be reproduced on a stable release of Emacs, such as 27, 28, or 29. (Unstable versions end in .50, .60, or .9x)

Expected behavior

When I have both graphical and terminal emacsclient processes open, and I run enable-theme in a terminal emacsclient, the theme should be loaded properly in all open graphical emacsclients. (I have verified that this is the case with a daemon launched as emacs -Q.)

Current behavior

When I run enable-theme in a terminal emacsclient, this display of the graphical emacsclient suddenly 'shrinks'. This is much easier to show with screenshots -

Before running enable-theme (left - a graphical emacsclient frame; right - emacsclient -nw): 20240313_17h32m49s_grim

After running enable-theme: 20240313_17h32m21s_grim

Steps to reproduce

Start emacs daemon with emacs --daemon=xyz
Start graphical emacsclient with emacsclient -s xyz --create-frame
Start terminal emacsclient with emacsclient -s xyz -nw
Run M-: (enable-theme doom-theme) in the terminal emacsclient
Observe the results depicted in the screenshots above

Note that this also happens with load-theme.

The issue happens for non-doom themes as well - replace Step 4 with M-x load-theme tango-dark and you'll see the same result.

Background

I encountered this bug because I was running (enable-theme doom-theme) on server-after-make-frame-hook, as a workaround for this solaire-mode issue.

System Information

https://0x0.st/HhCb.txt

hlissner commented 8 months ago

This is not a Doom bug, this is an issue with Emacs. If you load a theme, it loads the theme as if all frames were identical to the one you're loading it from. So, load a theme in GUI Emacs, and your terminal frames will try to use 16-bit colors (among other assumptions that can affect fonts/DPI/etc), which it will fail to do. The opposite has its own unpredictable quirks.

In fact, supporting users who want to have both terminal and GUI Emacs open at the same time is the 9th circle of hell for this reason, because themes (and faces) cannot be frame-local without a lot of arcane hackery.

In any case, I'll close this since this isn't anything Doom can do anything about it, but feel free to follow up here.

EDIT: I should mention, that the exact quirks that manifest from trying to loading themes across GUI/terminal Emacs vary wildly depending on the theme(s) you try, as well, making it that much harder to support downstream.

hlissner commented 8 months ago

Oh, hold on, this might be a matter of :font on the default face being incorrectly overwritten (on enable-theme). I'll reopen this while I investigate.

45mg commented 8 months ago

It doesn't seem to be an Emacs issue. I just tested the steps I wrote above again, replacing step 1 with emacs -Q --daemon=xyz, and running M-x load-theme tango-dark in step 4. The issue did not occur. So, loading the same theme in the same manner, but the issue only happens when Doom's config is loaded.

hlissner commented 8 months ago

See if this fixes the issue:

;;; in $DOOMDIR/config.el
(defadvice! fix-doom-init-fonts-h (fn &rest args)
  :around #'doom-init-fonts-h
  (if (daemonp)
      (set-frame-font doom-font t t t)
    (apply fn args)))

45mg commented 8 months ago

Yep - that does it!

hlissner commented 8 months ago

Could you try this snippet? (Replacing the snippet above)

;;; in $DOOMDIR/config.el
(defadvice! fix-doom-init-fonts-h (&optional reload)
  :override #'doom-init-fonts-h
  (let ((daemonp (daemonp)))
    (dolist (map `((default . ,doom-font)
                   (fixed-pitch . ,doom-font)
                   (fixed-pitch-serif . ,doom-serif-font)
                   (variable-pitch . ,doom-variable-pitch-font)))
      (condition-case e
          (when-let* ((face (car map))
                      (font (cdr map)))
            (dolist (frame (frame-list))
              (when (display-multi-font-p frame)
                (let (width height)
                  (when daemonp
                    (setq height (* (frame-parameter frame 'height)
                                    (frame-char-height frame))
                          width (* (frame-parameter frame 'width)
                                   (frame-char-width frame))))
                  (set-face-attribute face frame
                                      :width 'normal :weight 'normal
                                      :slant 'normal :font font)
                  (when daemonp
                    (modify-frame-parameters
                     frame `((height . ,(round height (frame-char-height frame)))
                             (width  . ,(round width  (frame-char-width frame)))))))))
            (let ((new-specs (doom--make-font-specs face font)))
              (custom-push-theme 'theme-face face 'user 'set new-specs)
              (put face 'face-modified nil)))
        ('error
         (ignore-errors (doom--reset-inhibited-vars-h))
         (if (string-prefix-p "Font not available" (error-message-string e))
             (signal 'doom-font-error (list (font-get (cdr map) :family)))
           (signal (car e) (cdr e))))))
    (when (fboundp 'set-fontset-font)
      (let* ((fn (doom-rpartial #'member (font-family-list)))
             (symbol-font (or doom-symbol-font
                              (cl-find-if fn doom-symbol-fallback-font-families)))
             (emoji-font (or doom-emoji-font
                             (cl-find-if fn doom-emoji-fallback-font-families))))
        (when symbol-font
          (dolist (script '(symbol mathematical))
            (set-fontset-font t script symbol-font)))
        (when emoji-font
          (when (version<= "28.1" emacs-version)
            (set-fontset-font t 'emoji emoji-font))
          (set-fontset-font t 'symbol emoji-font nil 'append)))
      (dolist (range '((#xe000 . #xf8ff) (#xf0000 . #xfffff)))
        (set-fontset-font t range "Symbols Nerd Font Mono")))
    (run-hooks 'after-setting-font-hook)))

45mg commented 8 months ago

No, that one doesn't fix the issue for me.

By the way, are you able to reproduce the issue on your end, or is it just me?

hlissner commented 8 months ago

Finally zeroed in on the issue. It should be fixed as of c20c2aa.

By the way, are you able to reproduce the issue on your end, or is it just me?

Yeah, I did finally manage to reproduce it.

45mg commented 8 months ago

While https://github.com/doomemacs/doomemacs/commit/c20c2aa36ed0be93beca3fc63eea2724f57af6a7 does indeed solve the issue on a fresh install of Doom, it doesn't work for my config - there must be something in my config messing things up. Which is a me problem, but... would you have any suggestions as to what kind of misconfiguration could cause this issue?

In the meantime, for anyone else in the same boat as me, that first snippet seems to work as a workaround.

hlissner commented 8 months ago

What have you set doom-font to? (and any of the other doom-*-font variables, if any?)

45mg commented 8 months ago

I have (setq doom-font (font-spec :family "Iosevka Nerd Font" :size 18)) in my config. None of those other variables are set.

45mg commented 8 months ago

Ok, I've found the lines in my config that cause the issue -

(setq doom-theme 'doom-moonlight) is sufficient to trigger the issue. ((setq doom-theme 'doom-one) is fine though.)
(setq persp-emacsclient-init-frame-behaviour-override t) is sufficient to trigger the issue.

Either of those lines in isolation is enough to trigger the issue. When I comment out both those lines, I don't see the issue.

@hlissner, may I ask that this issue be reopened, since we clearly haven't found the root cause yet?

doomemacs / doomemacs