Re: [E-devel] E CVS: libs/imlib2 raster
On Sat, 19 May 2007 20:34:41 +0200 Kim Woelders [EMAIL PROTECTED] babbled:
thats fine. problems didnt show up in my initial quick test, so i put the patch
in. i did wait until after aspara to get it time to settle and test :)
Sorry, but I'm backing this out.
After having spent about half a day trying to fix it I give up.
There is a problem with font sizing, at least for certain fonts, e.g.
/usr/share/e16/E-docs/rothwell.ttf, where the font size seems to get
scaled down somewhere along the way.
I got as far as to find out that if I remove the call to
FT_Set_Char_Size in imlib_font_find_face_in_fontset, things seem to
work, with one font in a font set, anyway.
I have no idea what I'm doing, so I think the author should have a look
at it.
Was this benchmarked in any way? I have a suspicion that
imlib_font_find_face_in_fontset is called all over the place and does
somewhat more work than the corresponding old code.
Also, I find the logic in imlib_font_find_face_in_fontset somewhat
unclear:
imlib_font_find_face_in_fontset()
{
...
repeat_upto_invalid:
while(something1)
{
maybe return;
}
if (something2)
goto repeat_upto_invalid;
return imlib_font_find_face_in_fontset() -- recursive!!!
}
If this patch goes in again I strongly suggest to first
1) Make it work.
2) Clean up the code logic so it's possible to actually read
what is going on.
3) Fix up indentation so that the code can actually be read.
4) Fix up indentation so it's consistent with the rest of imlib2.
Furthermore I suggest to
5) Many places, if not all - Not change fn to fn_list.
This patch makes ImlibFont a list by nature, and changing
the parameter name most places just adds noise making it
harder to evaluate the changes.
Finally I suggest to
6) Commit unrelated changes separately.
The polygon.c change is (I guess) about removing some (probably
bogus?) warnings.
7) Not make useless changes in unrelated code.
In color_helpers.c math.h is already included by color_helpers.h.
raster - You are most likely busy elsewhere. If you like I can handle
things if winfred(?) wants to have another go at it.
/Kim
Enlightenment CVS wrote:
Enlightenment CVS committal
Author : raster
Project : e17
Module : libs/imlib2
Dir : e17/libs/imlib2/src/lib
Modified Files:
api.c color_helpers.c font.h font_draw.c font_load.c
font_main.c font_query.c polygon.c
Log Message:
fontset patch from winfred
===
RCS file: /cvs/e/e17/libs/imlib2/src/lib/api.c,v
retrieving revision 1.11
retrieving revision 1.12
diff -u -3 -r1.11 -r1.12
--- api.c 15 Feb 2007 04:15:03 - 1.11
+++ api.c 6 May 2007 13:54:43 - 1.12
@@ -3078,16 +3078,21 @@
* @return NULL if no font found.
*
* Loads a truetype font from the first directory in the font path that
- * contains that font. The font name @p font_name format is
font_name/size. For
- * example. If there is a font file called blum.ttf somewhere in the
+ * contains that font. The font name @p font_name format is
font_name/size
+ * or a comma separated list of font_name/size elements.
+ * For example: if there is a font file called blum.ttf somewhere in the
* font path you might use blum/20 to load a 20 pixel sized font of
- * blum. If the font cannot be found NULL is returned.
+ * blum;nbsp; or blum/20, vera/20, mono/20 to load one or all fonts
+ * of the comma separated list. If the font cannot be found NULL is
returned.
*
**/
EAPI Imlib_Font
imlib_load_font(const char *font_name)
{
- return imlib_font_load_joined(font_name);
+ if(strchr(font_name, ',') != NULL)
+ return imlib_font_load_fontset(font_name);
+ else
+ return imlib_font_load_joined(font_name);
}
/**
@@ -3096,11 +3101,20 @@
EAPI void
imlib_free_font(void)
{
+ ImlibFont *fn;
+
if (!ctx)
ctx = imlib_context_new();
CHECK_PARAM_POINTER(imlib_free_font, font, ctx-font);
- imlib_font_free(ctx-font);
- ctx-font = NULL;
+
+ fn = (ImlibFont*)ctx-font; ctx-font = NULL;
+
+ if(fn-next_in_set == NULL)
+ {
+ imlib_font_free(fn);
+ return;
+ }
+ imlib_font_free_fontset(fn);
}
/**
===
RCS file: /cvs/e/e17/libs/imlib2/src/lib/color_helpers.c,v
retrieving revision 1.3
retrieving revision 1.4
diff -u -3 -r1.3 -r1.4
--- color_helpers.c 23 Feb 2005 03:02:41 - 1.3
+++ color_helpers.c 6 May 2007 13:54:43 - 1.4
@@ -1,3 +1,4 @@
+#include math.h
#include color_helpers.h
/*
* Color space conversion helper routines
===
RCS file:
Re: [E-devel] E CVS: libs/imlib2 raster
Sorry, but I'm backing this out.
After having spent about half a day trying to fix it I give up.
There is a problem with font sizing, at least for certain fonts, e.g.
/usr/share/e16/E-docs/rothwell.ttf, where the font size seems to get
scaled down somewhere along the way.
I got as far as to find out that if I remove the call to
FT_Set_Char_Size in imlib_font_find_face_in_fontset, things seem to
work, with one font in a font set, anyway.
I have no idea what I'm doing, so I think the author should have a look
at it.
Was this benchmarked in any way? I have a suspicion that
imlib_font_find_face_in_fontset is called all over the place and does
somewhat more work than the corresponding old code.
Also, I find the logic in imlib_font_find_face_in_fontset somewhat
unclear:
imlib_font_find_face_in_fontset()
{
...
repeat_upto_invalid:
while(something1)
{
maybe return;
}
if (something2)
goto repeat_upto_invalid;
return imlib_font_find_face_in_fontset() -- recursive!!!
}
If this patch goes in again I strongly suggest to first
1) Make it work.
2) Clean up the code logic so it's possible to actually read
what is going on.
3) Fix up indentation so that the code can actually be read.
4) Fix up indentation so it's consistent with the rest of imlib2.
Furthermore I suggest to
5) Many places, if not all - Not change fn to fn_list.
This patch makes ImlibFont a list by nature, and changing
the parameter name most places just adds noise making it
harder to evaluate the changes.
Finally I suggest to
6) Commit unrelated changes separately.
The polygon.c change is (I guess) about removing some (probably
bogus?) warnings.
7) Not make useless changes in unrelated code.
In color_helpers.c math.h is already included by color_helpers.h.
raster - You are most likely busy elsewhere. If you like I can handle
things if winfred(?) wants to have another go at it.
/Kim
Enlightenment CVS wrote:
Enlightenment CVS committal
Author : raster
Project : e17
Module : libs/imlib2
Dir : e17/libs/imlib2/src/lib
Modified Files:
api.c color_helpers.c font.h font_draw.c font_load.c
font_main.c font_query.c polygon.c
Log Message:
fontset patch from winfred
===
RCS file: /cvs/e/e17/libs/imlib2/src/lib/api.c,v
retrieving revision 1.11
retrieving revision 1.12
diff -u -3 -r1.11 -r1.12
--- api.c 15 Feb 2007 04:15:03 - 1.11
+++ api.c 6 May 2007 13:54:43 - 1.12
@@ -3078,16 +3078,21 @@
* @return NULL if no font found.
*
* Loads a truetype font from the first directory in the font path that
- * contains that font. The font name @p font_name format is
font_name/size. For
- * example. If there is a font file called blum.ttf somewhere in the
+ * contains that font. The font name @p font_name format is font_name/size
+ * or a comma separated list of font_name/size elements.
+ * For example: if there is a font file called blum.ttf somewhere in the
* font path you might use blum/20 to load a 20 pixel sized font of
- * blum. If the font cannot be found NULL is returned.
+ * blum;nbsp; or blum/20, vera/20, mono/20 to load one or all fonts
+ * of the comma separated list. If the font cannot be found NULL is
returned.
*
**/
EAPI Imlib_Font
imlib_load_font(const char *font_name)
{
- return imlib_font_load_joined(font_name);
+ if(strchr(font_name, ',') != NULL)
+ return imlib_font_load_fontset(font_name);
+ else
+ return imlib_font_load_joined(font_name);
}
/**
@@ -3096,11 +3101,20 @@
EAPI void
imlib_free_font(void)
{
+ ImlibFont *fn;
+
if (!ctx)
ctx = imlib_context_new();
CHECK_PARAM_POINTER(imlib_free_font, font, ctx-font);
- imlib_font_free(ctx-font);
- ctx-font = NULL;
+
+ fn = (ImlibFont*)ctx-font; ctx-font = NULL;
+
+ if(fn-next_in_set == NULL)
+ {
+ imlib_font_free(fn);
+ return;
+ }
+ imlib_font_free_fontset(fn);
}
/**
===
RCS file: /cvs/e/e17/libs/imlib2/src/lib/color_helpers.c,v
retrieving revision 1.3
retrieving revision 1.4
diff -u -3 -r1.3 -r1.4
--- color_helpers.c 23 Feb 2005 03:02:41 - 1.3
+++ color_helpers.c 6 May 2007 13:54:43 - 1.4
@@ -1,3 +1,4 @@
+#include math.h
#include color_helpers.h
/*
* Color space conversion helper routines
===
RCS file: /cvs/e/e17/libs/imlib2/src/lib/font.h,v
retrieving revision 1.1
retrieving revision 1.2
diff -u -3 -r1.1 -r1.2
--- font.h1 Nov 2004 09:45:31 - 1.1
+++ font.h6 May 2007 13:54:43 - 1.2
@@ -50,6 +50,7 @@
int references;
+ struct _Imlib_Font *next_in_set;
};
struct _Imlib_Font_Glyph
@@ -83,6 +84,12 @@
void
