Bug 26187 - Canvas initial lineWidth not in sync with graphics backends.
Summary: Canvas initial lineWidth not in sync with graphics backends.
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Nobody
Depends on:
Reported: 2009-06-04 08:22 PDT by Dean McNamee
Modified: 2009-06-09 17:18 PDT (History)
3 users (show)

See Also:

patch w/ layout test (2.57 KB, patch)
2009-06-04 08:24 PDT, Dean McNamee
oliver: review-
Details | Formatted Diff | Diff
New patch with more modern layout test (3.96 KB, patch)
2009-06-09 03:16 PDT, Dean McNamee
oliver: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dean McNamee 2009-06-04 08:22:04 PDT
The canvas rendering context has two pieces of state, the drawing context / graphics backend.  Canvas always defaults to a thickness of 1.  It happens that the CG backend also defaults to 1.  However, the Skia backend defaults to 0, and the Cairo backend defaults to 2.

This means that the follow line changes the style in both the Skia and Cairo backends:

ctx.lineWidth = ctx.lineWidth;

This is because ctx.lineWidth reports the <canvas> state, which is initially 1.  However, this is not actually querying the graphics backend.  Then setting it to 1 actually keeps them in sync.

My fix is simple, to set 1 to the graphics backend on creation of the canvas rendering context.

It might be something bigger / for the future to consider whether the Skia and Cairo backends should also default to a thickness of 1.
Comment 1 Dean McNamee 2009-06-04 08:24:26 PDT
Created attachment 30947 [details]
patch w/ layout test
Comment 2 Oliver Hunt 2009-06-04 12:42:00 PDT
Comment on attachment 30947 [details]
patch w/ layout test

The fix itself looks good but i don't like the layout test.  You should look at the design of the more modern canvas tests -- they're basically the same structure as JS ones so find a test which has the following

fast/canvas/test-name.html // html document will just have some boiler plate that includes a couple of js files, resoureces/test-name.js and then another js file
fast/canvas/resources/test-name.js // The actual test itself

I *think* the isPointInPath tests cases will be of this newer style.   The newer style is vastly more useful as we use getImagedata to anaylse the result of drawing rather than relying on pixel comparisons.  This is especially benefical given people typically do no run pixel tests, but also because it's much easier to see what is being tested.

Comment 3 Dean McNamee 2009-06-09 03:16:18 PDT
Created attachment 31088 [details]
New patch with more modern layout test
Comment 4 Oliver Hunt 2009-06-09 10:46:30 PDT
Comment on attachment 31088 [details]
New patch with more modern layout test

Comment 5 Brent Fulgham 2009-06-09 13:40:19 PDT
Landed in @r44542
Comment 6 Darin Fisher (:fishd, Google) 2009-06-09 16:50:05 PDT
Guys, this is missing expected test results.  The buildbots are not happy.
Comment 7 Brent Fulgham 2009-06-09 17:18:22 PDT
Added missing canvas-lineWidth-expected.txt in @r44548.