1662820 - Broken kerning with "Save to PDF" print destination, with "13px Arial" as font

Status: VERIFIED FIXED

(Core :: Printing: Output, defect, P2)

Description

[Daniel Holbert [:dholbert]]

Attachment: testcase 1

STR:

1. Ensure you have the new print UI enabled (about:config pref print.tab_modal.enabled = true)
2. View attached testcase (or original site https://www.htmldog.com/examples/pullquotes2.html )
3. Ctrl+P
4. Click "More Settings", and choose "Scale: 200%" (this is optional, but encouraged -- it simply makes the bug easier to see)
5. In the "Destination" dropdown, try "Save to PDF", and compare the preview to what we show for other printers/destinations (e.g. "Microsoft Print to PDF")

ACTUAL RESULTS:
With "Save to PDF", the kerning looks really broken. Some letter pairs look too close, others look too far. e.g. for me:

• In the word "The" on the 3rd line of text, "T" and "h" are too far apart, and "xtra" looks smooshed together with respect to its surroundings
• In the lorem-ipsum text, the following letter pairs look smooshed:
  'se' in consectetur
  'sm' in eiusmod
  'si' in nisi and sint
  ...and "aliquip" and "dolore" seem to have awkward extra spacing.

EXPECTED RESULTS:
Reasonable kerning, and reliable rendering regardless of which printer I choose (and especially reliable printing with our own built-in "Save to PDF" target).

Comment 1

[Daniel Holbert [:dholbert]]

I'm using Nightly 82.0a1 (2020-09-01) on Windows 10. I'm not sure yet if this affects other platforms.

jfkthame, do you know what might be going on & if this is a known issue?

Comment 2

[Daniel Holbert [:dholbert]]

Attachment: screenshot of testcase 1, previewed using Save to PDF, with some of the problem areas highlighted in red

Comment 3

[Daniel Holbert [:dholbert]]

Attachment: reference screenshot if I choose a different printer (nicer kerning)

Comment 4

[Daniel Holbert [:dholbert]]

Attachment: PDF: testcase 1, printed via "Save to PDF" (which shows the bug)

This bug persists in the actual PDF output, too -- it's not just a print-preview-time rendering glitch. See attached PDF which shows the same weird kerning.

Comment 5

[Daniel Holbert [:dholbert]]

Attachment: PDF: testcase 1, printed via "Microsoft Print to PDF" (no bug)

Comment 6

[Daniel Holbert [:dholbert]]

Note: the old print/print-preview UI isn't affected by this, because this is specific to the "Save to PDF" print target, which is only present in the new tab-modal print UI.

Comment 7

[Jonathan Kew [:jfkthame]]

That's.... appalling. It kinda looks like it's forcing integer-pixel glyph positioning, and failing to handle rounding nicely -- but we really don't want that at all, we should be using fractional-pixel positioning with no attempt to snap to a pixel grid (which is basically meaningless for PDF generation).

Part of the issue might be if we're asking the output device for its resolution, and getting something like 72dpi or 96dpi for the PDF destination. But regardless of that, we shouldn't be trying to snap glyph positions to it here. (And if we were going to do pixel-snapping, this would be a terrible attempt at it; looks like maybe each glyph is snapped independently of its neighbours, which gives bad results if one glyph shifts half-a-pixel right and the next shifts half-a-pixel left, for instance.)

There are probably properties of the context or drawtarget or something like that involved here... I'll try to dig around a bit if nobody else gets there first.

Comment 8

[Jonathan Kew [:jfkthame]]

This is related to the fact that we (by default) try to force Arial (and a few other "classic" fonts) to render in "GDI Classic" mode at smallish sizes.

If you remove Arial from the gfx.font_rendering.cleartype_params.force_gdi_classic_for_families list, and relaunch the browser, I believe it should behave better.

Comment 9

[Daniel Holbert [:dholbert]]

(In reply to Jonathan Kew (:jfkthame) from comment #8)

If you remove Arial from the gfx.font_rendering.cleartype_params.force_gdi_classic_for_families list, and relaunch the browser, I believe it should behave better.

Confirmed -- that does make it behave better.

Thanks for taking a look!

Comment 10

[Jonathan Kew [:jfkthame]]

The bad behavior here is triggered because the nsDeviceContextSpecWin we use when the Save to PDF destination is selected reports itself as having 72dpi resolution, so that layout happens based on a font size (in this example) of 9.75 device pixels. Arial really doesn't cope well with this.

For other destinations, we use 144dpi, so the layout is based on the font size being 19.5 device pixels, which works much better. (At sizes below 10px, it might still exhibit problems: e.g. try 7px Arial.)

A simple fix seems to be increasing the DPI returned by nsDeviceContextSpecWin::GetDPI(). Making it return 144 in all cases results in consistent behavior across destinations; but for the PDF destination this seems a bit arbitrary, and can still lead to some (much more minor) spacing irregularities due to "device-pixel" rounding effects. I'm inclined to suggest we return a higher resolution such as 300 here, which further reduces the effect on the PDF output.

Note that the nominal "dpi" reported by the context affects other things than just glyph rasterization mode and spacing; it also causes baseline snapping, which for a destination such as PDF is undesirable.

(It seems likely that output to other destinations would also be improved by increasing the DPI of the device context in those cases as well -- the current value of 144 bears little relation to most modern-day printers! -- but I'm more hesitant to touch that for now in case there are unpredictable side-effects.)

Comment 11

[Jonathan Kew [:jfkthame]]

Attachment: Bug 1662820 - Use a higher nominal DPI for the device context when generating PDF, to reduce unwanted device-pixel-snapping effects. r=jwatt

Comment 12

[Jonathan Watt [:jwatt]]

(In reply to Jonathan Kew (:jfkthame) from comment #10)

The bad behavior here is triggered because the nsDeviceContextSpecWin we use when the Save to PDF destination is selected reports itself as having 72dpi resolution

We're not exactly consistent. There are other places where we use 96:

https://searchfox.org/mozilla-central/search?q=96&path=widget%2Fwindows&case=true&regexp=false

A couple decades after the 72 values were added, devices have improved and that's probably no longer a good value for us to be using, indeed.

Note also that there's bug 1661294 for this issue, but we simply had more critical things to get done and that didn't make it.

Comment 13

[Jonathan Kew [:jfkthame]]

Yes, bug 1661294 would be related, though for "Save to PDF" there isn't really any such thing as "native resolution".

Ideally, perhaps we'd have some way of telling the rest of gecko to ignore the concept of "device pixels" completely and do resolution-independent layout and rendering, but that would be a much more extensive/invasive change. So for the PDF destination, at least, picking a value that's high enough for any dpi-dependent artifacts to go unnoticed should be OK for now.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c7c18b456034
Use a higher nominal DPI for the device context when generating PDF, to reduce unwanted device-pixel-snapping effects. r=jwatt

Comment 15

[Jonathan Kew [:jfkthame]]

Comment on attachment 9173967 [details]
Bug 1662820 - Use a higher nominal DPI for the device context when generating PDF, to reduce unwanted device-pixel-snapping effects. r=jwatt

Beta/Release Uplift Approval Request

• User impact if declined: Ugly rendering (erratic/broken character spacing) for some fonts (especially "classic" Windows fonts like Arial) in Save to PDF preview/output.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: See bug description (comment 0)
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Trivial update to the resolution assumed during layout for the PDF destination.
• String changes made/needed: none

Comment 16

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/c7c18b456034

Comment 17

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9173967 [details]
Bug 1662820 - Use a higher nominal DPI for the device context when generating PDF, to reduce unwanted device-pixel-snapping effects. r=jwatt

Approved for 81.0b7.

Comment 18

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/66856ed017c9

Comment 19

[Ryan VanderMeulen [:RyanVM]]

This was backed out from Beta due to causing bug 1659527.
https://hg.mozilla.org/releases/mozilla-beta/rev/f57d81054b78

Comment 20

[Oana Botisan, Desktop QA]

I managed to reproduce the issue on an older version of Nightly.
I retested everything using latest Nightly 83.0a1 and Firefox 82.0b2 on Windows 10 x64, Ubuntu 18.04 x64 and macOS 10.13. The issue is not reproducing anymore.