716402 - first in a row of identical letters positioned 1 px too far apart

Status: RESOLVED FIXED

(Core :: Layout: Text and Fonts, defect)

Description

[Stefan]

Attachment: Testcase

User Agent:  

Steps to reproduce:

1. Open the testcase (with default setting gfx.font_rendering.harfbuzz.scripts;3).


Actual results:

1. The gap between the first and the second '1' is one 1 px wider than the gaps of any other two consecutive 1's.


Expected results:

1. Gaps have same widths.

Comment 1

[Stefan]

Attachment: measurement

Comment 2

[Bill Gianopoulos [:WG9s]]

I am unable to reproduce an issue with current mozilla-central code under Linux.  Will try Windows also, but it seems this is an issue that has already been fixed and the fix will be released as soon as that code gets into the released product.

Comment 3

[Bill Gianopoulos [:WG9s]]

So this should be working at least by Firefox version 12 if not before.  I will try to track down the fix closer that that.

Comment 4

[Bill Gianopoulos [:WG9s]]

Hmm... but I see this is filed on trunk so there is something here I don't understand.  I can not duplicate this issue on current trunk under Linux.

Comment 5

[Bill Gianopoulos [:WG9s]]

Could you try this either in safe mode or with all add-ons disabled?

Comment 6

[Stefan]

Produced with 2012-01-08-03-10-24-mozilla-central cf8c9f9aeefc.

Comment 7

[Stefan]

(In reply to Bill Gianopoulos [:WG9s] from comment #2)
> I am unable to reproduce an issue with current mozilla-central code under
> Linux.

Can you prepare a screenshot?

Comment 8

[Stefan]

Can reproduce with current m-c tip revision 9a230265bad5.

Comment 9

[Stefan]

(In reply to Bill Gianopoulos [:WG9s] from comment #5)
> Could you try this either in safe mode or with all add-ons disabled?

Have no add-ons, tried also in safe-mode. The settings in~/.fonts.conf do not change the first gap width.

Comment 10

[Bill Gianopoulos [:WG9s]]

(In reply to Stefan from comment #7)
> (In reply to Bill Gianopoulos [:WG9s] from comment #2)
> > I am unable to reproduce an issue with current mozilla-central code under
> > Linux.
> 
> Can you prepare a screenshot?
'
Actually, you need to produce screen shots for what you consider a pass and fail for running your testcase.

Comment 11

[Stefan]

Attachment: screenshot last good nightly (pass)

Comment 12

[Stefan]

Attachment: screenshot first bad nightly (fail)

Comment 13

[Stefan]

last good nightly: 2010-12-08-03-mozilla-central 95452499f3d6
first bad nightly: 2010-12-09-03-mozilla-central 11e328a49e0a

Comment 14

[Bill Gianopoulos [:WG9s]]

Oh great now i need you to explain exactly what is the difference between the pass and fail screenshots because they look the same to me.

Comment 15

[Stefan]

(In reply to Bill Gianopoulos [:WG9s] from comment #14)
> they look the same to me.

1. Open the screenshots "pass" and "fail" in adjacent tabs and switch between them back and forth. The 1's number 2-9 seem to shift left/right.

2. Open the "measurement" screenshot. There a multiple red rectangles having all the same width. Every rectangle is left aligned with the pixel right after the stem of the 1's glyph. Only the rectangles between the figures 2-9 are also right-aligned with the rightmost stem-pixel of the adjacent glyph.

3. Enlarge the screenshots and count the pixels between the foots of the ones. The foot-gap is three pixel wide between 2-9 and four pixel wide between 1-2.

Comment 16

[Stefan]

(In reply to Stefan from comment #15)
> 3. Enlarge the screenshots

It is necessary to enlarge the screenshots, not the view of the testcase.

Comment 17

[Bill Gianopoulos [:WG9s]]

I still do not see any difference (hint if you want to show an issue it would be nice if on the fail case the to lines of 1's did NOT align with each other)

Comment 18

[Stefan]

(In reply to Bill Gianopoulos [:WG9s] from comment #17)
> I still do not see any difference (hint if you want to show an issue it
> would be nice if on the fail case the to lines of 1's did NOT align with
> each other)

Which of the three methods to perceive the difference given in Comment 15 did not work out for you?

Comment 19

[Stefan]

Attachment: revised Testcase

Comment 20

[Stefan]

Attachment: screenshot last good nightly (pass)

Comment 21

[Stefan]

Attachment: screenshot first bad nightly (fail)

Comment 22

[Stefan]

(In reply to Bill Gianopoulos [:WG9s] from comment #17)
> it would be nice if on the fail case the to lines of 1's did NOT align with
> each other)

The issue is not an alignment issue between the two lines (rows) of the original testcase. The second line was misleading, I have removed it.

The issue is the excess 1 px horizontal distance between one No. 1 and one No. 2 compared to the distances between the other horizontally adjacent ones.

Comment 23

[Stefan]

The first bad revision is:
changeset:   58959:11e328a49e0a
user:        Karl Tomlinson <[e-mail]>
date:        Thu Dec 09 20:29:39 2010 +1300
summary:     b=569770 part 9: use gfxHarfBuzzShaper when suitable r=jfkthame a=blocking

http://hg.mozilla.org/mozilla-central/rev/11e328a49e0a

http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=95452499f3d6&tochange=11e328a49e0a

Comment 24

[Karl Tomlinson (:karlt)]

I see the problem in attachment 586839 [details], but am not able to reproduce.

Can you tell me which font is used to render these characters, please?
This extension may be useful.
https://addons.mozilla.org/en-US/firefox/addon/fontinfo/

Is your locale "de"?

Just to be sure, you see this with Mozilla builds, right?

(In reply to Stefan from comment #9)
> Have no add-ons, tried also in safe-mode. The settings in~/.fonts.conf do
> not change the first gap width.

Did you try setting hintstyle to hintfull and hintnone?

Comment 25

[Stefan]

(In reply to Karl Tomlinson (:karlt) from comment #24)
> Can you tell me which font is used to render these characters, please?

fontinfo says it's "Times New Roman". According to lsof the fontfilename is "times.ttf". Fontforge says 

Fontname TimesNewRomanPSMT
Version  2.82

I can also reproduce the issue with Thorndale AMT and Arial (first gap smaller).

> Is your locale "de"?

"de_DE@euro" (std here seems to be "de_DE.utf8"). The issue does not depend on the locale setting.

> Just to be sure, you see this with Mozilla builds, right?

I use this one:

http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2012/01/2012-01-08-03-10-24-mozilla-central/firefox-12.0a1.en-US.linux-x86_64.tar.bz2

> (In reply to Stefan from comment #9)
> > Have no add-ons, tried also in safe-mode. The settings in~/.fonts.conf do
> > not change the first gap width.
> 
> Did you try setting hintstyle to hintfull and hintnone?

This is the .fonts.conf I use, but the wider gap is also visible w/o:

      1 <?xml version="1.0"?>
      2 <!DOCTYPE fontconfig SYSTEM "fonts.dtd">
      3 <fontconfig>
      4  <match target="font" >
      5   <edit mode="assign" name="rgba" >
      6    <const>none</const>
      7   </edit>
      8  </match>
      9  <match target="font" >
     10   <edit mode="assign" name="hinting" >
     11    <bool>false</bool>
     12   </edit>
     13  </match>
     14  <match target="font" >
     15   <edit mode="assign" name="hintstyle" >
     16    <const>hintnone</const>
     17   </edit>
     18  </match>
     19  <match target="font" >
     20   <edit mode="assign" name="antialias" >
     21    <bool>true</bool>
     22   </edit>
     23  </match>
     24 </fontconfig>

Comment 26

[Stefan]

(In reply to Karl Tomlinson (:karlt) from comment #24)
> Did you try setting hintstyle to hintfull and hintnone?

Yes, the wider gap does not depend on hinting.

Comment 27

[Karl Tomlinson (:karlt)]

(In reply to Stefan from comment #25)
> fontinfo says it's "Times New Roman". According to lsof the fontfilename is
> "times.ttf". Fontforge says 
> 
> Fontname TimesNewRomanPSMT
> Version  2.82

Thanks.  I can reproduce with that font, with hinting set to true or false.

Comment 28

[Karl Tomlinson (:karlt)]

This is what we're getting from hb_buffer_get_glyph_positions for the first 3 glyphs in a series of more than 3 "1"s at 16px.  I wonder why the x_offsets are non-zero.  The advances are reduced by the offsets.  I'd expect kerning to change advances, but not offsets.

(gdb) p posInfo[0]@3
$20 = {{
    x_advance = 504832, 
    y_advance = 0, 
    x_offset = 0, 
    y_offset = 0, 
    var = {
      u32 = 0, 
      i32 = 0, 
      u16 = {0, 0}, 
      i16 = {0, 0}, 
      u8 = "\000\000\000", 
      i8 = "\000\000\000"
    }
  }, {
    x_advance = 485376, 
    y_advance = 0, 
    x_offset = -19456, 
    y_offset = 0, 
    var = {
      u32 = 0, 
      i32 = 0, 
      u16 = {0, 0}, 
      i16 = {0, 0}, 
      u8 = "\000\000\000", 
      i8 = "\000\000\000"
    }
  }, {
    x_advance = 485376, 
    y_advance = 0, 
    x_offset = -19456, 
    y_offset = 0, 
    var = {
      u32 = 0, 
      i32 = 0, 
      u16 = {0, 0}, 
      i16 = {0, 0}, 
      u8 = "\000\000\000", 
      i8 = "\000\000\000"
    }
  }}

Disabling kerning by returning 0 from the kerning_func gives consistent advances and removes the offsets.  Comparing advances indicates that kerning was applied to the first advance, but was applied twice to the second advance, as well as to the offset.

(gdb) p posInfo[0]@3
$32 = {{
    x_advance = 524288, 
    y_advance = 0, 
    x_offset = 0, 
    y_offset = 0, 
    var = {
      u32 = 0, 
      i32 = 0, 
      u16 = {0, 0}, 
      i16 = {0, 0}, 
      u8 = "\000\000\000", 
      i8 = "\000\000\000"
    }
  }, {
    x_advance = 524288, 
    y_advance = 0, 
    x_offset = 0, 
    y_offset = 0, 
    var = {
      u32 = 0, 
      i32 = 0, 
      u16 = {0, 0}, 
      i16 = {0, 0}, 
      u8 = "\000\000\000", 
      i8 = "\000\000\000"
    }
  }, {
    x_advance = 524288, 
    y_advance = 0, 
    x_offset = 0, 
    y_offset = 0, 
    var = {
      u32 = 0, 
      i32 = 0, 
      u16 = {0, 0}, 
      i16 = {0, 0}, 
      u8 = "\000\000\000", 
      i8 = "\000\000\000"
    }
  }}

Comment 29

[Karl Tomlinson (:karlt)]

The numbers from hb_buffer_get_glyph_positions added together would actually lead to equally spaced glyphs, but I guess it's the rounding to integer pixels that we apply separately to advance and offset that causes the problem.

(HarfBuzz apparently applies the kerning half to the advance of the leading glyph and half to the following glyph.   The offset of the following glyph is adjusted by the same amount so that there is the correct distance between the glyphs.  It might be done this way so that each glyph shares the same change in space, as seen in text selection, for example.)

We can probably come up with a better way to do the rounding.

Comment 30

[Karl Tomlinson (:karlt)]

Reftest at http://hg.mozilla.org/try/rev/5b7c39ec6faf

Comment 31

[Karl Tomlinson (:karlt)]

As expected the reftest also fails on GDI because we also round to pixels there.

Comment 32

[Karl Tomlinson (:karlt)]

Attachment: improve inter-glyph pixel rounding

Comment 33

[Karl Tomlinson (:karlt)]

Attachment: improve inter-glyph pixel rounding v1.1

Removed an old comment that no longer applies, and made a couple of formatting changes to make things easier to read.

Comment 34

[Behdad Esfahbod]

Thanks Karl.  Your analysis is right.  I changed HarfBuzz to apply kerning (I think TrueType kerning only for now) half each side, for better cursor positioning.

Comment 35

[Stefan]

(In reply to Karl Tomlinson (:karlt) from comment #33)

The patch works and additionally alleviates the too tight kerning: Compare the P-A-distances in the Arial rendered words "SOPA" and "PIPA" in http://www.heise.de/tp/blogs/6/151249

Comment 36

[Stefan]

Attachment: screenshot patch 1.1 and kerning (P-A in "SOPA" and "PIPA")

Comment 37

[Jonathan Kew [:jfkthame]]

I pushed this to -inbound, as I'd like it to start baking before we do a harfbuzz update and other font-related stuff.

https://hg.mozilla.org/integration/mozilla-inbound/rev/0a9fd06dea54

Comment 38

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/0a9fd06dea54

Comment 39

[Karl Tomlinson (:karlt)]

reftest: https://hg.mozilla.org/integration/mozilla-inbound/rev/f80d6519ad29

Comment 40

[Matt Brubeck (:mbrubeck)]

reftest: https://hg.mozilla.org/mozilla-central/rev/f80d6519ad29