Bug 60331

Summary: Remove canvas usage of roundToDevicePixels
Product: WebKit Reporter: Matthew Delaney <mdelaney7>
Component: CanvasAssignee: Matthew Delaney <mdelaney7>
Status: RESOLVED FIXED    
Severity: Normal CC: mdelaney7, simon.fraser
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch simon.fraser: review+

Description Matthew Delaney 2011-05-05 19:26:23 PDT
roundToDevicePixels() was added in http://trac.webkit.org/changeset/14739 to canvas in order to remedy pixel cracks when at non-integral scales. This code path causes us to create clips for drawImage which causes the drawImage path to be much slower in CG. Chromium appears to ignore the method altogether by early returning from it. I don't believe it's necessary to worry about rounding to device pixels when drawing in the canvas' backing stores.
Comment 1 Matthew Delaney 2011-05-05 19:39:03 PDT
Created attachment 92531 [details]
Patch
Comment 2 Simon Fraser (smfr) 2011-05-05 22:34:28 PDT
Comment on attachment 92531 [details]
Patch

Were all these calls to roundToDevicePixels() added at the same time? Have you looked at the history to check why each was added?
Comment 3 Matthew Delaney 2011-05-09 11:22:35 PDT
Created attachment 92810 [details]
Patch
Comment 4 Simon Fraser (smfr) 2011-05-09 12:27:23 PDT
Comment on attachment 92810 [details]
Patch

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

> Source/WebCore/platform/graphics/cg/ImageCG.cpp:271
> -    CGContextDrawImage(context, GraphicsContext(context).roundToDevicePixels(FloatRect(0, 0, CGImageGetWidth(image), CGImageGetHeight(image))), image);
> +    CGContextDrawImage(context, FloatRect(0, 0, CGImageGetWidth(image), CGImageGetHeight(image)), image);

The change log says "This could only affect canvases at non-integral scale factors" but that doesn't seem to be true for this change.

> Source/WebCore/platform/graphics/cg/PatternCG.cpp:43
> +    CGRect rect = FloatRect(0, 0, CGImageGetWidth(platformImage), CGImageGetHeight(platformImage));

Ditto.
Comment 5 Matthew Delaney 2011-05-09 12:42:46 PDT
Created attachment 92829 [details]
Patch
Comment 6 Matthew Delaney 2011-05-09 13:23:13 PDT
Committed r86085: <http://trac.webkit.org/changeset/86085>