Bug 104367 – GraphicsContext::drawImageBuffer is inefficient

• Component: Platform
• Status: RESOLVED FIXED

[Stephen Chenney]

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.

[Stephen Chenney]

Created attachment 185096 [details]
Patch

We should now be calling at most 2 versions of drawImageBuffer or drawImage

[Build Bot]

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 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]

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]

Created attachment 186677 [details]
Patch

Modified to reflect Philip's suggestion, and to remove an unused method

[Stephen Chenney]

We should see how this does on ews.

[Stephen Chenney]

Committed r142123: <http://trac.webkit.org/changeset/142123>

[Eric Seidel (no email)]

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