67054 2011-08-26 13:02:59 -0700 Allow canvas backing store to be lazily allocated 2011-10-04 11:40:38 -0700 1 1 1 Unclassified WebKit Canvas 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 mdelaney7 mdelaney7 mdelaney7 webkit-bug-importer oldest_to_newest 457715 0 mdelaney7 2011-08-26 13:02:59 -0700 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. 457716 1 webkit-bug-importer 2011-08-26 13:03:54 -0700 <rdar://problem/10031538> 477285 2 109540 mdelaney7 2011-10-03 15:25:54 -0700 Created attachment 109540 Patch 477288 3 mdelaney7 2011-10-03 15:30:07 -0700 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). 477291 4 109540 darin 2011-10-03 15:33:06 -0700 Comment on attachment 109540 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. 477805 5 mdelaney7 2011-10-04 11:40:38 -0700 Committed r96624: <http://trac.webkit.org/changeset/96624> 109540 2011-10-03 15:25:54 -0700 2011-10-03 15:33:05 -0700 Patch bug-67054-20111003152554.patch text/plain 2402 mdelaney7 SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDk2NTQ3KQorKysgU291cmNlL1dlYkNvcmUvQ2hhbmdl TG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMTggQEAKKzIwMTEtMTAtMDMgIE1hdHRoZXcg RGVsYW5leSAgPG1kZWxhbmV5QGFwcGxlLmNvbT4KKworICAgICAgICBJbnZlc3RpZ2F0ZSByZW1v dmluZyBsaW5lV2lkdGgoKSBmcm9tIENhbnZhc1JlbmRlcmluZ0NvbnRleHQyRC5jcHAncyBjdG9y CisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD02NzA1NAor CisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIE5vIG5ldyB0 ZXN0czsgZG9lc24ndCBhZmZlY3QgYmVoYXZpb3IuCisKKyAgICAgICAgKiBodG1sL0hUTUxDYW52 YXNFbGVtZW50LmNwcDoKKyAgICAgICAgKFdlYkNvcmU6OkhUTUxDYW52YXNFbGVtZW50OjpjcmVh dGVJbWFnZUJ1ZmZlcik6IFJlbW92ZWQgdGhlIHNldHRpbmcgb2YgbGluZVdpZHRoLgorICAgICAg ICAqIGh0bWwvY2FudmFzL0NhbnZhc1JlbmRlcmluZ0NvbnRleHQyRC5jcHA6CisgICAgICAgIChX ZWJDb3JlOjpDYW52YXNSZW5kZXJpbmdDb250ZXh0MkQ6OkNhbnZhc1JlbmRlcmluZ0NvbnRleHQy RCk6IEFkZGVkIGluIGRlZmF1bHQKKyAgICAgICAgbGluZSB3aWR0aCBzZXR0aW5nIGFsb25nIHdp dGggb3RoZXIgZGVmYXVsdCBjb250ZXh0IHNldHRpbmdzLgorCiAyMDExLTA5LTIyICBEYXJpbiBB ZGxlciAgPGRhcmluQGFwcGxlLmNvbT4KIAogICAgICAgICBSZW5hbWUgbWFueSBvYnZpb3VzbHkt Y29ycmVjdCBjYWxsIHNpdGVzIGluIFdlYkNvcmUgdXNpbmcgcmVsZWFzZVJlZiB0byB1c2UgaXQg YnkgaXRzIG5ldyBuYW1lIGxlYWtSZWYKSW5kZXg6IFNvdXJjZS9XZWJDb3JlL2h0bWwvSFRNTENh bnZhc0VsZW1lbnQuY3BwCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9XZWJDb3JlL2h0bWwvSFRNTENh bnZhc0VsZW1lbnQuY3BwCShyZXZpc2lvbiA5NjUyMikKKysrIFNvdXJjZS9XZWJDb3JlL2h0bWwv SFRNTENhbnZhc0VsZW1lbnQuY3BwCSh3b3JraW5nIGNvcHkpCkBAIC00NTgsNiArNDU4LDcgQEAg dm9pZCBIVE1MQ2FudmFzRWxlbWVudDo6Y3JlYXRlSW1hZ2VCdWZmZQogICAgIG1faW1hZ2VCdWZm ZXItPmNvbnRleHQoKS0+c2NhbGUoRmxvYXRTaXplKGJ1ZmZlclNpemUud2lkdGgoKSAvIGxvZ2lj YWxTaXplLndpZHRoKCksIGJ1ZmZlclNpemUuaGVpZ2h0KCkgLyBsb2dpY2FsU2l6ZS5oZWlnaHQo KSkpOwogICAgIG1faW1hZ2VCdWZmZXItPmNvbnRleHQoKS0+c2V0U2hhZG93c0lnbm9yZVRyYW5z Zm9ybXModHJ1ZSk7CiAgICAgbV9pbWFnZUJ1ZmZlci0+Y29udGV4dCgpLT5zZXRJbWFnZUludGVy cG9sYXRpb25RdWFsaXR5KERlZmF1bHRJbnRlcnBvbGF0aW9uUXVhbGl0eSk7CisgICAgbV9pbWFn ZUJ1ZmZlci0+Y29udGV4dCgpLT5zZXRTdHJva2VUaGlja25lc3MoMSk7CiAKICNpZiBVU0UoSlND KQogICAgIEpTQzo6SlNMb2NrIGxvY2soSlNDOjpTaWxlbmNlQXNzZXJ0aW9uc09ubHkpOwpJbmRl eDogU291cmNlL1dlYkNvcmUvaHRtbC9jYW52YXMvQ2FudmFzUmVuZGVyaW5nQ29udGV4dDJELmNw cAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2ViQ29yZS9odG1sL2NhbnZhcy9DYW52YXNSZW5kZXJp bmdDb250ZXh0MkQuY3BwCShyZXZpc2lvbiA5NjUyMikKKysrIFNvdXJjZS9XZWJDb3JlL2h0bWwv Y2FudmFzL0NhbnZhc1JlbmRlcmluZ0NvbnRleHQyRC5jcHAJKHdvcmtpbmcgY29weSkKQEAgLTEy MiwxMCArMTIyLDYgQEAgQ2FudmFzUmVuZGVyaW5nQ29udGV4dDJEOjpDYW52YXNSZW5kZXJpbgog I2lmICFFTkFCTEUoREFTSEJPQVJEX1NVUFBPUlQpCiAgICAgQVNTRVJUX1VOVVNFRCh1c2VzRGFz aGJvYXJkQ29tcGF0aWJpbGl0eU1vZGUsICF1c2VzRGFzaGJvYXJkQ29tcGF0aWJpbGl0eU1vZGUp OwogI2VuZGlmCi0KLSAgICAvLyBNYWtlIHN1cmUgdGhhdCBldmVuIGlmIHRoZSBkcmF3aW5nQ29u dGV4dCgpIGhhcyBhIGRpZmZlcmVudCBkZWZhdWx0Ci0gICAgLy8gdGhpY2tuZXNzLCBpdCBpcyBp biBzeW5jIHdpdGggdGhlIGNhbnZhcyB0aGlja25lc3MuCi0gICAgc2V0TGluZVdpZHRoKGxpbmVX aWR0aCgpKTsKIH0KIAogdm9pZCBDYW52YXNSZW5kZXJpbmdDb250ZXh0MkQ6OnVud2luZFN0YXRl U3RhY2soKQo=