Bug 39329 - [WinCairo] Correct scaling for print preview
Summary: [WinCairo] Correct scaling for print preview
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other Windows XP
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-05-18 16:04 PDT by Brent Fulgham
Modified: 2010-05-20 12:15 PDT (History)
2 users (show)

See Also:


Attachments
Patch (8.84 KB, patch)
2010-05-18 17:06 PDT, Brent Fulgham
aroben: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Brent Fulgham 2010-05-18 16:04:55 PDT
[WinCairo] Correct scaling for print preview
Comment 1 Brent Fulgham 2010-05-18 17:06:45 PDT
Created attachment 56425 [details]
Patch
Comment 2 Brent Fulgham 2010-05-18 17:08:15 PDT
Cairo does not properly deal with Windows HDC's that have been scaled using MM_ANISOTROPIC mapping mode, and a WindowExt and ViewportExt setting. (see http://bugs.freedesktop.org/show_bug.cgi?id=28161)

Instead, reset the HDC's WorldTransform to unity, scale the cairo context to the desired scaling, and perform the drawing operation.
Comment 3 Martin Robinson 2010-05-18 18:33:45 PDT
Comment on attachment 56425 [details]
Patch

Brent asked me to do an informal review of this patch. I just have a couple
nits. Someone who knows a little bit more about Windows print preview and
printing should probably review this officially.

> +        * WebFrame.cpp:
> +        (WebFrame::drawHeader): Use pre-positioned context to simplify 
> +         tthis method.

'tthis' -> 'this' :)

> +static void setCairoToHDC(cairo_t* cr, HDC targetDC)

I think maybe this should have a more descriptive name as it touches
not only the Cairo context but the current world transform. Since this
patch is a work-around for a Cairo bug, maybe there should be a longish
comment at the call-site with a high-level overview of the problem (and
perhaps a link to the Cairo bug report, if it exists?).

This all looks pretty reasonable to me.
Comment 4 Adam Roben (:aroben) 2010-05-20 06:47:49 PDT
Comment on attachment 56425 [details]
Patch

WebKit/win/ChangeLog:8
 +          Cairo does not properly deal with Windows HDC's that have been
There's no apostrophe in "HDCs".

WebKit/win/ChangeLog:13
 +          Instead, reset the HDC's WorldTransform to unity, scale the
Here, too.

I think maybe you mean "identity" instead of "unity".

WebKit/win/WebFrame.cpp:2166
 +      int y = max(static_cast<int>(headerHeight) + pageRect.height(), m_pageHeight  -static_cast<int>(footerHeight));
Weird spacing around the - operator here.

WebKit/win/WebFrame.cpp:2172
 +  static XFORM buildXformFromCairo(HDC targetDC, cairo_t* cr)
I think WebKit naming conventions say this should be buildXFORMFromCairo.

Is there a better name we can use than "cr"?

WebKit/win/WebFrame.cpp:2247
 +  static void setCairoToHDC(cairo_t* cr, HDC targetDC)
Again, a better name than "cr" would be nice.

I agree that this function could use a better name.

WebKit/win/WebFrame.cpp:2252
 +      XFORM revertedCTM(passedCTM);
Using constructor syntax here is a little surprising.
Comment 5 Brent Fulgham 2010-05-20 12:15:15 PDT
Landed in http://trac.webkit.org/changeset/59855.