Last Comment Bug 433004 - Support canvas.getContext("2d").createImageData()
: Support canvas.getContext("2d").createImageData()
Status: RESOLVED FIXED
: dev-doc-complete, perf
Product: Core
Classification: Components
Component: Canvas: 2D (show other bugs)
: Trunk
: All All
: -- normal (vote)
: ---
Assigned To: Vladimir Vukicevic [:vlad] [:vladv]
:
Mentors:
http://www.whatwg.org/specs/web-apps/...
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2008-05-09 09:12 PDT by Jeff Walden [:Waldo] (remove +bmo to email)
Modified: 2009-05-12 08:17 PDT (History)
13 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
initial impl (2.69 KB, patch)
2008-06-05 17:41 PDT, Vladimir Vukicevic [:vlad] [:vladv]
no flags Details | Diff | Review
let's try that again, without the built-in integer overflow (2.96 KB, patch)
2008-06-05 17:48 PDT, Vladimir Vukicevic [:vlad] [:vladv]
roc: review+
roc: superreview+
Details | Diff | Review
a patch to implement a DOM object, clamping, the whole bit (26.35 KB, patch)
2008-08-14 10:45 PDT, Vladimir Vukicevic [:vlad] [:vladv]
no flags Details | Diff | Review

Description Jeff Walden [:Waldo] (remove +bmo to email) 2008-05-09 09:12:51 PDT
It'll speed up canvas ops that splash an ImageData on the canvas, because the canvas won't have to typecheck a bunch of JS values.  There are also wins in resolution independence, but I'm not familiar with the arguments over that particular wrinkle.

The spec as of this commit:

http://lists.whatwg.org/htdig.cgi/commit-watchers-whatwg.org/2008/000396.html

...requires throwing a type mismatch error if putImageData is called with an object which wasn't created by createImageData, but that seems a bit harsh at least for now -- we should wait for existing content to catch up to the spec on this.
Comment 1 Vladimir Vukicevic [:vlad] [:vladv] 2008-05-09 11:27:39 PDT
Wha?  I had no idea this was added... throwing the error seems like failure, given that the current spec was working fine.  Blah.  But yes, overall this seems like a good idea (adding a createImageData object); I don't buy the resolution independence argument, though.
Comment 2 Vladimir Vukicevic [:vlad] [:vladv] 2008-06-05 17:41:21 PDT
Created attachment 323967 [details] [diff] [review]
initial impl

Here's an initial implementation that just returns a JS object.  It doesn't do the clamping on pixel assignment as required by the spec.  We should grab this just to have most of source-compat with the spec, and then do a real native ImageData object with clamping and optimized drawing (backed by a gfxImageSurface) as a separate patch.
Comment 3 Vladimir Vukicevic [:vlad] [:vladv] 2008-06-05 17:48:38 PDT
Created attachment 323968 [details] [diff] [review]
let's try that again, without the built-in integer overflow

Ahem.
Comment 4 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2008-06-05 19:09:40 PDT
I don't get it, we're just going to throw this patch away when we do the real patch, so what's the value of taking this patch?

+  // ImageData createImageData(in float w, in float h);
+  void createImageData();

And even if there is value in just returning a JS object temporarily, why not make this "void createImageData(in float w, in float h)"?
Comment 5 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2008-07-21 04:46:07 PDT
Comment on attachment 323968 [details] [diff] [review]
let's try that again, without the built-in integer overflow

See comment #4.

Perhaps Eric would have time to do a real implementation here? It would be great to have the real DOM object here instead of poking the JS engine.
Comment 6 Vladimir Vukicevic [:vlad] [:vladv] 2008-07-21 10:13:10 PDT
Yeah, we talked about it; it's a bit complicated because it requires a scriptable helper and possibly some fast-paths to make performance not be abysmal..
Comment 7 Vladimir Vukicevic [:vlad] [:vladv] 2008-08-14 10:45:13 PDT
Created attachment 333786 [details] [diff] [review]
a patch to implement a DOM object, clamping, the whole bit

Here's a patch that implements the DOM object, clamping, the whole bit.  However, it is horribly, horribly slow -- the per-element access overhead is atrocious.  The only thing that's sped up here is putImageData with an ImageData Object -- my test goes from 259 ms -> 16 ms, so that's a decent speedup.  But pixel manipulation slows down by a factor of 17 or 3, depending on how you're accessing the data, and that doesn't seem like a good tradeoff.

I don't think putImageData was ever the bottleneck here, but instead the image manipulation was.. though people are arguing that there are cases where someone wants to compute once and put multiple times.  For that case, creating a second canvas, putting the data to that canvas, and then using drawImage with that canvas as a source will be fast end more flexible (you can use that canvas as a pattern, can transform it, etc.).

It's been suggested to me that creating a custom JS object would be a way to get back performance, but I don't think I can do this in time for fx 3.1 -- so I suggest we go with my original patch and try to get to this later.  The only thing we lose out on is clamping on assignment as opposed to at put time, which seems quite minor.
Comment 8 Vladimir Vukicevic [:vlad] [:vladv] 2008-08-14 10:46:12 PDT
Comment on attachment 323968 [details] [diff] [review]
let's try that again, without the built-in integer overflow

Let's just go with this for 3.1, to get a createImageData function that does mostly the right thing for compat.  See earlier comment about why we shouldn't do the real fix for now.
Comment 9 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2008-08-14 16:25:51 PDT
Comment on attachment 323968 [details] [diff] [review]
let's try that again, without the built-in integer overflow

Interesting.

You probably should send feedback to the list pointing out that, even if you have very fast transitions into and out of the DOM, JS array access might still be faster since the JS runtime gets complete control over data representation and compiler optimizations. Furthermore, having createImageData do the clamping isn't so bad since that can be very easily parallelized, whereas JS execution cannot --- so JS execution is likely to be the bottleneck, not createImageData. (Plus your point about using a temporary canvas if we're only changing a few pixels of the image data, although that might be a bit hard to implement in some cases.)
Comment 10 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2008-08-18 15:06:51 PDT
I thought about comment #9 some more and I'm not sure it's true after all.

If you went all-out and inlined the DOM ImageData array getter/setter into the JS code, the clamping would be basically free and you'd be accessing less memory so you'd probably still be winning with the DOM object even with unlimited parallelism available.

So basically we should go ahead with this and later enable tracing through the ImageData getter/setter for great justice.
Comment 11 Vladimir Vukicevic [:vlad] [:vladv] 2008-08-18 15:34:32 PDT
"go ahead with this" -- the JS bandaid, or the full DOM thing?  I think the full DOM thing is definitely the way to go, I'm just not sure if we can get it performant for 1.9.1.
Comment 12 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2008-08-19 11:42:41 PDT
the bandaid's OK
Comment 13 Vladimir Vukicevic [:vlad] [:vladv] 2008-09-02 13:29:58 PDT
Checked in; filed bug 453355 on doing the real pixel data object in the future.
Comment 14 Eric Shepherd [:sheppy] 2009-05-12 08:17:14 PDT
Added this article, which covers createImageData() among other things:

https://developer.mozilla.org/En/HTML/Canvas/Pixel_manipulation_with_canvas

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