inlyne icon indicating copy to clipboard operation
inlyne copied to clipboard

Fonts can appear faint / low contrast

Open CosmicHorrorDev opened this issue 1 year ago • 22 comments

See https://github.com/trimental/inlyne/issues/97#issuecomment-1540796548

@nicoburns Spinning this off into it's own issues since it seems distinct

Reference image

image

It looks like we do set the background color based on the theme. I'm not sure what causes the colors to look fainter for you. This is what I get

image

which is much more readable.

Here's the HTML we're rendering for reference
<pre style="background-color:#eff1f5;"><code class="language-rust"><span style="color:#b48ead;">use </span><span style="color:#4f5b66;">taffy::prelude::*;
</span><span style="color:#4f5b66;">
</span><span style="color:#a7adba;">// First create an instance of Taffy
</span><span style="color:#b48ead;">let mut</span><span style="color:#4f5b66;"> taffy = Taffy::new();
</span><span style="color:#4f5b66;">
</span><span style="color:#a7adba;">// Create a tree of nodes using `taffy.new_leaf` and `taffy.new_with_children`.
</span><span style="color:#a7adba;">// These functions both return a node id which can be used to refer to that node
</span><span style="color:#a7adba;">// The Style struct is used to specify styling information
</span><span style="color:#b48ead;">let</span><span style="color:#4f5b66;"> header_node = taffy
</span><span style="color:#4f5b66;">    .</span><span style="color:#96b5b4;">new_leaf</span><span style="color:#4f5b66;">(
</span><span style="color:#4f5b66;">        Style {
</span><span style="color:#4f5b66;">            size: Size { width: </span><span style="color:#96b5b4;">length</span><span style="color:#4f5b66;">(</span><span style="color:#d08770;">800.0</span><span style="color:#4f5b66;">), height: </span><span style="color:#96b5b4;">length</span><span style="color:#4f5b66;">(</span><span style="color:#d08770;">100.0</span><span style="color:#4f5b66;">) },
</span><span style="color:#4f5b66;">            ..Default::default()
</span><span style="color:#4f5b66;">        },
</span><span style="color:#4f5b66;">    ).</span><span style="color:#96b5b4;">unwrap</span><span style="color:#4f5b66;">();
</span><span style="color:#4f5b66;">
</span><span style="color:#b48ead;">let</span><span style="color:#4f5b66;"> body_node = taffy
</span><span style="color:#4f5b66;">    .</span><span style="color:#96b5b4;">new_leaf</span><span style="color:#4f5b66;">(
</span><span style="color:#4f5b66;">        Style {
</span><span style="color:#4f5b66;">            size: Size { width: </span><span style="color:#96b5b4;">length</span><span style="color:#4f5b66;">(</span><span style="color:#d08770;">800.0</span><span style="color:#4f5b66;">), height: </span><span style="color:#96b5b4;">auto</span><span style="color:#4f5b66;">() },
</span><span style="color:#4f5b66;">            flex_grow: </span><span style="color:#d08770;">1.0</span><span style="color:#4f5b66;">,
</span><span style="color:#4f5b66;">            ..Default::default()
</span><span style="color:#4f5b66;">        },
</span><span style="color:#4f5b66;">    ).</span><span style="color:#96b5b4;">unwrap</span><span style="color:#4f5b66;">();
</span><span style="color:#4f5b66;">
</span><span style="color:#b48ead;">let</span><span style="color:#4f5b66;"> root_node = taffy
</span><span style="color:#4f5b66;">    .</span><span style="color:#96b5b4;">new_with_children</span><span style="color:#4f5b66;">(
</span><span style="color:#4f5b66;">        Style {
</span><span style="color:#4f5b66;">            flex_direction: FlexDirection::Column,
</span><span style="color:#4f5b66;">            size: Size { width: </span><span style="color:#96b5b4;">length</span><span style="color:#4f5b66;">(</span><span style="color:#d08770;">800.0</span><span style="color:#4f5b66;">), height: </span><span style="color:#96b5b4;">length</span><span style="color:#4f5b66;">(</span><span style="color:#d08770;">600.0</span><span style="color:#4f5b66;">) },
</span><span style="color:#4f5b66;">            ..Default::default()
</span><span style="color:#4f5b66;">        },
</span><span style="color:#4f5b66;">        &amp;[header_node, body_node],
</span><span style="color:#4f5b66;">    )
</span><span style="color:#4f5b66;">    .</span><span style="color:#96b5b4;">unwrap</span><span style="color:#4f5b66;">();
</span><span style="color:#4f5b66;">
</span><span style="color:#a7adba;">// Call compute_layout on the root of your tree to run the layout algorithm
</span><span style="color:#4f5b66;">taffy.</span><span style="color:#96b5b4;">compute_layout</span><span style="color:#4f5b66;">(root_node, Size::</span><span style="color:#d08770;">MAX_CONTENT</span><span style="color:#4f5b66;">).</span><span style="color:#96b5b4;">unwrap</span><span style="color:#4f5b66;">();
</span><span style="color:#4f5b66;">
</span><span style="color:#a7adba;">// Inspect the computed layout using taffy.layout
</span><span style="color:#4f5b66;">assert_eq!(taffy.</span><span style="color:#96b5b4;">layout</span><span style="color:#4f5b66;">(root_node).</span><span style="color:#96b5b4;">unwrap</span><span style="color:#4f5b66;">().size.width, </span><span style="color:#d08770;">800.0</span><span style="color:#4f5b66;">);
</span><span style="color:#4f5b66;">assert_eq!(taffy.</span><span style="color:#96b5b4;">layout</span><span style="color:#4f5b66;">(root_node).</span><span style="color:#96b5b4;">unwrap</span><span style="color:#4f5b66;">().size.height, </span><span style="color:#d08770;">600.0</span><span style="color:#4f5b66;">);
</span><span style="color:#4f5b66;">assert_eq!(taffy.</span><span style="color:#96b5b4;">layout</span><span style="color:#4f5b66;">(header_node).</span><span style="color:#96b5b4;">unwrap</span><span style="color:#4f5b66;">().size.width, </span><span style="color:#d08770;">800.0</span><span style="color:#4f5b66;">);
</span><span style="color:#4f5b66;">assert_eq!(taffy.</span><span style="color:#96b5b4;">layout</span><span style="color:#4f5b66;">(header_node).</span><span style="color:#96b5b4;">unwrap</span><span style="color:#4f5b66;">().size.height, </span><span style="color:#d08770;">100.0</span><span style="color:#4f5b66;">);
</span><span style="color:#4f5b66;">assert_eq!(taffy.</span><span style="color:#96b5b4;">layout</span><span style="color:#4f5b66;">(body_node).</span><span style="color:#96b5b4;">unwrap</span><span style="color:#4f5b66;">().size.width, </span><span style="color:#d08770;">800.0</span><span style="color:#4f5b66;">);
</span><span style="color:#4f5b66;">assert_eq!(taffy.</span><span style="color:#96b5b4;">layout</span><span style="color:#4f5b66;">(body_node).</span><span style="color:#96b5b4;">unwrap</span><span style="color:#4f5b66;">().size.height, </span><span style="color:#d08770;">500.0</span><span style="color:#4f5b66;">); </span><span style="color:#a7adba;">// This value was not set explicitly, but was computed by Taffy
</span><span style="color:#4f5b66;">
</span></code></pre>

Notably I don't see anything that looks like it would impact the font width

Can you try setting a different font to see if it's only the font that is getting selected? You can do this by making a config file at ~/.config/inlyne/inlyne.toml or wherever that translates to for your system. You would just need

[font-options]
monospace-font = "{Insert desired font family}"

CosmicHorrorDev avatar May 09 '23 23:05 CosmicHorrorDev

I'm not convinced it's just the background colour. The text itself seems to be pale. The colour used for let in your screenshot is #b58fae, whereas the colour github is using length is #8250df. These are very different!

Can you try setting a different font to see if it's only the font that is getting selected? You can do this by making a config file at ~/.config/inlyne/inlyne.toml or wherever that translates to for your system

Will try this tomorrow.

nicoburns avatar May 09 '23 23:05 nicoburns

It could be a thickness thing. It does look like maybe some of the colour is getting anti-aliased away.

nicoburns avatar May 09 '23 23:05 nicoburns

I guess I wasn't very clear. I also don't think it's from the background color now

We do currently set a tolerance of 0.5 for the draw cache, so a lower value might lead to more accurate rendering of the glyphs. Although it doesn't really make sense that there would be such a universal impact

https://github.com/trimental/inlyne/blob/da76a06b6d68b74bf1e885bdce2e61587f07f3a8/src/renderer.rs#L157-L159

CosmicHorrorDev avatar May 09 '23 23:05 CosmicHorrorDev

@nicoburns can you confirm if this issue exists on current main? The switch to cosmic-text just got merged in today

CosmicHorrorDev avatar May 19 '23 01:05 CosmicHorrorDev

The following is a screenshot of main compared with the screenshots in https://github.com/trimental/inlyne/issues/97#issuecomment-1540796548:

Screenshot 2023-05-19 at 02 49 57

To me it looks like the contrast issue has been fixed, although:

  • The theme that inlyne is using isn't quite as clear as the one github is using
  • main seems to no longer be rendering code blocks using a monospace font

nicoburns avatar May 19 '23 01:05 nicoburns

  • main seems to no longer be rendering code blocks using a monospace font

Welp that's definitely a bug lol. It totally slipped by me in review since I set a custom monospace font in my inlyne.toml. I can dig into fixing that tomorrow

The theme that inlyne is using isn't quite as clear as the one github is using

Yeah the default light theme seems to be lower contrast for sure. These are all of the light themes we have bundled in from syntect. inspired-github seems to have much better contrast, but I don't like that it looks like they went with #ffffff for the background color although we could always forcibly override that. I believe that the inspired-github theme also uses different font-weights for some of the tokens which we haven't added support for yet, but that should be easy enough to handle

image

There's also always the possibility of us adding in a custom theme too

(Aside: Thanks for all of your help looking through these things :heart:. It's easy to miss issues after you've acclimated to them)

CosmicHorrorDev avatar May 19 '23 02:05 CosmicHorrorDev

The problem is with this line here https://github.com/pop-os/cosmic-text/blob/bfb5eefbfa869915e47824877af68a5307cf301c/src/attrs.rs#L156. Seems as though it'll sacrifice all text attributes to use a font with emojis.

trimental avatar May 19 '23 17:05 trimental

Hmm.... multiple fonts should match with that function though. There must be somewhere else ordering them.

nicoburns avatar May 19 '23 17:05 nicoburns

I think I've got a fairly decent fix here https://github.com/trimental/cosmic-text/commit/e1e9fb5215daa9dbd40998e16210a6ee3a61ff1f.

trimental avatar May 19 '23 18:05 trimental

It does seem like cosmic-text is lacking activity recently. It may be worth creating our own fork to apply fixes like this. Thoughts @CosmicHorrorDev.

trimental avatar May 19 '23 18:05 trimental

I'm AOK with patching with our own forks for now and trying to upstream stuff before a new inlyne release

Only real issue would be if they start heavily diverging from our forks, but that's unlikely

CosmicHorrorDev avatar May 19 '23 18:05 CosmicHorrorDev

Cosmic-text has historically been reasonably good at accepting PRs. Just a bit bursty in terms of development. There is also this thread on the Iced Discord server (https://discord.com/channels/628993209984614400/1027584473631555604) which has become the sort of unnofficial support forum for cosmic-text.

And a couple of open issues around font-fallback behaviour:

  • https://github.com/pop-os/cosmic-text/issues/114
  • https://github.com/pop-os/cosmic-text/issues/58

I suspect a more general algorithm implementation (rather than something that just works for monospace fonts) might be more likely to be accepted upstream.

nicoburns avatar May 19 '23 18:05 nicoburns

@nicoburns if possible could you test https://github.com/trimental/inlyne/pull/120 to affirm that monospace fallback is fixed?

trimental avatar May 20 '23 13:05 trimental

Yep, that branch selects a monospace font for code sections. However it still looks quite weird. Two issues I would point to:

  • The vertical distance between a line and it's predecessor differs between soft-wrapped lines and hard-wrapped lines.
  • Some of the letters seems too thin. Look at the es in header_node for example.
Screenshot 2023-05-20 at 14 41 16

nicoburns avatar May 20 '23 13:05 nicoburns

Seems git has scuffed the commit names but both https://github.com/trimental/inlyne/commit/8b441006757b45a6f6c5175b04ec015ec7a8e4a3 and https://github.com/trimental/inlyne/commit/ef8d5202add3678846025d8500155df9ba8e1d78. Should enable code bolding and fix the line spaces respectively.

As for the washed out appearance I believe this is due to this. There's no real way for us to determine whether to do srgb conversion or not. Perhaps we should just set the default theme to InspiredGithub for now or manually create a table for which themes need conversion.

trimental avatar May 20 '23 17:05 trimental

Perhaps we should just set the default theme to InspiredGithub for now

Already working on that :+1:

or manually create a table for which themes need conversion.

We only have a handful of bundled themes, so that should be doable although it's possible that future updates could cause things to get out of sync with the hardcoded values

CosmicHorrorDev avatar May 20 '23 17:05 CosmicHorrorDev

Seems git has scuffed the commit names but both https://github.com/trimental/inlyne/commit/8b441006757b45a6f6c5175b04ec015ec7a8e4a3 and https://github.com/trimental/inlyne/commit/ef8d5202add3678846025d8500155df9ba8e1d78. Should enable code bolding and fix the line spaces respectively.

Line spacing is fixed in that it is now even, but IMO the lines are now a little too close togther. A little spacing would be nice. However, I'm not seeing any bolding, and I'm still seeing weird glyph rendering where some characters and parts of the characters seem to be stroked more strongly than others:

Screenshot 2023-05-20 at 18 29 15

Perhaps this is just

As for the washed out appearance I believe this is due to this. There's no real way for us to determine whether to do srgb conversion or not. Perhaps we should just set the default theme to InspiredGithub for now or manually create a table for which themes need conversion.

That makes sense. An incorrect colour space is exactly what it looks like. I'd suggest that both of those fixes might be a good idea.

nicoburns avatar May 20 '23 17:05 nicoburns

Update: I get better font rendering by switching the font. This is Source Sans Pro:

Screenshot 2023-05-20 at 18 43 15

nicoburns avatar May 20 '23 17:05 nicoburns

Also works the other way with proportional fonts. Helvetica Neue doesn't look great:

Screenshot 2023-05-20 at 18 47 16

nicoburns avatar May 20 '23 17:05 nicoburns

Bolding doesn't occur with Base16OceanLight. InspiredGithub has it though. image

trimental avatar May 20 '23 18:05 trimental

in my case not only fonts, even images appear with wrong contrast and different colors than should be:

1694176554

1694176810

Above is inlyne, below is normal image opened with firefox

Any idea why this happens?

felixsanz avatar Sep 08 '23 12:09 felixsanz

There's a lot of potentially related discussion in this iced issue https://github.com/iced-rs/iced/issues/2254

CosmicHorrorDev avatar Mar 22 '24 00:03 CosmicHorrorDev