|Summary:||Allow canvas backing store to be lazily allocated|
|Product:||WebKit||Reporter:||Matthew Delaney <mdelaney7>|
|Component:||Canvas||Assignee:||Matthew Delaney <mdelaney7>|
|Version:||528+ (Nightly build)|
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 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.