290292 - [Postscript] Text will beyond the end of table cell when printing or previewing in xft enabled mozilla.

Status: RESOLVED FIXED

(Core :: Printing: Output, defect)

Description

[Leon Sha]

When previewing or printing the above URL, text will beyond the end of table
cell. I am using xft enabled mozilla. After the investigation, I found that it
is not so accurate when getting the width of string.

Comment 1

[Leon Sha]

Attachment: Patch

Comment 2

[Leon Sha]

Attachment: patch v 2

Comment 3

[Jungshik Shin]

Comment on attachment 181390 [details] [diff] [review]
patch v 2



>   FT_UInt glyph_index;

Althouhg it's not your code. Can you move the above declaration where it's
actually used (inside 'for' loop)? This part of the code is kinda polluted with
C-style declarations. 

>-  FT_Glyph glyph;
>   double origin_x = 0;
>+  double em_size = 0;
>+  double x_scale = 0;

The same here. When you move down em_size and x_scale, why don't you move down
origin_x as well? 


>+  em_size = 1.0 * face->units_per_EM;
>+  x_scale = face->size->metrics.x_ppem / em_size;
      double em_size = 1.0 * face->units_per_EM;
      double x_scale = face->size ..... ; 
      double origin_x = 0;

>   for (PRUint32 i=0; i<aLength; i++) {
>     glyph_index = FT_Get_Char_Index((FT_Face)face, aString[i]);
>-    if (FT_Load_Glyph(face, glyph_index, FT_LOAD_DEFAULT) ||
>-        FT_Get_Glyph(face->glyph, &glyph)) {     
>-      origin_x += FT_REG_TO_16_16(face->size->metrics.x_ppem/2 + 2);
>+    if (FT_Load_Glyph(face, glyph_index, FT_LOAD_NO_SCALE )) {     
>+      origin_x += face->size->metrics.x_ppem/2 + 2;
>       continue;
>     }
>-    origin_x += glyph->advance.x;
>-    FT_Done_Glyph(glyph);
>+    origin_x += (face->glyph->advance.x) * x_scale ;

Now that two lines are removed, it'd be clearer to write 

     if (FT_Load_Glyph(...)) 
	 origin_x += ..........;
     else 
	 origin_x += ..........;

Comment 4

[Leon Sha]

Attachment: patch with jshin1987's comments

Comment 5

[Jungshik Shin]

Comment on attachment 181729 [details] [diff] [review]
patch with jshin1987's comments

r=jshin
thanks for the patch. 

>+      origin_x += (face->glyph->advance.x) * x_scale ;

nit: pls, get rid of the extra space before ';'

>   float dev2app;
>   dev2app = dc->DevUnitsToAppUnits();
>   origin_x *= dev2app;

While you're at it, can you combine three lines above into one? 
(no need to upload a new patch)

Comment 6

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

The tree is going to be frozen until 1.8 / aviary 1.1 branches.  You may want to
ask for trunk approval.

Comment 7

[Asa Dotzler [:asa]]

Comment on attachment 181729 [details] [diff] [review]
patch with jshin1987's comments

a=asa

Comment 8

[Leon Sha]

Checking in nsFontMetricsPS.cpp;
/cvsroot/mozilla/gfx/src/ps/nsFontMetricsPS.cpp,v  <--  nsFontMetricsPS.cpp
new revision: 1.57; previous revision: 1.56
done

Comment 9

[Caleb]

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b2) Gecko/20050428
Firefox/1.0+

For some reason I can still reproduce it on the current BEAST build :/

Comment 10

[Leon Sha]

(In reply to comment #9)
> Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b2) Gecko/20050428
> Firefox/1.0+
> 
> For some reason I can still reproduce it on the current BEAST build :/
This bug is only for xft enabled postscript module.
It is not for windows version.
You may take a look at bug 109970 for print preview staff.

Comment 11

[Wolfgang Rosenauer [:wolfiR]]

sorry for asking again.
Should trunk be fixed to display the testcase correctly within print preview for
--enable-xft --disable-freetype2 GTK2 Linux builds?
If yes, this doesn't work for me. The testcase still shows two letters
overflowing the frame.

Comment 12

[Leon Sha]

(In reply to comment #11)
> sorry for asking again.
> Should trunk be fixed to display the testcase correctly within print preview for
> --enable-xft --disable-freetype2 GTK2 Linux builds?
> If yes, this doesn't work for me. The testcase still shows two letters
> overflowing the frame.

I have tried it on fedora core relese 3. It works OK.
Can you tell me which linux are you using and attach the screenshot?

Comment 13

[Wolfgang Rosenauer [:wolfiR]]

hmm, did test again and now it works. Seems that my previous build got
inconsistent. Sorry for the spam.