Bug 104367

Summary: GraphicsContext::drawImageBuffer is inefficient
Product: WebKit Reporter: Stephen Chenney <schenney>
Component: PlatformAssignee: Stephen Chenney <schenney>
Status: RESOLVED FIXED    
Severity: Normal CC: a.renevier, buildbot, cabanier, fmalita, krit, pdr, rniwa
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 104176    
Bug Blocks: 100026    
Attachments:
Description Flags
Patch
none
Patch none

Stephen Chenney
Reported 2012-12-07 06:07:37 PST
There are several versions of the GraphicsContext::drawImageBuffer method, each taking a different form for the src and dest rectangle specs. As written, the "least specific" version calls the version that is slightly more specific which calls the version that is slightly more specific which ..., until the most specific version is invoked to do the work. All of these calls could be short circuited with each non-final version invoking the final version directly. We would have fewer stack frame and less parameter shuffling.
Attachments
Patch (5.88 KB, patch)
2013-01-28 16:26 PST, Stephen Chenney 		no flags
DetailsFormatted DiffDiff
Patch (7.74 KB, patch)
2013-02-05 12:46 PST, Stephen Chenney 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Stephen Chenney
Comment 1 2013-01-28 16:26:37 PST
Created attachment 185096 [details] Patch We should now be calling at most 2 versions of drawImageBuffer or drawImage
Build Bot
Comment 2 2013-01-29 03:37:02 PST
Comment on attachment 185096 [details] Patch Attachment 185096 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://queues.webkit.org/results/16198105 New failing tests: fast/workers/worker-document-leak.html
Dirk Schulze
Comment 3 2013-01-29 08:12:46 PST
Comment on attachment 185096 [details] Patch I wish we had a Ctor for FloatRect that takes IntPoint and Intsize. But I guess it is not common enough to implement it. Do we have a performance test for that? Would be great if you can create one if not. r=me anyway.
Philip Rogers
Comment 4 2013-01-29 14:57:11 PST
There are essentially no other uses of FloatRect(IntRect(...)) outside GraphicsContext.cpp so I wonder if there's a better way. We could at least modify a couple of the FloatRect(IntRect(IntPoint(), ...)) ones to be FloatRect(FloatPoint(), FloatSize( )).
Stephen Chenney
Comment 5 2013-02-05 12:46:07 PST
Created attachment 186677 [details] Patch Modified to reflect Philip's suggestion, and to remove an unused method
Stephen Chenney
Comment 6 2013-02-05 12:46:46 PST
We should see how this does on ews.
Stephen Chenney
Comment 7 2013-02-07 07:57:23 PST
Committed r142123: <http://trac.webkit.org/changeset/142123>
Eric Seidel (no email)
Comment 8 2013-03-01 02:52:13 PST
Comment on attachment 186677 [details] Patch Cleared review? from attachment 186677 [details] so that this bug does not appear in http://webkit.org/pending-review. If you would like this patch reviewed, please attach it to a new bug (or re-open this bug before marking it for review again).
Note You need to log in before you can comment on or make changes to this bug.