Bug 20578 - [CAIRO] pattern of a canvas-element changes after modifications on canvas-element
: [CAIRO] pattern of a canvas-element changes after modifications on canvas-ele...
Status: RESOLVED FIXED
: WebKit
New Bugs
: 528+ (Nightly build)
: PC All
: P2 Normal
Assigned To:
: http://philip.html5.org/tests/canvas/...
:
:
:
  Show dependency treegraph
 
Reported: 2008-08-31 11:30 PST by
Modified: 2009-07-16 15:28 PST (History)


Attachments
Patch v1 (6.42 KB, patch)
2009-06-23 02:10 PST, Shinichiro Hamaji
no flags Review Patch | Details | Formatted Diff | Diff
Patch v2 (6.45 KB, patch)
2009-06-23 18:33 PST, Shinichiro Hamaji
eric: review-
Review Patch | Details | Formatted Diff | Diff
Patch v3 (6.86 KB, patch)
2009-06-26 04:29 PST, Shinichiro Hamaji
no flags Review Patch | Details | Formatted Diff | Diff
Patch v4 (6.81 KB, patch)
2009-07-09 22:45 PST, Shinichiro Hamaji
oliver: review+
Review Patch | Details | Formatted Diff | Diff


Note

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


Description From 2008-08-31 11:30:01 PST
If you create a pattern of a canvas-element

var pattern = ctx.createPattern(canvas, 'no-repeat');

and modify the canvas after that, the pattern is changed too.
------- Comment #1 From 2009-06-23 02:10:41 PST -------
Created an attachment (id=31714) [details]
Patch v1

 LayoutTests/ChangeLog                              |   18 +++++++
 .../fast/canvas/canvas-pattern-modify-expected.txt |   13 +++++
 LayoutTests/fast/canvas/canvas-pattern-modify.html |    6 ++
 LayoutTests/fast/canvas/canvas-pattern-modify.js   |   52 ++++++++++++++++++++
 WebCore/ChangeLog                                  |   15 ++++++
 .../platform/graphics/cairo/ImageBufferCairo.cpp   |   19 +++++++-
 6 files changed, 122 insertions(+), 1 deletions(-)
------- Comment #2 From 2009-06-23 17:42:52 PST -------
(From update of attachment 31714 [details])
Isn't there an easier/cleaner way to copy an image in Cairo? 

Is memcpy the right allocator to use?
------- Comment #3 From 2009-06-23 18:33:41 PST -------
Created an attachment (id=31764) [details]
Patch v2

 LayoutTests/ChangeLog                              |   18 +++++++
 .../fast/canvas/canvas-pattern-modify-expected.txt |   13 +++++
 LayoutTests/fast/canvas/canvas-pattern-modify.html |    6 ++
 LayoutTests/fast/canvas/canvas-pattern-modify.js   |   52 ++++++++++++++++++++
 WebCore/ChangeLog                                  |   15 ++++++
 .../platform/graphics/cairo/ImageBufferCairo.cpp   |   22 ++++++++-
 6 files changed, 125 insertions(+), 1 deletions(-)
------- Comment #4 From 2009-06-23 18:41:00 PST -------
(In reply to comment #2)
> (From update of attachment 31714 [details] [review])
> Isn't there an easier/cleaner way to copy an image in Cairo? 

I searched cairo API and web, but couldn't find an convenient API for this. In the following discussion, a person is saying that "cairo isn't providing a "copy surface" primitive".

http://lists.cairographics.org/archives/cairo/2007-June/010877.html

> Is memcpy the right allocator to use?

Ah, I think using cairo_paint would be better as it would work even if the source surface has different internal representation. I've uploaded another patch which changes this. Thanks.
------- Comment #5 From 2009-06-26 03:25:46 PST -------
(From update of attachment 31764 [details])
I think this should be broken into a "copySurface" static inline.  That function should have a comment pointing to the discussion about how cairo is lacking this call.

Otherwise this looks great.

I think it would be nice to fix the one nit about breaking this out into its own function with a comment about how cairo doesn't provide this natively.  Eventually cairo is likely to provide this, and when it does, we can replace your copySurface function with a call to the native cairo call.
------- Comment #6 From 2009-06-26 04:29:29 PST -------
Created an attachment (id=31926) [details]
Patch v3
------- Comment #7 From 2009-06-26 04:30:21 PST -------
(From update of attachment 31926 [details])
Thanks for the review! I've created a new static inline function with some comments.
------- Comment #8 From 2009-07-09 22:45:13 PST -------
Created an attachment (id=32550) [details]
Patch v4

Fixing a style error for copySurface. Though Eric kindly said that he'll fix this, I fixed it to save Eric's time a bit.
------- Comment #9 From 2009-07-16 00:02:55 PST -------
(From update of attachment 32550 [details])
r=me
------- Comment #10 From 2009-07-16 15:07:21 PST -------
Will land.
------- Comment #11 From 2009-07-16 15:28:02 PST -------
Committed r45989