1720152 - Print preview for remote frames is blurry on macOS, Windows with Fission

Status: RESOLVED FIXED

(Core :: Printing: Output, defect)

Description

[Julian Descottes [:jdescottes]]

STRs:

• enable fission
• navigate to https://bug1695806-blurry-root.glitch.me/
• print/save as PDF

ER: Both the text from the same-process & from the remote frame should be rendered similarly

AR: The text from the remote frame is blurry

Comment 1

[Julian Descottes [:jdescottes]]

Attachment: print_preview_of_test_case.png

Comment 2

[Julian Descottes [:jdescottes]]

Still need to verify linux, but I have the same behavior on Windows 10

Comment 3

[Julian Descottes [:jdescottes]]

It seems that the same-process frame is still rendered as text, while cross process frames are rendered as images.
That probably explains the blurriness.

Emilio: do you know if this limitation is something that can be fixed or should we account for it in our tests?

Comment 4

[Emilio Cobos Álvarez (:emilio)]

Matt did most of the graphics work, I'm not sure how feasible it'd be to turn the oop iframe into something that generates proper PDF output...

Comment 5

[Emilio Cobos Álvarez (:emilio)]

Chris, fyi, this might need fission triage

Comment 6

[Chris Peterson [:cpeterson]]

Tentatively tracking this bug for Fission M8. The blurry text in the OOP frames look pretty bad.

Comment 7

[Matt Woodrow (:mattwoodrow)]

I can reproduce this on Windows using the Microsoft Print to PDF mode, but not with the regular print to pdf.

I would expect this to work, we're collecting recordings of the drawing commands from each process and then replaying them all to the printing surface.

Comment 8

[Matt Woodrow (:mattwoodrow)]

It looks like this is a cairo bug, at least for the Windows 'Microsoft Print to PDF' mode.

We replay the nested Gecko recordings into cairo recording surfaces, so the cairo recording surface for the iframe gets drawn into the recording surface for the tab. That takes a copy-on-write snapshot of the inner recording surface.

Finally, we replay the outer recording surface into the win32 printing surface. Generally this should just recurse into nested recording surfaces, but in this case the nested surface is of type snapshot, not recording, and the code doesn't check for this - https://searchfox.org/mozilla-central/rev/aa67c5a592026cf42e15b02371259d808d086eb3/gfx/cairo/cairo/src/win32/cairo-win32-printing-surface.c#1122

Switching that check to use _cairo_surface_get_source to unwrap the snapshot and get the wrapped recording surface makes things work correctly.

Comment 9

[Matt Woodrow (:mattwoodrow)]

Bob, do you know why we're replaying into a recording surface, and then replaying that into the win32 surface?

Having these multiple levels seems likely part of the reason that we're hitting corner cases that cairo isn't handling.

Comment 10

[Bob Owen (:bobowen)]

(In reply to Matt Woodrow (:mattwoodrow) from comment #9)

Bob, do you know why we're replaying into a recording surface, and then replaying that into the win32 surface?

Having these multiple levels seems likely part of the reason that we're hitting corner cases that cairo isn't handling.

I'm not entirely sure that is causing the problem.
It appears to be the creation of a snapshot or the way the snapshot doesn't take account of recording surfaces here:
https://searchfox.org/mozilla-central/rev/42ae3bea104c37a9986c6f18d17bd9ddb387129c/gfx/cairo/cairo/src/cairo-pattern.c#422

Or as you say the printing surface not handling the snapshot properly.

Even if we could somehow remove a level, wouldn't another level of nested iframes would cause a similar issue?

Comment 11

[Chris Peterson [:cpeterson]]

Matt, do you have cycles to work on this bug? We are hoping to fix it for our Fission M8 milestone in Firefox 92 (Nightly freeze date 2021-08-05).

Comment 12

[Matt Woodrow (:mattwoodrow)]

(In reply to Bob Owen (:bobowen) from comment #10)

(In reply to Matt Woodrow (:mattwoodrow) from comment #9)

Bob, do you know why we're replaying into a recording surface, and then replaying that into the win32 surface?

Having these multiple levels seems likely part of the reason that we're hitting corner cases that cairo isn't handling.

I'm not entirely sure that is causing the problem.
It appears to be the creation of a snapshot or the way the snapshot doesn't take account of recording surfaces here:
https://searchfox.org/mozilla-central/rev/42ae3bea104c37a9986c6f18d17bd9ddb387129c/gfx/cairo/cairo/src/cairo-pattern.c#422

Or as you say the printing surface not handling the snapshot properly.

Even if we could somehow remove a level, wouldn't another level of nested iframes would cause a similar issue?

I was thinking that if we were replaying our moz2d recording directly into a win32 print surface, then all iframe surfaces would be created using CreateSimilarDrawTarget and we'd avoid cairo recording surfaces.

Unfortunately, cairo's win32 print surface implementation of create_similar_surface just creates a recording surface, so yes, I think we'd just hit this again with another nesting level :)

Comment 13

[Matt Woodrow (:mattwoodrow)]

Attachment: Bug 1720152 - Recurse into replay for dependencies, rather than using a temp surface. r?jrmuizel!,bobowen!

Comment 14

[Chris Peterson [:cpeterson]]

This bug is a soft blocker for Fission M8. We'd like to fix it before our M8 Release experiment, but we won't delay the experiment waiting for it.

Comment 15

[Daniel Holbert [:dholbert]]

Hi Matt -- I think this is ready to land except for maybe adding a test per [1]. Just want to be sure it's still on your radar. Thanks!

[1] https://phabricator.services.mozilla.com/D120050#3914827

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/de2e92cbb94d
Recurse into replay for dependencies, rather than using a temp surface. r=jrmuizel,bobowen

Comment 17

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/29833 for changes under testing/web-platform/tests

Comment 18

[Dorel Luca [:dluca]]

Backed out changeset de2e92cbb94d (bug 1720152) for WPT failures in html/browsers/windows/iframe-cross-origin-print.sub.html. CLOSED TREE

Log:
https://treeherder.mozilla.org/logviewer?job_id=346677884&repo=autoland&lineNumber=2291

Push with failures:
https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&revision=de2e92cbb94df78617f535d0b800ab4ac82cca2a

Backout:
https://hg.mozilla.org/integration/autoland/rev/0cde05528b97aa8eb34d3b5d74d479f4dee2ed52

Comment 19

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Upstream PR was closed without merging

Comment 20

[Hiroyuki Ikezoe (:hiro)]

Setting S3 for now (to be removed from Layout untriage list) since this is Fission specific.

Comment 21

[Chris Peterson [:cpeterson]]

Matt says he believes the failure is in the existing printing WPT, not the new one he just added. He suspects it’s a positioning/offset issue and will debug it this week.

Comment 22

[Hiroyuki Ikezoe (:hiro)]

Note that I haven't seen this blurry failure on printing WPTs. The WPT printing stuff outputs two PDFs (a test case and a reference case) and compare them, it runs through printing output, it's not results of print preview. Adding an OOP iframe case in printpreview_helper.xhml could catch this blur?

Note that during running a WPT print test (iframe-cross-origin-print.sub.html) I actually can see there are two borderless windows rendering different results (I guess it's previews, but I can't tell it's blur or not since it's too small), but it doesn't directly output to the PDFs, I believe.

Comment 23

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/69bbb46eebb1
Recurse into replay for dependencies, rather than using a temp surface. r=jrmuizel,bobowen,emilio

Comment 24

[Chris Peterson [:cpeterson]]

Setting status-firefox92=wontfix because this Fission fix looks too big to uplift to Beta 92.

Comment 25

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/69bbb46eebb1

Comment 26

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot