(partial patch) Fixing some issues with fontconfig
Dale Magee <[email protected]> Mon, 8 Oct 2018 15:04:39 +1100
| Newsgroups | gmane.comp.video.xine.devel |
|---|---|
| Message-ID | <[email protected]> |
Hi
I've just been messing around with tracking down the cause and trying to
fix a couple of issues I've found WRT the way fontconfig works in xine-lib.
The attached patch is what I think it should be doing, but I'm no C
coder and this code causes a segfault at the moment. I've tried a bunch
of different things to fix it but I obviously don't understand what's
going on and I've run out of time and patience today (it's now 2am). If
somebody could explain why it causes a segfault and how to fix that I'd
be very grateful as this might help me understand C better (particularly
memory allocation and freeing) :)
This patch addresses 2 issues that I've discovered when compiling with
fontconfig enabled:
1. When using fontconfig it's not possible for xine to use the
cetus font for OSD, because osd_set_font tries to load a font using
freetype (and hence fontconfig) first. When osd_set_font_freetype2
passes 'cetus' as the font name to osd_lookup_fontconfig, fontconfig
loads a system default font because it can't find a font named cetus.
This means that play/pause icons in the OSD are replaced with the
corresponding characters (">" and "<").
2. When compiled with fontconfig you can not use a filename in
subtitles.separate.font_freetype, because when this filename is passed
to fontconfig it looks by font name rather than filename and returns the
system default font.
Here's what my patch is trying to do:
0. abstract out the bit of osd_set_font which searches for a matching
native font into a new function osd_find_native_font (to be DRY).
1. osd_set_font_freetype2 first tries to use fontname as a filename and
load it (to support filenames in subtitles.separate.font_freetype).
2. osd_set_font_freetype2 then looks for a native xine font with the
provided name. If it finds it, it immediately returns 0 so that
osd_set_font can load the native xine font (i.e xine fonts are now used
as preference over freetype fonts. This should be fine because according
to the FAQ the native fonts are better. e.g for subtitles they have
borders).
3. if the 2 above steps fail, osd_set_font_freetype2 passes the font
name to fontconfig, which may find the font or return the system default
font (i.e previous behaviour).
I'd love to hear any thoughts and feedback.
Thanks for your time :)
Dale Magee
_______________________________________________
xine-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/xine-devel
fontconfig_behaviour.patch
(text/x-patch, 3.7 KB)
--- a/src/xine-engine/osd.c
+++ b/src/xine-engine/osd.c
@@ -44,9 +44,7 @@
#define LOG_MODULE "osd"
#define LOG_VERBOSE
-/*
#define LOG
-*/
#include <xine/xine_internal.h>
#include "xine-engine/bswap.h"
@@ -1013,6 +1011,30 @@ static int osd_renderer_unload_font(osd_renderer_t *this, char *fontname ) {
return ret;
}
+/*
+ try to find a native xine font
+*/
+static int osd_find_native_font(osd_object_t *osd, const char *fontname, int size,osd_font_t *font) {
+
+ int best = 0;
+ int found = 0;
+
+ font = osd->renderer->fonts;
+ while( font ) {
+
+ if( !strcasecmp(font->name, fontname) && (size>=font->size)
+ && (best<font->size)) {
+ found = 1;
+ best = font->size;
+ lprintf ("best: font->name=%s, size=%d\n", font->name, font->size);
+ }
+ font = font->next;
+ }
+ return found;
+
+}
+
+
#ifdef HAVE_FT2
# ifdef HAVE_FONTCONFIG
@@ -1046,6 +1068,7 @@ static int osd_lookup_fontconfig( osd_object_t *osd, const char *const fontname,
if ( fs->nfont != 0 ) {
FcChar8 *filename = NULL;
+
FcPatternGetString(fs->fonts[0], FC_FILE, 0, &filename);
if ( ! FT_New_Face(osd->ft2->library, (const char*)filename, 0, &osd->ft2->face) ) {
FcFontSetDestroy(fs);
@@ -1076,10 +1099,6 @@ static int osd_lookup_fontconfig( osd_object_t *osd, const char *const fontname,
static int osd_lookup_xdg( osd_object_t *osd, const char *const fontname ) {
const char *const *data_dirs = xdgSearchableDataDirectories(&osd->renderer->stream->xine->basedir_handle);
- /* try load font from current directory or from an absolute path */
- if ( FT_New_Face(osd->ft2->library, fontname, 0, &osd->ft2->face) == FT_Err_Ok )
- return 1;
-
if ( data_dirs )
while( (*data_dirs) && *(*data_dirs) ) {
FT_Error fte = FT_Err_Ok;
@@ -1116,8 +1135,28 @@ static int osd_set_font_freetype2( osd_object_t *osd, const char *fontname, int
FT_Done_Face (osd->ft2->face);
osd->ft2->face = NULL;
}
-
+
+ /*
+ for storing font found by osd_find_native_font, not used
+ */
+ osd_font_t *font = NULL;
+
do { /* while 0 */
+
+ /*
+ try to load font from current directory or from an absolute path
+ we want to do this before trying osd_lookup_fontconfig
+ (which doesn't handle filenames)
+ */
+ if ( FT_New_Face(osd->ft2->library, fontname, 0, &osd->ft2->face) == FT_Err_Ok )
+ break;
+
+ /*
+ don't use freetype if there's a native xine font which matches:
+ */
+ if (osd_find_native_font(osd,fontname,size,font))
+ return 0;
+
#ifdef HAVE_FONTCONFIG
if ( osd_lookup_fontconfig(osd, fontname, size) )
break;
@@ -1156,30 +1195,17 @@ static int osd_set_font( osd_object_t *osd, const char *fontname, int size) {
if ( ! osd_set_font_freetype2(osd, fontname, size) )
#endif
{ /* If the FreeType2 loading failed */
- osd_font_t *font;
- int best = 0;
+ osd_font_t *font = NULL;
osd->font = NULL;
- ret = 0;
-
- font = osd->renderer->fonts;
- while( font ) {
-
- if( !strcasecmp(font->name, fontname) && (size>=font->size)
- && (best<font->size)) {
- ret = 1;
- osd->font = font;
- best = font->size;
- lprintf ("best: font->name=%s, size=%d\n", font->name, font->size);
- }
- font = font->next;
- }
-
+ ret = osd_find_native_font(osd, fontname,size,font);
+
if( ret ) {
- /* load font if needed */
- if( !osd->font->loaded )
- ret = osd_renderer_load_font(osd->renderer, osd->font->filename);
- if(!ret)
- osd->font = NULL;
+ osd->font = font;
+ /* load font if needed */
+ if( !osd->font->loaded )
+ ret = osd_renderer_load_font(osd->renderer, osd->font->filename);
+ if(!ret)
+ osd->font = NULL;
}
}