623974 - Letter spacing problem with D2D enabled and ClearType disabled

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[zgf1]

User-Agent:       Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b9pre) Gecko/20110107 Firefox/4.0b9pre
Build Identifier: Minefield 4.0b9pre (2011-01-07)

The issue with letter spacing occurs mostly after the letter 'm' (low caps)

Firefox4 ct off, d2d on:
http://zgf1.e-net24.pl/screensender/img/img_25752785.png

Firefox4 ct off, d2d off:
http://zgf1.e-net24.pl/screensender/img/img_25798368.png


WWW:
http://sport.onet.pl/boks/adamek-o-walce-z-kliczka-tak-robia-prawdziwi-wojow,1,4099489,wiadomosc.html




4.0b9pre (2011-01-05):
http://zgf1.e-net24.pl/screensender/img/img_4081469.png

Internet Explorer 8:
http://zgf1.e-net24.pl/screensender/img/img_4108442.png

Look at the words: spRZedam, dreamER, booMBo, wgRAne, geMIni...

Spacing issues.

Reproducible: Always

Steps to Reproduce:
1.Disable CT
2.Enable D2D
3.Go to www

Comment 1

[Jonathan Kew [:jfkthame]]

This is a continuation of bug 594889, which tried to address this issue, and made things better in some cases but did not completely fix it, as shown in the screenshots here.

Comment 2

[Jonathan Kew [:jfkthame]]

Attachment: patch, use GDI aliased spacing in DW when ClearType is off

Bug 594889 was basically doing the right thing, but for non-ClearType text I think we'll get better results by using "GDI aliased" metrics than the "natural" option. For example, a waterfall of small sizes of Times has much more consistent spacing this way.

Comment 3

[Jonathan Kew [:jfkthame]]

Attachment: screenshot of Times, 8px to 18px, no ClearType, unpatched trunk rendering

Comment 4

[Jonathan Kew [:jfkthame]]

Attachment: screenshot of Times, 8px to 18px, no ClearType, with patch to use aliased GDI widths

For comparison, here are screenshots of a Times waterfall, "before" and "after" this patch.

Comment 5

[Bas Schouten (:bas.schouten)]

Comment on attachment 502126 [details] [diff] [review]
patch, use GDI aliased spacing in DW when ClearType is off

We decide whether to do grayscale or aliased rendering based on the GASP table, isn't this what we should base our decision on here?

Comment 6

[Bas Schouten (:bas.schouten)]

See http://mxr.mozilla.org/mozilla-central/source/gfx/cairo/cairo/src/cairo-dwrite-font.cpp#349

Comment 7

[Jonathan Kew [:jfkthame]]

(In reply to comment #5)
> Comment on attachment 502126 [details] [diff] [review]
> patch, use GDI aliased spacing in DW when ClearType is off
> 
> We decide whether to do grayscale or aliased rendering based on the GASP table,
> isn't this what we should base our decision on here?

I don't think so. The choice we're making here isn't between GDI Aliased and GDI grayscale spacing, it's between GDI Aliased and GDI ClearType Natural Widths. Whether the rendering is aliased or grayscale makes little or no difference to the metrics, AFAIK; the big difference (in some fonts) comes when CT Natural Widths is enabled, but we never paint the glyphs that way with DirectWrite, so we should never use those metrics.

(In reply to comment #6)
> See
> http://mxr.mozilla.org/mozilla-central/source/gfx/cairo/cairo/src/cairo-dwrite-font.cpp#349

Out of curiosity, I hacked the do_grayscale() function here to always return TRUE. To my surprise, the ONLY change in the Times Waterfall example was to the first (8px) line, which became grayscale rather than aliased; but all the sizes from 9px to 17px remained aliased; only at 18px did grayscale kick in again. Arial showed similar behavior; Tahoma also similar, except that grayscale kicks in at 17px.

So - I'm not sure what is going on here, but the question of whether we use grayscale rendering or purely bi-level, aliased rendering does not seem to be determined solely by that function. But in any case, I think the proper option to give the DWrite GDI-compatible metrics functions is always FALSE, as we're never using DWrite to simulate GDI ClearType Natural Widths rendering.

Comment 8

[zgf1]

Jonathan, I downloaded your today's build and it looks very good at first glimpse, I'll test more tomorrow.

http://zgf1.e-net24.pl/screensender/img/img_30121780.png
http://zgf1.e-net24.pl/screensender/img/img_30159330.png

Thanks.

Comment 9

[neb1236]

Confirming improvement, I added it to my previous screenshot:

http://img341.imageshack.us/img341/8553/attachment2 [review]v.png

We are now really close to GDI, letter Spacing is exactly the same, and word spacing is very slightly bigger (5px, on 550px long text in my example, it's below 1%)
Good work

Comment 10

[Jonathan Kew [:jfkthame]]

zgf1, why did you change this bug to RESOLVED:FIXED? The patch here has not landed (or even received r+) yet; the bug isn't fixed until the patch lands (unless something else has changed in the meantime).

Re-opening, as I don't think this is actually fixed on trunk - a successful tryserver build is not a trunk resolution.

Comment 11

[zgf1]

Sorry, I thought that the problem has been fixed.

Comment 12

[zgf1]

Website rendering problem with hardware acceleration enabled.


Acceleration on:
http://zgf1.e-net24.pl/screensender/img/img_42757487.png



Acceleration off:
http://zgf1.e-net24.pl/screensender/img/img_42881414.png


WWW:
http://www.benchmark.pl/

Comment 13

[Jonathan Kew [:jfkthame]]

(In reply to comment #11)
> Sorry, I thought that the problem has been fixed.

I believe the patch here will fix the spacing problem - and your testing of the tryserver build seems to confirm this (thanks!) - but until the patch lands in the tree (which requires explicit approval-2.0 or blocking-2.0 status) it's not actually FIXED.

Comment 14

[Jonathan Kew [:jfkthame]]

Checked in:
http://hg.mozilla.org/mozilla-central/rev/6d621e3fc42e

Comment 15

[neb1236]

(In reply to comment #14)
> Checked in:
> http://hg.mozilla.org/mozilla-central/rev/6d621e3fc42e

The OS/Version is not Windows XP like you just modified but Windows 7 (and Vista)
I just checked the latest tinderbox build and everything looks fine.

(In reply to comment #12)
> Website rendering problem with hardware acceleration enabled.
> 
> Acceleration on:
> http://zgf1.e-net24.pl/screensender/img/img_42757487.png
> 
> Acceleration off:
> http://zgf1.e-net24.pl/screensender/img/img_42881414.png
> 
> WWW:
> http://www.benchmark.pl/

This issue is not due to letter spacing but line spacing, DW seems to increase it a bit, This happens only on text with no style, add a line-height: in CSS and the problem will be solved.
But I don't know who wrong here, designer or Firefox.

Comment 16

[Jonathan Kew [:jfkthame]]

(In reply to comment #15)
> (In reply to comment #14)
> > Checked in:
> > http://hg.mozilla.org/mozilla-central/rev/6d621e3fc42e
> 
> The OS/Version is not Windows XP like you just modified but Windows 7

Sorry, that was unintentional - result of a (webkit) browser bug at my end, I believe.

Comment 17

[neb1236]

(In reply to comment #15)
> (In reply to comment #12)
> > Website rendering problem with hardware acceleration enabled.
> > 
> > Acceleration on:
> > http://zgf1.e-net24.pl/screensender/img/img_42757487.png
> > 
> > Acceleration off:
> > http://zgf1.e-net24.pl/screensender/img/img_42881414.png
> > 
> > WWW:
> > http://www.benchmark.pl/
> 
> This issue is not due to letter spacing but line spacing, DW seems to increase
> it a bit, This happens only on text with no style, add a line-height: in CSS
> and the problem will be solved.
> But I don't know who's wrong here, designer or Firefox.

Forgot to mention, this bug this is not linked with ClearType on/off, should we file a bug?

Comment 18

[Bas Schouten (:bas.schouten)]

(In reply to comment #17)
> (In reply to comment #15)
> > (In reply to comment #12)
> > > Website rendering problem with hardware acceleration enabled.
> > > 
> > > Acceleration on:
> > > http://zgf1.e-net24.pl/screensender/img/img_42757487.png
> > > 
> > > Acceleration off:
> > > http://zgf1.e-net24.pl/screensender/img/img_42881414.png
> > > 
> > > WWW:
> > > http://www.benchmark.pl/
> > 
> > This issue is not due to letter spacing but line spacing, DW seems to increase
> > it a bit, This happens only on text with no style, add a line-height: in CSS
> > and the problem will be solved.
> > But I don't know who's wrong here, designer or Firefox.
> 
> Forgot to mention, this bug this is not linked with ClearType on/off, should we
> file a bug?

The screenshot was definitely taken with ClearType off ...

Comment 19

[neb1236]

(In reply to comment #18)
> (In reply to comment #17)
> > (In reply to comment #15)
> > > (In reply to comment #12)
> > > > Website rendering problem with hardware acceleration enabled.
> > > > 
> > > > Acceleration on:
> > > > http://zgf1.e-net24.pl/screensender/img/img_42757487.png
> > > > 
> > > > Acceleration off:
> > > > http://zgf1.e-net24.pl/screensender/img/img_42881414.png
> > > > 
> > > > WWW:
> > > > http://www.benchmark.pl/
> > > 
> > > This issue is not due to letter spacing but line spacing, DW seems to increase
> > > it a bit, This happens only on text with no style, add a line-height: in CSS
> > > and the problem will be solved.
> > > But I don't know who's wrong here, designer or Firefox.
> > 
> > Forgot to mention, this bug this is not linked with ClearType on/off, should we
> > file a bug?
> 
> The screenshot was definitely taken with ClearType off ...

Yes ClearType is off here
Sorry, it seems I wasn't clear enough, This bug shown by those screenshot can be reproduced with ClearType On (http://img535.imageshack.us/img535/6249/sanstitre1ka.png). And as an example if you look at the left Menu list, the last Item disappeared pushed down by a wider Line spacing. Line spacing problem can be easily tested with any raw text btw.
So we have line spacing problem in DW with no connection with ClearType (Don't think it's related to this present bug)

Comment 20

[Jonathan Kew [:jfkthame]]

It is expected that DW may give somewhat different font metrics/line spacing from GDI. (See also bug 578620, for example.) That shouldn't be a problem, as layout should normally accommodate varying font sizes. I haven't studied this example enough to figure out whether there's actually a layout bug here, or just poor website design (e.g. making rigid assumptions about text size and spacing). A simplified testcase would make it easier to analyze.

In any case, if there's a problem here (and if it's not already covered by existing DW bugs - see bug 549115), it should be handled in a separate bug.

Comment 21

[Rich Parker]

Is this just verified as "looking good", or has someone compared with Firefox 3.6 or another browser pixel by pixel to make sure it's identical?

Comment 22

[neb1236]

Comparison between Firefox 3.6 and 4.0b10pre with different font and size:
http://img413.imageshack.us/img413/6783/sanstitre2yyc.png

It's not pixel exact placement but, close enough I think (notice it's a test with no space since word spacing is a bit bigger). If needed I can make a more advanced comparison.

Comment 23

[Rich Parker]

Could this affect the truncation of bookmark names?  I have several that started getting cut off with ellipses in Beta 10.  For example, "Student Loan Corporation" now shows as "Student Loan Corporati...".  It seems like Firefox may be miscalculating the size of the text and truncating unnecessarily.

Comment 24

[Jonathan Kew [:jfkthame]]

There's a problem related to sizing of the Bookmarks menu, but I don't think it's this; there's something else mysterious going on. See bug 622507.

Comment 25

[Rich Parker]

Okay, I already logged 629014 anyway.  Thanks.