Bug 42272 – Canvas: Fast-path for assigning the same color string as before to fillStyle or strokeStyle

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Andreas Kling]

When assigning a color string to fillStyle or strokeStyle, we take a detour via a CanvasStyle object.

We could add an early return fast-path when assigning the same string as before that does the check without creating a CanvasStyle object.

[Andreas Kling]

Created attachment 61541 [details]
Proposed patch

Benchmarking with Hixie's fillRect video test:
http://hixie.ch/tests/adhoc/perf/video/002.html

BEFORE:

Elapsed wall-clock time: 853ms (ideal: 640ms).
Elapsed non-idle time: 213ms (ideal: 0ms).
Speed: 17.58fps (ideal: 25.00fps).

AFTER:

Elapsed wall-clock time: 819ms (ideal: 640ms).
Elapsed non-idle time: 179ms (ideal: 0ms).
Speed: 18.32fps (ideal: 25.00fps).

[Eric Seidel (no email)]

Attachment 61541 [details] did not build on mac:
Build output: http://webkit-commit-queue.appspot.com/results/3548050

[Andreas Kling]

(In reply to comment #2)
> Attachment 61541 [details] did not build on mac:
> Build output: http://webkit-commit-queue.appspot.com/results/3548050

cc1plus: warnings being treated as errors
/Users/eseidel/Projects/MacEWS/WebCore/bindings/js/JSCanvasRenderingContext2DCustom.cpp:54: warning: unused parameter 'exec'
distcc[13899] ERROR: compile /Users/eseidel/Projects/MacEWS/WebCore/bindings/js/JSCanvasRenderingContext2DCustom.cpp on localhost failed

[Andreas Kling]

Created attachment 61546 [details]
Proposed patch v2

Less kludgy version - since assigning an empty string to a CanvasRenderingContext2D property has no effect we don't need the bools in the previous patch.

[Darin Adler]

Comment on attachment 61546 [details]
Proposed patch v2

>  JSValue JSCanvasRenderingContext2D::strokeStyle(ExecState* exec) const
>  {
>      CanvasRenderingContext2D* context = static_cast<CanvasRenderingContext2D*>(impl());
> +
>      return toJS(exec, context->strokeStyle());        
>  }

Stray change here.

[Andreas Kling]

Committed r63344: <http://trac.webkit.org/changeset/63344>