Closed Bug 581087 Opened 9 years ago Closed 9 years ago

REFTEST TEST-UNEXPECTED-FAIL | file:///c:/talos-slave/mozilla-central-win7-opt-u-reftest-d2d/build/reftest/tests/layout/reftests/forms/input-text-size-1.html |

Categories

(Core :: Graphics, defect)

x86
Windows 7
defect
Not set

Tracking

()

RESOLVED FIXED
Tracking Status
blocking2.0 --- beta4+

People

(Reporter: jrmuizel, Assigned: jfkthame)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

One version has a caret the other does not.
Assignee: nobody → bas.schouten
Would either of you guys be willing to look at this? The textbox appears to be
wider on D2D than without. Allowing one extra character to slip in, it's not
the caret, it's a '|' character.
This happens because the DirectWrite font metrics APIs do not provide the maxAdvance and averageCharWidth values (as GDI did), and so our dwrite code fakes them using the em-size for maxAdvance, and the width of 'x' for aveCharWidth.

Most of the time, the true values from the font are not really needed, but the TextControlFrame code looks at these two fields to decide if the font is "fixed-pitch" and behaves differently in response; for non-fixed-pitch fonts, some padding is added to the control width. See http://mxr.mozilla.org/mozilla-central/source/layout/forms/nsTextControlFrame.cpp#279.
This resolves the problem by reading the 'hhea' and 'OS/2' tables to get the maxAdvance and aveCharWidth values, if possible (it falls back to the old "fake" values if the font tables are not available -- this shouldn't happen for valid TrueType/OpenType fonts).
Attachment #462782 - Flags: review?(bas.schouten)
Comment on attachment 462782 [details] [diff] [review]
patch, v1 - get true maxAdvance and aveCharWidth in dwrite font metrics

One important comment, from MSDN: 'It is possible for a NULL tableContext to be returned, especially if the implementation performs direct memory mapping on the whole file. Nevertheless, always release it later, and do not use it as a test for function success.'

So we should still release it even if tableContext is NULL.
Attachment #462782 - Flags: review?(bas.schouten) → review+
Attached patch patch, v1.1Splinter Review
Updated for review comment. Also made a similar change to gfxDWriteFontEntry::ReadCMAP(), where the same issue applies. (Marking for re-review so you can check I did the appropriate thing, Bas.)
Assignee: bas.schouten → jfkthame
Attachment #462782 - Attachment is obsolete: true
Attachment #462842 - Flags: review?(bas.schouten)
Requesting blocking-2.0 as we need this for D2D-reftests to pass on Win7.
blocking2.0: --- → ?
Comment on attachment 462842 [details] [diff] [review]
patch, v1.1

Looks good to me!
Attachment #462842 - Flags: review?(bas.schouten) → review+
blocking2.0: ? → beta4+
http://hg.mozilla.org/mozilla-central/rev/df514ee6ac84
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.