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
Product: WebKit
Classification: Unclassified
Component: New Bugs
: 528+ (Nightly build)
: PC All
: P2 Normal
Assigned To: Adam Barth
http://philip.html5.org/tests/canvas/...
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2008-08-31 11:30 PDT by Dirk Schulze
Modified: 2009-07-16 15:28 PDT (History)
3 users (show)

See Also:


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

Note You need to log in before you can comment on or make changes to this bug.
Description Dirk Schulze 2008-08-31 11:30:01 PDT
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 Shinichiro Hamaji 2009-06-23 02:10:41 PDT
Created attachment 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 Eric Seidel 2009-06-23 17:42:52 PDT
Comment on attachment 31714 [details]
Patch v1

Isn't there an easier/cleaner way to copy an image in Cairo? 

Is memcpy the right allocator to use?
Comment 3 Shinichiro Hamaji 2009-06-23 18:33:41 PDT
Created attachment 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 Shinichiro Hamaji 2009-06-23 18:41:00 PDT
(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 Eric Seidel 2009-06-26 03:25:46 PDT
Comment on attachment 31764 [details]
Patch v2

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 Shinichiro Hamaji 2009-06-26 04:29:29 PDT
Created attachment 31926 [details]
Patch v3
Comment 7 Shinichiro Hamaji 2009-06-26 04:30:21 PDT
Comment on attachment 31926 [details]
Patch v3

Thanks for the review! I've created a new static inline function with some comments.
Comment 8 Shinichiro Hamaji 2009-07-09 22:45:13 PDT
Created attachment 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 Oliver Hunt 2009-07-16 00:02:55 PDT
Comment on attachment 32550 [details]
Patch v4

r=me
Comment 10 Adam Barth 2009-07-16 15:07:21 PDT
Will land.
Comment 11 Adam Barth 2009-07-16 15:28:02 PDT
Committed r45989