Bug 42495

Summary:

Canvas: Re-use existing CanvasStyle objects for color strings

Product:

WebKit

Reporter:

Andreas Kling <kling>

Component:

WebCore Misc.

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED WONTFIX

Severity:

Normal

CC:

oliver, webkit.review.bot

Priority:

Keywords:

Performance

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Attachments:

Description	Flags
Proposed patch	kling: review-

Andreas Kling

Reported 2010-07-16 22:40:18 PDT

Setting fillStyle or strokeStyle on a CanvasRenderingContext2D is still quite slow. We can optimize this further by re-using the current state().m_fillStyle or state().m_strokeStyle to house the new RGBA value.

Attachments
Proposed patch (5.01 KB, patch) 2010-07-16 23:00 PDT, Andreas Kling	kling: review-	Details Formatted Diff Diff
View All Add attachment proposed patch, testcase, etc.

Andreas Kling

Comment 1 2010-07-16 23:00:10 PDT

Created attachment 61876 [details] Proposed patch Benchmarking with Hixie's skeleton test: http://hixie.ch/tests/adhoc/perf/video/002.html BEFORE: Elapsed wall-clock time: 819ms (ideal: 640ms). Elapsed non-idle time: 179ms (ideal: 0ms). Speed: 18.32fps (ideal: 25.00fps). AFTER: Elapsed wall-clock time: 810ms (ideal: 640ms). Elapsed non-idle time: 170ms (ideal: 0ms). Speed: 18.52fps (ideal: 25.00fps).

WebKit Review Bot

Comment 2 2010-07-16 23:03:48 PDT

Attachment 61876 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1 WebCore/html/canvas/CanvasRenderingContext2D.cpp:149: More than one command on the same line [whitespace/newline] [4] Total errors found: 1 in 4 files If any of these errors are false positives, please file a bug against check-webkit-style.

Andreas Kling

Comment 3 2010-07-16 23:06:33 PDT

(In reply to comment #2) > WebCore/html/canvas/CanvasRenderingContext2D.cpp:149: More than one command on the same line [whitespace/newline] [4] Double semi-colon at end of line.

Oliver Hunt

Comment 4 2010-07-17 09:35:46 PDT

Comment on attachment 61876 [details] Proposed patch Do you have perf numbers?

Andreas Kling

Comment 5 2010-07-17 10:01:17 PDT

On second thought, I'm not sure this is such a great idea. It's adding a bit too much confusion to justify the barely measurable gain.

Andreas Kling

Comment 6 2010-07-17 13:11:12 PDT

Comment on attachment 61876 [details] Proposed patch r-, not worth the added complexity.

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