Re: [Devel] FIX? [was: Non-integer bbox for T1 font?]

[Tom Kacvinsky]

I just looked at the use of T1_CoordArray: it was only used in parse_font_bbox.
The same function (under a different name: CID_ToFixedArray) is used in only
parse_font_bbox from the cid module.  So I think these can be replaced
to calls of {T1,CID}_ToFixedArray.  The only problem is this:

t1objs.c:      root->ascender     = (FT_Short)face->type1.font_bbox.yMax;
t1objs.c:      root->descender    = (FT_Short)face->type1.font_bbox.yMin;
t1objs.c:      root->height       = ( ( root->ascender - root->descender ) * 12 
) / 10;
t1objs.c:      root->max_advance_width = (FT_Short)face->type1.font_bbox.xMax;
t1objs.c:      root->max_advance_height = root->height;
t1objs.c:      root->height       = ( ( root->ascender - root->descender ) * 12 
) / 10;

These values are FT_Shorts, and as type1.font_bbox.yMax is an FT_Fixed, the cast
is *bad* thing to do.  The least significant byte (the fractional part) is used,
when the most significant byte (the integer part) should be used instead.

Perhaps root->ascender should be assigned as follows:

  root->ascender     = face->type1.font_bbox.yMax >> 16;

etc...

Thoughts?

On Tue, 6 Mar 2001, Tom Kacvinsky wrote:

> The FontBBox is is set up to take 16.16 fixed values.  However, the code (w/o 
> my
> patch) was taking in 16 bit shorts.  This is because T1_ToCoordArray 
> typescasts
> (resulting in a demotion -- ugh!) the results of t1_tofixed to a 16 bit short.
> Changing the code to use T1_ToFixedArray bypasses this typecast and uses the
> full 16.16 fixed numbers returned by t1_tofixed.  So that is the fix, I think.
>
> As an aside:
>
> I think that if T1_ToCoordArray is going to fill in arrays of 16 bit shorts, 
> the
> code where the assignment is made should take the results of t1_tofixed, shift
> right 16 bits, and with 0xFFFF, cast the result to a 16 bit short and then 
> make
> the assignment...
>
> But I need to investigate more and see exactly how T1_ToCoordArray is used.
>
> On Tue, 6 Mar 2001, Werner LEMBERG wrote:
>
> >
> > > Attached is patch fro t1load.c that places 16.16 fixed numbers (16
> > > bits of integer, 16 bits of fraction) into the font bbox array.  To
> > > get the integer part in your code, just shift right 16 bits.  There
> > > will be some rounding errors (rounding up/down when it should be
> > > vice versa), but i imagine you can get around that...
> > >
> > > Let me know if that fixes your problem.  I would appreciate comments
> > > from others as well.
> >
> > You say that non-integer bboxes are valid, so I think it would be a
> > good idea to have that in FreeType also...  Feel free to fix that :-)
> >
>
>
> _______________________________________________
> Devel mailing list
> address@hidden
> http://www.freetype.org/mailman/listinfo/devel
>