Bug 67054

Summary: Allow canvas backing store to be lazily allocated
Product: WebKit Reporter: Matthew Delaney <mdelaney7>
Component: CanvasAssignee: Matthew Delaney <mdelaney7>
Status: RESOLVED FIXED    
Severity: Normal CC: mdelaney7, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch darin: review+

Description Matthew Delaney 2011-08-26 13:02:59 PDT
I noticed that the lazy construction of a canvas (i.e. lazy allocation of backing store, etc.) is thwarted by setting the lineWidth in the CRC2D ctor. This was added in https://bugs.webkit.org/show_bug.cgi?id=26187 to rectify the issue that skia and cairo graphics contexts start with linewidth equal to 0 and 2 respectively where canvas needs them to be 1.

I imagine there's a better solution here that allows us to continue to have lazy construction/allocation for canvas. In particular, the case that sucks is when a page creates a bunch of canvases (e.g. pages that use cufonts), then get their 2d contexts, then resize the canvases, then use them. This has each canvas getting a 300x150 backing store created then throw right away once the width/height are changed.
Comment 1 Radar WebKit Bug Importer 2011-08-26 13:03:54 PDT
<rdar://problem/10031538>
Comment 2 Matthew Delaney 2011-10-03 15:25:54 PDT
Created attachment 109540 [details]
Patch
Comment 3 Matthew Delaney 2011-10-03 15:30:07 PDT
For the record, setting lineWidth() ends up grabbing the graphics context of the ImageBuffer to set the line width on. Grabbing the graphics context requires creation of the ImageBuffer.

Instead, I added the line width setter call to createImageBuffer alongside the other setter calls for other default settings. This ensures the platform graphics context gets the correct default value (for all platforms) and removes the dependency between HTMLCanvasElement creation and ImageBuffer creation which was time consuming and often a waste of memory (when the width and height were called shortly thereafter in most cases).
Comment 4 Darin Adler 2011-10-03 15:33:06 PDT
Comment on attachment 109540 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=109540&action=review

r=me on this; it’s OK to land this as is, but I think the fix isn’t quite right

> Source/WebCore/ChangeLog:3
> +        Investigate removing lineWidth() from CanvasRenderingContext2D.cpp's ctor

This is a really confusing name for the patch!

The patch itself seems clearly like a good idea.

> Source/WebCore/html/HTMLCanvasElement.cpp:461
> +    m_imageBuffer->context()->setStrokeThickness(1);

It seems to me that it’s a bug that the a GraphicsContext starts with a stroke thickness that is platform-dependent. I understand that the different graphics libraries have different defaults, but the point of the platform layer is to abstract away such differences. The code that sets the thickness to 1 should be in the platform directory instead of here in the canvas code.
Comment 5 Matthew Delaney 2011-10-04 11:40:38 PDT
Committed r96624: <http://trac.webkit.org/changeset/96624>