bugThe FreeType Project - Bugs: bug #47708, pcf signed fixes commit 03ccfd00...

 
 

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

bug #47708: pcf signed fixes commit 03ccfd00 breaks grub-mkfont

Submitted by:  None
Submitted on:  Fri 15 Apr 2016 06:46:29 PM UTC  
 
Severity:  3 - Normal Item Group:  Incorrect behaviour
Status:  Fixed Privacy:  Public
Assigned to:  Werner LEMBERG <wl> Open/Closed:  Closed
Planned Release:  2.6.4

( Jump to the original submission)

Sun 26 Jun 2016 06:30:03 AM UTC, comment #9: 

OK, patch is now in git.  Thanks for the report!

Werner LEMBERG <wl>
Project AdministratorIn charge of this item.
Sat 25 Jun 2016 03:03:25 PM UTC, comment #8: 

OK, I've just done the following to test the font with native X11 PCF support (on my 64bit GNU/Linux box), assuming that the font is located in directory `/home/foo/bar', and that no other `unifont*.pcf' is installed.

  mkfontdir /home/foo/bar
  xset fp+ /home/foo/bar
  xset fp rehash
  xfd -fn "-gnu-unifont-medium-r-normal-sans-0-0-75-75-c-0-iso10646-1"

Everything's fine; the whole font can be displayed without errors, so it must be valid, I believe.

Alexei, please apply your patch.

Werner LEMBERG <wl>
Project AdministratorIn charge of this item.
Fri 24 Jun 2016 05:28:59 PM UTC, comment #7: 

Werner commented:

> You patch looks reasonable.  However: does the Korean example works with X11 utilities also?  Or to ask differently: Why does the X11 code work with signed integers?  Is this a 16 vs. 32bit issue?  What about other, older CJK PCF fonts?  Is there a possibility that unifont is an invalid PCF, using wrong offsets (namely unsigned ones)?


I cannot find any other PCF font with this broad Unicode coverage. Please help.

Alexei Podtelezhnikov <podtelez>
Project Member
Fri 24 Jun 2016 04:16:49 AM UTC, comment #6: 

I still think encodingOffset should be read as unsigned short even though this is not how it is described in the unofficial specs or used in the original X.org file. The signed offset does not make sense to me.

Alexei Podtelezhnikov <podtelez>
Project Member
Fri 24 Jun 2016 03:44:07 AM UTC, comment #5: 

Sorry, I'm not as familiar as you are with freetype2; I could only do so much initially. I also now see only 57086 glyphs at top left index, not sure where the others come from.

I also see some of the most relevant information for this bug report when missing along the way -- this was using freetype2 2.6.1-0.1ubuntu1 on Ubuntu 16.04;

After applying the patch I originally submitted; I can also use
ftstring -r 96 -m "국민투표에서 잔류와 탈퇴 진영간에 우" 12 Downloads/unifont-8.0.01.pcf.gz

to display Korean with no issues (didn't need the extra two hunks provided by Alexei). Without the patch, all of the Korean glyphs used in the command above show NUL boxes.

However, before applying the patch, ftview does display all glyphs correctly... They must use a different, unaffected code path.

The best I could do so far aside from ftview/ftstring was as presented in the original report -- using grub-mkfont to convert the font, and regardless -- stopping at exactly 32768 looked very suspect to me. I looked at both grub-mkfont itself and freetype, it just so happened that reverting freetype (since grub hadn't otherwise changed meaningfully) corrected the issue, hence warranted some bisection.

Mathieu Trudel <cyphermox>
Fri 24 Jun 2016 01:46:44 AM UTC, comment #4: 

I failed miserably using Korean:

ftstring -r 96 -m "국민투표에서 잔류와 탈퇴 진영간에 우" 12 Downloads/unifont-8.0.01.pcf.gz

until I applied this:

--- a/src/pcf/pcfread.c
+++ b/src/pcf/pcfread.c
@@ -849,7 +849,7 @@ THE SOFTWARE.
     int           firstCol, lastCol;
     int           firstRow, lastRow;
     FT_ULong      nencoding;
-    int           encodingOffset;
+    FT_UShort     encodingOffset;
     int           i, j;
     FT_ULong      k;
     PCF_Encoding  encoding = NULL;
@@ -922,14 +922,14 @@ THE SOFTWARE.
       for ( j = firstCol; j <= lastCol; j++ )
       {
         if ( PCF_BYTE_ORDER( format ) == MSBFirst )
-          encodingOffset = FT_GET_SHORT();
+          encodingOffset = FT_GET_USHORT();
         else
-          encodingOffset = FT_GET_SHORT_LE();
+          encodingOffset = FT_GET_USHORT_LE();

-        if ( encodingOffset > -1 )
+        if ( encodingOffset != 0xFFFF )
         {
           encoding[k].enc   = i * 256 + j;
-          encoding[k].glyph = (FT_UShort)encodingOffset;
+          encoding[k].glyph = encodingOffset;

           FT_TRACE5(( "  code %d (0x%04X): idx %d\n",
                       encoding[k].enc, encoding[k].enc, encoding[k].glyph ));

Alexei Podtelezhnikov <podtelez>
Project Member
Thu 23 Jun 2016 08:59:55 PM UTC, comment #3: 

Still waiting for the OP to reply...

Werner LEMBERG <wl>
Project AdministratorIn charge of this item.
Thu 23 Jun 2016 07:09:45 PM UTC, comment #2: 

It does look strange that this code uses signed encodingOffset and then rejects negative values. Perhaps this matters in 16-bit mode, where grub can operate.

I'd use unsigned int encodingOffset and FT_GET_USHORT.

Alexei Podtelezhnikov <podtelez>
Project Member
Sat 16 Apr 2016 06:35:44 AM UTC, comment #1: 

I can't reproduce the problem.  Using ftview from current git, I can easily access all 57087 glyphs – I don't know how you get 57248 glyphs; if you look into the corresponding BDF file, you can clearly see the line

    CHARS 57086

(the additional glyph is `.notdef', inserted by FreeType itself).

Note that I've downloaded the BDF and PCF files from

    https://ftp.gnu.org/gnu/unifont/unifont-8.0.01/

On my GNU/Linux box, both 32bit and 64bit builds return the same, correct result if compiled with gcc 4.8.5.

Werner LEMBERG <wl>
Project AdministratorIn charge of this item.
Fri 15 Apr 2016 06:46:29 PM UTC, original submission:  

I've tracked down an issue in generating unicode.pf2 in grub (using grub-mkfont) to commit 03ccfd004fcfd3fab4a0c0cea307bdcb0cd79129:

[pcf] Signedness fixes.

Looks like some of the encodingOffset changes make it so that only the first 32768 glyphs from unifont.pcf are being read.

That said, I've attached a smaller patch which appears to solve the issue and doesn't require reverting the full commit. More details follow.

There should be 57248 by my count on Ubuntu 16.04; using unifont 8.0.01; specifically using /usr/share/fonts/X11/misc/unifont.pcf.gz to generate unicode.pf2 with grub-mkfont:

[14:36:10] mtrudel@moloch:~git/freetype/freetype2 G[u][freetype2.git>4e757eb] $ ../../grub/grub/build-grub-mkfont -v -o ../unicode.pf2 /usr/share/fonts/X11/misc/unifont.pcf.gz          
Font name: Unifont Regular 16
Max width: 16
Max height: 16
Font ascent: 14
Font descent: 2
Number of glyph: 32768

[14:35:59] mtrudel@moloch:~git/freetype/freetype2 G[u][freetype2.git>4e757eb] $ LD_PRELOAD=objs/.libs/libfreetype.so ../../grub/grub/build-grub-mkfont -v -o ../unicode.pf2 /usr/share/fonts/X11/misc/unifont.pcf.gz
Font name: Unifont Regular 16
Max width: 16
Max height: 16
Font ascent: 14
Font descent: 2
Number of glyph: 57248

build-grub-mkfont was built from grub master, as above I used unifont 8.0.01-1 as available on Ubuntu 16.04. Commit 4e757eb visible in my prompt is after reverting 03ccfd00 from VER-2-6-1 and fixing conflicts.

Please see the attached patch.

Anonymous

 

Attached Files
file #36943:  0001-Revert-pcf-Signedness-fixes.patch added by None (1KiB - 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 cyphermox (Posted a comment)
  • -email is unavailable- added by podtelez (Posted a comment)
  • -email is unavailable- added by wl (Posted a comment)
  • -email is unavailable- added by cyphermox
  • -email is unavailable- added by cyphermox (cyphermox)
  •  

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

    Only logged-in users can vote.

     

     

     

    Follow 8 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2016-06-26 wl StatusWorks For Me => Fixed
        Open/ClosedOpen => Closed
        Planned ReleaseNone => 2.6.4
    2016-04-16 wl StatusNone => Works For Me
        Assigned toNone => wl
    2016-04-15 cyphermox Carbon-Copy- => Added mathieu@canonical.com
    2016-04-15 cyphermox Carbon-Copy- => Added cyphermox
    2016-04-15 None Attached File- => Added 0001-Revert-pcf-Signedness-fixes.patch, #36943

    Back to the top


    Powered by Savane 3.7