bugThe FreeType Project - Bugs: bug #56404, 32bit build renders glyph with...

 
 

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

bug #56404: 32bit build renders glyph with coordinate overflow after varfont precision increase commit

Submitter:  Dominik Röttsches <drott>
Submitted:  Tue 28 May 2019 11:02:25 AM UTC
   
 
Severity:  3 - Normal Item Group:  None
Status:  Fixed Privacy:  Public
Assigned to:  wl Open/Closed:  Closed
Planned Release:  2.10.1

Jump to the original submission

Wed 29 May 2019 06:18:42 AM UTC, comment #6: 

Thanks a lot, Dominik and Ben, for the analysis and the fix!  I've now removed the cast to unsigned completely.

Werner LEMBERG <wl>
Group administrator
Wed 29 May 2019 04:41:13 AM UTC, comment #5: 

Now that I think about it, it's probably better for both FT_fixedToInt and FT_fixedToFdot6 to use signed (arithmetic) shifts since they're both right shifts and shouldn't run into the warning problems of signed left shifts.

bungeman <bungeman>
Tue 28 May 2019 09:02:49 PM UTC, comment #4: 

I was wondering why this wasn't happening in 64 bit builds, as the same logic should apply -- the resulting FT_Pos 'unrounded' should be missing some high sign bits and look like a (very) large positive number. I think what is happening is that due to FT_MULFIX_ASSEMBLER being set, FT_MulFix is casting its (FT_Long) parameter values to FT_Int32, which makes things work (there are enough sign bits still around that this truncation makes things work out). Awkwardly, if FT_MULFIX_ASSEMBLER isn't set on an architecture where long is 64 bit, there will currently be an issue similar to the one reported here, but the points will be really far off in space, since the portable code does not cast to FT_Int32.

bungeman <bungeman>
Tue 28 May 2019 08:38:19 PM UTC, comment #3: 

I looks like I managed to upload the wrong signed_shift.patch, note that the change to FT_fixedToInt isn't needed (as explained before). It's just FT_fixedToFdot6 that needs tweaking here.

bungeman <bungeman>
Tue 28 May 2019 08:23:32 PM UTC, comment #2: 

It turns out the issue is with the unsigned shifts on signed values.

In the original reproduction case in TT_Vary_Apply_Glyph_Deltas, on the line

unrounded[i].x += FT_fixedToFdot6( point_deltas_x[i] );

when i = 0, unrounded.x[0] = 64000, point_deltas_x[0] = -1048800

point_deltas_x_tmp = 0xffefff20 (in 16.16 this is -15.995)
point_deltas_x_tmp += 0x200 => 0xfff00120 (in 16.16 this is -16.004)
point_deltas_x_tmp >> 10 => 0x3ffc00 (in 26.6 this is 65520)

unrounded.x = 0xfa00 (in 26.6 this is 1000)
unrounded.x += 0x3ffc00 => 0x40f600 (in 26.6 this is 66520)

The issue being that the sign bits didn't work out. FT_fixedToInt seems ok, since it casts to FT_Short after shifting down 16 bits (on a 32 or 64 bit type) so the unsigned shift keeps sign bits in the resulting value. With FT_fixedToFdot6 the bits don't always work out that way since FT_Pos is 'signed long' and FT_ULong is 'unsigned long'. As a result, negative deltas are likely to cause this issue on the 'unrounded' values.

I've attached signed_shift.patch which fixes this particular reproduction. Due to the fun with signed shifts I know they are avoided, but this at least points out where the issue lies.

(file #46994)

bungeman <bungeman>
Tue 28 May 2019 12:54:31 PM UTC, comment #1: 

An alternative is to apply deltas to scaled paths.

Alexei Podtelezhnikov <podtelez>
Group Member
Tue 28 May 2019 11:02:25 AM UTC, original submission:  

Steps to reproduce:

1) Build FreeType as 32bit version:
In FT dir:
CFLAGS=-m32 ./configure --with-zlib=no --with-png=no --with-harfbuzz=no && make

2) Build ft-multi as 32bit version:
In ft-demos:
CFLAGS=-m32 make

(Needs 32bit X11 libs installed, e.g. apt install x11-dev:i386)

3) Run
$ bin/ftmulti -d "99" -f 2 40 variabletest_matching.ttf

(Font attached, font originally part of web platform tests, see https://github.com/web-platform-tests/wpt/blob/master/css/css-fonts/matching/resources/variabletest_matching.ttf )

Actual:
With width axis set to 99, top most, first block glyph extends to the far right and outside of the screen.

Expected:
Top most, first block glyphs shrinks to a narrower size than its size at width axis set to 100.


This happens after 1178227b39a4eeecb
[truetype] Increase precision of font variation (#54371) and does not occur before that CL, and does not happen before.

Screenshots attached for expected and actual.

Causes failures on Chromium bots for Windows 7 variable fonts font matching tests, compare:
https://test-results.appspot.com/data/layout_results/win7-rel/100744/webkit_layout_tests%20%28with%20patch%29/layout-test-results/results.html


Dominik Röttsches <drott>

 

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

Attached Files
file #46994:  signed_shift.patch added by bungeman (724B - text/x-patch)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by wl (Posted a comment)
  • -email is unavailable- added by bungeman (Updated the item)
  • -email is unavailable- added by podtelez (Posted a comment)
  • -email is unavailable- added by drott
  • -email is unavailable- added by drott (Submitted the item)
  •  

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

    Date Changed by Updated Field Previous Value => Replaced by
    2019-05-29 wl StatusNone Fixed
        Assigned toNone wl
        Open/ClosedOpen Closed
        Planned ReleaseNone 2.10.1
    2019-05-28 bungeman Attached File- Added signed_shift.patch, #46994
    2019-05-28 drott Carbon-Copy- Added -email is unavailable-
    2019-05-28 drott Attached File- Added variabletest_matching.ttf, #46988
        Attached File- Added overflow_variable_matching_actual.png, #46989
        Attached File- Added overflow_variable_matching_expected.png, #46990

    Back to the top

    Powered by Savane 3.13-4448.
    Corresponding source code