Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix font fallbacks #7448

Merged
merged 8 commits into from
Oct 7, 2023
Merged

Fix font fallbacks #7448

merged 8 commits into from
Oct 7, 2023

Conversation

hpfr
Copy link
Contributor

@hpfr hpfr commented Sep 21, 2023

See commit messages for details. Please comment if you can confirm this is an improvement, as font configuration varies from user to user and is somewhat dependent on environment and operating system. I’m fairly confident it’s good, though. To test, please remove any workarounds you may have made from your configs. Checking out my branch in your Doom installation and starting a new Emacs instance should be all you need to test. You can leave your current Emacs instance running if you have a bunch of stuff you’re working on. Just make sure to return to master when you’re done testing.


  • I searched the issue tracker and this hasn't been PRed before.
  • My changes are not on the do-not-PR list for this project.
  • My commits conform to the git conventions.
  • My changes are visual; I've included before and after screenshots.
  • n/a Any relevant issues or PRs have been linked to.

Copy link
Member

@hlissner hlissner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, but would you mind removing the (core) scope from the git messages (scope should be omitted for changes to Doom's core).

@hpfr
Copy link
Contributor Author

hpfr commented Sep 21, 2023

Ah yeah, forgot about that, done. I added a bunch of issues this closes and figured we could give people some time to test and confirm, but I suppose it is easier to just merge, let them test by upgrading Doom, and reopen issues if necessary.

@hpfr
Copy link
Contributor Author

hpfr commented Sep 21, 2023

Whoops, rebased

@fosskers
Copy link
Contributor

This fixes the issue for me.

@hpfr
Copy link
Contributor Author

hpfr commented Sep 21, 2023 via email

@George9000
Copy link

Please see my reply in #3296 This did not fix my reported issue and broke the workaround.

@fosskers
Copy link
Contributor

@hpfr Who is "you" in this case?

@vancleve
Copy link

vancleve commented Sep 22, 2023

Hi @hpfr. I've tried your PR and it improves things somewhat but not completely.

I'll show below screenshots for whether I set the doom-unicode-font and whether I use my hack from #7036 (comment) for both doom master and for this PR. The upshot is that this PR does seem to be fixing some of the emoji issues but as you can see there are still a few emoji characters that don't display despite being present in the font Apple Color Emoji. These same emoji are displayed for some reason when I both set the doom-unicode-font and when I use my hack. For doom master, I bizarrely only get these emoji to display properly with my hack but without setting doom-unicode-font. When I set doom-unicode-font to JuliaMono in that case, some of the emoji are coming from JuliaMono instead of from Apple Color Emoji.

The PR does seem to fix the file icons to Symbols Nerd even when the doom-font has the same glyphs, which is nice.

I do have trouble setting the greek and calligraphic characters with the PR. With master, setting doom-unicode-font to JuliaMono results in all JuliaMono calligraphic characters but under this PR only some of the characters are JuliaMono and the rest are Apple Symbol.

Master:
image

This PR:
image

Doom info:
image

@hpfr
Copy link
Contributor Author

hpfr commented Sep 23, 2023

@hpfr Who is "you" in this case?

@fosskers Did you see my edit? You must not use email notifications, but someone else sent several screenshots before deleting all their messages. I replied before they deleted them.

under this PR only some of the characters are JuliaMono and the rest are Apple Symbol

@vancleve I think that’s good news! Doom defaults doom-symbol-fallback-font-families to include Apple Symbols. The current implementation has the first found font in that list override doom-unicode-font. You can take a few approaches: include JuliaMono before Apple Symbols, unset the variable entirely, or (set-fontset-font t 'symbol "JuliaMono") in your config. Let me know if that takes care of it!

@hpfr
Copy link
Contributor Author

hpfr commented Sep 23, 2023

I tried a fresh install by cloning your fork with the latest commit. This did not fix my issue. In fact, it seemed to cause a regression as neither just the setq doom-unicode-font nor adding the workaround posted above successfully changed the unicode font to the heavier weight, larger size. I kept getting a spindly, small font size. To get the larger effect, as in the picture below, I had to go back to the doom master branch and use the workaround above.

@George9000 I don’t have enough to work with here. I don’t know your operating system or what your full configuration (doom-font?) looks like.

Time to bring in heavier artillery. It took a while and resulted in #7457, but I finally got a sandbox working with Doom’s mostly-baked profiles feature1. Sorry for the complicated procedure, but I need to have some confidence that issues aren’t introduced by user configuration.

If you run into issues along the way, you can stop and post them here. I’m not sure how robust this is yet.

  1. Create profiles.el in your Doom config directory (most likely ~/.config/doom/ or ~/.doom.d) with the following contents:
;;; profiles.el -*- mode: lisp-data; lexical-binding: t; -*-

((sandbox
  ;; worktree for Doom default branch
  (user-emacs-directory . "~/.config/emacs-doom-upstream/")
  (doom-user-dir . "~/.config/doom-sandbox/")))
  1. Clone Doom to ~/.config/emacs-doom-upstream/ and checkout my branch. You can also use a Git worktree if you are confortable with them.

  2. Create init.el in ~/.config/doom-sandbox/ with appropriate contents. You can enable more modules, but there’s no need. All of Doom’s font setup is in its core.

;;; init.el --- Doom sandbox -*- lexical-binding: t; -*-
(doom! :completion vertico
       ;; :editor evil
       :config (default +bindings))
  1. Create config.el in ~/.config/doom-sandbox/ with appropriate contents for testing:
;;; config.el --- Doom sandbox -*- lexical-binding: t; -*-
(load-theme 'modus-operandi t)
(setq doom-font (font-spec :family "Fira Mono" :size 12)
      doom-variable-pitch-font (font-spec :family "Fira Sans"))
  1. Run doom profiles sync and DOOMPROFILE=sandbox doom sync.

  2. Run emacs --profile sandbox. You should end up in a minimal Doom environment.

  3. Time to debug. Insert whatever characters you want to check. view-hello-file, emoji-search (Emacs 29), and list-charset-chars may be useful. Note that tofu is expected with no configuration for many scripts. You can mess around with set-fontset-font as you like and reset everything to your defined doom-*-font setup any time with doom/reload-font. If you get really lost, just quit and restart the sandbox.

  4. If you notice issues, run describe-char with the cursor at the character. In addition, run eval-expression with (describe-fontset nil)2. Copy whatever you think is relevant (or everything) somewhere so you can upload it when you’re done.

  5. Run doom/info and pastebin the results somewhere (or use GitHub file attachments) along with relevant output from the previous step. Also include your sandbox settings from config.el, of course.

  6. Post here with your findings, including a link to the pastebin and screenshots if applicable.

  7. ???

  8. Profit!

Footnotes

  1. Henrik, what’s it gonna take to get you to declare a treewide “Non Bug Fix PR Review on Hold” after the Corfu PR so we can see Doom v3 this year?

  2. This is so I don’t have to guess whether people made sure to select the default fontset with completing-read

@hpfr
Copy link
Contributor Author

hpfr commented Sep 23, 2023

@George9000 to start, I would just make sure your ELEMENT OF character is actually coming from JuliaMono with describe-char. I have experimented in my sandbox with a configuration like so:

(load-theme 'modus-operandi t)
(setq doom-font (font-spec :family "Noto Sans Mono" :size 16)
      doom-variable-pitch-font "Source Sans 3:"
      doom-unicode-font (font-spec :family "JuliaMono" :size 20 :weight 'medium))

I can change the size or weight of JuliaMono, eval-defun, doom/reload-font, and see the size or weight of JuliaMono change live. The weights are subtle, though.

@George9000
Copy link

@hpfr Results:

First, I cloned the latest doom commit to ~/.config/emacs
Then I ran the config I use but stripped down, which can be seen here. Doom info can be expanded below. File used in screenshots below is here

doom/info
generated  Sep 24, 2023 17:04:24
system     MacOS 12.7 Darwin 21.6.0 arm64 mac
emacs      29.1 HEAD e5f196da5 ~/.config/emacs/
doom       3.0.0-pre PROFILE=_@0 grafted, HEAD -> master, origin/master, origin/HEAD
           844a82c 2023-09-22 23:18:31 +0200 ~/.config/doom/
shell      /bin/zsh
features   ACL GLIB GMP GNUTLS IMAGEMAGICK JSON LCMS2 LIBXML2 MODULES NATIVE_COMP
           NOTIFY KQUEUE PDUMPER RSVG SQLITE3 THREADS TOOLKIT_SCROLL_BARS TREE_SITTER
           WEBP XIM ZLIB
traits     gui server-running envvar-file
modules    :config use-package :completion company vertico :ui doom doom-dashboard
           hl-todo modeline ophints (popup +defaults) (vc-gutter +pretty)
           vi-tilde-fringe workspaces :editor (evil +everywhere) file-templates fold
           snippets :emacs dired electric undo vc :checkers syntax :tools (eval
           +overlay) lookup lsp magit tree-sitter :os macos :lang emacs-lisp (julia
           +tree-sitter +lsp) markdown org sh :config (default +bindings +smartparens)
packages   (doct) (svg-tag-mode) (treesit-auto) (julia-ts-mode :recipe (:host github
           :repo ronisbr/julia-ts-mode)) (doom-nano-modeline :recipe (:host github
           :repo ronisbr/doom-nano-modeline)) (julia-mode :pin
           a20367f2826b26084abd2f403219218813007123)
unpin      julia-repl

Doom master with my config

doom_master_unicode_math_glyphs_my_config

Then I followed your instructions to set up a profiles.el in ~/.config/doom, cloned your doom fork/branch to ~/.config/emacs-doom-upstream/ and created an init.el and config.el in ~/.config/doom-sandbox/. Following your steps 5 and 6, here is the result.

hpfr fork with suggested config

hpfr_branch_unicode_math_glyphs_firamono_16

I then tried modifying your config with my fonts like this:

;;; config.el --- Doom sandbox -*- lexical-binding: t; -*-
(load-theme 'modus-operandi t)
;(setq doom-font (font-spec :family "Fira Mono" :size 16)
;      doom-variable-pitch-font (font-spec :family "Fira Sans"))

(setq doom-font (font-spec :family "TheSansMono" :size 14 :weight 'medium))
(setq doom-unicode-font (font-spec :family "JuliaMono" :size 16 :weight 'medium))
hpfr_branch_unicode_math_glyphs_sansmono_juliamono

Last I tried appending my workaround after the lines above:

(when (display-graphic-p)
(dolist (charset '(unicode))
  (set-fontset-font (frame-parameter nil 'font)
    charset (font-spec :family "JuliaMono" :size 16 :weight 'medium))))

This had no effect. The most problematic glyphs are those in the first two lines: the "element of" symbol and the "ring operator" symbol used for function composition in Julia. The rendering is quite spindly or tiny using the hpfr branch and more robust on the current doom master branch. describe-char output here

describe-fontset output for my initial config and for the hpfr config.

In conclusion, I prefer the current doom master rendering over the proposed hpfr branch merge.

@hpfr
Copy link
Contributor Author

hpfr commented Sep 25, 2023

Please don’t use <details> for pasting your Doom info, it still pollutes the issue search so this issue is returned when people search for any module or package you had installed. Attach or pastebin a text file.

Thanks for taking the time to debug. Based on your describe-fontset output, you’re having the same issue as @vancleve. I thought some more about the current setup and decided to just rename doom-unicode-font. A fallback font for all of Unicode is not very realistic. In fact, it is impossible for a single font to support all of Unicode. One OpenType font can only hold 2¹⁶-1 glyphs. And evidently, as @vancleve and @George9000 and @tecosaur demonstrate, everyone is using doom-unicode-font for symbols from Julia Mono 😆. So I’ve renamed it to doom-symbol-font accordingly.

I added a similar variable, doom-emoji-font, for emoji. This enabled making the fallback families variables constants. Those are now internal variables for Doom to provide platform specific fallbacks when the corresponding user-facing variables are unset. I think that was always the intent, as they aren’t documented anywhere.

Please try the latest push. All you have to do is change doom-unicode-font to doom-symbol-font and remove any modifications to doom-symbol-fallback-font-families and doom-emoji-fallback-font-families.

@hpfr
Copy link
Contributor Author

hpfr commented Sep 25, 2023

Emacs Lisp doesn’t enforce constants, so I’ve dropped the breaking change verbiage for that commit. Don’t take that as license to set them! 😄

@George9000
Copy link

I've used the latest commit from your fork/branch. Then set fonts as directed:

config_fonts

Result:

example_jl

Note the continued problem with "element of" and "ring operator" symbols. The ring operator / compose symbol is particularly bad.

epsilon_compose_symbol

Even other symbols in that file seem to have better / bolder rendering in doom master as seen in the prior reply. Again, prefer doom master as it currently stands. Yes, this requires a couple lines of workaround but the result is pleasing.

@hpfr
Copy link
Contributor Author

hpfr commented Sep 26, 2023

Even other symbols in that file seem to have better / bolder rendering in doom master as seen in the prior reply. Again, prefer doom master as it currently stands. Yes, this requires a couple lines of workaround but the result is pleasing.

I get the impression you think I don’t care that this PR doesn’t work for you and have the power to merge it regardless. This is not the case. I obviously want these fixes to work for everyone and am working to ensure that they do. There are issues with master’s current implementation; I wouldn’t have needed to write the PR if there weren’t. A few people besides myself have confirmed it fixes their issues as well. Thanks for being patient as I try to eliminate quirks for platforms I don’t have access to.

Please make sure you are on latest and have disabled your workarounds. Attach or gist your (describe-fontset nil) output. Let me know which characters you are having issues with and what font is being used by using describe-char. I suspect it is Apple Symbols.

I made a profile with your config. I tweaked some minor things to make it reproducible and published the fork at https://github.com/hpfr/george9000-doom/tree/repro. I’m not on macOS, but I mocked the Apple Symbols scenario by adding Noto Sans Symbols 2 to doom-symbol-fallback-font-families. Here is what I see:

image

JuliaMono is taking precedence for me.

@vancleve if you could get back to us with whether the latest push fixes your Apple Symbols issues, that would be greatly appreciated. That would give me some confidence that the issue is specific to @George9000 in some way.

@George9000
Copy link

Mea culpa. I was using git clone --depth 1 with your repo and not getting the fix-font-fallbacks branch. Corrected that just now.

Issues resolved. Appreciate your considerable efforts to address problems raised.

describe-fontset output

describe-char output for three characters

example

closeup

@hpfr
Copy link
Contributor Author

hpfr commented Sep 27, 2023

Glad to hear it. Thanks for sticking with me.

I think this PR is actually ready now. A report from a Windows user would be ideal, but I think Windows should be isomorphic to macOS from the perspective of this PR (both have a fallback symbol font).

Edit: one remaining question I have is whether the variable rename should actually be marked breaking. I think the alias ensures no one’s configs will break.

@vancleve
Copy link

Hi @hpfr! Sorry, busy at work, but coming back to emacs now :-).

Thanks for all this hard work!

I followed your instructions to setup the sandbox using the most recent version of the PR.

Things are looking very good though there is one issue and one question with a few of the emoji I've just randomly put in this test file

math_characters.txt
image

  1. 🕳🗯 characters are not displayed despite being present in Apple Color Emoji
image
  1. 🗨 character is represented by JuliaMono instead of Apple Color Emoji. I assume this is just going to happen when the symbol font specifies characters also specified by the emoji font?

describe-char: 🕳🗨🗯
(describe-fontset nil)
init.el
config.el
doom/info

@hpfr hpfr mentioned this pull request Sep 27, 2023
4 tasks
Nerd Fonts assign icons to code points in these Unicode Private Use
Areas. `doom-unicode-font` is now available again as a user-defined
fallback of last resort for non-PUA Unicode code points.

Ref: https://github.com/ryanoasis/nerd-fonts/wiki/Glyph-Sets-and-Code-Points/f12c615e4d2a411d5e9b3cc57533cf4866e6e3f2#overview
d4dec35 removed Apple Color Emoji, and it wasn’t clear how Symbola
was set if this variable was nil.
No font supports all of Unicode or anywhere near it. It’s not even
really possible with current font formats. Therefore, rename
`doom-unicode-font` to `doom-symbol-font`. Only set it as a fallback for
characters in the `symbol` and `mathematical` scripts.
This parallels doom-symbol-font for emoji. In the process, refactor
symbol and emoji font setup to take advantage of these parallels.
Symbols fonts vary widely in their coverage of Unicode symbols blocks.
Emoji fonts are generally guaranteed to cover the small subset of
symbols code points that have assigned emoji representations, so fall
back to them when symbols fonts are lacking.

Ref: https://en.wikipedia.org/w/index.php?title=Variant_form_(Unicode)&oldid=1175107681#Blocks_with_standardized_variation_sequences
These are undocumented internal variables for basic platform-specific
fallbacks. Now that doom-symbol-font and doom-emoji-font exist, make
them constant.
@hpfr
Copy link
Contributor Author

hpfr commented Sep 28, 2023

Sorry, busy at work

No worries. Life takes priority. Thanks for taking another look.

🕳🗯 characters are not displayed despite being present in Apple Color Emoji

Thanks, I can reproduce this. U+1F573 HOLE and U+1F5EF RIGHT ANGER BUBBLE are part of the symbol script in Emacs, and if your symbol font lacks support for these characters, they will show up as tofu like this. I have pushed new commits setting up the emoji font as a fallback for the symbol script now. Let me know how things look on your end. I was also able to tidy up the code a bit.

🗨 character is represented by JuliaMono instead of Apple Color Emoji. I assume this is just going to happen when the symbol font specifies characters also specified by the emoji font?

Yes, this is one of the complexities of Unicode. Certain code points, like U+1F5E8 LEFT SPEECH BUBBLE, existed before emoji as symbols, but also have designated emoji representations. In fact, the previous two characters that were showing up as tofu are also part of this class; it just so happens that Julia Mono only covers 🗨 of the three. Appending U+FE0{E,F} VARIATION SELECTOR 1{5,6} to the symbol code point indicates to the text renderer to prefer a {text,emoji} font. Without a selector, Unicode defines a default presentation, which is text in the case of 🗨. Many applications ignore the default, though. Try appending VARIATION SELECTOR 16 (use insert-char) to 🗨. On my system, this changes the font from Julia Mono to Noto Color Emoji. If you backspace, the variation selector will be deleted and the font will switch back (many text composers ensure a backspace deletes the entire arbitrarily long composition in one go, but not Emacs).

If you want the default representation without a variation selector to prefer emoji, (set-fontset-font t 'symbol doom-emoji-font nil 'prepend) should work (after setting doom-emoji-font of course). I am not sure this should be included in Doom, though. It’s kind of a matter of personal preference. For example, this could make certain arrows show up as emoji, which wouldn’t be desirable for programming ligatures. That forces you to configure preferences for individual character ranges, and I don’t think doom-init-fonts-h should get too into the weeds with that. This is a hard problem to solve generally, which is ultimately why the variation selectors exist.

Here is some reading material for anyone interested:

Variation selectors are the tip of the iceberg. For more fun, try insert-char family and follow that up with emoji-search family. The latter requires Emacs 29 and will prompt you for all the different family character sequences defined in Unicode that compose to a single glyph representation. describe-char or backspace on one of those (or that “eye in speech bubble“ in your test file) to see some more emoji complexity.

@vancleve
Copy link

Wow, very informative @hpfr! Thanks again and looking forward to seeing this in doom master!

@hlissner hlissner merged commit 9863985 into doomemacs:master Oct 7, 2023
@hlissner
Copy link
Member

hlissner commented Oct 7, 2023

Great work! Thanks again for your help!

@hpfr
Copy link
Contributor Author

hpfr commented Oct 10, 2023

The emoji script was added in 28 so this is breaking Doom for 27 users. See #7495, if any 27 users can test that would be appreciated

hlissner pushed a commit that referenced this pull request Nov 23, 2023
317cea5 assumed Emacs 28, but Doom still tries to support 27.
Attempting to use the undefined emoji script on 27 results in an error.
Prior to 28, emoji are part of the symbol script, which the following
`set-fontset-font` call already handles.

Amend: 317cea5
Ref: #7448
Fix: #7505
@hpfr hpfr mentioned this pull request Jan 30, 2024
24 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment