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

Unicode combining characters do not show up #109

Closed
retupmoca opened this issue Jun 9, 2021 · 7 comments · Fixed by #121
Closed

Unicode combining characters do not show up #109

retupmoca opened this issue Jun 9, 2021 · 7 comments · Fixed by #121
Assignees

Comments

@retupmoca
Copy link

It looks like unicode combining characters such as https://www.fileformat.info/info/unicode/char/0303/index.htm do not render correctly.

Modifying the example starter project as a test (see https://gist.github.com/retupmoca/68a4852a28b7cf6d2c39a90d7670212f for the full modified file), I added:

  // plain UTF-8 output of a random combining character:
  // https://www.fileformat.info/info/unicode/char/0303/index.htm
  std::cout << "done\xcc\x83:\n";

  auto summary = [&] {
    auto content = vbox({
        // converted UTF-8
        hbox({text(to_wstring(std::string("- done\xcc\x83:   "))), text(L"3") | bold}) | color(Color::Green),
        // UTF-16 / UCS-2
        hbox({text(L"- active\x0303: "), text(L"2") | bold}) | color(Color::RedLight),
        hbox({text(L"- queue:  "), text(L"9") | bold}) | color(Color::Red),
    });
    return window(text(L" Summary "), content);
  };

When I run the modified program, I see:
image

That is, the combining character renders correctly in my terminal if I directly output it, but displaying it via FTXUI does not appear to work.

@ArthurSonzogni ArthurSonzogni self-assigned this Jun 9, 2021
@ArthurSonzogni
Copy link
Owner

Thanks for reporting this. I will take a look.

@ArthurSonzogni
Copy link
Owner

ArthurSonzogni commented Jun 9, 2021

A ftxui::Pixel is:

struct Pixel {
  wchar_t character = U' ';
  bool blink = false;
  bool bold = false;
  bool dim = false;
  bool inverted = false;
  bool underlined = false;
  Color background_color = Color::Default;
  Color foreground_color = Color::Default;
};

So, a single wchar_t is not enough to store combining characters.
I am actually quite surprised this fallback so nicely. ;-)

Not fully sure how to overcome this issue.

@ArthurSonzogni
Copy link
Owner

Note that the character you are using can also be represented via a single character:

  auto document = text(L"- activẽ: ");
  auto screen = Screen::Create(Dimension::Full(), Dimension::Fit(document));
  Render(screen, document);
  screen.Print();

output
Capture d’écran du 2021-06-09 21-12-32

This doesn't solve this issue, but this might solve your particular problem?

@retupmoca
Copy link
Author

retupmoca commented Jun 9, 2021

Yeah, most of the common graphemes are available as a single codepoint - it's not currently a major issue for me, but I wanted to report it for completeness :)

ArthurSonzogni added a commit that referenced this issue Jun 10, 2021
Add a note about:
#109
@ArthurSonzogni
Copy link
Owner

I added a note into the README.

At first glance, this seems difficult to support. I will try to learn more about this unicode feature. If its specification is not too insane, there might be a way to support "multiple character per pixels" feature.

@magiblot
Copy link

In case it may help: magiblot/tvision#26

@ArthurSonzogni
Copy link
Owner

In case it may help: magiblot/tvision#26

This helps a lot! Sounds like you had the exact same problem and you solved it. Thank you!

ArthurSonzogni added a commit that referenced this issue Jun 19, 2021
Modify the ftxui::Pixel. Instead of storing a wchar, store a
std::wstring. Now a single pixel can store multiple codepoints.
If a codepoint is of size <=0, it will be appended to the previous
pixel.

Only ftxui::text() is supported. ftxui::vtext support still needs to be
added.

This fixes: #109
ArthurSonzogni added a commit that referenced this issue Jun 25, 2021
Modify the ftxui::Pixel. Instead of storing a wchar, store a
std::wstring. Now a single pixel can store multiple codepoints.
If a codepoint is of size <=0, it will be appended to the previous
pixel.

Only ftxui::text() is supported. ftxui::vtext support still needs to be
added.

This causes the following CPU and memory regression:
- Memory: Pixel size increases by 200% (16 byte => 48byte).
- CPU:    Draw/Second decrease by 62.5% (16k draw/s => 6k draw/s on 80x80)

Both regressions are acceptable. There are still two orders of magnitude
(100x) before the levels where performance/memory concerns begins.

This fixes: #109
ArthurSonzogni added a commit that referenced this issue Jun 25, 2021
Modify the ftxui::Pixel. Instead of storing a wchar, store a
std::wstring. Now a single pixel can store multiple codepoints.
If a codepoint is of size <=0, it will be appended to the previous
pixel.

Only ftxui::text() is supported. ftxui::vtext support still needs to be
added.

This causes the following CPU and memory regression:
- Memory: Pixel size increases by 200% (16 byte => 48byte).
- CPU:    Draw/Second decrease by 62.5% (16k draw/s => 6k draw/s on 80x80)

Both regressions are acceptable. There are still two orders of magnitude
(100x) before the levels where performance/memory concerns begins.

This fixes: #109
ArthurSonzogni added a commit that referenced this issue Jun 25, 2021
Modify the ftxui::Pixel. Instead of storing a wchar, store a
std::wstring. Now a single pixel can store multiple codepoints.
If a codepoint is of size <=0, it will be appended to the previous
pixel.

Only ftxui::text() is supported. ftxui::vtext support still needs to be
added.

This causes the following CPU and memory regression:
- Memory: Pixel size increases by 200% (16 byte => 48byte).
- CPU:    Draw/Second decrease by 62.5% (16k draw/s => 6k draw/s on 80x80)

Both regressions are acceptable. There are still two orders of magnitude
(100x) before the levels where performance/memory concerns begins.

This fixes: #109
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants