swaywm / sway

i3-compatible Wayland compositor
https://swaywm.org
MIT License
14.64k stars 1.11k forks source link

Japanese/Chinese characters change title height which can cause screen flashing #4992

Closed kuon closed 3 years ago

kuon commented 4 years ago

I realize this issue has been discussed before:

https://github.com/swaywm/sway/issues/3114 https://github.com/swaywm/sway/issues/4107 https://github.com/swaywm/sway/issues/3719 https://github.com/swaywm/sway/issues/4179

Even as the issue is known, I will describe my case:

The problem is that when switching from asian an non asian characters in title bar, it will resize all title bars, and this resizes every windows, which causes flashing for many program.

The problem can be reproduced the following way:

Each time the tab is switched, the title bar is resized and firefox flashes.

I understand fontconfig can be adjusted, but it is often nicer to have asian characters a little bigger than latin characters.

This is why I reopen a new issue and propose the following solutions:

Using sway 1.4

emersion commented 4 years ago

How does i3 handle this case?

mstoeckl commented 4 years ago

i3 (with a pango font) appears to maintain a constant title height, center text vertically, and truncate anything out of bounds: the critical lines of code are i3/src/render.c#L26, i3/src/x.c#L619, L706, and i3/libi3/font.c#L113. This matches what I've observed in practice with window titles involving large characters.

In case the changing title height is incredibly annoying for anyone, I've been running with a small patch that effectively freezes the titlebar height, although the vertical alignment and other details do not match i3.

--- a/sway/tree/container.c
+++ b/sway/tree/container.c
@@ -512,7 +523,7 @@ void container_calculate_title_height(struct sway_container *container) {
        int height;
        int baseline;
        get_text_size(cairo, config->font, NULL, &height, &baseline, 1,
-                       config->pango_markup, "%s", container->formatted_title);
+                       config->pango_markup, "M");
        cairo_destroy(cairo);
        container->title_height = height;
        container->title_baseline = baseline;
travankor commented 4 years ago

I think this affects swaybar, too.

6A61736F6E206E61646572 commented 4 years ago

@travankor I haven't noticed Japanese characters affecting swaybar in the height direction

travankor commented 4 years ago

Not Japanese, but I notice this for other Asian languages, like Tamil and Thai (look at Tibetan for an extreme example). These languages use (vertical) ligatures and have some characters that are disproportionally long compared to the "normal-sized" characters, which might be what causes them to increase height.

Anyway, I think the specific international font comes down to what your font configuration is and what your preferred viewing size for the font is.


For swaybar, it seems like fonts can make the bar larger, but swaybar never shrinks unless you restart sway (tested with i3status-rust). So swaybar's behavior matches the third proposed "fix" for OP's problem.

Personally, I like i3's behavior the most.

vvrein commented 4 years ago

I have same thing with some unicode characters: https://site.tort.icu/bin/raw/swaytitle.gif

Test url: https://github.com/pldubouilh/gossa

First title is a little bit higher: image
image

While there is pretty small difference in titles, there is an annoying issue with window flicker when title changes (watch gif)

vvrein commented 4 years ago

My vote is for

allow setting title bar height in the configuration and stick to it, even if it truncates the text

oranenj commented 4 years ago

Providing a configuration option seems... suboptimal.

Is it possible to provide a sane default that works for 99% of the cases and perhaps results in truncation in some edge cases like the crazy diacritic stacking people sometimes do? I think you could do that by finding the tallest real-world characters in unicode and calculating the font's title height based on that.

oranenj commented 4 years ago

After a very quick test with different ways to do the calculation, for me the best results are produced by calculating the height using a static value, and re-calculating the baseline according to the title, like so:

diff --git a/sway/tree/container.c b/sway/tree/container.c
index 56cdee1d..5ea6e0dc 100644
--- a/sway/tree/container.c
+++ b/sway/tree/container.c
@@ -512,10 +512,14 @@ void container_calculate_title_height(struct sway_container *container) {
        int height;
        int baseline;
        get_text_size(cairo, config->font, NULL, &height, &baseline, 1,
-                       config->pango_markup, "%s", container->formatted_title);
-       cairo_destroy(cairo);
+                       config->pango_markup, "%s", u8"Baseline");
        container->title_height = height;
+
+       get_text_size(cairo, config->font, NULL, &height, &baseline, 1,
+                       config->pango_markup, "%s", container->formatted_title);
+
        container->title_baseline = baseline;
+       cairo_destroy(cairo);
 }

I personally would be perfectly happy with this fix as far as its logic goes, though of course my implementation of it is not acceptable :-)

oranenj commented 4 years ago

I think the above logic would work for setting defaults at least, let me try to make it into an acceptable patch...

kuon commented 4 years ago

What about a min-height? If set to a large enough value, it would prevent the issue and prevent truncation. I think having an height or min-height configuration is the way to go. It won't change behaviour for existing users and allow an additional and optional configuration

oranenj commented 4 years ago

@kuon my problem with that is that it forces non-latin users to work around a bug with configuration that they might not even be aware exists and that a min-height wastes space and makes non-Japanese text look bad.

@ddevault closed my pull request, but I still think "bleeding" larger characters into the padding is an improvement, because it at least replaces the distracting behaviour with something that I think most people would find acceptable. The question is getting the right values for the initial size so that it works for most people by default, and fixing the issue my patch had that of how to center the Japanese characters properly.

oranenj commented 4 years ago

After some more experimentation I managed to come up with a set of changes that appears to keep non-English text centered while allowing it to "leak" into the titlebar padding. This still is "anglocentric" in that it uses latin text to calculate the base text height, but users who need more space for larger characters can simply increase the vertical padding to provide more room for characters to expand into.

kuon commented 4 years ago

If we want the fix to be based on font metrics, the only correct way to do it is to keep current behaviour and just don't "shrink back" the height.

That's because we never know what kind of "large emoji" might be displayed with which font, this means we can't know beforehand the correct height.

This is solution 3 in my original post.

oranenj commented 4 years ago

That might be an acceptable solution, though it has the problem that if someone creates a webpage with some crazy unicode diacritics in the title, sway will permanently expand the titlebar size.

I'm currently running a version of my patch that stops the resizing and renders at least Japanese acceptably. It's probably not correct nor the best possible solution, but it's heaps better than the current behaviour. Screenshots for comparison: Japanese Latin text

kuon commented 4 years ago

Yes, very good remark, crazy unicode can be an issue. How would something like this renders?

\ \ \ \ \  

Ģ̷̛̛̖͙͎̠̦̞̙̞̰͌͐͛͆̆̍́͆̏͌̈́̑̀̈́̈́̈́̚̚͜l̸̨̡̧̧̛͚̰̞̠̦̫̙̪̺͔̼̳̥̜̝̹̹̓̓̃̓͐̽̐͋̌̽́̇̒͊̅̿͂̋͆̈́̉̚͜͜͝͝ͅį̴̧̛̛͔̘̮̟̯̩͕̫̰͈̼̺̖̘͙̖̱̟͙͈̮̞̀̆̈́̔̍̄̍͒̒̅́͑̀̌̂͛̇̕͜͝͝ͅt̷̨̨̢̨̛̜̤͈͎͙̭̬̝̥̺̭̻̝̩̻̜̯̗͍͈̯̥̩̰́͒̽̌̃̾͂̀̓̑̋̎̈́̒̍̌̇̈́͛̈́̓̚̚͜͝͠͝͝͝͝ͅͅͅċ̷̢̛͓̲̞͈̭͚͕͖̥̭̣̞̩̯͉͛͑̆̑̒͐̆̇͜͝h̵̡̛̟̮̥̱̤͎͉͍̙͉̖̤͚̠̜̩̳̳̩̼͕͍̞͍̻̘̄̄̀̂͛̌̋̆́̈́͛̃̓͜ͅͅͅe̵̡̢̢̢̢̨̛̗̬̦̞̠̬͉͇͉̝̹͚̜̣͚̯̞̩̫̤͔͕̮̜͊̀̂̇͛̽͗̐́̾́́̇̅́ͅd̶̛̙̟̲͖̫̮̭̽̈́̀̽̈́̽͛͂̎̒̃́̇͋̓͂̈̉̋̈̈̈́͂̐͋̂̿͂͘̚͝͠ ̸̨̡̫͍̼̫̺̞̘̜̮͙͙̹͇̲̖̠͔̳̦̲̦̜͚̤̙̳͕͖̝͎̍͐̊̂̓͆͊t̸̢̧̡̡̢͖̮̹̟̜̱̠̬͎̞͍͎̲͈̬̖͇͚̤̟̫̘̾̂̓̃͑̅͊̈́͂̇̿͝ͅͅȇ̵̡̡͔̗̼̫͙͈̼͍͙͖̼̳̭̱̯͓̺̗͓͍̖͍͓̩̻̎͊̐̃̅̈̀̄͋̋͆͆͒̐̂̈́͗͐͐͆̏̓̚͘͝͝x̷̡̨̨̡̣̤̮͇̜͍̭̘̗̺͇̘̼̤̠͕̓̔͋́̈́̅̐̆̇̀̅̔͆͘͘͜͠ͅţ̴̨̢̢̢̧̛̛͔̞̳̟̺͖̫̪̟̣̹̜͙̻̟͎̭̘̭͓̏̀͋́̓̀̎͊̌̌̌̿̎͂͗̐͛͆́͂̅̋̈́̕̚͜͠͝͝͝ ̴̰̯͓̤̖̠̫̳̤̙͔̼̖̯͇͔͖̝̱̪̖̬̺͖͉̬̱̱͍̣̽̾͒͂͐̄̎̅̎̈́̉̈́̽̆̏̇̈́́̀͑́̽̎̂͐̒̈͜͠͝l̴̛͓̹̠̞̼̱̲̘̥̉̍̈́̑̍̀̈́̓̈́̓̅̈́̓̓̂̃͆͂̕͠ͅi̸̡̡̧̡̢̢͕̼̝̺̰̯͚̼̗̜͓̰͖̭͙̯͈̲̤͍̱͚̣̱̘͆k̴̡͎̪̠̬͕̦̰̻͙͎͕̞͙̱̮̘͖̦̫̹̲̗̺͊ẻ̷̛̱͔̟̱͌̃̉́̆̿̒͆́̓̇̀̚͠͝ͅ ̸̢̢̢͍̹͈͚͉̲͇̩̤̣̱̱̗̤̺̗̦̭͋͊̔̈́͊̍̃̃̐̍̒̓̍͆͂͊̀̅̒̋̓̚͘͘͠͠ͅt̶̡̲͚̜̻̠̲͙̬͓͉̝͇̙̘̪̮̠̤̞͚̼͚͎͒̋͐̊̅̀̈́̌̌̒́̀͌͊͘̕̕͜͠͝͠ͅh̵̡̧͔̹̰̗̝̼͇̻̼̖̙̱̙͕̼̰̟̰̤̻̼͖̙͋̏̈͊̋͒͆͒̍̓̆̈́̅̅͘͜į̸̡̧̻̳͙͈̩͉͉͈̦̖̣͈̥̪̲̠̣͍͈̤̦̣̏̍́̌̓͂͛̑́͝͝͠ș̶̡̡̧̛͈̬̙̠̦̘̿̓̄̀̅̿̾͐͗̓̈̇́̿̈́̽̑̑̓͠?̶̧̢̢̧͎̺̞͈͔̟͓͎̠̩̳̙͕͍͖̻̠̭͍̳̳̓̾̓́̀̔̚ͅ

\ \ \ \ \ \ \ \ \   You can generate more with those tools:

oranenj commented 4 years ago

I tested that. Interestingly enough vanilla sway seems to limit the text height to about the same as with Japanese text, so it doesn't result in a crazy high title bar. On the other hand, it definitely does not render anywhere close to correct, with or without my patch.

travankor commented 4 years ago

If one of these solutions gets chosen, it would be nice to apply that behavior to swaybar, too. Right now, the title bar and swaybar have inconsistent behavior.

oranenj commented 4 years ago

I'm doing some more testing. It seems some languages misbehave even more noticeably on vanilla sway: 20200315_16h24m25s_grim Adding some debug prints, pango returns some seriously weird height/baseline values for that text:

2020-03-15 16:20:51 - [sway/tree/container.c:532] Text metrics for ASCII h/b: 17/13, this title: 33/22

Clearly the text height is nowhere near 33 pixels, so what on earth are those values? Why is the baseline so high compared to latin text? Os sway perhaps misinterpreting the information Pango provides somehow?

oranenj commented 4 years ago

It'll be a while before I'll have time to look into this again, but it seems using the pixel size of pango's "ink rectangle" might be what sway needs to use; it provides the actual height in rendered pixels. With my font settings, a bunch of latin text is usually around 12-14 pixels, Japanese text might be 15-16, and telugu seems to be around 11. In general it seems like fixing this properly requires pretty much rewriting the font size logic.

Given that the actually rendered sizes don't seem to be that far apart in practice and that rendering very tall text is hopelessly broken already, fixing the 99% case could be done by using the rendered pixel size instead of whatever the "logical" size is.

In my testing using a static titlebar size will work for at least Japanese, English, Hangul, Telugu and Devanagari, and people who the corner case will always be able to increase the titlebar padding to give their characters more space. Sway could also just get rid of configurable vertical titlebar padding since it'll be meaningless and provide a configuration value for the titlebar height.

The default titlebar height will necessarily involve a bit of guesswork since you can't calculate it dynamically and produce reasonable results, but even just arbitrarily hardcoding it at the (IIRC) 17+4 padding that it is currently would work fine for most languages.

vvrein commented 4 years ago

Dear @ddevault :)
Сould you give a couple of tips what could be done further.

https://github.com/swaywm/sway/compare/3078f232581d1dcd548810370c193c6d235d2e82...vvrein:master

Here I "threw some crutches into the code", the purpose was to add ability custom title height from config.
I have no any knowledge of C/C++, and all changes was made "by following the example".

For now, it seems to be working on my pc.
I met some problem in sway/tree/container.c line 516
The problem is that when there is no titlebar_v_height in config file - titles disappears.

I think there should be done something like these:

        int titlebar_v_height = config->titlebar_v_height;
    if (titlebar_v_height >= 0) {
        container->title_height = titlebar_v_height;
        return;
    }

I will test this ^ approach a little bit later.

It would be ideal to bring this to the state of possibility of merging this to master, if sway team interested in.

Here how this is looks like: (there is nothing special there, just works)
image

oranenj commented 4 years ago

@vvrein I'm using a similar patch myself, though it also removes a bunch of size calculations since they become unnecessary as a result of the vertical size being configurable; though I don't actually configure it since the default value that's hardcoded in current sway (17) happens to work just fine.

I suppose I should work it into a submittable patch, but I haven't really had time to work on this lately.

vvrein commented 4 years ago

@oranenj

though it also removes a bunch of size calculations

I don't think that we actually need to remove something, since most users [I think] will use sway "as is", without deep configurations.
And, if once there would be some fix, for somehow bind titlebar_height to 99% people's need's out of the box - separate setting in config file may serve needs of 1% that want esoteric settings like "totally disabled title bar" or "thin like the blade title bar"

oranenj commented 4 years ago

@vvrein the size calculations become useless if the size is configurable, so it's just removing dead code. I'm not sure if making the titlebar size configurable is the best solution, but I think any solution is better than no solution.

vvrein commented 4 years ago

@oranenj As for me, any solutions that I can't control - are dubious. Since I may want to set title height right to 9px, because I want :)

And any auto calculations suppose to have some cons in UTF world.
f.e. while there is only one window with title like this https://github.com/swaywm/sway/issues/4992#issuecomment-599116214 -- all titles would have same crazy height in ~100px

I suppose there might be one good solution in setting title bar height once at sway start enough to display font from conf file, and never recalculate this value.
f.e. with font pango:Misc Fixed 12 title height would be set once from equation 12 > title_height > 12*2 It seems that this way is implemented in i3 https://github.com/swaywm/sway/issues/4992#issuecomment-584447422 @mstoeckl am I right?

But, I think our opinions are pretty one-sided, and we need whether more opinions, or make exact copy i3-wm's behavior. Or both :)

ddevault commented 4 years ago

A config option which allows you to specify an explicit, fixed titlebar height seems reasonable to me.

emersion commented 4 years ago

Yeah, the idea looks good to me too.

oranenj commented 4 years ago

I'll make a PR with the patch I'm running that does essentially that so we can bikeshed the exact form the parameter needs to take, then.

vvrein commented 4 years ago

@oranenj Could you consider approach to make configurable only titlebar height, and in case when there is no such option in conf file - calculate titlebar height based on font conf file option. This would be more useful, I think.

somini commented 4 years ago

@oranenj Could you consider approach to make configurable only titlebar height, and in case when there is no such option in conf file - calculate titlebar height based on font conf file option. This would be more useful, I think.

Wouldn't this be a auto option, which would be the default? There's already plenty of other options like this.

vvrein commented 4 years ago

@somini yes, that would be. Right now, there is no auto, there is 17 px hardcoded, https://github.com/swaywm/sway/pull/5189/commits/43cb2e3f4d0c4cdea468d4d951dfb635da3e73f9#diff-fc41e85ea74168f3b164b822e8987468R247 if no titlebar_max_text_height option found in config file.

vvrein commented 4 years ago

@oranenj I have compiled sway with your PR, seems everything is ok, thank you!
BTW, there was regression https://github.com/swaywm/sway/issues/5193 in the latest commits, here is fix https://github.com/swaywm/sway/pull/5194

vvrein commented 4 years ago

Buuut, seems there is an issue with titles, if text goes beyond the allowed limit. https://tort.icu/bin/raw/rec-issues-4992.mp4 image

And here whats going on when we set just title height: https://tort.icu/bin/raw/rec-issues-4992-1.mp4 image

Title w/o emoji image

And that’s why I'm complained here about the height of the text.

travankor commented 4 years ago

swaybar is still affected with the patch

travankor commented 4 years ago

Also marks are showing up strangely with the patch (bottom half is cropped out) 2020-04-09-02:57:49AM

vvrein commented 4 years ago

Here's patch that allows set titlebar_height (with some tmp debug logging) https://github.com/swaywm/sway/compare/master...vvrein:master

But it still shifts down text, when in title appears some "large" symbol, like this image Can anyone help me to find where in the code text position is set, or something related to the text position?

I want to make text would stick to its position, without shifting in case if in it are large symbols/emoji/etc. And everything above or below titlebar height would be truncated.

I've did some research on https://github.com/swaywm/sway/blob/master/sway/desktop/render.c#L396-L637 but without any luck. Seems that there only text box is set

cathay4t commented 4 years ago

Inspired by #3114, the workaround could be adding a Chinese/Japanese characters to all window titles. For example, Chinese user could:

for_window [title=".*"] title_format ゜%title゜
WhyNotHugo commented 4 years ago

How about a config setting to make the title-bar CJK-friendly-sized.

  1. User set cjk_titlebars = true
  2. Calculate the height of text based on the current font-settings for any CJK character (e.g.: 한글).
  3. Use that as a default.

It seems slightly better than a fixed config value for the title-bar size, mostly because it doesn't need to be manually recalculated whenever the font settings change (also, how would I manually recalculate that?).

Functionally it works, but the implementation idea is quite a hack. Maybe someone can use it as inspiration for something better...?

Making the recurrence text on step 2 configurable might help cover even more edge cases, and be slightly cleaner.

lae commented 4 years ago

Been following this thread for a while, but just want to post another use case that's been bothering me more lately. Quite a few Twitter users and other website content have emoji in their names/titles, and switching to/from those tabs in a browser initiates resizes, too.

WhyNotHugo commented 4 years ago

FWIW, the workaround provided by @cathay4t seems to work quite reliable for sites with CJK or emoji.

lae commented 4 years ago

I've actually been using that workaround for a couple months, but several emoji kept causing issues. One of the edits I made in my previous comment was an improvement over that workaround by wrapping emoji/degree symbol with a <span alpha='1'></span> but some emoji (namely 🐿) just ended up adding excessive padding during normal use, so I edited it out.

I ended up just specifying multiple different fonts (title_format "<span font-family='Noto Sans CJK JP, Symbola'>%title</span>") for title bars and that's working out a lot better for me (at the expense of not using a monospace font).

hwadii commented 3 years ago

The CJK font used by default if it's installed is IPA Gothic (I think), and one workaround I've been using is specifying two fonts in the sway config, like so

font pango:Inconsolata, IPA Gothic 11.5

By default, it seems like IPA Gothic is 13px which is causing the bar to change size when CJK chars are rendered. Forcing its size at 11.5 is enough to not cause the title bars to change size.

WhyNotHugo commented 3 years ago

I'm trying to adopt @hwadii's workaround. I used fc-match to figure out which font is used as a fallback for those kind of characters:

$ fc-match :lang=zh-cn
NotoSansCJK-Regular.ttc: "Noto Sans CJK SC" "Regular"

These settingss presents the issue discussed here:

font Cantarell 11

These three settings yield larger fonts, and titlebars don't shift in height when there are CJK characters, but everything is rendered in Noto Sans, including latin characters:

font pango:Cantarell 11, Noto Sans CJK SC 10
font Noto Sans CJK SC 10
font Cantarell 11, Noto Sans CJK SC 10

I've no idea why Cantarell 11 is ignored in these approaches; any ideas? Would it be best to open a separate issue for this?

Exagone313 commented 3 years ago

@WhyNotHugo

According to sway.5 manual:

Regardless of whether pango markup is enabled, font should be specified as a pango font description.

According Pango documentation I read that your understanding of the format of a font description is incorrect, you can only put commas between font names. That would explain why does only the second font work, as there is no font exactly named Cantarell 11.

FAMILY-LIST is a comma separated list of families optionally terminated by a comma

WhyNotHugo commented 3 years ago

Would a PR that makes both the titlebar height and its baseline configurable be accepted?

Depending on the title's content, the baseline (as calculated right now) varies, so the title content moves up and down as the titlebar's character sets (or font families) change . In my case, it moves down a few px when there's an emoji, and a few px more when there's Chinese characters.

I honestly can't think of a way to work around this, since the baseline calculation is very closely tied to the height calculation, so if one is static, I think other should be too.

WhyNotHugo commented 3 years ago

I've opened #6433 to illustrate the problem mentioned in the above comment.

WhyNotHugo commented 3 years ago

A config option which allows you to specify an explicit, fixed titlebar height seems reasonable to me.

Alright, I've figured out how to get this done without assuming locale or any additional parameters.

Would you prefer that the fallback be (a) the current behaviour or (b) a fixed default value?

fluix-dev commented 3 years ago

Would you prefer that the fallback be (a) the current behaviour or (b) a fixed default value?

Given the issue with various locales/additional parameters it seems like (a) is the correct choice because what value would be chosen by us for (b). If your solution has an associated sway command, say smart_titlebar_height on then (b) could work with a px argument after on/off which the user explicitly chooses.

hollunder commented 3 years ago

I really wish the title bar had a fixed height as in i3 because this is really really annoying. 20210818_18h07m08s_grim 20210818_18h07m01s_grim The above is the default font, the emotes are from Discord running in Firefox. As you can see there is quite a difference in height, I measured it to be 7 px in this case, but it is variable. Due to the title bar changing height the entire screen content is jumping up and down by that much, which is extremely jarring visually.