search

contour-terminal / contour

Modern C++ Terminal Emulator
http://contour-terminal.org/
Apache License 2.0
2.32k stars 101 forks source link

Chosing wrong font in surround text when U+25A0 is involved #1419

Closed christianparpart closed 5 months ago

christianparpart commented 5 months ago

See screenshot:

image

printf "Swi \u25a0 Swi\n"

Not sure yet why this is happening. I think it looks like it must have been wrong all the time, because the font file is chosen based on text segmentation rules, which are largely taking from the Unicode segmentation algorithms (script, emoji, ...) plus some additional rules on top like SGR change and blank grid cells.

In the above example, as soon as U+25A0 (■) printed, it's right AND left side are using the wrong font file.

Maybe this is because that was the only font delivering U+25A0, but we are having that character in our builtin box drawing logic - it should have been rendered using that rather than that bad font.

Solution: we probably forgot to segment based on custom codepoint ranges (that we render manually!).

cqexbesd commented 5 months ago

It's not just that char as you probably can guess:

image

0000000   [   D   E   B   U   G   ]       a   d   d   i   n   g       I
         5b  44  45  42  55  47  5d  20  61  64  64  69  6e  67  20  49
0000020   O   :       a       =       0   x   1   e   e   1   f   0
         4f  3a  20  61  20  3d  20  30  78  31  65  65  31  66  30  20
0000040   v   a   l       =     027 267   Q   :  \0       N   5 021 035
         76  61  6c  20  3d  20  17  b7  51  3a  00  20  4e  35  11  1d
0000060 001 030  \n
         01  18  0a

Not entirely sure how the end of the string comes out like that at all, aside from the font issue, though with that many control characters I don't want to say it's wrong without much careful study...

christianparpart commented 5 months ago

@cqexbesd hey. Is this still the case for you on latest master? It contains a tweak on text segmentation, which affects font selection. In other words, space split text must be text shaped individually.

christianparpart commented 5 months ago

Um, if at all, it should have fixed yours and mine. I will check ASAP.

cqexbesd commented 5 months ago

I'm still running 0.4.2-master-601185e4 so let me update and retry.

cqexbesd commented 5 months ago

Yes you are right. Sorry about that - didn't even think it might have just been fixed when I saw your issue! It is fixed on current master and the output looks like a good attempt at the binary data, unlike the example above.

It's not my issue but I think it can be closed.

christianparpart commented 5 months ago

@cqexbesd great to hear. and yeah. it's my issue that i noticed. and I didn't close the ticket myself either, because I fixed something else that also affects this ticket (implicitly). Enjoy the day ☀️