Canvases passed to mozPrintCallback should be prescaled by the printers pixel ratio to ensure highest print quality.

First shot. Only works when e10s is disabled. Needs further investigation.

The e10s related issues I was experiencing don't seem to be related to my patches. It is https://bugzilla.mozilla.org/show_bug.cgi?id=1217194.

Comment on attachment 8720670 [details] [diff] [review] Part1: Pre scale canvases with print callbacks Robert, asking you to review this since you also reviewed the original patch that introduced the mozPrintCallback event.

Comment on attachment 8720670 [details] [diff] [review] Part1: Pre scale canvases with print callbacks Review of attachment 8720670 [details] [diff] [review]: ----------------------------------------------------------------- Can you explain these changes in a bit more detail? With comments in the code, preferably, and maybe a test?

Added printing reftest.

This patch resizes a canvas before printing by the printer pixel ratio than scales it down to original size while painting. This trick is usually done in JavaScript (see http://www.html5rocks.com/en/tutorials/canvas/hidpi/) to render a canvas at high quality on HiDPI screens, where window.devicePixelRatio is used as the scaling factor. When printing, the actual resolution is only known when the document is prepared to be send to the printer. So we need to scale the canvas to match the printers resolution right before we handle the mozPrintCallback and apply an inverted scale on the layer before painting.

Added comments to the code.

(In reply to Tobias Schneider [:tobytailor] from comment #6) > This patch resizes a canvas before printing by the printer pixel ratio than > scales it down to original size while painting. This trick is usually done > in JavaScript (see http://www.html5rocks.com/en/tutorials/canvas/hidpi/) to > render a canvas at high quality on HiDPI screens, where > window.devicePixelRatio is used as the scaling factor. When printing, the > actual resolution is only known when the document is prepared to be send to > the printer. So we need to scale the canvas to match the printers resolution > right before we handle the mozPrintCallback and apply an inverted scale on > the layer before painting. When we're actually printing, those print callbacks get passed a canvas which actually generates a print-compatible surface, i.e. that drawing should be resolution-independent (or at the print resolution) already. Right? So this patch is only to get better quality output during print preview?

Please correct me if I'm wrong, but AFAIK such resolution-independent printing surface is only available on windows (via cairo_win32_surface_create). I also forgot to mention that the canvas gets scaled using its computed dimension (via CSS using relative units). This allows PDF.js to render over the fully available paper space in highest quality.

(In reply to Tobias Schneider [:tobytailor] from comment #9) > Please correct me if I'm wrong, but AFAIK such resolution-independent > printing surface is only available on windows (via > cairo_win32_surface_create). We're calling cairo_surface_create_similar to create the canvas surface. It looks like that would work well on Mac (Quartz) but not with the cairo PDF backend which I guess is the other one that matters...

Comment on attachment 8724984 [details] [diff] [review] Part1 (v2): Pre scale canvases with print callbacks Review of attachment 8724984 [details] [diff] [review]: ----------------------------------------------------------------- ::: layout/generic/nsHTMLCanvasFrame.cpp @@ +361,5 @@ > + float scale = 1.0; > + // If we are printing and the canvas specified a mozPrintCallback handler, > + // it got resized in nsSimplePageSequenceFrame to match the printers > + // resolution. We inverse the scale to paint the canvas at its intended size. > + if (type == nsPresContext::eContext_Print && element->GetMozPrintCallback()) { The coupling between this code and the code in nsSimplePageSequence is troubling; we're relying on this code running exactly when some distantly related code in nsSimplePagesequence runs. I think we should pass the scale factor into InitializeWithSurface and just use that here.

> We're calling cairo_surface_create_similar to create the canvas surface. It looks like that would work well on Mac (Quartz)... Right, but what would make this resolution-independent? > The coupling between this code and the code in nsSimplePageSequence is troubling; we're relying on this > code running exactly when some distantly related code in nsSimplePagesequence runs. I think we should > pass the scale factor into InitializeWithSurface and just use that here. Sounds reasonable.

(In reply to Tobias Schneider [:tobytailor] from comment #12) > > We're calling cairo_surface_create_similar to create the canvas surface. It looks like that would work well on Mac (Quartz)... > > Right, but what would make this resolution-independent? It could, depending on the cairo backend implementation. Looking at the cairo PDF backend, it doesn't handle cairo_surface_create_similar properly, so that wouldn't work. So I support the idea of your patch, modulo the comment I made.

Moved scaling logic to InitializeWithSurface and CanvasLayer.

Comment on attachment 8726285 [details] [diff] [review] Part1 (v3): Pre scale canvases with print callbacks Review of attachment 8726285 [details] [diff] [review]: ----------------------------------------------------------------- Better, thanks! Why do we need to change CanvasLayer? Your previous patch didn't, can't we continue using that approach? ::: dom/canvas/CanvasRenderingContext2D.h @@ +717,4 @@ > // Member vars > int32_t mWidth, mHeight; > > + float mDeviceXScale, mDeviceYScale; Remove trailing whitespace

> Why do we need to change CanvasLayer? Your previous patch didn't, can't we continue using that approach? We need to apply the scaling at some point. Doing it in CanvasLayer seemed to be the most reasonable place for me. Would you rather keep that part in nsHTMLCanvasFrame?

Sorry for the delay here. So the only thing that actually sets the scale factor is nsSimplePageSequenceFrame::PrePrintNextPage, right? And it uses GetPrintPreviewScale. Is this only a problem during print preview, or also during actual printing? Is the canvas during actual printing pixel based or infinite resolution? Why does the scale matter with an infinite resolution canvas?

This seems to be platform dependent. AFAIK, on Mac OS X it uses a pixel based canvas for printing. But even for infinite resolution surfaces, content gets rasterized at some point (e.g. when blend modes are used, see https://bugzilla.mozilla.org/show_bug.cgi?id=1105557). Also notice that prescaling for printing purposes not only takes the printers resolution into account, but also the CSS scaling. Since the printers pixel ratio/page size is not exposes to user content (opposed to the screen size/pixel ratio), apps like PDF.js set a CSS width/height of 100% to fit to final printing dimension. So what this patch is doing is, resizing the canvas to its computed CSS size multiplied by the printers scaling. Together with the printerPixelRatio as exposed via https://bugzilla.mozilla.org/show_bug.cgi?id=1247422, an app can set a proper transformation before drawing to match the printers resolution. This is basically the same as applying the tricks described in http://www.html5rocks.com/en/tutorials/canvas/hidpi/ to printing instead of a High DPI screen.

Update patch to actually include the CSS scaling.

Comment on attachment 8736597 [details] [diff] [review] Part1 (v4): Pre scale canvases with print callbacks Review of attachment 8736597 [details] [diff] [review]: ----------------------------------------------------------------- This doesn't sound like a good idea. It's not compatible with existing mozPrintCallback users, and it shouldn't be necessary in the first place. The rasterization steps that you're concerned happen in our own drawing code, probably inside cairo though I haven't seen proof of that. As such, it's up to us to use the right resolution in that place. We shouldn't require web content to make changes to get this right.