381333 - Improve font handling and text display in OS/2 Thebes builds

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Peter Weilbacher]

Followup to bug 371504:
- investigate which font really gets used
- why on Dave's system text by default was displayed weird
  (as in attachment 265256 [details])
- can we implement some kind of font replacement scheme where a
  glyph is taken from the unicode font (or another installed
  font) if not available in the current font.

Comment 1

[Dave Yeo]

I've done some more testing. Definitely here there are problems with WarpSans and Postscript fonts.
Method to test menu fonts. Open a PM app, I used e.exe. While holding alt key drag'n'drop WarpSans from the font pallette to the menu bar on e.exe. (for some reason here the mouse cursor does not change but drag'n'drop does work). Watch font change. Open firefox and menus are unreadable. Repeat with truetype font and postscript font opening and closing Firefox in between tests.
Results here are I only get readable menus with Truetype fonts.

Comment 2

[Peter Weilbacher]

Attachment: [checked in] Fix missing glyphs

Two small improvements with this patch that I have just checked in:

- Take the same fix as in bug 377923 for OS/2
- Really draw missing glyph boxes. For the glyph ID returned by FreeType
  (always 0 for missing glyph) IsSimpleGlyph() returns true, so we need
  to make advance negative as a trick to reach the second if. The correct
  advance is then set by the call to SetMissingGlyph().

Comment 3

[Peter Weilbacher]

Attachment: [checked in] Improve weight matching when creating cairo font

Just checked in this patch that hopefully improves matching of font weights when CSS weights like "bolder" are used (e.g. the <strong> elements on the Minefield Start Page).

Comment 4

[Peter Weilbacher]

Attachment: [checked in] fix font scaling

This is what I just checked in to fix some stupid scaling hacks that I had originally implemented. I am pretty sure that I had tried this approach (scaling with face->size->metrics.x_scale/.yscale) before and it had not worked, now it does. :-)

This should fix
- the size of the selection area
- the size of the caret/cursor in URL bar and input fields
- the height of menu items that do not have an icon in front

Comment 5

[Peter Weilbacher]

Hmm, since my last build (of a week ago) Umlaut characters don't display any more or display as missing glyph characters. In German texts also other characters are missing or displayed as spaces. Probably something to do with the new textframe code that was turned on on 2007-06-20, the last SeaMonkey nightly build of 2007-06-20 09:48 was still good...

I also don't like the spaces themselves. For some fonts they are OK because I multiply them by a factor of 2 but for some they are now too wide, especially in titles. Perhaps I should test for the space width and increase it only if it is smaller than the width of an x...

Comment 6

[Peter Weilbacher]

I checked in two more fixes today.

The first one fixed some stupidity from last week's checkins and adds a few comments to gfxOS2Fonts::GetMetrics(). It also reverts back to using the space width that the font tells us we should use. This improves text display at larger font sizes and for monospace fonts. At the same time I am not at all happy with the display at small sizes where words are not well separated any more. Not sure how to solve this...

The second checkin fixes display of non-ASCII chars (and simplifies the function calls). It basically rehashes the fix for bug 377942 on OS/2 and adapts the function of the gfxOS2FontGroup class to match those of gfxPangoFontGroup after bug 357637.

Comment 7

[Peter Weilbacher]

Attachment: [checked in] Metrics rewrite, part 1

After fiddling with this for the last few weeks I have come to the conclusion that most of the metrics computation that I had implemented before was badly done or wrong.

This patch now
. corrects metrics (especially x width and height, space width,
  by loading without scaling to get font units)
. corrects offsets for strikeout, super/subscript
. adds adjusted size computation
. rearranges debug outputs to be a bit more useful when switched on

I think this should increase readability of texts quite a bit, especially at smaller font sizes, mostly due to corrected space widths.

Comment 8

[Peter Weilbacher]

Attachment: [checked in] Metrics rewrite, part 2

This second part uses the second font scaling method from
   http://freetype.sourceforge.net/freetype2/docs/tutorial/step2.html
Instead of rounding/truncating the font units I think we should better make use of the fact that the metrics are of type gfxFloat (=double) and so we can use proper scaling factors to scale metrics from font units to pixels. I think this is also a bit "cleaner" than using these macros for everything.

I forgot to say that the first patch has the side effect that the Acid2 test displays correctly again instead of having an indented chin as before. :-)

I will check both patches in tomorrow.

Comment 9

[Peter Weilbacher]

Attachment: Hackish try to enable kerning

In principle kerning should make the text display better, especially for words like "Tor" or "VAN" where otherwise the gap between adjacent chars is a bit too large.

This is now a first try to enable it, but there are two problems:
- We can only set the character advance when we already know the next
  character. The current loop is not prepared to do that so this patch
  simply sets the advance twice (once without kerning, once with kerning
  if necessary). :-(
- Type 1 fonts (like HELV.PFB) need to get their metrics file attached
  to the FreeType font face, before kerning info is accesible. This means
  we would have to pass the filename into either gfxOS2Fonts::GetMetrics()
  or gfxOS2FontGroup::CreateGlyphRunsFT() -- the latter would probably be
  very slow -- and then try to find the metrics file (which can have a
  .PFM, .AFM, or .OFM extension, or be located somewhere else, like in a
  PFM subdir).

Well, the old OS/2 GFX code says that it takes into account kerning but I don't actually see that happening, so this is low priority. Perhaps it would be worth to try and enable Pango support instead.

Comment 10

[Peter Weilbacher]

Comment on attachment 277237 [details] [diff] [review]
[checked in] Metrics rewrite, part 1

Checked this into trunk (in two parts).

Comment 11

[Peter Weilbacher]

Comment on attachment 277239 [details] [diff] [review]
[checked in] Metrics rewrite, part 2

Didn't find any problems with this scaling approach since yesterday, so I checked it in.

Comment 12

[Peter Weilbacher]

Attachment: testcase for crash with font-size: 0

Comment 13

[Peter Weilbacher]

Attachment: [checked in] patch to fix font-size:0 crash

This is the patch to fix the crash with the testcase I just attached. I just checked it into trunk.

Comment 14

[Peter Weilbacher]

Attachment: glyph replacement, 1st draft

As I have already noted here and there, I have been working a bit on glyph replacement in OS/2 Thebes builds. What is mean by that is that on pages which are not simply ASCII or Western, the primary font may not contain all the characters necessary to display all text, so one has to search for a font that does contain the missing glyphs and use that to paint the remaining characters.

This is the first cut at this: for each new fontGroup I append a few fonts that are good candidates for replacement glyphs (like Unicode fonts). These are then easily searched when constructing the textRun. I also tried the approach (that is commented out as REALLY_DESPERATE_FONT_MATCHING in this patch) to just append every font installed in the system to the font list and search them all, but apart from slowing down complex pages quite a bit, this has the drawback that I don't know which font is serif, sans, or monospaced, and one should really only replace missing glyphs within the same kind of font.

Just so that others can try this I'm attaching this patch which is a first draft. I have to do a lot more testing (and remove debugging stuff) but on the main Wikipedia page it gets me quite far already with the right fonts installed. I think we should also tweak the default setup (especially the font.name-list prefs for x-unicode) a bit to improve things for some of the other free Unicode fonts available.

As a small addition I just tried today, to force replacing of WarpSans with Workplace Sans in the code. The former cannot be displayed in Thebes builds, and the latter provides a good look-alike. The only drawback that I found is that with this font (which can be downloaded from http://www.cs-club.org/~alex/creative/fonts/) it doesn't paint bold stuff in bold, but that should be solvable, too. This experiment is also active in the patch.

Comment 15

[Dave Yeo]

replying to comment #14
Your patch does not cleanly apply to a 2 day old checkout or todays checkout. eg
I:\mozilla >patch -p0 < glyph_replacement_2007-11-11.diff
patching file `gfx/thebes/src/gfxOS2Fonts.cpp'
Hunk #2 FAILED at 603.
1 out of 2 hunks FAILED -- saving rejects to gfx/thebes/src/gfxOS2Fonts.c#

and does not look simple to apply by hand though I will try if I find time.

Comment 16

[Peter Weilbacher]

Oh, yes, sorry. Forgot about the patch that went in with bug 385417. I hope to improve this patch soon anyway once I have the remaining non-working cases debugged (like Tamil and some other languages, that aren't displayed currently).

Comment 17

[Peter Weilbacher]

Attachment: [checked in] glyph replacement, cleaned up version

OK, second, cleaned-up version of the glyph replacement patch. This version doesn't mix in the stuff from bug 381330 any more, fixed some weirdness with non-plane1 characters (now they should at least display the missing glyph symbol).

To notice the full power of this patch one needs to tweak the font settings a bit from the defaults, especially add enough good Unicode fonts to the font.name-list.*.x-unicode settings (and possibly also add some extra non-Unicode fonts to list.*.x-user-def). I will try to improve the defaults with another patch.

Also note that this is still rather primitive, it doesn't make use of font caches the way it should and the other platforms do. But it gives me something displayed on many more "exotic" pages then I hoped it would.

Will check this into CVS in a minute.

Comment 18

[Peter Weilbacher]

Attachment: [checked in] implement kerning

This patch actually seems to work to implement kerning. To test, one currently needs to have TrueType fonts (for Type1 fonts I would need to implement loading the metrics file) and set browser.display.auto_quality_min_font_size to something low (like 10 or so). By default only the text in the UI or text > 60px gets kerned.

Comment 19

[Peter Weilbacher]

Attachment: Adjust default font settings in all.js

To make better use of the glyph replacement scheme that I implemented we should adjust the default font settings. At least the ones for Unicode. This is a first try which lists some more of the popular fonts mentioned in the newsgroup.

The current version still leaves in all the "Tms Rmn" and "Helv" settings   which are two fonts we cannot display (stupid OS/2 .fon format). I thought   about replacing them with "Times New Roman" and "Helvetica" but I am not sure what happens when those are not installed -- unlike the .fon variants they are not guaranteed to be!
Fontconfig will also automatically replace "Helv" with "Helvetica" or any other font that contains the string "helv" but for "Tms Rmn" that doesn't work...

I'm also not sure why some of the languages (like Korean, Japanese, and Chinese) have the sans-serif font as default, even though only Helv is preset for that while for serif the special Times New Roman variants for CJK were preset which should be much more capable to display Asian characters. I have changed those Asian languages to serif for now.

Comment 20

[Peter Weilbacher]

Comment on attachment 291533 [details] [diff] [review]
[checked in] implement kerning

I just checked this into trunk.

Comment 21

[Peter Weilbacher]

Attachment: Adjust default font settings in all.js, v2

I learned (by reading the PM Developer's Reference docs and asking on comp.os.os2.programmer.misc) that Times New Roman and Helvetica are installed by default these days and one can normally expect them to be there. So I replaced the remaining Tms Rmn and Helv entries in all.js with those Type1 fonts that we can display.

Mike, can you take a look? I would like to hear from you why the Asian languages were set to use sans-serif by default, even though the sans-serif font was less likely to contain those characters. And why languages like Cyrillic had "Tms Rmn" set for sans-serif... I also changed those languages that had sans-serif selected by default to serif to have all languages behave the same. (Or should I change all to sans-serif which is supposedly better for on-screen reading?)

Comment 22

[Mike Kaply [:mkaply]]

The Asian defaults were set that way because at the time it was requested by the localizers for those countries.

Do we know for a fact that this change will work on the appropriate version of OS/2?

Incidentally, the reason we used Helv and Tms Rmn was for speed. bitmap fonts are tons faster on OS/2

Comment 23

[Peter Weilbacher]

(In reply to comment #22)
> The Asian defaults were set that way because at the time it was requested by
> the localizers for those countries.

Hmm, did Asian OS/2 version ship with a different Helv font, one that had all/most those CJK characters? Because the Helv that I have doesn't seem to have them, and so CJK would almost always be taken from the replacement/Unicode font. Or did those people just set their sans font to something more capable?

> Do we know for a fact that this change will work on the appropriate version of
> OS/2?
> 
> Incidentally, the reason we used Helv and Tms Rmn was for speed. bitmap fonts
> are tons faster on OS/2

In the font rendering scheme that I had to implement, to be able to use the cairo font backend, I use FreeType. And with that I can only handle Type1 or TrueType fonts, so Helv and Tms Rmn are out. :-( Yes, that may well be a reason why current trunk is (much) slower for some pages than 1.8 branch was.

[At least I haven't found any way to load bitmap fonts in OS/2 format. Perhaps there was support for that format built into the Innotek Font Engine? With its sources closed I have no way to tell and nobody else seems to know about it...]

Comment 24

[Peter Weilbacher]

(In reply to comment #22)
> Do we know for a fact that this change will work on the appropriate version of
> OS/2?

Oh, forgot to answer this bit...
I think so, the PM reference, available online here
http://www.warpspeed.com.au/cgi-bin/inf2html.cmd?..%5Chtml%5Cbook%5CToolkt40%5CPM5.INF+495
says that Times New Roman and Helvetica are supplied. Perhaps this was only true for Warp4 but as we don't officially support Warp3 any more that should be fine.

Btw, that section of the PM docs also says that if you call for "Tmn Rmn" you would still get "Times New Roman"! Not sure if that is really true though...

Comment 25

[Mike Kaply [:mkaply]]

I'm also thinking about the Asian Os/2s as well. Those font choices were picked specifically for them.

Comment 26

[Mike Kaply [:mkaply]]

Sorry, missed the first response.

So Asian versions of Os/2 use font substitution to fill in the missing characters. There are some fonts that have all characters consistently and those are the ones they wanted to be the default.

Honestly I'm not sure all this matters that much anymore. I doubt we have many users of this in Japan/China. 

Comment 27

[Peter Weilbacher]

But perhaps you are right, it's better to check. But I'm more optimistic regarding the user base. :-) E.g. OS2.jp still seems quite active. There was also somebody from China who contacted me just last week... Will contact them to find out what they know about these issues.

Comment 28

[Peter Weilbacher]

OK, by now I have confirmed that "Times New Roman" and "Helvetica" are available on any system: the Fixpak file listings of any language under ftp://service.software.ibm.com/ps/products/os2/fixes/v4warp show that TNR*{OFM,PFB} and HELV*.{OFM,PFB} are present.

I also remembered that the Innotek Font Engine wouldn't work with "Helv" and "Tms Rmn", either, so they implemented a replacement font list in the registry under HKLM\SOFTWARE\InnoTek\InnoTek Font Engine\Fonts. Those two fonts are listed there by default. If this had not worked for many people we would have heard a lot more complaints. I have now released a new Fontconfig version that has the replacement from those two fonts to "Times New Roman" and "Helvetica" hardcoded. So it doesn't matter any more which of those we list in all.js.

A response I got from Korea indicates that some other fonts, that are so far not listed, are popular there. So I want to wait for further responses from Asia before checking in refinements to the default font lists.

Comment 29

[Peter Weilbacher]

Attachment: all.js changes v3

I have gotten some feedback from Korean and Japanese users and that indicates that the font sets in this selection should work on their systems. As we cannot use the OS/2 API any more (that would do the replacement as indicated by Mike in comment 26 automatically), we should choose as default the fonts that will have the most characters. That's the "Times New Roman WT/MT" family and so I'm changing some sans-serif language defaults to serif.

Since the last patch I have implemented an automatic substitution of Tms Rmn -> Times New Roman and Helv -> Helvetica, so we don't need to change those entries any more.

Comment 30

[Peter Weilbacher]

Comment on attachment 309279 [details] [diff] [review]
all.js changes v3

Mike, does this make sense to you? I would like to use this in beta 5 if you don't see a problem.

Comment 31

[Walter Meinl]

I'm not sure, if a new bug should be opened. What I see is that when I choose truetype fonts opposed to ofm fonts (Times Roman, Helvetica and Courier) that the spacing between lines seems to be sth. like 1 1/2 or 2. Testcase load e.g. http://bonsai.mozilla.org/cvsquery.cgi?branch=HEAD&file=mozilla/&date=day with font 'Times Roman' or with 'DejaVu serif' or this bug381333 with 'Courier' or 'DejaVu sans mono'. (Just in case it would not be too complicated to get a similar spacing for truetype fonts)

Comment 32

[Peter Weilbacher]

Yes, please open a new bug with a screenshot, and perhaps an excerpt of prefs.js that contains your font settings.

Comment 33

[Peter Weilbacher]

Attachment: [checked in] all.js changes v4

I didn't even remember that this was still open, but as it is we can as well fix the missing default fonts for MathML. (I tested that the TTF versions of the STIX fonts work well under OS/2 with current a trunk build, Deja Vu is quite good, too, but lacks stretching.)

Comment 34

[Peter Weilbacher]

Comment on attachment 318899 [details] [diff] [review]
[checked in] all.js changes v4

OS/2 only change in cross-platform file.

Comment 35

[Damon Sicore (:damons)]

Comment on attachment 318899 [details] [diff] [review]
[checked in] all.js changes v4

a1.9+=damons

Comment 36

[Peter Weilbacher]

Comment on attachment 318899 [details] [diff] [review]
[checked in] all.js changes v4

Checked this into trunk.

Comment 37

[Peter Weilbacher]

OK, there are still some problems, especially with languages that need font shaping. At some point I also want to rewrite gfxOS2Fonts/gfxOS2FontGroup to use the font replacement mechanism that is now used on Windows and Mac (to access all system fonts instead of just those in the *.font.* + *.font-list.* prefs). But currently, I don't have time to investigate either. I'll open a new bug for that when/if I do.