GNOME Bugzilla – Bug 346942
gnome-font-viewer doesn't render complex scripts correctly
Last modified: 2014-01-14 22:12:53 UTC
I'm not sure what gome-font-viewer is doing, but seems like it's accessing FreeType directly instead of using Pango. That makes it not render Persian text correctly for example, and it makes the text non-selectable too.
Created attachment 68592 [details] shot The letters should be connected, and the layout right-to-left and right-aligned..
Behdad: as the pango maintainer, would you know how to use Pango here? The program needs to render the sample text using the particular font file, which may not be on the fontconfig path or may only be accessible via gnome-vfs. I can create an FT_Face for the font quite easily (what the app does currently), but I don't see a way to get Pango to render using that font. As this is a font previewer, we don't want to be doing any font substitution even if the font in question lacks coverage (it might be worth picking different sample text in this case).
With PangoFT2 and PangoXft you probably can use pango_ft2_font_map_set_default_substitute() and pango_xft_set_default_substitute() to set FC_FILE/FC_INDEX on the pattern, to force the font file to be used, or FC_FT_FACE to point to the face. For PangoCairo, I'm trying to come up with a generic solution and so far this is what I've come up with: Add a new PangoCairoFontMap implementation that uses a single font face: PangoFontMap * pango_cairo_font_map_create_for_cairo_font_face (cairo_font_face_t *); Such a font map then can be used to create a context and then a layout that will only use the specificed font face. You can then use cairo-ft.h to create a cairo_font_face_t for your FT_Face. Filed that as bug 347237
Jamed, does that make sense?
As for fallbacks, we already have pango attributes to disable fallback. That would trigger hexboxes. I'm going to add more attributes to disable hexboxes too, eventually. But with the above approach, the hexbox you get will use the same font.
Behdad: sorry for not replying earlier. I think mail for your last three comments on this bug never got through (see http://blogs.gnome.org/view/ovitters/2006/07/11/0). The pango_xft_set_default_substitute() would affect other rendering though, right? That'd make things a bit more complicated for the other text rendering in the font viewer app (I still want the rest of the widgets to render normally). So I suppose the pangoft2 method (possible now) or pangocairo method (possible with bug 347237) are worth trying. So I suppose the rendering code would look something like this: 1. create FT_Font for the file being previewed. 2. create a PangoFT2FontMap with pango_ft2_font_map_new() 3. set DPI of font map with pango_ft2_font_map_set_resolution() (to what?) 4. set a default substitute function for the font map to a function that ensures that said font gets used. 5. create a PangoContext with pango_ft2_font_map_create_context() 6. create a PangoLayout with the context, making sure that the "fallback" PangoAttribute is set to false for the entire run of text 7. Render the layout with pango_ft2_render_layout() or pango_ft2_render_layout_subpixel() (which one?) I guess the fact that this isn't too obvious isn't too bad, since font viewers are pretty much the only apps that'd want to use pango this way.
(In reply to comment #6) > Behdad: sorry for not replying earlier. I think mail for your last three > comments on this bug never got through (see > http://blogs.gnome.org/view/ovitters/2006/07/11/0). No problem. Thanks for the detailed reply. > The pango_xft_set_default_substitute() would affect other rendering though, > right? That's true, because with the Xft backend you cannot create a new fontmap for the same display. However, you can use a magic string as the font family and match FC_FAMILY in the susbtitution function before overwriting the matched font file/face. > So I suppose the pangoft2 method (possible now) or pangocairo method (possible > with bug 347237) are worth trying. With the pangoft2 you still cannot use a textview but that's not a big deal. > So I suppose the rendering code would look something like this: > > 1. create FT_Font for the file being previewed. > 2. create a PangoFT2FontMap with pango_ft2_font_map_new() > 3. set DPI of font map with pango_ft2_font_map_set_resolution() (to what?) to what you get from pango_cairo_context_get_resolution() from a context created by gtk_widget_create_pango_context(), such that you match the resolution of user's desktop. > 4. set a default substitute function for the font map to a function that > ensures that said font gets used. > 5. create a PangoContext with pango_ft2_font_map_create_context() > 6. create a PangoLayout with the context, making sure that the "fallback" > PangoAttribute is set to false for the entire run of text > 7. Render the layout with pango_ft2_render_layout() or > pango_ft2_render_layout_subpixel() (which one?) First one. If you read the docs for the latter, it's not really that different. Only the positioning parameters are in subpixel accuracy, but the final rendering is still rounded to pixel boundaries. > I guess the fact that this isn't too obvious isn't too bad, since font viewers > are pretty much the only apps that'd want to use pango this way. Exactly. Pango wants to be really sure that it can find a suitable font for the text and defeating that is a bit hard.
*** Bug 442750 has been marked as a duplicate of this bug. ***
Any updates regarding this issue? this is really an annoying bug for Arabic script users.
(In reply to comment #9) > Any updates regarding this issue? this is really an annoying bug for Arabic > script users. I'm still making up my mind about the right API to add. Owen didn't like my previously proposed API, with good reason. I'm working on it, hopefully this will be fixed this cycle.
So I'm working on a solution that I hope to get into tomorrow's pango devel release. James, do you mind if I hack gnome-font-viewer to use my new API? And are you planning a release tomorrow?
Any plans for this to get in gnome 2.20?
I've not found a way to implement the needed feature in Pango yet. This is constantly brought up again and again when Owen and I are talking. Hopefully we find a solution for 2.20, but no promises at this point.
Interesting thing I found today, font-viewer of serif project (http://code.google.com/p/serif/source/browse/#svn/fontview/trunk) does use pango to render fonts (it gets Arabic rendered right) without installing the font.
Khaled, It does that by using fontconfig API to add app-specific fonts. That works for files on your system but not for more general types of streams... Though, thinking about it, using the ~/.gvfs fuse mounts we can always use that trick.
-> gnome-utils gnome-font-viewer has now moved to gnome-utils, reassigning.
Created attachment 255145 [details] [review] Patch using HarfBuzz Since Pango does not (yet) have a API to use a cairo_font_face_t for rendering, HarfBuzz is used directly instead to do the actual text layout, but Pango is still (ab)use to do text itemization. The thumbnailer still does not do complex text, but I don’t think it actually need to, so it was left alone. This should fix Bug 109269 as well.
Created attachment 255146 [details] [review] Patch to fix showing fonts with non-Unicode charmaps This is not a new issue, but it seems the use of HarfBuzz exposed it.
I think the patches should have been submitted against Sushi, but I don’t know how to test that code there.
*Very* neat. Thanks Khaled! Humm. James, you still maintainer here?
Any chance of getting those patches for the next GNOME release?
Cosimo, can you please take these patches? I support them.
Review of attachment 255145 [details] [review]: Hi Khaled, sorry it took me so long to review this. Code looks mostly good, apart from some memory leaks that I found below. The original code for SushiFontWidget lives here [1] though, so any modification will need to be made on both copies. [1] https://git.gnome.org/browse/sushi/tree/src/libsushi/sushi-font-widget.c ::: src/sushi-font-widget.c @@ +102,3 @@ + * pango will split runs at font change */ + context = pango_cairo_create_context (cr); + gint i; This should be free-d with pango_attr_list_unref() @@ +108,3 @@ + text, 0, strlen (text), + attr_list, NULL); + *glyphs = NULL; Missing space before paren. @@ +111,3 @@ + + /* reorder the items in the visual order */ + base_dir = pango_find_base_dir (text, -1); pango_reorder_items() will return a new list, so here you're leaking the original items list. @@ +153,3 @@ + } + + Can use g_list_free_full() here instead.
Review of attachment 255146 [details] [review]: Looks good to me.
Created attachment 266217 [details] [review] Patch using HarfBuzz, V2 Here a new patch that should address the comments above.
Review of attachment 266217 [details] [review]: This looks good to me, thanks for the quick update. Do you have a GNOME git account, or should I push these for you?
(In reply to comment #26) > Review of attachment 266217 [details] [review]: > > This looks good to me, thanks for the quick update. > Do you have a GNOME git account, or should I push these for you? I have an account, should I push both commits?
Pushed both patches.