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

Can not render U+FF08 symbol #1457

Closed
luokuning opened this issue Mar 12, 2019 · 18 comments
Closed

Can not render U+FF08 symbol #1457

luokuning opened this issue Mar 12, 2019 · 18 comments

Comments

@luokuning
Copy link

Hi there, I found that kitty can not render a symbol '\uff08', which should be '(':

image

Here is a screenshot about the macOS origin terminal app:

image

My environment is macOS 10.14.3.

@kovidgoyal
Copy link
Owner

As far as I can tell this is a bug in CoreText -- it renders fine on linux. Sadly I am not a CoreText expert, so I dont know where to look to find a workaround. For anyone that is interested, the relevant code is in core_text.m in the render_glyphs function. You can use --debug-font-fallback to see which font is bein used to render, which seems to be Ping Fang SC on my mac.

@luokuning
Copy link
Author

My current font is Monaco. This bug is kind of annoying in some case.😅

@laggardkernel
Copy link

It seems the character is rendered but it's not located in the center, which causes right part of the content is truncated. The same happened on the character, making the word spacing between and too large.

@iamcco
Copy link

iamcco commented Jan 4, 2020

same issue on mac os 10.14.3

@Camouflager
Copy link

Not only U+FF08 but most of the full width characters have either vertical or horizontal shifts, regardless of fonts. For example most of the chars (if not all of the fullwidth chars) in U+FF00-U+FFEF.
Also I've noticed some "ligatures for coding" from Iosevka also have similarly weird vertical shifts but I don't know if they are relevant to this issue.

Kitty is by far the smoothest terminal emulator I can find on macOS. But this problem makes it pretty troublesome for CJK users. I'd like to dig into this a little bit and I presume I should maybe try to play around with the APIs of CoreText a bit? Any help will be appreciated.

@kovidgoyal
Copy link
Owner

kovidgoyal commented Mar 29, 2020 via email

@Camouflager
Copy link

Camouflager commented Apr 5, 2020

@kovidgoyal I managed to print out the entire render_buf when rendering the glyph for U+FF08, and it looks just fine. I suppose the bug may be somewhere else?

update:
I commented out right_shift_canvas in core_text.m and it renders correctly.

@kovidgoyal
Copy link
Owner

I dont see where else it could be. The render buf is simply rendered. If it were not being rendered properly it would affect other characters as well (all rendering happens via that same codepath)

@Camouflager
Copy link

It seems that the code block that does glyph centering is not in sync with the block in freetype.c, which may be the source of the problem

@kovidgoyal
Copy link
Owner

kovidgoyal commented Apr 5, 2020 via email

@Camouflager
Copy link

I mean that the two blocks that does the same thing is different. In core_text.m a boolean called center_glyphs in not used, which seems like an oversight...

@kovidgoyal
Copy link
Owner

freetype are completely different in the APIs the y expose and the
amount of control they offer. FreeType gives far more, in CoreText it
simply leaves it up to the OS to draw the glyph properly.

@Camouflager
Copy link

Camouflager commented Apr 5, 2020

if (num_cells > 1) {
Maybe center_glyph should be included in this condition check? I don't know what it does so... Full width character uses 2 cells but contains only 1 glyph, so unexpected glyph centering (I don't know what exactly it is for) happens in such cases.

@kovidgoyal
Copy link
Owner

Does including it fix the issue?

@Camouflager
Copy link

Yes, it does

@kovidgoyal
Copy link
Owner

OK I'll look at it in detail when I have a moment, thanks.

@kovidgoyal
Copy link
Owner

I dont see how that could possibly work. The only time center_glyph is
set to true is for PUA characters followed by spaces. That is not the
case here. And the following patch has no effect as expected:

diff --git a/kitty/core_text.m b/kitty/core_text.m
index 15d58c46..3b58fe4f 100644
--- a/kitty/core_text.m
+++ b/kitty/core_text.m
@@ -505,7 +505,7 @@ do_render(CTFontRef ct_font, bool bold, bool italic, hb_glyph_info_t *info, hb_g
         Region src = {.bottom=cell_height, .right=canvas_width}, dest = {.bottom=cell_height, .right=canvas_width};
         render_alpha_mask(render_buf, canvas, &src, &dest, canvas_width, canvas_width);
     }
-    if (num_cells > 1) {
+    if (num_cells > 1 || (center_glyph && num_cells)) {
         // center glyphs
         CGFloat delta = canvas_width - br.size.width;
         if (delta > 1) right_shift_canvas(canvas, canvas_width, cell_height, (unsigned)(delta / 2.f));

The reason that center_glyph is not used in core_text is that all
multi-cell glyphs are centered already.

@kovidgoyal
Copy link
Owner

Unless you mean turning off the centering fixes it. Which would imply the bounding rect returned by coretext for these glyphs is wrong.

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

No branches or pull requests

5 participants