Bug 104367 - GraphicsContext::drawImageBuffer is inefficient
Summary: GraphicsContext::drawImageBuffer is inefficient
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Stephen Chenney
Depends on: 104176
Blocks: 100026
  Show dependency treegraph
Reported: 2012-12-07 06:07 PST by Stephen Chenney
Modified: 2013-03-01 02:52 PST (History)
7 users (show)

See Also:

Patch (5.88 KB, patch)
2013-01-28 16:26 PST, Stephen Chenney
no flags Details | Formatted Diff | Diff
Patch (7.74 KB, patch)
2013-02-05 12:46 PST, Stephen Chenney
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Stephen Chenney 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.
Comment 1 Stephen Chenney 2013-01-28 16:26:37 PST
Created attachment 185096 [details]

We should now be calling at most 2 versions of drawImageBuffer or drawImage
Comment 2 Build Bot 2013-01-29 03:37:02 PST
Comment on attachment 185096 [details]

Attachment 185096 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://queues.webkit.org/results/16198105

New failing tests:
Comment 3 Dirk Schulze 2013-01-29 08:12:46 PST
Comment on attachment 185096 [details]

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.
Comment 4 Philip Rogers 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( )).
Comment 5 Stephen Chenney 2013-02-05 12:46:07 PST
Created attachment 186677 [details]

Modified to reflect Philip's suggestion, and to remove an unused method
Comment 6 Stephen Chenney 2013-02-05 12:46:46 PST
We should see how this does on ews.
Comment 7 Stephen Chenney 2013-02-07 07:57:23 PST
Committed r142123: <http://trac.webkit.org/changeset/142123>
Comment 8 Eric Seidel (no email) 2013-03-01 02:52:13 PST
Comment on attachment 186677 [details]

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).