bugThe FreeType Project - Bugs: bug #43392, load glyph can return...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

bug #43392: load glyph can return uninitialized vertical metric/bbox data

Submitter:  None
Submitted:  Thu 09 Oct 2014 06:12:18 PM UTC
   
 
Severity:  3 - Normal Item Group:  Incorrect behaviour
Status:  Fixed Privacy:  Public
Assigned to:  wl Open/Closed:  Closed
Planned Release:  2.5.4

Thu 16 Oct 2014 05:22:17 AM UTC, comment #3: 

I no longer can remember why I've set `linearVertAdvance' in that way :-)  I'm thus following your advice, changing the code accordingly, thanks.

Werner LEMBERG <wl>
Group administrator
Tue 14 Oct 2014 10:17:40 PM UTC, comment #2: 

The patch takes care of the garbage data, but doesn't address the inconsistency with FT_Get_Advance.  These are still returning different values. With the new code the vertical advance is too small, and the glyphs overlap slightly.

The approach I used was to set vertAdvance = 0 (previously, the default value coming in accidentally was 0) in ttsbit.c.

Then, in TT_Load_Glyph, I make two changes.  I change the computation of linearVertAdvance to set linearVertAdvance = loader.vadvance, and then add an additional 'sanity check' for vertical metrics paralleling the one for horiAdvance:


if ( !glyph->metrics.vertAdvance && glyph->linearVertAdvance )
  glyph->metrics.vertAdvance =
    FT_MulFix( glyph->linearVertAdvance,
               size->root.metrics.y_scale );


This results in a vertAdvance that matches that returned from FT_Get_Advance, in this font at any rate.

I'm not sure why the linearVertAdvance is currently being computed as it is.  The value I get from the existing computation seems incorrect, its ~ -135 units. This code was introduced in commit f186ba682d72cc38bc0dc09b07cdd5ef6cf98482, but I don't know where it originated. Perhaps it deals with some other situation that this font doesn't encounter.

Doug Felt <dougfelt>
Sun 12 Oct 2014 06:34:31 AM UTC, comment #1: 

Thanks for your very detailed analysis.

I've implemented a fix in the git repository.  Please test whether this is sufficient.

Werner LEMBERG <wl>
Group administrator
Thu 09 Oct 2014 06:12:18 PM UTC, original submission:  

Although clients shouldn't be asking for vertical metrics from fonts that don't support it, this should not return random/unitialized values.

This can occur in FT 2.5.3 when requesting vertical metrics from a font with embedded sbit data and no vertical metrics tables.  One such font is kochi-mincho-subst.ttf.

The problem occurs in truetype load_sbit_image (ttgload.c). The sfnt's load_sbit_image function is called with an uninitialized TT_SBit_MetricsRec.  This can call through to tt_face_load_sbit_image in ttsbit.c.  If the table type is EBLC this calls tt_sbit_decoder_init, which sets the decoder to point at the uninitialized metrics struct, then calls tt_sbit_decoder_load_image. If the index format is 1, this does not load metrics directly, but goes to tt_sbit_decoder_load_bitmap. Here, if glyph_format is 2, this calls tt_sbit_decoder_load_metrics with 0 for 'big'. This sets the horizontal metrics in the decoder, but leaves the vertical metrics untouched, so they retain their uninitialized values.

Back in load_sbit_image, the error value is 0, so the metrics data, including the uninitialized vertical bearing and vertical advance, is copied into the glyph metrics.  If FT_LOAD_VERTICAL_LAYOUT has been called, the glyphs' bitmap_left/_top values are set with these uninitialized values as well.

When this returns to TT_Load_Glyph, if the font is scalable, a loader is initialized and the glyph outline bbox is read via TT_Load_Glyph_Header. Metrics are read there, this time using TT_Get_H/VMetrics, which in this case synthesizes vertical metrics from the os2 table.  These metrics are used to set the linear advances. There's a failsafe to set the horizontal advance if it is 0, but nothing to set the vertical advance, or the other metrics.

The end result is a glyph metrics field in which the vertical bearing and advance can be random values.  If the FT_LOAD_VERTICAL_LAYOUT flag is set, the bitmap_left and _top values are random as well.  This means that a subsequent call to FT_Glyph_Get_CBox, since this is a bitmap glyph, returns a garbage bounding box.

Note that FT_Get_Advance, if LOAD_ADVANCE_FAST_CHECK returns true, will return the advances from TT_Get_V/HMetrics.  This is why FT_Get_Advance will return a reasonable advance for vertical metrics from this font, though TT_Load_Glyph does not.

While it's incorrect for clients to ask for vertical metrics from fonts that do not support them, I believe it's an error to return uninitialized data, particularly with no warning. This is especially unexpected since FT_Get_Advance can be used to get a usable vertical advance in this case (e.g. in HarfBuzz).

Anonymous

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by dougfelt (Posted a comment)
  • -email is unavailable- added by wl (Posted a comment)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

     

    Follow 4 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2014-10-12 wl StatusNone Fixed
        Assigned toNone wl
        Open/ClosedOpen Closed
        Planned ReleaseNone 2.5.4

    Back to the top

    Powered by Savane 3.13-d3ae.
    Corresponding source code