bugThe FreeType Project - Bugs: bug #27364, vertical metrics calculation wrong

 
 

bug #27364: vertical metrics calculation wrong

Submitted by:  bram tassyns <bramt>
Submitted on:  Tue 01 Sep 2009 08:34:24 AM UTC  
 
Severity: 3 - NormalItem Group: Incorrect behaviour
Status: FixedPrivacy: Public
Assigned to: Werner LEMBERG <wl>Open/Closed: Closed
Planned Release: 2.3.10

Add a New Comment(Rich Markup)
   

You are not logged in

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

 

(Jump to the original submission Jump to the original submission)

Wed 02 Sep 2009 11:08:05 AM UTC, comment #12:

Thanks. I've extended it with comments from below. Now in git.

Werner LEMBERG <wl>
Project AdministratorIn charge of this item.
Wed 02 Sep 2009 08:03:54 AM UTC, comment #11:

:s

ok, let's try this once more.

ChangeLog:

Improved vertical metrics calculation #27364.

  • src/base/ftobjs.c (ft_synthesize_vertical_metrics): vertBearingX is now calculated as described by the Adobe PDF Spec. Estimate for vertBearingY now also takes small glyphs completely above or below the baseline into account.
  • src/cff/cffgload.c (cff_slot_load): vertBearingX is now calculated as described by the Adobe PDF Spec. Vertical metrics information was always ignored when FT_CONFIG_OPTION_OLD_INTERNALS was not defined.
  • src/truetype/ttgload.c (compute_glyph_metrics): vertBearingX is now calculated as described by the Adobe PDF Spec.
bram tassyns <bramt>
Wed 02 Sep 2009 07:51:30 AM UTC, comment #10:

The ChangeLog file :-)

Werner LEMBERG <wl>
Project AdministratorIn charge of this item.
Wed 02 Sep 2009 07:42:18 AM UTC, comment #9:

Which file ?

bram tassyns <bramt>
Wed 02 Sep 2009 07:23:51 AM UTC, comment #8:

Thanks. However, you mention that you've fixed a bug in handling the vmtx for CFF OTF...

Ideally, I would like to have a ChangeLog entry which follows the format given in this file. I know this is cumbersome, but it helps a lot later on.

Werner LEMBERG <wl>
Project AdministratorIn charge of this item.
Wed 02 Sep 2009 07:20:30 AM UTC, comment #7:

There were a few local changes that were not part of this fix that I removed from the v2 patch file.

ChangeLog:

Updated vertical metrics calculation:
- Values not defined by the font (like vertBearingX) are now calculated as described by the Adobe PDF spec.
- Estimate for vertBearingY for fonts without vertical metrics improved to take small glyphs completely above or below the baseline into account.

bram tassyns <bramt>
Wed 02 Sep 2009 07:09:28 AM UTC, comment #6:

This time, it has worked, thanks. Have you massaged the resulting diff file afterwards? At two places there were missing empty lines...

Please provide a ChangeLog entry (as a simple file or comment, no diff please), and I'll commit it.

BTW, `ftstring' supports vertical display of glyphs; try the `V' key. Actually, I've never used it, so maybe there are bugs lurking around.

Regarding the calculation of vertBearingY, I can't remember the exact details (it's David's code). If you find something simpler which works, please don't hesitate to fix it.

Werner LEMBERG <wl>
Project AdministratorIn charge of this item.
Wed 02 Sep 2009 06:31:27 AM UTC, comment #5:

Lets try this again :D, new patch attached.

(file #18669)

bram tassyns <bramt>
Tue 01 Sep 2009 08:58:38 PM UTC, comment #4:

Actually, I ask you to resend the patch -- it gets indeed applied incorrectly.

It's probably to easiest to compare the old and new directory, saying

diff -u -r <olddir> <newdir>

Werner LEMBERG <wl>
Project AdministratorIn charge of this item.
Tue 01 Sep 2009 08:51:58 PM UTC, comment #3:

Thanks. The most important option is `-u' to create a unified diff, providing some context (this is, lines before and after the change) which reduces the probability of applying the patch to the wrong spot. Another useful option is `-r to make diff compare two directories recursively.

`diff --help' should give you a list of all available options.

Werner LEMBERG <wl>
Project AdministratorIn charge of this item.
Tue 01 Sep 2009 05:24:28 PM UTC, comment #2:

Added the patch created by diff. I'm not used to using this tool though, what's the best way to create a diff file that you can apply directly to your source tree ?

(file #18667)

bram tassyns <bramt>
Tue 01 Sep 2009 03:29:19 PM UTC, comment #1:

This looks very promising! Thanks for your efforts.

Can you please post a patch, using `git diff', for example?

Werner LEMBERG <wl>
Project AdministratorIn charge of this item.
Tue 01 Sep 2009 08:34:24 AM UTC, original submission:

I've experienced some problems with the calculation of vertBearingX. The calculation of this value is not defined in the OTF font spec so FT does a 'best effort' attempt. However this value is defined in the PDF and postscript spec and that algorithm is better then the one FT currently uses.

FreeType: use the middle of the bounding box as the X coordinate of the vertical origin
Adobe PDF spec: use the middle of the horizontal advance vector as the X coordinate of the vertical origin

FreeType's algorithm goes wrong when you have a really small glyph (like the dot at the end of the sentence) with large bearings. With the FreeType algorithm this dot gets centered on the baseline, with the PDF algorithm it gets the correct location (in the top right). Note that this is a serious issue, it's like printing the dot at the end of a roman sentance at the center of the textline instead of on the baseline like it should. So i believe the PDF spec's algorithm should be used in FreeType as well. See fix below.

The vertBearingY for the glyph described above is also very strange if no vmtx information is present.
Since the height of the bbox is not representable for the height of the glyph visually (the whitespace up to the baseline is a part of the glyph). The fix below also includes some code for a better estimate for vertBearingY.

I also discovered another bug that causes the vmtx to be ignored for OTF CFF fonts. (That's why I noticed that the default vertBearingY was strange for fonts without vmtx :D).

I hope these changes will make vertical text a little more reliable.

Questions:

Is there a way to test the displaying of vertical metrics with only the utilities in ft2demos ?

Why is the calculation of vertBearingY and vertAdvance in ttgload.c so complex ? Can't the
block starting at 1586 be replaced by:

glyph->metrics.vertBearingY = loader->pp3.y - bbox.yMax;
glyph->metrics.vertAdvance = loader->pp3.y - loader->pp4.y;

(linear calculation is still necessary though)

Fixes:

cffgload.c:2714:

has_vertical_info = FT_BOOL( face->vertical_info &&
face->vertical.number_Of_VMetrics > 0 &&
face->vertical.long_metrics );

to

#ifdef FT_CONFIG_OPTION_OLD_INTERNALS

has_vertical_info = FT_BOOL( face->vertical_info &&
face->vertical.number_Of_VMetrics > 0 &&
face->vertical.long_metrics );

#else

has_vertical_info = FT_BOOL( face->vertical_info &&
face->vertical.number_Of_VMetrics > 0 );
#endif

cffgload.c:2807

if ( has_vertical_info )
metrics->vertBearingX = -metrics->width / 2;
else
ft_synthesize_vertical_metrics( metrics,
metrics->vertAdvance );

to

if ( has_vertical_info )
metrics->vertBearingX = metrics->horiBearingX - ( metrics->horiAdvance / 2 );
else
ft_synthesize_vertical_metrics( metrics,
metrics->vertAdvance );

ftobjs.c:2398

FT_BASE_DEF( void )
ft_synthesize_vertical_metrics( FT_Glyph_Metrics* metrics,
FT_Pos advance )
{
/* the factor 1.2 is a heuristical value */
if ( !advance )
advance = metrics->height * 12 / 10;

metrics->vertBearingX = -( metrics->width / 2 );
metrics->vertBearingY = ( advance - metrics->height ) / 2;
metrics->vertAdvance = advance;
}

to

FT_BASE_DEF( void )
ft_synthesize_vertical_metrics( FT_Glyph_Metrics* metrics,
FT_Pos advance )
{
int theHeight = metrics->height;

/* compensate for glyph with bbox above/below the baseline */
if ( 0 > metrics->horiBearingY )
{
if ( theHeight < metrics->horiBearingY )
theHeight = metrics->horiBearingY;
}
else if ( 0 < metrics->horiBearingY )
theHeight -= metrics->horiBearingY;

/* the factor 1.2 is a heuristical value */
if ( !advance )
advance = theHeight * 12 / 10;

metrics->vertBearingX = metrics->horiBearingX - ( metrics->horiAdvance / 2 );
metrics->vertBearingY = ( advance - theHeight ) / 2;
metrics->vertAdvance = advance;
}

ttgload.c:1581

cut and past the block from 1679 here to ensure all metrics besides the vertical ones are completely calculated before starting with the vertical ones:

/* adjust advance width to the value contained in the hdmx table */
if ( !face->postscript.isFixedPitch &&
IS_HINTED( loader->load_flags ) )
{
FT_Byte* widthp;

widthp = tt_face_get_device_metrics( face,
size->root.metrics.x_ppem,
glyph_index );

if ( widthp )
glyph->metrics.horiAdvance = *widthp << 6;
}

/* set glyph dimensions */
glyph->metrics.width = bbox.xMax - bbox.xMin;
glyph->metrics.height = bbox.yMax - bbox.yMin;

ttgload.c:1674

glyph->metrics.vertBearingX = ( bbox.xMin - bbox.xMax ) / 2;

to

glyph->metrics.vertBearingX = glyph->metrics.horiBearingX - ( glyph->metrics.horiAdvance / 2 );

bram tassyns <bramt>

 

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

Attach File(s):
   
   
Comment:
   

Attached Files
file #18669:  Patch_v2.txt added by bramt (6KiB - text/plain)
file #18667:  Patch.txt added by bramt (3KiB - text/plain)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by wl (Posted a comment)
  • -unavailable- added by bramt (Submitted the item)
  •  

    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 7 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Wed 02 Sep 2009 11:08:05 AM UTCwlOpen/ClosedOpen=>Closed
      Planned ReleaseNone=>2.3.10
    Wed 02 Sep 2009 11:08:04 AM UTCwlStatusNeed Info=>Fixed
    Wed 02 Sep 2009 06:31:26 AM UTCbramtAttached File-=>Added Patch_v2.txt, #18669
    Tue 01 Sep 2009 05:24:28 PM UTCbramtAttached File-=>Added Patch.txt, #18667
    Tue 01 Sep 2009 03:29:19 PM UTCwlStatusNone=>Need Info
      Assigned toNone=>wl

    Back to the top


    Powered by Savane 3.1-cleanup1