Implement ImageData constructor

RESOLVED FIXED in mozilla29

Status

()

Core
DOM
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: emk, Assigned: emk)

Tracking

({dev-doc-complete})

unspecified
mozilla29
dev-doc-complete
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

3 years ago
See bug 959925 for the use case. .createImageData is unavailable from workers.
This isn't in the spec, though.
(Assignee)

Comment 2

3 years ago
Actually it is.
http://www.w3.org/html/wg/drafts/2dcontext/html5_canvas/#hitregionoptions
> [Constructor (Uint8ClampedArray data, unsigned long width, unsigned long height)]   
> interface ImageData {
>   readonly attribute unsigned long width;
>   readonly attribute unsigned long height;
>   readonly attribute Uint8ClampedArray data;
> };
(Assignee)

Comment 3

3 years ago
Oh, yet another discrepancy between WHATWG and W3C :(
http://www.whatwg.org/specs/web-apps/current-work/multipage/the-canvas-element.html#hitregionoptions
(Assignee)

Comment 4

3 years ago
OTOH, WHATWG's ImageData has resolution but W3C's one doesn't.
I don't understand why W3C bugzilla 21045 and 21046 were closed.
(Assignee)

Updated

3 years ago
I think we should do this, fwiw.
(In reply to comment #5)
> I think we should do this, fwiw.

Yeah, and fix the whatwg spec to add a ctor there too!
(Assignee)

Comment 7

3 years ago
The problem is, the WHATWG spec has an attribute what the W3C spec does not.
How the constructor should be?
[Constructor (Uint8ClampedArray data, unsigned long width, unsigned long height, optional double resolution)]?
[Constructor (ImageDataInit initDict)]?
Or whatever else?
Keywords: dev-doc-needed

Updated

3 years ago
Keywords: dev-doc-needed

Updated

3 years ago
Keywords: dev-doc-needed
resolution's going away; the constructor as specced on the w3c side is probably ok.
Actually the W3C one's arguments don't make much sense... I'll spec something on the WHATWG side.
Done. There's two constructors:

   new ImageData(w, h); // returns a blank ImageData of the given dimensions

   new ImageData(data, w[, h]); // returns a new ImageData that references the given data, using the given width

See: http://www.whatwg.org/html#dom-imagedata
No longer blocks: 959925
(Assignee)

Comment 11

3 years ago
Created attachment 8361752 [details] [diff] [review]
Implement ImageData constructor

https://tbpl.mozilla.org/?tree=Try&rev=953f0204d579
Assignee: nobody → VYV03354
Status: NEW → ASSIGNED
(Assignee)

Comment 12

3 years ago
Created attachment 8362161 [details] [diff] [review]
Implement ImageData constructor

https://tbpl.mozilla.org/?tree=Try&rev=e696d5f383d3
Attachment #8361752 - Attachment is obsolete: true
Attachment #8362161 - Flags: review?(khuey)
(Assignee)

Comment 13

3 years ago
ping?
Attachment #8362161 - Flags: review?(khuey) → review?(bzbarsky)
Comment on attachment 8362161 [details] [diff] [review]
Implement ImageData constructor

>+  if (aWidth <= 0 || aHeight <= 0) {

They're unsigned, so just == 0, please.

>+  if (length <= 0 || length & 3) {

Likewise.  Also, "% 4" would be clearer than "& 3" here, I think.

>+  length >>= 2;

length /= 4, please.  The compiler will strength-reduce as needed, and the code is clearer.

>+  if (aWidth <= 0) {

Again, just ==.  Also, please file a spec issue; the spec doesn't do this check, but should.

r=me with that
Attachment #8362161 - Flags: review?(bzbarsky) → review+
(Assignee)

Comment 15

3 years ago
(In reply to Boris Zbarsky [:bz] from comment #14)
> >+  if (aWidth <= 0) {
> 
> Again, just ==.  Also, please file a spec issue; the spec doesn't do this
> check, but should.

It is covered by the text "If length is not an integral multiple of sw," in the spec. If sw is zero, length will never be an integral multiple of sw. Our explicit zero check is just an implementation detail.

https://hg.mozilla.org/integration/mozilla-inbound/rev/4a801fc05819
Flags: in-testsuite+
> It is covered by the text "If length is not an integral multiple of sw," in the spec.

I think that assumes an unwarranted level of mathematical sophistication on the part of spec readers.  The spec should just have an explicit check for 0 here before that test.
(Assignee)

Comment 17

3 years ago
Hm, indeed I forgot the check until the test crashed despite that programmers must be careful about the zero division. Filed <https://www.w3.org/Bugs/Public/show_bug.cgi?id=24429>.
https://hg.mozilla.org/mozilla-central/rev/4a801fc05819
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29

Comment 19

3 years ago
This API was implemented incorrectly.
The spec says that imagedata should keep a reference to the data but it seems to create a copy.
Flags: needinfo?(bzbarsky)

Comment 20

3 years ago
(In reply to Hixie (not reading bugmail) from comment #9)
> Actually the W3C one's arguments don't make much sense... I'll spec
> something on the WHATWG side.

The W3C level 2 version should just be a copy of the WhatWG. Not sure what was there before...
(Assignee)

Comment 21

3 years ago
(In reply to Rik Cabanier from comment #19)
> This API was implemented incorrectly.
> The spec says that imagedata should keep a reference to the data but it
> seems to create a copy.

Our implementation doesn't create a copy.
 < var a=new Uint8ClampedArray([1,2,3,4]);var i=new ImageData(a,1);i.data.set([5,6,7,8]);a;
 > Uint8ClampedArray [ 5, 6, 7, 8 ]
I even added an automated test to verify that. Why do you think it creates a copy?

(In reply to Rik Cabanier from comment #20)
> (In reply to Hixie (not reading bugmail) from comment #9)
> > Actually the W3C one's arguments don't make much sense... I'll spec
> > something on the WHATWG side.
> 
> The W3C level 2 version should just be a copy of the WhatWG. Not sure what
> was there before...

Because I referred a wrong version on the W3C site.
https://www.w3.org/Bugs/Public/show_bug.cgi?id=24298#c6
I agree with Masatoshi Kimura.  The code (which is _really_ simple), and basic tests, all indicate the constructor takes a reference.  Why do you think it creates a copy?
Flags: needinfo?(bzbarsky) → needinfo?(cabanier)

Comment 23

3 years ago
I looked at the wrong constructor.
Sorry about that!
Flags: needinfo?(cabanier)
Doc updated:
https://developer.mozilla.org/en-US/docs/Web/API/ImageData
https://developer.mozilla.org/en-US/docs/Web/API/ImageData.ImageData
https://developer.mozilla.org/en-US/Firefox/Releases/29
Keywords: dev-doc-needed → dev-doc-complete
You need to log in before you can comment on or make changes to this bug.