too much space above and below line when using Cambria Math font

RESOLVED FIXED in Firefox 41

Status

()

Core
Layout: Text
RESOLVED FIXED
7 years ago
2 years ago

People

(Reporter: Jaan Vajakas, Assigned: fredw)

Tracking

(Depends on: 1 bug)

unspecified
mozilla41
x86
Windows XP
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox41 fixed)

Details

(URL)

Attachments

(4 attachments, 8 obsolete attachments)

494 bytes, text/html
Details
81.01 KB, image/png
Details
1.07 KB, text/html
Details
11.50 KB, patch
Details | Diff | Splinter Review
(Reporter)

Description

7 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.6) Gecko/20100625 Firefox/3.6.6 ( .NET CLR 3.5.30729)
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.6) Gecko/20100625 Firefox/3.6.6 ( .NET CLR 3.5.30729)

There is extra space above and below lines containing the Unicode blackboard bold letter R in the default font.

Reproducible: Always

Steps to Reproduce:
1. Open the file R.html in Firefox
2. (Optionally) make the Firefox window narrower, causing text to be wrapped on more lines, making the problem more visible and showing that only lines containing blackboard R have extra space
Actual Results:  
There is more than one line height of extra space above and below each line containing the blackboard R character.

Expected Results:  
No extra space above and below lines containing the blackboard R character.
(Reporter)

Comment 1

7 years ago
Created attachment 477881 [details]
the test document that rendered incorrectly
(Reporter)

Comment 2

7 years ago
R.html is the attachment I added

Comment 3

7 years ago
Not seeing this on Linux, either 3.6.8 or latest trunk (Mozilla/5.0 (X11; Linux i686; rv:2.0b7pre) Gecko/20100922 Firefox/4.0b7pre).

Have you tested in a beta? It may already be fixed.
This probably depends on the exact font that \mathbb{R} is coming from on Jaan's system.  The font could just have broken metrics.....
(Reporter)

Comment 5

7 years ago
Created attachment 478739 [details]
test page for the font Cambria Math

It seems that on my computer Firefox's default font is Times New Roman but the as Times New Roman does not contain the blackboard R character it uses Cambria Math as substitute. This attachment contains Cambria Math text without special characters and on my computer it is rendered with the same extra space above and below each line. Internet Explorer 8 does not display this extra space.
(Reporter)

Comment 6

7 years ago
@dolphinling: I also tested with Firefox Portable 3.6.10 and Firefox Portable 4.0 beta 6. They both had same problem with Cambria Math but Firefox 4 used another font for the blackboard R so Firefox 4 rendered the first document correctly (on the other hand, the font that FF4 used was sans serif which does not fit as well with Times as Cambria Math).
(Reporter)

Comment 7

7 years ago
I tested on a Windows 7 machine and the situation with the Cambria Math test page was the same: Firefox 3.6.10 had huge line spacing, IE8 had not. However, on Windows 7 Firefox used another font for blackboard bold R so the first test page showed correctly.
I assume that on the Cambria Math page Firefox 4 has the same spacing issue?

Thanks for hunting that down, by the way!
Status: UNCONFIRMED → NEW
Ever confirmed: true
Summary: too much space above and below ℝ → too much space above and below line when using Cambria Math font
(Reporter)

Comment 9

7 years ago
@Boris: Yes, all configurations that I tested (FF4beta6 on Windows XP, FF 3.6.6 on Windows XP and FF 3.6.10 on Windows XP and Windows 7) had the Cambria Math page spacing issue.
I think you'll find that FF4beta on Win7 will _not_ have the same issue (perhaps depending on your hardware). It appears that this happens with the GDI-based font backend, which is the only one present in FF3.6 and is the only option on WinXP; but it does not happen with the DirectWrite backend, which will normally be used by default on Win7 unless the hardware isn't adequate to support it.

The issue arises because the usWinAscent and usWinDescent fields in the OS/2 table of Cambria Math have very large values, designed to accommodate greatly-enlarged versions of some of the math glyphs, and the GDI font code is using these fields to determine line spacing. The sTypoAscender and sTypoDescender fields have more "normal" values, and these are what the DWrite code uses.

We should probably consider modifying the GDI code to use the typographic metrics, though we'll need to watch carefully for other effects of such a change.
(Reporter)

Comment 11

7 years ago
Now I tested FF 4.0 beta 6 on Windows 7 and for some reason it had the same problem with the Cambria Math page. Even setting gfx.font_rendering.directwrite.enabled to true in about:config (it was false by default) and restarting Firefox did not change anything.
Is your graphics hardware possibly blacklisted?  Does about:support have anything useful to say about directwrite?
(Reporter)

Comment 13

7 years ago
CORRECTION: Now I tested again on the same Win7 machine and FF4.0b6 works correctly with the Cambria Math page if gfx.font_rendering.directwrite.enabled is set to true (a Firefox restart seems to be needed for this setting to apply). Maybe I did not restart Firefox properly the previous time (maybe I forgot some window open). So the hypothesis that this is a GDI-only bug is probably valid.
Duplicate of this bug: 609656

Comment 15

6 years ago
I was going to file a new bug on this, since we I noticed a giant insertion point displaying in ranges of Cambria Math text on slides in PowerPoint Web App, but fortunately came across this bug before I filed a dup. I'd like to throw in another vote in favor of fixing this.

Comment 16

6 years ago
(In reply to comment #15)
> I was going to file a new bug on this, since we I noticed a giant insertion
> point displaying in ranges of Cambria Math text on slides in PowerPoint Web
> App, but fortunately came across this bug before I filed a dup. I'd like to
> throw in another vote in favor of fixing this.

Diana, are you using 3.6 or 4.0beta?  If you could attach the build number you're using that would be really helpful (enter about: in the url bar).  If you're using 4.0beta, it would be useful to know whether DirectWrite is enabled as the metrics will be different in that case (enter about:support in the url bar).
See comment 10 (and 13). This issue occurs with GDI font rendering, and does not occur with DirectWrite, because the APIs (GetOutlineTextMetrics for GDI vs IDWriteFontFace::GetMetrics for DW) we're using base their results on different metrics fields within the font.
Created attachment 515891 [details] [diff] [review]
WIP - patch to read font metrics from sfnt tables under GDI

I started on a patch for this some time ago; just refreshed it for current trunk. This still needs significant testing, though.

Comment 19

6 years ago
Thanks for the comments, John and Jonathan. To answer John's questions, I see this repro in 4.0b12 without DirectWrite enabled on Windows 7.
Duplicate of this bug: 521678
Bug 643781 comment 15 observes that DirectWrite uses the USE_TYPO_METRICS bit in fsSelection to decide whether to use win or typo metrics from the os/2 table.
(InitMetricsFromSfntTables uses the hhea table.)
Duplicate of this bug: 676291
Created attachment 590264 [details]
Quadruple Spacing Thunderbird 10 Beta 4 with Cambria Math screenshot

Windows 7 64 bit. Thunderbird 7 on Alienware laptop:
If I set gfx.font_rendering.directwrite.enabled to true the problem goes away but then Thunderbird runs really slowly.

Cannot reproduce this problem on Firefox 12 Nightly

Comment 24

5 years ago
This bug still exists in 
Mozilla/5.0 (Windows NT 5.1; rv:14.0) Gecko/20120330 Firefox/14.0a1

In the case that Cambria Math is set as font-family there probably can't be done anything other than fixing / changing the font parsing code,
but in the case that Cambria Math is used as a substitute font it would make sense to just skip Cambria Math when searching for a substitute font.
(Because for most characters there are alternatives to using Cambria Math.)
Blocks: 739804

Comment 25

5 years ago
By following this comment https://bugzilla.mozilla.org/show_bug.cgi?id=754452#c25 I tried on FF13/Nightly on Win 7 with 'gfx.direct2d.disabled' set to true, I'm able to display the extra space above and below the line of the testcase.

In addition if you set 'gfx.font_rendering.directwrite.enabled' to true as in comment https://bugzilla.mozilla.org/show_bug.cgi?id=676291#c6 the text is displayed correctly again.

Comment 26

5 years ago
Hi,
I don't exactly use Cambria Math font but Verdana Sans Serif.
I get the same problem (too much space above and below character).
But in my case it's just one character which causes this. It's the round multiply dot (⋅).

I use Windows XP 32 bit.
The strange thing though is that I get the error on my laptop but not at my desktop machine (also Win XP 32 bit). Both use the current Firefox 13.
I don't get the error when I install Firefox 12.

Comment 27

5 years ago
i am experiencing this problem with FF13 on vista.  these characters in particular:
↖↗↘↙

it seems that cambria math is being used as the substitute font (used the "font information 0.1" add-on to verify this).  is there a way around this until the bug is fixed?

did not have the issue in previous versions of FF.

Comment 28

5 years ago
I confirm this appeared for me with FF13 (winXP32), I noticed it with ⋅. It affects selection too, so I can't select anything on the lines above or below, for some (variable) distance to the left and right of the character.

Comment 29

5 years ago
Hi,

installed "fontinfo 0.1" add on too and can confirm now that "Cambria Math" is being used besides Times New Roman (standard and bold) and Verdana (standard and bold).
On the desktop machine where I don't experience the problem only "Cambria" font is used (though I don't know why that is). 
Does anyone know how I could set my Laptop to use "Cambria" instead of "Cambria Math"? Thank you in advance.

Comment 30

5 years ago
Note that bug 739804 should eliminate the use of Cambria Math for fallback for the problem characters mentioned here.  The problem with explicit use of Cambria Math under GDI remains.
(Assignee)

Updated

4 years ago
See Also: → bug 947650
(Assignee)

Updated

3 years ago
Blocks: 947650
(Assignee)

Updated

3 years ago
See Also: bug 947650
The line-height fixes in bug 643781 (incorporating the WIP patch from here) should resolve this, I believe.
Depends on: 643781
Argh, wrong bug # - I meant bug 657864.
Depends on: 657864
No longer depends on: 643781
(Assignee)

Comment 33

3 years ago
(In reply to Jonathan Kew (:jfkthame) from comment #31)
> The line-height fixes in bug 643781 (incorporating the WIP patch from here)
> should resolve this, I believe.

Thanks. That seems to work for me for Cambria Math but not for the other math fonts (Latin Modern, TeX Gyre * and STIX Word). See attachment 8344237 [details].
Hmm, at least in the case of Latin Modern (I didn't check all the others yet), it seems that we'd need to use the sTypo* metrics, but the font doesn't have the USE_TYPO_METRICS flag set in fsSelection. Currently, I wrote the patch to only override hhea metrics with OS/2 sTypo* metrics if that flag is set.

We could try respecting the sTypo* metrics in all cases; that should help here. Pushed https://tbpl.mozilla.org/?tree=Try&rev=25f55a896db1 to see how that fares on unit tests...
(In reply to Jonathan Kew (:jfkthame) from comment #34)
> Hmm, at least in the case of Latin Modern (I didn't check all the others
> yet), it seems that we'd need to use the sTypo* metrics, but the font
> doesn't have the USE_TYPO_METRICS flag set in fsSelection. Currently, I
> wrote the patch to only override hhea metrics with OS/2 sTypo* metrics if
> that flag is set.
> 
> We could try respecting the sTypo* metrics in all cases; that should help
> here. Pushed https://tbpl.mozilla.org/?tree=Try&rev=25f55a896db1 to see how
> that fares on unit tests...

Well, regardless of what tryserver thinks, running this locally on OS X I'm seeing some fonts with excessively tight line spacing - including such basics as Lucida Grande and Verdana, which makes b.m.o pages look bad. :( So I don't think we'll be able to use this; too many poorly-constructed fonts out there.
(Assignee)

Comment 36

3 years ago
(In reply to Jonathan Kew (:jfkthame) from comment #35)
> Well, regardless of what tryserver thinks, running this locally on OS X I'm
> seeing some fonts with excessively tight line spacing - including such
> basics as Lucida Grande and Verdana, which makes b.m.o pages look bad. :( So
> I don't think we'll be able to use this; too many poorly-constructed fonts
> out there.

So you mean only respecting sTypo* metrics when explicitly asked? I'm not sure this solves the issue with the other math fonts, anyway. It would be great to find what the problem exactly is, so that we can report that to the STIX and GUST projects.
In Latin Modern, the hhea ascent/descent are the same as the usWinAscent/Descent values in OS/2 (i.e. large enough for the full extent of the largest glyphs), while the sTypo* values are appropriate for "normal" line spacing.

The question is whether we should favor the hhea or OS/2 sTypo metrics, and the answer is...unclear, in general.

If the foundry were to set the USE_TYPO_METRICS flag, we could use that as an indication that OS/2 sTypo should override hhea.

Another option would be to check for the presence of a MATH table, and always prefer the sTypo metrics in this case. I've pushed another try job using this approach; I suspect this may actually work. https://tbpl.mozilla.org/?tree=Try&rev=bd1406635401
(Assignee)

Comment 38

3 years ago
Relying on the USE_TYPO_METRICS sounds best (we should report that to MATH font authors) but relying on the MATH table seems OK for now.

However, I tried the patches with https://hg.mozilla.org/try/rev/97b4c599b7d0 and that didn't work. I want to try again with a fresh build. What are exactly the patches that should be taken?
"Didn't work" as in couldn't build with the patch, or didn't give good results?

Try current m-c with the patches from bug 657864:
  https://bugzilla.mozilla.org/attachment.cgi?id=8369407 (windows/dwrite)
  https://bugzilla.mozilla.org/attachment.cgi?id=8369399 (windows/gdi & macosx)
  https://bugzilla.mozilla.org/attachment.cgi?id=8369405 (reftest fixes, optional)

plus https://hg.mozilla.org/try/rev/97b4c599b7d0 to respect sTypo metrics in MATH fonts.
(Assignee)

Comment 40

3 years ago
(In reply to Jonathan Kew (:jfkthame) from comment #39)
> "Didn't work" as in couldn't build with the patch, or didn't give good
> results?

The latter.

> 
> Try current m-c with the patches from bug 657864:
>   https://bugzilla.mozilla.org/attachment.cgi?id=8369407 (windows/dwrite)
>   https://bugzilla.mozilla.org/attachment.cgi?id=8369399 (windows/gdi &
> macosx)
>   https://bugzilla.mozilla.org/attachment.cgi?id=8369405 (reftest fixes,
> optional)
> 
> plus https://hg.mozilla.org/try/rev/97b4c599b7d0 to respect sTypo metrics in
> MATH fonts.

That were the patches I took. I tried again with a fresh build as well as with the Linux builds from the try server output, but that does not change the result. See http://www.maths-informatique-jeux.com/ulule/mathml_torture_test/ascent-descent.html for a testcase that does not involve MathML and uses WOFF Open Source MATH fonts.
Ah, you're testing on Linux: those patches didn't touch the Linux backend. (That testcase looks good here on OS X with patches, FWIW. I'll check on WinXP shortly.)

We should be able to do a similar fix in the FT2 backend for Linux; I'll post a further patch in a while...
(Assignee)

Comment 42

3 years ago
(In reply to Jonathan Kew (:jfkthame) from comment #41)
> Ah, you're testing on Linux: those patches didn't touch the Linux backend.
> (That testcase looks good here on OS X with patches, FWIW. I'll check on
> WinXP shortly.)
> 
> We should be able to do a similar fix in the FT2 backend for Linux; I'll
> post a further patch in a while...

ah, ok, yes I was testing on Linux. I wanted to try on my Windows 7 VM but the try server build was not ready yet. Thanks.
(Assignee)

Comment 43

3 years ago
I'm guessing this should be marked a duplicate of bug 947650 or the dependency should be changed, since the most recent patches are now worked out on bug 947650.
(Assignee)

Updated

3 years ago
Blocks: 1014498
(Assignee)

Updated

3 years ago
Duplicate of this bug: 1014498
Duplicate of this bug: 356826
(Assignee)

Comment 46

3 years ago
@Jonathan: Do you know the status with these ascent/descent bugs on Windows? I updated my test http://fred-wang.github.io/MathFonts with the latest GUST fonts but I still see the problem (although I thought it didn't show up when I tested with the pre-release versions). I'm not sure I understand everything...
Flags: needinfo?(jfkthame)
(Assignee)

Updated

3 years ago
Flags: needinfo?(jfkthame)
(Assignee)

Comment 47

2 years ago
Created attachment 8603759 [details] [diff] [review]
GDI: use typo metrics when a MATH table is present or when USE_TYPO_METRICS is specified.

This is an attempt to extract the fix for this bug from attachment 8369639 [details] [diff] [review]. From a quick verification with the MathML torture test, that seems indeed to remove the excessive ascent/descent from all these math fonts. However, I'm asking feedback as I'm not sure I understand everything in this code / patch.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=bb71dd51cd70
Attachment #8603759 - Flags: feedback?(karlt)
Attachment #8603759 - Flags: feedback?(jfkthame)
Comment on attachment 8603759 [details] [diff] [review]
GDI: use typo metrics when a MATH table is present or when USE_TYPO_METRICS is specified.

These are the right concepts, but all the metrics you need except
HasFontTable(TRUETYPE_TAG('M','A','T','H')) are available in OUTLINETEXTMETRIC
[1], so there is no need to fetch the os/2 table separately.

The part that is tricky about using these metrics is that they are not
rounded to pixels for us.  We want to emulate IE, but I don't know exactly
what it does.  Attachment 520939 [details] observes that ceil rounding is not consistent
with IE.  I suspect a sane strategy would be to round line height, max height (ascent + descent), and ascent to the nearest pixel, ensure line height is at least max height, and then set internal leading, external leading, and max descent from differences in the other metrics.
Attachment #8603759 - Flags: feedback?(karlt) → feedback+
(Assignee)

Comment 49

2 years ago
Created attachment 8604497 [details]
testcase with STIX2 beta
(Assignee)

Comment 50

2 years ago
Created attachment 8604498 [details]
Rendering of 8604497 on Windows 7 with Firefox, Chrome and IE
(Assignee)

Comment 51

2 years ago
(In reply to Karl Tomlinson (ni?:karlt) from comment #48)
> OUTLINETEXTMETRIC
> [1], so there is no need to fetch the os/2 table separately.

It looks like the reference [1] was lost...
(In reply to Karl Tomlinson (ni?:karlt) from comment #48)
> OUTLINETEXTMETRIC
> [1], so there is no need to fetch the os/2 table separately.

[1] https://msdn.microsoft.com/en-us/library/windows/desktop/dd162755%28v=vs.85%29.aspx

> I suspect a sane strategy would be to round line height, max
> height (ascent + descent), and ascent to the nearest pixel

Or perhaps rounding line height, ascent, and descent would give a better balance around the baseline.
(In reply to Frédéric Wang (:fredw) from comment #50)
> Created attachment 8604498 [details]
> Rendering of 8604497 on Windows 7 with Firefox, Chrome and IE

I'm suprised at the large line-height with IE.
It's as if it is using the win ascent/descent, and yet I assume IE doesn't use these metrics with Cambria Math.

STIXMath_115.otf has USE_TYPO_METRICS set.
(Assignee)

Comment 54

2 years ago
(In reply to Karl Tomlinson (ni?:karlt) from comment #53)
> I'm suprised at the large line-height with IE.
> It's as if it is using the win ascent/descent, and yet I assume IE doesn't
> use these metrics with Cambria Math.
> 
> STIXMath_115.otf has USE_TYPO_METRICS set.

OK, I tested again on Windows 7 and the excessive ascent/descent:

- happens with Firefox release
- does not happen with Cambria Math or GUST fonts (Latin Modern & TeX Gyre) with IE 11
- does not happen with Cambria Math or GUST fonts (Latin Modern & TeX Gyre) with Firefox with the GDI patch to use typo metrics.
- always happen with STIX2 beta and Asana Math.

Actually, as I remember we were not able to make Asana Math work, even with the Direct2d backend. So I suspect there is a different issue here, but I do not know what. I'll now try STIX2 on my Windows 8 partition.
(Assignee)

Comment 55

2 years ago
Created attachment 8604824 [details]
Rendering of 8604497 on Windows 8 with Firefox and IE

Top: STIX2Beta
Bottom: Cambria Math
Left: Firefox 36
Right: IE 11
-----------------------

So as I suspected, STIX2 beta does not render correctly even with the DWrite backend.
(In reply to Karl Tomlinson (ni?:karlt) from comment #53)
> STIXMath_115.otf has USE_TYPO_METRICS set.

I suspect I'm wrong here.  I was depending on what fontconfig indicated in its "Really use Typo metrics" check box.  However ttx reports differently.

  <OS_2>
    <version value="3"/>
    <xAvgCharWidth value="706"/>
    <usWeightClass value="400"/>
    <usWidthClass value="5"/>
    <fsType value="00000000 00000000"/>
    <ySubscriptXSize value="650"/>
    <ySubscriptYSize value="600"/>
    <ySubscriptXOffset value="0"/>
    <ySubscriptYOffset value="75"/>
    <ySuperscriptXSize value="650"/>
    <ySuperscriptYSize value="600"/>
    <ySuperscriptXOffset value="0"/>
    <ySuperscriptYOffset value="350"/>
    <yStrikeoutSize value="51"/>
    <yStrikeoutPosition value="283"/>
    <sFamilyClass value="0"/>
    <panose>
      <bFamilyType value="2"/>
      <bSerifStyle value="2"/>
      <bWeight value="6"/>
      <bProportion value="3"/>
      <bContrast value="5"/>
      <bStrokeVariation value="4"/>
      <bArmStyle value="5"/>
      <bLetterForm value="2"/>
      <bMidline value="3"/>
      <bXHeight value="4"/>
    </panose>
    <ulUnicodeRange1 value="10100000 00000000 00000010 11111111"/>
    <ulUnicodeRange2 value="01000010 00000000 11111101 11111111"/>
    <ulUnicodeRange3 value="00000010 00000000 00000000 00100000"/>
    <ulUnicodeRange4 value="00000000 00000000 00000000 00000000"/>
    <achVendID value="STIX"/>
    <fsSelection value="00000000 01000000"/>
    <fsFirstCharIndex value="32"/>
    <fsLastCharIndex value="65535"/>
    <sTypoAscender value="706"/>
    <sTypoDescender value="-220"/>
    <sTypoLineGap value="43"/>
    <usWinAscent value="2200"/>
    <usWinDescent value="1700"/>
    <ulCodePageRange1 value="01100000 00000000 00000000 10011111"/>
    <ulCodePageRange2 value="00000000 00000000 00000000 00000000"/>
    <sxHeight value="473"/>
    <sCapHeight value="657"/>
    <usDefaultChar value="32"/>
    <usBreakChar value="32"/>
    <usMaxContex value="3"/>
  </OS_2>

Still, that doesn't explain why the presence of the MATH table is not triggering use of the typo metrics through the GDI patch.
(Assignee)

Comment 57

2 years ago
(In reply to Karl Tomlinson (ni?:karlt) from comment #56)
> I suspect I'm wrong here.  I was depending on what fontconfig indicated in
> its "Really use Typo metrics" check box.  However ttx reports differently.

Yes, it seems that FontForge is wrong here. Using ttx to set the correct bit on STIX2 Beta fixes the issue.

> Still, that doesn't explain why the presence of the MATH table is not
> triggering use of the typo metrics through the GDI patch.

Yes, I don't know why it fails. Anyway, we probably want to remove this test when the MATH fonts have the correct bit set.
(Assignee)

Comment 58

2 years ago
(In reply to Frédéric Wang (:fredw) from comment #57)
> (In reply to Karl Tomlinson (ni?:karlt) from comment #56)
> > I suspect I'm wrong here.  I was depending on what fontconfig indicated in
> > its "Really use Typo metrics" check box.  However ttx reports differently.
> 
> Yes, it seems that FontForge is wrong here. Using ttx to set the correct bit
> on STIX2 Beta fixes the issue.

So the problem is that Fontforge always check "Really use Typo metrics" when loading fonts with OS/2 version < 4 but never set the USE_TYPO_METRICS bit when saving a font with OS/2 version < 4. STIX2Beta has OS/2 version set to 3. If you upgrade to version 4, the problem goes away. I submitted https://github.com/fontforge/fontforge/pull/2274
(Assignee)

Comment 59

2 years ago
Created attachment 8606198 [details] [diff] [review]
GDI: use typo metrics when USE_TYPO_METRICS is specified.

After more testing and discussion with Karl, it seems that OUTLINETEXTMETRIC provides bad typo metrics for CFF fonts (it works for TrueType font, though). So this patch goes back to loading the OS/2 table with gfxFontEntry::AutoTable.

I also adapted Karl's reftest from 657864 to verify that the typo metrics are really used.
Assignee: nobody → fred.wang
Status: NEW → ASSIGNED
(Assignee)

Updated

2 years ago
Attachment #8603759 - Flags: feedback?(jfkthame)
(Assignee)

Comment 60

2 years ago
Created attachment 8606223 [details] [diff] [review]
GDI: use typo metrics when USE_TYPO_METRICS is specified
Attachment #477881 - Attachment is obsolete: true
Attachment #515891 - Attachment is obsolete: true
Attachment #8603759 - Attachment is obsolete: true
Attachment #8604498 - Attachment is obsolete: true
Attachment #8604824 - Attachment is obsolete: true
Attachment #8606198 - Attachment is obsolete: true
(Assignee)

Comment 61

2 years ago
Created attachment 8606602 [details] [diff] [review]
GDI: use typo metrics when USE_TYPO_METRICS is specified
Attachment #8606223 - Attachment is obsolete: true
(Assignee)

Comment 62

2 years ago
Comment on attachment 8606602 [details] [diff] [review]
GDI: use typo metrics when USE_TYPO_METRICS is specified

Try result:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=c70175aa4130

I need to update the expected result of lineheight-metrics-1.html as it now passes with Windows XP. This is because the test uses a font with "use_typo_metrics" but I assume the rounding is still incorrect otherwise.
Attachment #8606602 - Flags: review?(karlt)
Comment on attachment 8606602 [details] [diff] [review]
GDI: use typo metrics when USE_TYPO_METRICS is specified

>+        // Using the equivalent values from oMetrics provides inconsistent
>+        // results, so we instead rely on OS2Table.

Please explicitly state that the inconsistent results are "with CFF fonts".

>+# Fonts with known typo-height = f.os2_typoascent + f.os2_typoascent and
>+# line-height = winascent + windescent = f.hhea_ascent + f.hhea_descent,
>+# such that typo-height is much smaller than line-height.

typoHeight here is f.os2_typoascent - f.os2_typodescent + f.os2_typolinegap,
which is really a line height, so please correct or remove the formula, and rename to typoLineHeight and rename "lineHeight" to winHeight (or macLineHeight or hheaLineHeight).
Attachment #8606602 - Flags: review?(karlt) → review+
(Assignee)

Comment 64

2 years ago
Created attachment 8606861 [details] [diff] [review]
Patch final version
Attachment #8606602 - Attachment is obsolete: true
(Assignee)

Comment 65

2 years ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=c70175aa4130
Keywords: checkin-needed

Comment 66

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/35ca44343ed8
Keywords: checkin-needed

Comment 67

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/2d2577321beb

Comment 68

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/fc32848983e7
https://hg.mozilla.org/mozilla-central/rev/35ca44343ed8
https://hg.mozilla.org/mozilla-central/rev/2d2577321beb
https://hg.mozilla.org/mozilla-central/rev/fc32848983e7
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox41: --- → fixed
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla41

Comment 70

2 years ago
https://hg.mozilla.org/mozilla-central/rev/e94dc650c901
(Assignee)

Comment 71

2 years ago
@karl: do you think we should / it is possible to have this patch for Firefox 38 ESR?
Flags: needinfo?(karlt)
ESR has a policy which tries to keep changes to a minimum.  I assume that is so that enterprises can test and know that they'll continue to get the same results for the support period.  So I don't think a change like this would be accepted for ESR.

https://wiki.mozilla.org/Release_Management/ESR_Landing_Process#What_should_land_on_mozilla-esr31.2Fmozilla-esr38

"Security and some major stability fixes when they're landed/merged onto mozilla-beta, or fixes for regressions specific to the ESR."
Flags: needinfo?(karlt)
You need to log in before you can comment on or make changes to this bug.