Bug 33022 – [WinCairo] WebKit Plugins Are Not Rendered During Printing

• Component: WebKit Misc.
• Status: RESOLVED FIXED

[Brent Fulgham]

The attached ZIP file contains a simple test plugin that renders a blue box in the <embed> area.  The first screenshot shows how this looks when rendered with Safari.

If Safari (or WinLauncher) attempts to print the page, the blue boxes are not rendered in the print context (see second screenshot).

[Brent Fulgham]

Created attachment 45607 [details]
Zip of Visual Studio 2005 test plugin

This project builds a very simple WebKit plugin that renders a blue box inside the provided <embed> region.

[Brent Fulgham]

Created attachment 45608 [details]
On-Screen Rendering of Plugin

The plugin properly renders blue boxes in the <embed> regions.

[Brent Fulgham]

Created attachment 45609 [details]
Print Output from Safari

A print output PDF created by Safari showing the same content as the on-screen PNG.  The blue boxes are missing from the output.

[Brent Fulgham]

Created attachment 45688 [details]
Print Output using artificially large empty header/footer.

This PDF was generated by printing the example plugin using a header and footer height of 166 (specified in the WebUIDelegate::webViewHeaderHeight and WebUIDelegate::webViewFooterHeight calls).

The plugin's blue rectangles are drawn in the areas set aside for header/footer, but are hidden inside WebKit's print region.

[Brent Fulgham]

Artificially extending the header and footer allows the plugin output to display (inside those regions).  However, the layout looks very wrong, probably because of related Bug 32909.

[Brent Fulgham]

The problem boils down to improper synchronization between the windows world XFORM and the Cairo context.  The use of a Cairo "cairo_win32_printing_surface" caused the coordinate systems to be incompatible, resulting in the strange printing issues.

Fixes are as follows:
1.  Handle the case of a 0 scaling factor, which happens when a print preview context (from Windows) is passed to WebKit for display.
2.  Switch from using a cairo_win32_printing_surface (which is an EMF-backed surface) to the same cairo_win32_surface used for all other WebKit drawing.

With this patch. plugins display in their (mostly) correct location (but see remaining Bug 32909) with good print quality.  Print output quality for text and <canvas> logic looks good as well.

[Brent Fulgham]

Created attachment 45808 [details]
Revise Cairo-based printing to properly handle plugins

[WebKit Review Bot]

style-queue ran check-webkit-style on attachment 45808 [details] without any errors.

[Adam Roben (:aroben)]

(In reply to comment #6)
> 2.  Switch from using a cairo_win32_printing_surface (which is an EMF-backed
> surface) to the same cairo_win32_surface used for all other WebKit drawing.

Does this result in increased memory usage when spooling?

[Brent Fulgham]

(In reply to comment #9)
> (In reply to comment #6)
> > 2.  Switch from using a cairo_win32_printing_surface (which is an EMF-backed
> > surface) to the same cairo_win32_surface used for all other WebKit drawing.
> 
> Does this result in increased memory usage when spooling?

Yes.  The cairo_win32_printing_surface uses considerably less memory than the native cairo_surface.  I'll take another look at whether I can get the various CTM's to play nicely.

[Brent Fulgham]

Created attachment 45837 [details]
Revised Cairo-based printing to handle plugins properly.

I made another attempt to use the cairo_win32_print_surface type, but found that it was not properly handling certain drawing effects (e.g., a dashed-border box rendered only its corners).

I think we should revisit the print_surface with Cairo 1.9 (or I guess 2.0 when released?) since they've done a lot of work on that backend surface.

[WebKit Review Bot]

style-queue ran check-webkit-style on attachment 45837 [details] without any errors.

[Adam Roben (:aroben)]

Comment on attachment 45837 [details]
Revised Cairo-based printing to handle plugins properly.

> +    HDC screenDC = ::GetDC(0);

I think you need to call ReleaseDC before this goes out of scope.

[Brent Fulgham]

(In reply to comment #13)
> (From update of attachment 45837 [details])
> > +    HDC screenDC = ::GetDC(0);
> 
> I think you need to call ReleaseDC before this goes out of scope.

Doh!  You're right.  Fixed locally.

[Eric Seidel (no email)]

Comment on attachment 45837 [details]
Revised Cairo-based printing to handle plugins properly.

Adam is still probably your best reviewer.  Since this has local changes it won't ever be possible to commit as is (not that you ever wanted it to be), so marking cq-.

[Peter Kasting]

Not sure why eseidel added me

[Brent Fulgham]

(In reply to comment #16)
> Not sure why eseidel added me

Perhaps your general awesomeness?

[Brent Fulgham]

The problem (at least on the WinCairo side of things) was due to two issues:
1.  The World Transform applied to the device context used by the plugin did not take scaling or translation (due to margins and header/footer) into account.  This was corrected under separate Bug 32909.
2.  When using a cairo_win32_printing_surface, order of operations can affect the final output.  In this case, since the plugin draws directly to the HDC of the printer, while WebKit largely draws to the metafile-backed cairo surface, the result of sequencing the metafile content after the plugin drawing caused the blank regions of the page (set aside for the plugin) to overlay the plugin drawings.  This problem is corrected in this bug.

[Brent Fulgham]

Created attachment 46095 [details]
Patch to properly flush cairo state when drawing plugins

Revised Cairo-based printing to handle plugins properly.  Deals with metafile/direct device drawing behavior of the Cairo port, but does not address the CoreGraphics issue.

[WebKit Review Bot]

style-queue ran check-webkit-style on attachment 46095 [details] without any errors.

[Adam Roben (:aroben)]

Comment on attachment 46095 [details]
Patch to properly flush cairo state when drawing plugins

> +#if PLATFORM(CAIRO)
> +    // Must flush drawings up to this point to the backing metafile, otherwise the
> +    // plugin region will be 

.......don't leave me hanging!

>  static HDC hdcFromContext(PlatformGraphicsContext* pctx)
>  {
>      cairo_surface_t* surface = cairo_get_target(pctx);
> -    return cairo_win32_surface_get_dc(surface);
> +    HDC hdc = cairo_win32_surface_get_dc(surface);
> +
> +    SetGraphicsMode(hdc, GM_ADVANCED);
> +
> +    return hdc;
>  }

This change isn't mentioned in your ChangeLog.

It seems strange that this "getter"-style function would modify the HDC that it's returning. Do we really need to do this every time we retrieve the HDC? Can we just do it once when the HDC is created?

> -    float scale = static_cast<float>(printRect.width()) / static_cast<float>(pageRect.width());
> +    const float scale = scaleFactor(printDC, pageRect);

We don't normally mark non-reference local variables "const".

r=me

[Brent Fulgham]

Landed in http://trac.webkit.org/changeset/52995.

[Brent Fulgham]

(In reply to comment #21)
> (From update of attachment 46095 [details])
> > +#if PLATFORM(CAIRO)
> > +    // Must flush drawings up to this point to the backing metafile, otherwise the
> > +    // plugin region will be 
> 
> .......don't leave me hanging!

Whoops!  Fixed.

> >  static HDC hdcFromContext(PlatformGraphicsContext* pctx)
> >  {
> >      cairo_surface_t* surface = cairo_get_target(pctx);
> > -    return cairo_win32_surface_get_dc(surface);
> > +    HDC hdc = cairo_win32_surface_get_dc(surface);
> > +
> > +    SetGraphicsMode(hdc, GM_ADVANCED);
> > +
> > +    return hdc;
> >  }
> 
> This change isn't mentioned in your ChangeLog.
> 
> It seems strange that this "getter"-style function would modify the HDC that
> it's returning. Do we really need to do this every time we retrieve the HDC?
> Can we just do it once when the HDC is created?

You are right; this wasn't necessary and has been removed from the code I just landed.

> > -    float scale = static_cast<float>(printRect.width()) / static_cast<float>(pageRect.width());
> > +    const float scale = scaleFactor(printDC, pageRect);
> 
> We don't normally mark non-reference local variables "const".

Changed.

[Brent Fulgham]

I have corrected the problem for the Cairo build on Windows.  However, the CoreGraphics logic is still not working.  I have opened Bug 33391 to document the issue there.

[Brent Fulgham]

Oops:  Landed roll-back of unwanted change in http://trac.webkit.org/changeset/52997.