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

RESOLVED FIXED in mozilla12

Status

()

defect
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: kdevel, Assigned: karlt)

Tracking

({regression})

Trunk
mozilla12
x86_64
All
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(6 attachments, 4 obsolete attachments)

Reporter

Description

8 years ago
Posted file Testcase (obsolete) —
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.
Reporter

Updated

8 years ago
Attachment #586822 - Attachment mime type: text/plain → text/html
Reporter

Comment 1

8 years ago
Posted image measurement
Reporter

Updated

8 years ago
OS: Other → Linux
Hardware: Other → x86_64
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.
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.
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.
Could you try this either in safe mode or with all add-ons disabled?
Reporter

Comment 6

8 years ago
Produced with 2012-01-08-03-10-24-mozilla-central cf8c9f9aeefc.
Reporter

Comment 7

8 years ago
(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?
Reporter

Comment 8

8 years ago
Can reproduce with current m-c tip revision 9a230265bad5.
Reporter

Comment 9

8 years ago
(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.
(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.
Reporter

Comment 11

8 years ago
Posted image screenshot last good nightly (pass) (obsolete) —
Reporter

Updated

8 years ago
Attachment #586831 - Attachment description: last good nightly → screenshot last good nightly (pass)
Reporter

Comment 12

8 years ago
Posted image screenshot first bad nightly (fail) (obsolete) —
Reporter

Comment 13

8 years ago
last good nightly: 2010-12-08-03-mozilla-central 95452499f3d6
first bad nightly: 2010-12-09-03-mozilla-central 11e328a49e0a
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.
Reporter

Comment 15

8 years ago
(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.
Reporter

Comment 16

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

It is necessary to enlarge the screenshots, not the view of the testcase.
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)
Reporter

Comment 18

8 years ago
(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?
Reporter

Comment 19

8 years ago
Posted file revised Testcase
Attachment #586822 - Attachment is obsolete: true
Reporter

Comment 20

8 years ago
Attachment #586831 - Attachment is obsolete: true
Reporter

Comment 21

8 years ago
Attachment #586832 - Attachment is obsolete: true
Reporter

Updated

8 years ago
Attachment #586837 - Attachment mime type: text/plain → text/html
Reporter

Comment 22

8 years ago
(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.
Reporter

Comment 23

8 years ago
The first bad revision is:
changeset:   58959:11e328a49e0a
user:        Karl Tomlinson <karlt+@karlt.net>
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
Component: Untriaged → Layout: Text
Product: Firefox → Core
QA Contact: untriaged → layout.fonts-and-text
Assignee

Comment 24

8 years ago
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?
Reporter

Comment 25

8 years ago
(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>
Reporter

Comment 26

8 years ago
(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.
Assignee

Comment 27

8 years ago
(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.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Assignee

Comment 28

8 years ago
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"
    }
  }}
Assignee

Updated

8 years ago
Keywords: regression
Assignee

Comment 29

8 years ago
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.
Blocks: 605043
Assignee

Comment 30

8 years ago
Reftest at http://hg.mozilla.org/try/rev/5b7c39ec6faf
Assignee: nobody → karlt
Assignee

Comment 31

8 years ago
As expected the reftest also fails on GDI because we also round to pixels there.
OS: Linux → All
Assignee

Comment 32

8 years ago
Attachment #587917 - Flags: review?(jfkthame)
Assignee

Comment 33

8 years ago
Removed an old comment that no longer applies, and made a couple of formatting changes to make things easier to read.
Attachment #587917 - Attachment is obsolete: true
Attachment #587917 - Flags: review?(jfkthame)
Attachment #588255 - Flags: review?(jfkthame)

Comment 34

8 years ago
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.
Attachment #588255 - Flags: review?(jfkthame) → review+
Reporter

Comment 35

8 years ago
(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
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
Target Milestone: --- → mozilla12
https://hg.mozilla.org/mozilla-central/rev/0a9fd06dea54
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.