Bug 70852 - Setting up a HiDPI base-level GraphicsContext should be more straightforward for WebKit2
Summary: Setting up a HiDPI base-level GraphicsContext should be more straightforward ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Beth Dakin
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-10-25 14:26 PDT by Beth Dakin
Modified: 2011-10-25 16:09 PDT (History)
3 users (show)

See Also:

Attachments
Patch (9.32 KB, patch)
2011-10-25 14:36 PDT, Beth Dakin 		webkit-ews: commit-queue-
Details | Formatted Diff | Diff
Another patch (9.50 KB, patch)
2011-10-25 14:59 PDT, Beth Dakin 		mitz: review+
Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Beth Dakin 2011-10-25 14:26:10 PDT 
Right now there are three places in WebKit2 where we have to set up a GraphicsContext to act as a base-level HiDPI GraphicsContext. Once in DrawingAreaImpl::display() which sets up the context for the main web page. Then twice in WebPage.cpp for snapshotting functions. I wanted to improve those three places so that they set up a HiDPI context in a way that is more straightforward.
Comment 1 Beth Dakin 2011-10-25 14:36:51 PDT 
Created attachment 112405 [details]
Patch
Comment 2 Early Warning System Bot 2011-10-25 14:55:11 PDT 
Comment on attachment 112405 [details]
Patch

Attachment 112405 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/10208488
Comment 3 WebKit Review Bot 2011-10-25 14:55:50 PDT 
Comment on attachment 112405 [details]
Patch

Attachment 112405 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/10209391
Comment 4 Beth Dakin 2011-10-25 14:59:44 PDT 
Created attachment 112412 [details]
Another patch
Comment 5 mitz 2011-10-25 15:03:38 PDT 
Comment on attachment 112412 [details]
Another patch

View in context: https://bugs.webkit.org/attachment.cgi?id=112412&action=review

> Source/WebCore/platform/graphics/cg/GraphicsContextCG.cpp:1597
> +    scale(FloatSize(deviceScaleFactor, deviceScaleFactor));

A slightly neater way to do this would be to keep this common part in the cross-platform applyDeviceScaleFactor() and have that one call a private platformApplyDeviceScaleFactor() that would have a no-op definition in GraphicsContext.cpp and a CG-specific definition in GraphicsContextCG.cpp, but I think this is okay for now.
Comment 6 Beth Dakin 2011-10-25 15:27:37 PDT 
(In reply to comment #5)
> (From update of attachment 112412 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=112412&action=review
> 
> > Source/WebCore/platform/graphics/cg/GraphicsContextCG.cpp:1597
> > +    scale(FloatSize(deviceScaleFactor, deviceScaleFactor));
> 
> A slightly neater way to do this would be to keep this common part in the cross-platform applyDeviceScaleFactor() and have that one call a private platformApplyDeviceScaleFactor() that would have a no-op definition in GraphicsContext.cpp and a CG-specific definition in GraphicsContextCG.cpp, but I think this is okay for now.

Oh, I like that! I'll do that! Thanks for the review, Dan!
Comment 7 Beth Dakin 2011-10-25 16:09:30 PDT 
Committed change with revision 98406.