107020 - changes to nsFontMetricsGTK.cpp for anti-aliased scaled bitmap fonts

Status: VERIFIED FIXED

(Core :: Internationalization, defect)

Description

[kill this account]

Since the checkin for bug 90813 is ~4500 lines I have broken it down into
sub bugs to make the checking process more managable.

This bug covers the changes to nsFontMetricsGTK.cpp needed for
anti-aliased scaled bitmap fonts.

Comment 1

[kill this account]

Attachment: changes to nsFontMetricsGTK and nsRenderingContext

Comment 2

[kill this account]

Katakai-san: do you have time to review this code?

Comment 3

[kill this account]

Attachment: unix.js (prefs file) changes

Comment 4

[kill this account]

Attachment: patch with fixes to make MathML compile

Comment 5

[Masaki Katakai]

I have succeeded the build with the patch and it seems working fine.
I've started looking the codes.

I have one question, how are these change working with Xft anti-
aliasing? Brian has been working on Xft codes.

Comment 6

[kill this account]

this bug has no relationship with Xft in any way.

Comment 7

[Masaki Katakai]

Hi Brian,

Yes, I know. I mean if Xft codes come, how these codes and
Xft codes are working together, e.g. all scalling by Xft or
bitmap scalling,scalling for small size by bitmap-scalling
and for large size by Xft... is it customizable?

Comment 8

[kill this account]

1st: it is not clear that Xft would be a desireable thing to add to mozilla

2nd: the interesting part of Xft only applys when there are TrueType fonts
     this only applies when there are not.

Comment 9

[kill this account]

There is no debate; we want to use FreeType2 to render TrueType fonts on 
Linux/Unix.

It has not been finalized (although I have very strong opinions) if we will

 1) use Xft to access FreeType2 to open the font and then do all the rest 
    directly with FreeType2
or
 2) directly open the font with FreeType2 and do all the rest 
    directly with FreeType2.


The order of font preference should be:

1) Bitmap fonts since these are often hand tuned.

2) Outline scaled fonts. The order between these is not clear;
   AA by the client
   AA by the server - right not there is not way to control this
   non-AA by the server - results are okay but not great

3) AA Bitmap Scaled since these tend to be more blurry

4) non-AA bitmap scaled (worst choice)

Comment 10

[Masaki Katakai]

I have done purify testing today and verified no errors and
warnings with the patch. I'll continue code review.

Comment 11

[kill this account]

Katakai-san: thanks for the purify testing. I've been trying to do this but
had a link error that I only recently found out how to workaround.

Comment 12

[kill this account]

Katakai: when you are done reviewing please re-assign this bug to me.

Comment 13

[Masaki Katakai]

1. With debugging version, I got the following assertion when I visit
www.goo.ne.jp.

###!!! Break: at file nsXFontNormal.cpp, line 128
###!!! ASSERTION: zero width: 'gdk_text_width(mGdkFont, aString, aLength)', file
nsXFontNormal.cpp, line 128

2. BadMatch happens

when I try to use bitmap which has only 18 point size,
I got the following assertion and X error.

unscaled_width=18 mUnscaledMax.width=17
###!!! ASSERTION: unexpected glyph width: 'unscaled_width<=mUnscaledMax.width',
file nsXFontAAScaledBitmap.cpp, line 395
###!!! Break: at file nsXFontAAScaledBitmap.cpp, line 395
X Error of failed request:  BadMatch (invalid parameter attributes)
  Major opcode of failed request:  73 (X_GetImage)
  Serial number of failed request:  1794
  Current serial number in output stream:  1794

setup (for example)
  mkdir /tmp/p
  cd /tmp/p
  cp /usr/X11R6/lib/X11/fonts/75dpi/lu*18* .
  mkfontdir .
  sed 's/lucida/katakai/g' fonts.dir > 1
  mv 1 fonts.dir
  xset +fp `pwd`
	
start Mozilla and set -b&h-katakai-... for western fonts.
The 18 point bitmap should be used as scaled bitmap for all sizes.

3.

In nsFontGTK::~nsFontGTK(), I see the following codes,

  if (mFont && (mAABaseSize==0)) {
    gdk_font_unref(mFont);
  }

What will happen when mAABaseSize!=0?

4. Text in text field is not drawn within the text field

Try to compose Japanese text in text field. The text should be drawn inside
the text field. I'll attach the snapshot.

Comment 14

[Masaki Katakai]

Attachment: japanese text is drawn on outside of text field

Comment 15

[Masaki Katakai]

Sorry, one correction,

1. I visited www.infoseek.co.jp not www.goo.ne.jp.

Comment 16

[kill this account]

Attachment: patch with fixes

Comment 17

[kill this account]

> ###!!! Break: at file nsXFontNormal.cpp, line 128
> ###!!! ASSERTION: zero width: 'gdk_text_width(mGdkFont, aString, aLength)', 

The font is returning zero width for 0xA0 (nbsp). Looks like our converter
does not exactly match the font.

> ###!!! ASSERTION: unexpected glyph width:'unscaled_width<=mUnscaledMax.width',
> file nsXFontAAScaledBitmap.cpp, line 395
> ###!!! Break: at file nsXFontAAScaledBitmap.cpp, line 395
> X Error of failed request:  BadMatch (invalid parameter attributes)

Interesting, seems 

  max_bounds.rbearing - min_bounds.lbearing 

does not give the largest width so I changed this to

  MAX(usfi->max_bounds.rbearing,usfi->max_bounds.width) 
  - MIN(usfi->min_bounds.lbearing, 0)

> In nsFontGTK::~nsFontGTK(), I see the following codes,
> 
>   if (mFont && (mAABaseSize==0)) {
>     gdk_font_unref(mFont);
>   } 

changed to:

    if (mFont) {
      gdk_font_unref(mFont);
    }

> 4. Text in text field is not drawn within the text field

I was using the wrong GC.

------------------

I also fixed/changed:
  
  1) if the window is near the right edge a BadMatch error can occur

  2) changed calloc to nsMemory::Alloc

Comment 18

[kill this account]

Katakai-san: thanks for your help so far. Kindly try this new patch, thanks

Comment 19

[kill this account]

Attachment: patch with fixes (just nsFontMetricsGTK and nsRenderingContext)

Comment 20

[Masaki Katakai]

Hmm... the changes

changed to:

    if (mFont) {
      gdk_font_unref(mFont);
    }


seems to cause gdk assertion at exiting Mozilla.

Gdk-CRITICAL **: file gdkfont.c: line 234 (gdk_font_unref): assertion
`private->ref_count > 0' failed.

Comment 21

[kill this account]

Attachment: for reference: full gtk/ patch

Comment 22

[kill this account]

Attachment: patch; just nsFontMetricsGTK and nsRenderingContextGTK

Katakai: would you kindly try this patch? thanks

Comment 23

[Masaki Katakai]

r=katakai

Comment 24

[Brendan Eich [:brendan]]

Comment on attachment 57235 [details] [diff] [review]
patch; just nsFontMetricsGTK and nsRenderingContextGTK

Indentation drifts rightward at the then-part's closing brace, doesn't recover
till the outer else (not shown) closes:

+      if (!mCurrentFont->GetXFontIs10646()) { // keep 8 bit path fast
	 // 8 bit data with an 8 bit font
-	 nsRenderingContextGTK::my_gdk_draw_text (mSurface->GetDrawable(),
-						  mCurrentFont->GetGDKFont(),
mGC,
-						  x, y, aString, aLength);
+	 NS_ASSERTION(xFont->IsSingleByte(),"wrong string/font size");
+	 xFont->DrawText8(mSurface->GetDrawable(), mGC, x, y, aString,
aLength);
	 }
	 else {
	   // we have 8 bit data but a 16 bit font
-	   Widen8To16AndDraw(mSurface->GetDrawable(),
mCurrentFont->GetGDKFont(), mGC,
+	   NS_ASSERTION(!xFont->IsSingleByte(),"wrong string/font size");
+	   Widen8To16AndDraw(mSurface->GetDrawable(), xFont, mGC,
						x, y, aString, aLength);
	 }

possible copy-paste error?

Clue me in: isn't GTK2 gonna do anti-aliased fonts for us, so we won't have to?
 Most platforms provide their own solutions, but I'm too many years out of the
X world to know why GTK lags.

Rubber-stampy sr=[e-mail]

/be

Comment 25

[kill this account]

checked in

Comment 26

[kill this account]

> possible copy-paste error?

fixed

> isn't GTK2 gonna do anti-aliased fonts for us, so we won't have to?

By "anti-aliased fonts" do you mean TrueType fonts?

If so, this bug does anti-aliasing on preexisting X fonts; eg: non-TrueType.
Hence the name "Anti-Aliased Bitmap Scaled" (AASB) fonts. Since AASB works
with existing fonts it means it will benefit users immediately and does not 
require them to find/download TrueType fonts. Personally, I think this has a 
limited life but my manager, ftang, is very excited about this. Momoi and
the IQA testers thought AASB produced good results.

As a font developer I would prefer to focus on a long term solution: TrueType
fonts. Now that FreeType2 is "relatively" stable this is possible.

> Most platforms provide their own solutions, but I'm too many years out of the
> X world to know why GTK lags.

To my knowledge, all other X TrueType solutions are based on Xft which is a
fine "bridge" solution for getting TrueType into X. Xft is a bridge library
because it uses XLFD and XFontStruct and describes TrueType fonts as iso10646 
(Unicode) fonts. Using XLFD and XFontStruct to describe a font was okay in 
1985 when all we had were simple / smallish encodings like iso8859-1 (~256 
chars), JISX0208 (~6000 chars), etc. Fonts typically had glyphs for all the 
characters in the encoding. TrueType fonts do not follow those rules, instead:

 1) TrueType fonts use Unicode for the encoding (~65000 chars)
  
 2) TrueType fonts do *not* contain glyphs for all the characters in Unicode

If:

 1) moz were just concerned with displaying one language
 
 2) moz could ignore Numeric Character References, Named Entities, and 
    Microsoft additions like smart quotes, etc.
    
 3) moz users did not mind the occasional blank character

then XLDF would be fine. But as a moz i18n engineer I strive to make the 
text readable and hence need to know *exactly* what chars a font 
does/does-not have. XLFD makes getting the list of characters from a
Unicode (iso10646) font *extremely* expensive. To get the list of 
characters moz would need to *load* the font and *download* the *entire* 
list of per-glyph metrics just to see if there was a particular character. 
Imagine doing this for every TrueType font on the system. I have seen this
take almost 90 seconds on my 1GHz PC when directly accessing the fonts (ie:
no X server in the middle which would slow things down.)
This search happens in the middle of displaying a page. I don't imagine 
that users would tolerate the 90 second plus pause. Searching thru every 
single font for a particular character happens enough that it cannot be 
ignored.

If anyone knows of a solution that moz can use (ie: does not have these
problems) please let me know.

Comment 27

[Roland Mainz]

bad news:
I have asked around some people with Xterminal instatation to test the "new"
Zilla. The responses are devastating, ranging from "bad performance" to
"unuseable"... ;-(

Having the Xterminal on the same switch is still good, but if the latency
increases (for example, different network/other building or a lab at the other
side of the city) this feature starts to hurt performance at lot (the worst
comment says that she can "count the rendering of the single chars...") ... ;-(

I strongly recommend to turn this feature _OFF_ by default and add a prefs GUI
"checkbox" to enable this feature "on demand", and figure out if we can
implement a better solution for this problem ...

Comment 28

[Roland Mainz]

Can't we use the render extension for this (I am aware that RENDER is not
available on all platforms, but it may provide a clean solution without hurting
the performance). 
Looking at |XRenderAddGlyphs| - is there any problem which prevents us from
using this for AASB glyphs, too ?

Comment 29

[kill this account]

Can I get contact info for these people?

Yes, render would help but does a Xterminal have (or will it ever have) render?

Comment 30

[Adam D. Moss]

The quality of these peoples' experiences might have been
negatively affected by the ViewManager bug spanning several
days recently (now fixed) which caused an enormous number
of superflous paints.  That would magnify any additional
pain of aasb overhead.

Is it as painful on the very latest nightly?  Additionally,
do they really see each character painted one at a time?  I'd
have thought that the aasb code only does a server round-trip
once per string rather than once per character, but I haven't
verified.

Comment 31

[kill this account]

The aasb code does one XGetImage/XPutImage per run of text (same font).

Without talking to the user its hard to tell what "count the rendering of the 
single chars..." means except that there was rendering problems. It might or
might not be related. Either way it needs to be addressed. Hence the need
for contact info. 

Is there a bug open on this?

Comment 32

[Roland Mainz]

bstell wrote:
> Can I get contact info for these people?

Sure, via PM.
--> Be sure not to post the email adresses in public areas, most people are very
picky about that (they fear the spam bots... =:-) <-- !!

> Yes, render would help but does a Xterminal have (or will it ever have) 
> render?

The newer, Xfree86-one based ship with RENDER enabled (see http://www.ltsp.org/
for example). Commercial ones may follow, but some of them may not follow unless
RENDER will "officially" pop-up in X.org sources ...

----

> The aasb code does one XGetImage/XPutImage per run of text (same font).

Looking at
http://lxr.mozilla.org/mozilla/source/gfx/src/gtk/nsRenderingContextGTK.cpp#1585
- there are some cases where we draw strings char-by-char, right ?

> Without talking to the user its hard to tell what "count the rendering of the 
> single chars..." means except that there was rendering problems.

See above.

> It might or
> might not be related. Either way it needs to be addressed. Hence the need
> for contact info. 

> Is there a bug open on this?

Not yet. I can harass the others to open one... at least we should avoid
shipping 0.9.7 with this issue or we may have some customer escalations in the
queue ...

Comment 33

[Frank Tang]

Roland Mainz:
Does those people with Xtermnianl installation you asked to test have RENDER
with their Xterminal ?
Is the performance of the same build "usable" if they turn off the aasb by
changing the pref?

Comment 34

[Roland Mainz]

Frank Tang wrote:
> Does those people with Xtermnianl installation you asked to test have RENDER
> with their Xterminal ?

Partially. Those Linux-based are running (AFAIK) all Xfree4.x which means that
RENDER should be available on these machines ...

> Is the performance of the same build "usable" if they turn off the aasb by
> changing the pref?

Yup. I created a Solaris SPARC build (non-debug), tested it myself, packaged it
for easy installation and made it available for testing incl. instructions what
they should test ...

Comment 35

[kill this account]

Is there a bug open on this?

To disable aasb just set this pref to false and restart moz:
http://lxr.mozilla.org/seamonkey/source/modules/libpref/src/unix/unix.js#225

    pref("font.scale.aa_bitmap.enable", true);

Comment 36

[kill this account]

the control in nsFontMetricsGTK::PickASizeAndLoad for deciding to scale needs 
work.

Right not it *only* anti-alias scales if there *is* a scalable X font.
Instead it should scale if there is *no* outline scalablable X font.

Comment 37

[kill this account]

Attachment: prototype code

this code may need some work

Comment 38

[kill this account]

Attachment: patch; fix the code to pick a font type

Comment 39

[kill this account]

Attachment: patch; same as attachment 60779 but ignore whitespace to make the review easier

Comment 40

[Roland Mainz]

Comment on attachment 60779 [details] [diff] [review]
patch; fix the code to pick a font type

r=[e-mail]

Comment 41

[Chris Waterson]

rs=waterson, contingent on one more review from pavlov, blizzard, or rbs.

Comment 42

[Stuart Parmenter]

Comment on attachment 60780 [details] [diff] [review]
patch; same as attachment 60779 [details] [diff] [review] but ignore whitespace to make the review easier

r=pavlov

Comment 43

[kill this account]

checked in

Comment 44

[Yuying Long]

The aasb fonts on 12-13 trunk build.

Mark it as verified.