Closed Bug 1201874 Opened 4 years ago Closed 4 years ago

PDF printing issues on Mac OS X (PDF display issues on OS X with Cairo canvas)

Categories

(Core :: Graphics, defect)

41 Branch
x86_64
macOS
defect
Not set

Tracking

()

VERIFIED FIXED
mozilla49
Tracking Status
firefox41 + wontfix
firefox42 + wontfix
firefox43 + wontfix
firefox44 - wontfix
firefox45 - wontfix
firefox46 --- wontfix
firefox47 --- wontfix
firefox48 --- verified
firefox49 --- verified
firefox-esr45 --- wontfix

People

(Reporter: pauly, Assigned: pchang)

References

Details

(Keywords: regression, Whiteboard: [gfx-noted])

Attachments

(3 files)

FF 41b7, OS X 10.10.5

STR:
1. Open http://rsb.info.nih.gov/ij/docs/pdfs/examples.pdf
2. Press Command + P
3. Choose to "Open PDF in Preview"

Actual results:
Rendering issues (huge arrows) both in preview and on the printed paper - see attached

Expected results:
The PDF should be printed properly

Note: Doesn't reproduce on Win 7, Ubuntu 12.04
Doesn't reproduce on FF 40.0.3
Keywords: regression
FF 41b1, 41b2 - preview shows blank pages (bug 1188752)
FF 41b3 - problem is reproducible
Blocks: 1188752
This is a recent regression so tracking. The arrow artifacts in the print preview is not good. Let's try to see if we can get this fixed before 41 ships.
Bas, not sure if you are the right person here, but would you be able to help investigate or find an owner? This is a recent regression in 41 and I was hoping to get somebody to look into it and determine whether it would be easy to fix this before 41 goes live. Thanks!
Flags: needinfo?(bas)
(In reply to Ritu Kothari (:ritu) from comment #4)
> Bas, not sure if you are the right person here, but would you be able to
> help investigate or find an owner? This is a recent regression in 41 and I
> was hoping to get somebody to look into it and determine whether it would be
> easy to fix this before 41 goes live. Thanks!

Hrm, if this is OS X only it's a little tricky for me to work on. This is a very interesting bug though, I don't have an idea about what causes it yet but I'll think about it more.
Flags: needinfo?(bas)
Just to be clear - we got the regression range to point to bug 1188752?  Bas, I know you don't have a Mac, but drive this and work with somebody on the team that has access to it.
Assignee: nobody → bas
Component: PDF Viewer → Graphics
Product: Firefox → Core
Whiteboard: [gfx-noted]
Milan, we will gtb Beta9 (last beta) tomorrow. Is there any chance we can get a fix in time before then?
Flags: needinfo?(milan)
Doubt it.
Flags: needinfo?(milan)
Paul, would you be able to try out a few different PDFs and let us know whether this happens every single time with every PDF you try? Thanks.
Flags: needinfo?(paul.silaghi)
I also see this on OS X 10.10.4 on Beta with the example pdf. 
It isn't every PDF, and here it seems to be caused by the particular arrow symbol used in the PDF.
(In reply to Ritu Kothari (:ritu) from comment #9)
> Paul, would you be able to try out a few different PDFs and let us know
> whether this happens every single time with every PDF you try? Thanks.
This is not happening with every PDF, I'm sure it's related to something, but I don't know what.
Here are some other examples that reproduce the problem (you just need to compare the browser view with the print preview):
http://mpdf1.com/examples/example_invoice.pdf
http://mpdf1.com/examples/mpdf57demo.pdf - page 9
Flags: needinfo?(paul.silaghi)
So far, we do not have a huge number of reports of this issue from our Mac end users. Based on a few tries by Liz, QE team, while this issue is bad, it is not a release blocker for 41. I hope we get a fix in time for 42.
Printing uses Cairo for canvas - if I force cairo canvas on OS X, I don't need to do a print preview - it looks wrong displayed in the browser.  Both the in browser and print (as mentioned above) work on Windows and Linux, when forcing cairo canvas.  So, it's still platform specific, but it seems to be OS X + Cairo only.
Summary: PDF printing issues on Mac OS X → PDF printing issues on Mac OS X (PDF display issues on OS X with Cairo canvas)
Bas, any news on this bug? Thanks
Flags: needinfo?(bas)
Wonder if it's related to another printing bug Lee is currently looking at.
Assignee: bas → lsalzman
Flags: needinfo?(bas) → needinfo?(lsalzman)
(In reply to Milan Sreckovic [:milan] from comment #15)
> Wonder if it's related to another printing bug Lee is currently looking at.

The recent transform issues we were dealing with on Windows in bug 1205854 were confined to Cairo's Windows printing surface code, and were more specifically with problems with bugs in GDI. So I wouldn't expect any relationship with the problems for now.

I tried looking at the supplied example pdf to see if I could spot anything obviously unusual with it, but it didn't looked like a lot of binary encoded stuff that I was not able to make any sense of.

As I don't have a Mac to test with this is best assigned to someone who can more readily reproduce it.
Assignee: lsalzman → nobody
Flags: needinfo?(lsalzman)
This mostly fixes the problem, and it seems to make sense - we need the full transform in the context when pushing the path.  However, with this fix, I now see (minor) differences between the printed and cairo-displayed canvas, so I'll look a bit more.
Attachment #8672024 - Flags: feedback?(lsalzman)
Don't think this is the right patch.  There is clearly a problem with the transform, but I think the change above is probably just pushing things off the page so that we don't see it being wrong.
Also, to confirm, the extras are indeed the outlines of the small arrows.
Comment on attachment 8672024 [details] [diff] [review]
(wip) Apply the transform before we push the path.

As discussed elsewhere, the CTM probably should not affect the path, and the problem must be something else.
Attachment #8672024 - Flags: feedback?(lsalzman) → feedback-
Too late for 42 now. However, tracking for 43 as we would be happy to take a patch in this release.
This is probably wontfix for 43 and 44.
OK, let's aim for 45. If you find a fix and want to uplift it please let me or Ritu know.
Wontfix'ing for 44. Going by the number of FF versions this is getting wontfix'd I think we might be better off not tracking it.
Just like Ritu said in comment #24.
Assuming wontfix for Firefox 45 at this point. Milan, who is working on resolving this regression?
Flags: needinfo?(milan) → needinfo?(howareyou322)
I'm working on this issue.
Assignee: nobody → howareyou322
Status: NEW → ASSIGNED
Flags: needinfo?(howareyou322)
(In reply to Milan Sreckovic [:milan] from comment #13)
> Printing uses Cairo for canvas - if I force cairo canvas on OS X, I don't
> need to do a print preview - it looks wrong displayed in the browser.  Both
> the in browser and print (as mentioned above) work on Windows and Linux,
> when forcing cairo canvas.  So, it's still platform specific, but it seems
> to be OS X + Cairo only.

I can reproduce it on OSX by switching to cairo canvas. But now we already switch skia as canvas backend form bug 932958 and I didn't see this problem in FF46(release). Milan, do we still need to address this issue?
Flags: needinfo?(milan)
Yes - this is for printing, and forcing Cairo canvas is just a way to reproduce the problem without printing.  But, printing is still using Cairo, so we need it for that reason.  However, if we come up with the fix that takes care of printing, but somehow still doesn't fix the Cairo canvas (when forced), that's probably OK.
Flags: needinfo?(milan)
(In reply to Milan Sreckovic [:milan] from comment #29)
> Yes - this is for printing, and forcing Cairo canvas is just a way to
> reproduce the problem without printing.  But, printing is still using Cairo,
> so we need it for that reason.  However, if we come up with the fix that
> takes care of printing, but somehow still doesn't fix the Cairo canvas (when
> forced), that's probably OK.
Thanks for the explanation. I'm checking the cairo path.
I'm able to reproduce this issue with a simple stroketext test with cairo backend.
(In reply to Peter Chang[:pchang] from comment #31)
> I'm able to reproduce this issue with a simple stroketext test with cairo
> backend.
http://jsfiddle.net/vNWn6/358/
This is the simple stroketext test case.

For the text stroke, the path was calculated based on the scaledFont[1] with a scale factor as 10.
I can get the correct result if I reset the scale factor of scaledFont. The scale factor '10' of the  MacFont was initialized during gfxMacFont constructor. Now I'm checking the behavior of FF 40.

[1]https://dxr.mozilla.org/mozilla-central/source/dom/canvas/CanvasRenderingContext2D.cpp#3763
I saw the same value of scaledFont in FF40 which was not able to reproduce this stroketext issue. Therefore, this should not be related to scaledFont. But I could see the different path[1] between FF49 and FF40 build. And I didn't find suspicious changes to cause different path calculation.

[1]https://dxr.mozilla.org/mozilla-central/source/dom/canvas/CanvasRenderingContext2D.cpp#3763
This issue could be verified by the stroketext test case http://jsfiddle.net/vNWn6/358/ on OSX.
Please change gfx.canvas.azure.backends to cairo first.

Request regression window to speedup debugging.
(In reply to Milan Sreckovic [:milan] from comment #35)
> This is the range I get:
> https://hg.mozilla.org/mozilla-central/
> pushloghtml?fromchange=b7ee8e13145a&tochange=c223b8844264
> 
> and inbound:
> 
> https://hg.mozilla.org/integration/mozilla-inbound/
> pushloghtml?fromchange=600c02fb4be271987e90f6fc0e61f33c05baa1d7&tochange=9fa3
> 12e065e1e9da9b5cb3514c3b40dcc7f65539
> 
> so I would imagine this patch:
> https://hg.mozilla.org/integration/mozilla-inbound/rev/beb4dcffcc0f
> 
> Peter, does backing out that particular patch fix the problem for you?
Yes, this problem was fixed after reverting above patch. Checking which part caused the problem.
https://reviewboard.mozilla.org/r/54734/#review51384

::: gfx/cairo/cairo/src/cairo-quartz-font.c:573
(Diff revision 1)
>  					font->base.scale.yx,
>  					-font->base.scale.xy,
>  					-font->base.scale.yy,
>  					0, 0);
>  
> -    ctFont = CTFontCreateWithGraphicsFont (font_face->cgFont, 0.0, NULL, NULL);
> +    ctFont = CTFontCreateWithGraphicsFont (font_face->cgFont, 1.0, NULL, NULL);

If we passed the size parameter as 0, then it created the CTFont with default size 12\[1\] which caused this problem. 


[1]https://developer.apple.com/library/ios/documentation/Carbon/Reference/CTFontRef/#//apple_ref/c/func/CTFontCreateWithGraphicsFont
(Perhaps interesting, Skia font host uses 0/default size in most of their calls to this method.)
Comment on attachment 8755679 [details]
MozReview Request: Bug 1201874 - create core text font with correct size,r?lsalzman

https://reviewboard.mozilla.org/r/54734/#review51520
Attachment #8755679 - Flags: review?(lsalzman) → review+
https://hg.mozilla.org/mozilla-central/rev/0edb40641826
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
Verified fixed FX 49.0a1 (2016-06-02) OS X 10.11.
Status: RESOLVED → VERIFIED
Peter, Lee, are you going to request an uplift to 48? 
It doesn't seem too risky
Flags: needinfo?(lsalzman)
Flags: needinfo?(howareyou322)
Comment on attachment 8755679 [details]
MozReview Request: Bug 1201874 - create core text font with correct size,r?lsalzman

Approval Request Comment
[Feature/regressing bug #]:No
[User impact if declined]:Users might see some artifacts in the printing content
[Describe test coverage new/current, TreeHerder]: try looks good
[Risks and why]: low risk, it just changes to correct font configuration for printing and won't cause any stability
[String/UUID change made/needed]:no
Flags: needinfo?(lsalzman)
Flags: needinfo?(howareyou322)
Attachment #8755679 - Flags: approval-mozilla-beta?
Attachment #8755679 - Flags: approval-mozilla-aurora?
Comment on attachment 8755679 [details]
MozReview Request: Bug 1201874 - create core text font with correct size,r?lsalzman

This patch fixes the regression and is verified. Take it in 48 beta 4 and aurora.
Attachment #8755679 - Flags: approval-mozilla-beta?
Attachment #8755679 - Flags: approval-mozilla-beta+
Attachment #8755679 - Flags: approval-mozilla-aurora?
Attachment #8755679 - Flags: approval-mozilla-aurora+
Verified fixed on FX 48.0b4 (2016-06-27) on OSx 10.10.5.
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.