Re: GDI32: [1/5] freetype.c add more font selecting&linking mechanism support.

[Huw D M Davies]

On Tue, Mar 07, 2006 at 09:10:12PM +0900, Byeong-Sik Jeon wrote:
> ChangeLog:
>    init_system_links support .ttc file.
>    add "Tahoma" registry value for CJK font link.
>    find_face_from_filename support .ttc file

I think you could spilt out the 'add Tahoma registry value' bit from
this patch to make it a little smaller.
> --- 000/dlls/gdi/freetype.c	2006-03-07 19:09:53.000000000 +0900
> +++ 001/dlls/gdi/freetype.c	2006-03-07 20:28:14.000000000 +0900
> @@ -909,14 +935,16 @@
>      SYSTEM_LINKS *font_link, *system_font_link;
>      CHILD_FONT *child_font;
>      static const WCHAR Tahoma[] = {'T','a','h','o','m','a',0};
> - static const WCHAR tahoma_ttf[] = {'t','a','h','o','m','a','.','t','t','f',0};
> - static const WCHAR System[] = {'S','y','s','t','e','m',0};
> - static const char Tahoma_ttf[] = {'t','a','h','o','m','a','.','t','t','f',0};

Why don't you just use "tahoma.ttf" in the code where you call
find_face_from_filename, there's no point in initializing a char
string like this.
> @@ -990,15 +1046,15 @@
>         that Tahoma has */
>  system_font_link = HeapAlloc(GetProcessHeap(), 0, sizeof(*system_font_link));
> - system_font_link->font_name = strdupW(System);
> - system_font_link->font_name = strdupW(defSystem);

Please don't use defSystem here.  That's supposed to be a fallback for
System.  Please leave it as it was.

GDI32: [1/5] freetype.c add more font selecting&linking mechanism support.

[Byeong-Sik Jeon]

2006-03-07 (화), 14:22 +0000, Huw D M Davies 쓰시길:
> On Tue, Mar 07, 2006 at 09:10:12PM +0900, Byeong-Sik Jeon wrote:
> > ChangeLog:
> >    init_system_links support .ttc file.
> >    add "Tahoma" registry value for CJK font link.
> >    find_face_from_filename support .ttc file
>
> I think you could spilt out the 'add Tahoma registry value' bit from
> this patch to make it a little smaller.

Yes. I understand. add_reg_value_system_font_link() will be removed.
> > --- 000/dlls/gdi/freetype.c	2006-03-07 19:09:53.000000000 +0900
> > +++ 001/dlls/gdi/freetype.c	2006-03-07 20:28:14.000000000 +0900
> > @@ -909,14 +935,16 @@
> >      SYSTEM_LINKS *font_link, *system_font_link;
> >      CHILD_FONT *child_font;
> >      static const WCHAR Tahoma[] = {'T','a','h','o','m','a',0};
> > - static const WCHAR tahoma_ttf[] = {'t','a','h','o','m','a','.','t','t','f',0};
> > - static const WCHAR System[] = {'S','y','s','t','e','m',0};
> > - static const char Tahoma_ttf[] = {'t','a','h','o','m','a','.','t','t','f',0};
>
> Why don't you just use "tahoma.ttf" in the code where you call
> find_face_from_filename, there's no point in initializing a char
> string like this.

Yes. will be change to   static const char Tahoma_ttf[] = "tahama.ttf";
> > @@ -990,15 +1046,15 @@
> >         that Tahoma has */
> >  system_font_link = HeapAlloc(GetProcessHeap(), 0, sizeof(*system_font_link));
> > - system_font_link->font_name = strdupW(System);
> > - system_font_link->font_name = strdupW(defSystem);
>
> Please don't use defSystem here.  That's supposed to be a fallback for
> System.  Please leave it as it was.

I can't understand. defSans, defFixed, defSerif is already using in the WineEngCreateFontInstance().
this is different case?

Thanks.

GDI32: [1/5] freetype.c add more font selecting&linking mechanism support.

[Huw D M Davies]

On Tue, Mar 07, 2006 at 11:58:01PM +0900, Byeong-Sik Jeon wrote:
> 2006-03-07 (화), 14:22 +0000, Huw D M Davies 쓰시길:
> > Why don't you just use "tahoma.ttf" in the code where you call
> > find_face_from_filename, there's no point in initializing a char
> > string like this.
>
> Yes. will be change to   static const char Tahoma_ttf[] = "tahama.ttf";

Just remove the variable completely and call find_face_from_filenae("tahoma.ff")
> > > - system_font_link->font_name = strdupW(System);
> > > - system_font_link->font_name = strdupW(defSystem);
> >
> > Please don't use defSystem here.  That's supposed to be a fallback for
> > System.  Please leave it as it was.
>
> I can't understand. defSans, defFixed, defSerif is already using in the WineEngCreateFontInstance().
> this is different case?

The def* strings are supposed to be the default fallback names for the
various fonts.  Here you just want to use the string "System", so
please leave it as it was.

Huw.

GDI32: [1/5] freetype.c add more font selecting&linking mechanism support.

[Huw D M Davies]

On Tue, Mar 07, 2006 at 03:01:34PM +0000, Huw D M Davies wrote:
> The def* strings are supposed to be the default fallback names for the
> various fonts.  Here you just want to use the string "System", so
> please leave it as it was.

Actually, what you really want to use is SystemW[].  The def* strings
should go at some point, but that's another story.

Huw.