pop-os / cosmic-term

WIP COSMIC terminal emulator
GNU General Public License v3.0
343 stars 53 forks source link

Broken rendering for braile fonts #66

Open snaggen opened 7 months ago

snaggen commented 7 months ago

When I tested the system monitor bottom (https://clementtsang.github.io/bottom/nightly/troubleshooting/) it doesn't render the graphs. On the troubleshooting page for bottom it says:

It's possible that your graphs won't look great out of the box due to the reliance on braille fonts to draw them. One example of this is seeing a bunch of missing font characters, caused when the terminal isn't configured properly to render braille fonts.

https://clementtsang.github.io/bottom/nightly/troubleshooting/

snaggen commented 7 months ago

Tested with gnome-terminal and wezterm and both renders it correct. I also installed a braile font, as per the troubleshooting page, https://yudit.org/download/fonts/UBraille/ , but still broken rendering.

snaggen commented 7 months ago

Installing Iosevka font fixed the issue. My guess is that other terminals are less strict about the fallback fonts needing to be mono space or something like that. Is it desirable to fallback to non-monospace fonts if no monospace font have the glyph? Not sure this really is a bug, but if other terminals display something and cosmic-term doesn't it may be considered an issue.

MoSal commented 7 months ago

Installing Iosevka font fixed the issue.

Is the issue fixed if the font is not selected in config?

Also, can you try with this patch applied to cosmic-text

 src/font/fallback/mod.rs | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/src/font/fallback/mod.rs b/src/font/fallback/mod.rs
index 001243cfe..300a010d3 100644
--- a/src/font/fallback/mod.rs
+++ b/src/font/fallback/mod.rs
@@ -62,6 +62,10 @@ impl<'a> FontFallbackIter<'a> {
         default_families: &'a [&'a Family<'a>],
         scripts: &'a [Script],
     ) -> Self {
+        if !scripts.is_empty() {
+            dbg!(scripts)
+        }
+
         Self {
             font_system,
             font_match_keys,

And this for cosmic-term (change path to wherever is cosmic-text in your system)

diff --git a/Cargo.toml b/Cargo.toml
index 9aadbd1..2eb5ef6 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -38,6 +38,9 @@ wgpu = ["libcosmic/wgpu"]
 libc = { git = "https://gitlab.redox-os.org/redox-os/liblibc.git", branch = "redox_0.2.151" }
 smithay-client-toolkit = { git = "https://github.com/pop-os/client-toolkit", branch = "wayland-resize" }

+[patch.'https://github.com/pop-os/cosmic-text.git']
+cosmic-text = { path = "../cosmic-text" }
+
 [profile.release-with-debug]
 inherits = "release"
 debug = true

and see if any dbg lines get printed? (I'm suspecting the answer is no).

Is it desirable to fallback to non-monospace fonts if no monospace font have the glyph?

So, as of two days ago, that was the desire, but didn't exactly happen. And it didn't happen because the opposite of what you describe actually happened, as in almost always falling back to a non-monospace font or fonts.

Things should now work closer to what you describe. And the fallback order is script-aware. But the next thing is to make it also glyph-aware (shouldn't be a big change). This will also help avoiding falling back to multiple fonts, which may help more in cases like #43, and maybe possibly here too. Especially if no scripts are passed to the fallback iter.

IMHO, the goal should still be trying as hard as possible to fallback to a monospace font, but try to improve the order/selection process.

snaggen commented 7 months ago

I had JetBrains Mono as the selected font, so it worked as only fallback.

As for the patch, I will try to get some time to test it tomorrow.

MoSal commented 7 months ago

I had JetBrains Mono as the selected font, so it worked as only fallback.

Cool. That probably means my change to cosmic-text actually helped, since without it, fallback probably wouldn't have picked up Iosevka.

snaggen commented 7 months ago

I added the

        if !scripts.is_empty() {
            log::debug!("scripts: {scripts:?}");
        }

And this is the output with loglevel debug

[2024-01-18T20:13:39Z DEBUG cosmic_text::font::fallback] scripts: [Braille]
[2024-01-18T20:13:39Z DEBUG cosmic_text::font::fallback] failed to find family 'Noto Sans'
[2024-01-18T20:13:39Z DEBUG cosmic_text::font::fallback] failed to find family 'FreeSans'
[2024-01-18T20:13:39Z DEBUG cosmic_text::font::fallback] failed to find family 'Noto Sans Mono'
[2024-01-18T20:13:39Z DEBUG cosmic_text::font::fallback] failed to find family 'DejaVu Sans Mono'
[2024-01-18T20:13:39Z DEBUG cosmic_text::font::fallback] failed to find family 'FreeMono'
[2024-01-18T20:13:39Z DEBUG cosmic_text::font::fallback] failed to find family 'Noto Sans Symbols'
[2024-01-18T20:13:39Z DEBUG cosmic_text::font::fallback] failed to find family 'Noto Sans Symbols2'
[2024-01-18T20:13:39Z DEBUG cosmic_text::font::fallback] failed to find family 'Noto Color Emoji'
[2024-01-18T20:13:39Z DEBUG cosmic_text::font::fallback] Failed to find any fallback for [Braille] locale 'sv-SE': '⠈││'

Not sure what font other terminals are able to use for Braille. Will see if I'm able to query my font caches.

snaggen commented 7 months ago

"DejaVu Sans" seems to be the source for the Braille according to my gucharmap

Edit: Seems like I have a bunch of fonts containing Braille, DejaVu Sans, DejaVu Serif, Symbola, Noto Sans Symbol 2, and a font actually called "Braille".

MoSal commented 7 months ago

@snaggen If you change the font weight in settings, does anything change?

snaggen commented 7 months ago

Switched my font weight to Normal, and now it finds the font [2024-01-18T20:34:41Z DEBUG cosmic_text::font::fallback] scripts: [Braille] [2024-01-18T20:34:41Z DEBUG cosmic_text::font::fallback] Failed to find script fallback for [Braille] locale 'sv-SE', used 'DejaVu Sans': '⠈⠉││'

snaggen commented 7 months ago

When I have Iosevka installed and set my default weight to "Extra Light", it works and I dont see the 'Failed to find script...' line. I only see this

[2024-01-18T20:42:49Z DEBUG cosmic_text::font::fallback] scripts: [Braille]
MoSal commented 7 months ago

@snaggen

So for fallback, cosmic-text matches on exact weight. I loosened that requirement for the Monospace fallback case. That's why fallback works with Monospace (Iosevka) if the weight does not match. And doesn't if non-Monospace fallback is required.

@jackpot51 Fixing that would require changing the default behavior of cosmic-text for all use-cases. I don't have the confidence to touch that.

snaggen commented 6 months ago

This seems to be somewhat solved by the fix in https://github.com/pop-os/cosmic-term/issues/104 . However, when I tried to run https://github.com/ClementTsang/bottom without the "Correct" font, getting a braille font with an suboptimal weight, it seems the width gets a bit off causing the UI to look a bit weird. Not sure this is solvable or worth solving.