Bug 42267 – Canvas: Make assigning the same fillStyle or strokeStyle a fast no-op

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Andreas Kling]

Right now we always call into the GraphicsContext which is a fairly expensive operation (at least for Qt.)
This can easily be avoided for cases where you set the fillStyle or strokeStyle to the same value it already is.

[Andreas Kling]

Created attachment 61529 [details]
Proposed patch

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

BEFORE:

Elapsed wall-clock time: 905ms (ideal: 640ms).
Elapsed non-idle time: 265ms (ideal: 0ms).
Speed: 16.57fps (ideal: 25.00fps).

AFTER:

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

[Andreas Kling]

This can be made even faster by skipping the CanvasStyle construction path for color strings. This will make it hard to drop the custom JS bindings for fillStyle and strokeStyle though. ;(

[Darin Adler]

Comment on attachment 61529 [details]
Proposed patch

> +    if (m_type == RGBA)
> +        return m_rgba == other.m_rgba;
> +
> +    if (m_type == Gradient)
> +        return m_gradient == other.m_gradient;
> +
> +    if (m_type == ImagePattern)
> +        return m_pattern == other.m_pattern;
> +
> +    if (m_type == CMYKA)
> +        return m_cmyka.c == other.m_cmyka.c
> +            && m_cmyka.m == other.m_cmyka.m
> +            && m_cmyka.y == other.m_cmyka.y
> +            && m_cmyka.k == other.m_cmyka.k
> +            && m_cmyka.a == other.m_cmyka.a;

I'd suggest using a switch here instead of if statements.

> +        bool operator==(const CanvasStyle&) const;

Typically it's better style to use a friend function rather than a member function for ==, since it allows conversions to be performed on both the left and right side of an expression, whereas a member function only allows conversions on the right side.

r=me despite that

[Andreas Kling]

Comment on attachment 61529 [details]
Proposed patch

Setting cq-, will make Darin's suggested adjustments.

[Andreas Kling]

Committed r63327: <http://trac.webkit.org/changeset/63327>