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

 
 

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

Submitted by:  None
Submitted on:  Thu 09 Oct 2014 06:12:18 PM UTC  
 
Severity: 3 - NormalItem Group: Incorrect behaviour
Status: FixedPrivacy: Public
Assigned to: Werner LEMBERG <wl>Open/Closed: Closed
Planned Release: 2.5.4

Add a New Comment(Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

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>
Project AdministratorIn charge of this item.
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:

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>
Project AdministratorIn charge of this item.
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.)

Attach File(s):
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

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

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 4 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Sun 12 Oct 2014 06:34:31 AM UTCwlStatusNone=>Fixed
      Assigned toNone=>wl
      Open/ClosedOpen=>Closed
      Planned ReleaseNone=>2.5.4

    Back to the top


    Powered by Savane 3.1-cleanup1