Bug 43718 - Canvas: Fast-path for setting the already set color with setStrokeColor() and setFillColor()
: Canvas: Fast-path for setting the already set color with setStrokeColor() and...
Status: RESOLVED FIXED
: WebKit
WebCore Misc.
: 528+ (Nightly build)
: All All
: P2 Normal
Assigned To:
:
: Performance
:
:
  Show dependency treegraph
 
Reported: 2010-08-09 05:01 PST by
Modified: 2010-08-09 09:03 PST (History)


Attachments
Proposed patch (5.07 KB, patch)
2010-08-09 05:06 PST, Andreas Kling
no flags Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2010-08-09 05:01:19 PST
Some JS demos, for example "Monster" at Chrome Experiments, use the WebKit-specific color setters of CanvasRenderingContext2D.

For this particular demo, roughly 5% of the calls to setFillColor() have the same color as is already set on the context. We should detect this and make an early return before creating any CanvasStyle objects (or calling into GraphicsContext.)
------- Comment #1 From 2010-08-09 05:06:32 PST -------
Created an attachment (id=63887) [details]
Proposed patch
------- Comment #2 From 2010-08-09 05:07:01 PST -------
The "Monster" demo mentioned in bug description:
http://www.chromeexperiments.com/detail/monster/
------- Comment #3 From 2010-08-09 06:23:13 PST -------
(From update of attachment 63887 [details])
Looks good.
------- Comment #4 From 2010-08-09 06:56:55 PST -------
(From update of attachment 63887 [details])
Clearing flags on attachment: 63887

Committed r64980: <http://trac.webkit.org/changeset/64980>
------- Comment #5 From 2010-08-09 06:57:05 PST -------
All reviewed patches have been landed.  Closing bug.
------- Comment #6 From 2010-08-09 09:03:55 PST -------
A side comment:

I think it's dangerous to overload and treat floating point values as RGBA or CMYKA just based on the number of arguments. Thus, these functions should have RGBA and CMYKA in their names. Similarly, the create functions for CanvasStyle should have RGBA and CMYKA and such in their names too.

The reason the functions in CanvasStyle used overloading historically is that they were constructors. Tis is one advantage of the create functions over direct calls to the constructors. We can leave the overloading trick to the constructors only.

CanvasRenderingContext2D has to do this overloading because that's in the specification, but internally we should not repeat that mistake.