Re: Minor logging improvements to freetype.c

[Huw D M Davies]

On Mon, May 31, 2004 at 09:47:35PM +0100, Mike Hearn wrote:
> Mike Hearn 
> Change some TRACEs into WARNs, WARN on skipping font due to wrong format.
> Index: dlls/gdi/freetype.c
>
> RCS file: /home/wine/wine/dlls/gdi/freetype.c,v
> retrieving revision 1.60
> diff -u -r1.60 freetype.c
> --- dlls/gdi/freetype.c	20 Apr 2004 01:12:18 -0000	1.60
> +++ dlls/gdi/freetype.c	31 May 2004 20:47:29 -0000
> @@ -316,20 +316,21 @@
>    }
>
> if(!FT_IS_SFNT(ft_face)) { /* for now we'll skip everything but TT/OT */
> -
>        WARN("skipping font file %s, not TrueType/OpenType font\n", debugstr_a(file));
>  pFT_Done_Face(ft_face);
>  return FALSE;
> }
>  if(!pFT_Get_Sfnt_Table(ft_face, ft_sfnt_os2) ||
>     !pFT_Get_Sfnt_Table(ft_face, ft_sfnt_hhea) ||
>           !(pHeader = pFT_Get_Sfnt_Table(ft_face, ft_sfnt_head))) {
> -
>    TRACE("Font file %s lacks either an OS2, HHEA or HEAD table.\n"
> -
>    WARN("Font file %s lacks either an OS2, HHEA or HEAD table.\n"
> "Skipping this font.\n", debugstr_a(file));
>  pFT_Done_Face(ft_face);
>  return FALSE;
>
> }
>    if(!ft_face->family_name || !ft_face->style_name) {
> -
>        TRACE("Font file %s lacks either a family or style name\n", debugstr_a(file));
> -
>        WARN("Font file %s lacks either a family or style name\n", debugstr_a(file));
>        pFT_Done_Face(ft_face);
>        return FALSE;
>    }

You could argue that these three should really be FIXMEs
> @@ -369,13 +370,13 @@
>                        (*insertface)->font_version,  pHeader->Font_Revision);
>              if(fake_family) {
> -
>                TRACE("This font is a replacement but the original really exists, so we'll skip the replacement\n");
> -
>                WARN("This font is a replacement but the original really exists, so we'll skip the replacement\n");
>                HeapFree(GetProcessHeap(), 0, StyleW);
>                pFT_Done_Face(ft_face);
>                return FALSE;
>            }
>            if(pHeader->Font_Revision <= (*insertface)->font_version) {
> -
>                TRACE("Original font is newer so skipping this one\n");
> -
>                WARN("Original font is newer so skipping this one\n");
>                HeapFree(GetProcessHeap(), 0, StyleW);
>                pFT_Done_Face(ft_face);
>                return FALSE;

I don't think these two should be warnings.  It's perfectly possible
to have two copies of the same font installed and you don't want to
confuse users by issuing warnings about something where we actually do
the right thing.

Huw.

Minor logging improvements to freetype.c

[Mike Hearn]

On Tue, 01 Jun 2004 11:42:56 +0100, Huw D M Davies wrote:
> You could argue that these three should really be FIXMEs

My reading of the code is that these indicate bodged/corrupt fonts rather
than known bugs in the Wine code. Am I wrong?
> I don't think these two should be warnings.  It's perfectly possible to
> have two copies of the same font installed and you don't want to confuse
> users by issuing warnings about something where we actually do the right
> thing.

Users don't see warnings normally anyway, I changed them because
sometimes I do a warn+all trace to see if anything shows up and the
program using different fonts to what I (perhaps) expect might qualify.

Well, I am not all that bothered really :) Alexandre can decide.

thanks -mike

Minor logging improvements to freetype.c

[Huw D M Davies]

On Tue, Jun 01, 2004 at 07:09:16PM +0100, Mike Hearn wrote:
> On Tue, 01 Jun 2004 11:42:56 +0100, Huw D M Davies wrote:
> > You could argue that these three should really be FIXMEs
>
> My reading of the code is that these indicate bodged/corrupt fonts rather
> than known bugs in the Wine code. Am I wrong?

No, these trigger when either the font isn't a ttf or if the font is
missing some tables that Wine relies on.  Windows is better at dealing
with this than we are and so it really is a Wine bug.
> > I don't think these two should be warnings.  It's perfectly possible to
> > have two copies of the same font installed and you don't want to confuse
> > users by issuing warnings about something where we actually do the right
> > thing.
>
> Users don't see warnings normally anyway, I changed them because
> sometimes I do a warn+all trace to see if anything shows up and the
> program using different fonts to what I (perhaps) expect might qualify.

I'd still argue that these should be TRACEs, since the situation is
perfectly normal and we handle the case correctly.

Huw.

Minor logging improvements to freetype.c

[Mike Hearn]

On Tue, 2004-06-01 at 20:45 +0100, Huw D M Davies wrote:
> No, these trigger when either the font isn't a ttf or if the font is
> missing some tables that Wine relies on.  Windows is better at dealing
> with this than we are and so it really is a Wine bug.

OK, I understand now.
> I'd still argue that these should be TRACEs, since the situation is
> perfectly normal and we handle the case correctly.

In that case Alexandre, disregard that patch.