Some public APIs using FT_Int/FT_UInt should be improved in future (Re: [ft-devel] Atari/PureC port status)

[mpsuzuki]

Hi all,

Just I've finished the outline to clear implicit cast warnings
on LP32 and LP64 platforms by AtariST/PureC and Mac OS X/gcc
for ppc64. The whole patch is attached, but it's too large for
quick review, I will split it to atomic bits with detaileddescription.

# for brief tracking, please check the revision log in GNU arch
# repository:
#
#       archive URL:
#       http://gyvern.ipc.hiroshima-u.ac.jp/~mpsuzuki/arch-2007.03.14
#
#       version name:
#       freetype2--mps-osx-lp64--0.2.1.5
#
# there are 60 atomic changesets. each changesets are designed
# to insert no new warnings.

During the checking of Atari port, I found that some public
APIs use FT_Int/FT_UInt to interchange the values greater
than 16bit. There are 4 types:

------------------------------------------------------------------
A) Some 32bit (or more, in future extension?) flags are
   interchanged by FT_UInt.

   Example: FT_Get_Advance(), FT_Get_Advance() receive
   flags by FT_UInt type, but FT_ADVANCE_FLAG_FAST_ONLY
   (=0x20000000UL) etc cannot be passed by FT_UInt on 16bit
   system.

   Although the extension from FT_UInt to portable 32bit
   integer makes 16bit systems troubled, I think it
   should be extended. To keep ILP32/LP64 systems unchanged,
   using FT_UInt32 would be most appropriate.

------------------------------------------------------------------
B) Some integer values derived from FT_Pos are interchanged
   by FT_Int.

   Example: FT_Bitmap_Convert() called by FT_Bitmap_Embolden()

   FT_Bitmap_Embolden() receive the strength of embolding
   by FT_Pos-typed variables: xStrength and yStrength
   (3rd and 4th arguments). In FT_Bitmap_Embolden(),
   they are rounded to 2 integer variables like:

    FT_Int          xstr, ystr;

    [snip]

    xstr = FT_PIX_ROUND( xStrength ) >> 6;
    ystr = FT_PIX_ROUND( yStrength ) >> 6;

   As ftbitmap.h tells, x/yStrength are 26.6 fixed point
   floatings, so x/ystr should cover 26bit integer, so
   FT_Int on 16bit system cannot cover. Afterwards, an
   alignment to pass FT_Bitmap_Convert() is calculated
   aslike:

    switch ( bitmap->pixel_mode )
    {
    case FT_PIXEL_MODE_GRAY2:
    case FT_PIXEL_MODE_GRAY4:
      {
        FT_Bitmap  tmp;
        FT_Int     align;


        if ( bitmap->pixel_mode == FT_PIXEL_MODE_GRAY2 )
          align = ( bitmap->width + xstr + 3 ) / 4;
        else
          align = ( bitmap->width + xstr + 1 ) / 2;

        FT_Bitmap_New( &tmp );

        error = FT_Bitmap_Convert( library, bitmap, &tmp, align );

    So, changing the internal variables x/ystr is insufficient,
    it is expected to extend the 4th argument of FT_Bitmap_Convert()
    to cover 26bit integer (FT_Int on 16bit system is insufficient).

    However, I think it's not reasonable to pass such a large
    value greater than 16bit range. I want to insert the warnings
    for overflow cases and keep the APIs as they are, and
    discuss this issue again in future when incompatible API
    update is scheduled.

------------------------------------------------------------------
C) Most APIs interchange the glyph index by FT_UInt, although
   the number of glyphs in a face is typed as FT_Long.

   Most public APIs, FT_Load_Glyph(), FT_Get_Glyph_Name() etc
   receive the glyph index by FT_UInt. On 16bit system, using
   FT_UInt restricts the glyph index to 16bit, although the
   number of glyphs (FT_Long) is still 32bit. I think using
   FT_ULong (or FT_Long) would be more consistent.

   But, BDF/PCF are only font formats that can bring 32bit
   glyph index to FT2, so it is questionable if the extension
   of glyph index in public APIs is so worthful that we decide
   to break the API compatibility on 16bit platform.

   # TrueType/OpenType cmap 10/12/14 can store 32bit glyph
   # index internally, but public glyph index of TrueType/
   # OpenType is still restricted to 16bit, I think.
   # Some people insist as they saw Chinese TTFs included
   # the glyphs more than 64k, but I've never seen.

------------------------------------------------------------------
D) Some bitmap-image related properties mix int and long types.

   Example: pfr_slot_load_bitmap() versus FT_GlyphSlot

   FT_GlyphSlot has 2 integer parameters:

                typedef struct  FT_GlyphSlotRec_
                {

                [snip]

                        FT_Int  bitmap_left;
                        FT_Int  bitmap_top;

                [snip]

                } FT_GlyphSlotRec;

   FT_Bitmap members are similar, but more directly
   (not FT_Int but int)

                typedef struct  FT_Bitmap_
                {
                        int  rows;
                        int  width;
                        int  pitch;

                [snip]

 
   But pfr_slot_load_bitmap() tries to write FT_Long values
   to there.

        /* get the bitmap metrics */
        {
                FT_Long  xpos, ypos, advance;
                FT_UInt  xsize, ysize, format;
                FT_Byte*  p;

        [snip]

                error = pfr_load_bitmap_metrics( &p, stream->limit,
                                advance,
                                &xpos, &ypos,
                                &xsize, &ysize,
                                &advance, &format );
                if ( !error )
                {
                        glyph->root.bitmap.width = (FT_Int)xsize;
                        glyph->root.bitmap.rows  = (FT_Int)ysize;
???===>                 glyph->root.bitmap.pitch = (FT_Long)( xsize + 7 ) >> 3;

                [snip]

======>                 glyph->root.bitmap_left = xpos;
======>                 glyph->root.bitmap_top  = ypos + ysize;

   I'm not sure why pitch is casted to FT_Long.

   According to PFR spec sheet, theoretically PFR can
   include extremely high resolution bitmap upto 24bit
   length (16.7M pixel, I don't think it's realistic),
   x/ypos must be typed as long (or greater). This is big
   contrast with embedded bitmap TrueType limited to 8bit
   length.

   PFR is the exceptional format which is capable to such
   extreme case? The BDF/PCF drivers in FreeType2 restricts
   bounding box properties to signed short, like this:

        /* Expect the BBX field next. */
        if ( ft_memcmp( line, "BBX", 3 ) == 0 )
        {

                [snip]
                glyph->bbx.width    = _bdf_atos( p->list.field[1], 0, 10 );
                glyph->bbx.height   = _bdf_atos( p->list.field[2], 0, 10 );
                glyph->bbx.x_offset = _bdf_atos( p->list.field[3], 0, 10 );
                glyph->bbx.y_offset = _bdf_atos( p->list.field[4], 0, 10 );

   I think BDF spec sheet does not restrict these properties
   to 16bit integer, but these restrictions were introduced
   for pragmatic simplification. I don't think there's urgent
   need to extend them to long (the maximum integer type which
   is portable in ANSI-C).

   In summary, PFR is the only implementation I could find,
   which should store pixel size in 32bit and requests the
   32bit extension of FT_GlyphSlot and FT_Bitmap (it does
   not change anything for ILP32 systems, but 16bit system
   would be changed). But, it is very questionable if the
   support of huge bitmap in PFR is worthful for 16bit
   platforms, in comparison with breaking binary compatibility
   with previous releases. Breaking the compatibility of
   public data type (not only function interface) would be
   a remarkable impact. Thus, I want to keep the public
   types as they are, and I want to insert the warnings for
   the overflow cases.
------------------------------------------------------------------

About the discussion to extend FT_UInt to FT_UInt32 (or FT_ULong)
for case B, C and D, I want to postpone them until the day when FT2
or FT3 define new API set including incompatible changes. But the
case A is exceptional, because this is a bug that a FT2 client
cannot set the flags appropriately.

Please let me hear the comments! If breaking compatibility should be
avoided even in case A, I propose to insert some C preprocessor
macro for 16bit systems to prevent passing the overflowing flags.

Regards,
mpsuzuki

lp32_lp64.patch.bz2
Description: Binary data