Add ImageBitmapOptions to ImageBitmap's spec #741

xidachen · 2016-02-25T15:15:43Z

This change adds ImageBitmapOptions to the ImageBitmap spec. The original proposal for ImageBitmapOptions can be found here: https://wiki.whatwg.org/wiki/ImageBitmap_Options.

domenic · 2016-02-26T01:34:34Z

source

@@ -89418,10 +89418,19 @@ typedef (<span>HTMLImageElement</span> or
         <span>CanvasRenderingContext2D</span> or
         <span>ImageBitmap</span>) <dfn>ImageBitmapSource</dfn>;

+enum ImageOrientation { "none", "flipY" };


<dfn>s around ImageOrientation, PremultiplyAlpha, and ColorspaceConversion at their definition sites (these next three lines)

<span>s around them at their usage sites (inside the dictionary)

domenic · 2016-02-26T01:50:18Z

Overall looking pretty reasonable on a technical level, especially given how this feature has been vetted through discussions on the wiki for a long time. Just lots of little editorial awesomeness of the HTML spec for you to get used to :).

Don't forget to add your name to the acknowledgments!!

domenic · 2016-02-26T19:41:44Z

source

+enum <dfn>ColorspaceConversion</dfn> { "<span data-x="dom-ColorspaceConversion-none">none</span>", "<span data-x="dom-ColorspaceConversion-default">default</span>" };
+
+dictionary <dfn>ImageBitmapOptions</dfn> {
+  <span>ImageOrientation</span> <span data-x="dom-ImageBitmapOptions-imageOrientation">imageOrientation</span> = "<code data-x="dom-ImageOrientation-none">none</code>";


span, not code; we are already inside a <pre>, and the code makes them orange.

domenic · 2016-02-26T19:52:55Z

Done with another round of review. I imagine this will be the last one and then I can merge. Yay!

domenic · 2016-02-26T22:12:42Z

@xidachen not sure if you are still working, but just wanted to let you know that despite all the "Done" emails, you don't seem to have uploaded the new commit ^_^

xidachen · 2016-02-28T02:15:04Z

@annevk Thank you for your comments. I have made changes in the latest commit. Please review.

annevk · 2016-02-28T16:54:02Z

Here we have a subdfn data-x="dom-createImageBitmap", if I break into two lines, should I have one subdfn for each line? That seems would complicate things. In the latest commit, I merge these two lines into one, but if we would prefer two lines, maybe we could have one line with subdfn and the other line doesn't?

I checked for precedent and the first line should have the subdfn. I still think it would be better to have two lines, since IDL has them too.

annevk · 2016-02-28T16:55:21Z

source

+  <ref spec=EXIF></p>
+
+  <p>If the value of the <dfn><code data-x="dom-ImageBitmapOptions-colorspaceConversion">
+  colorspaceConversion</code></dfn> member of <var>options</var> is


You cannot wrap directly after a start tag as that introduces whitespace between the start of the element and its contents. You'll need to wrap at data-x, even though that makes the first line short.

annevk · 2016-02-28T17:01:11Z

Something else, I don't really understand how the additional requirements fit into the constructor algorithm. Currently the algorithm is just a big <dl>. How do these new steps fit in?

Unless I missed something it seems like you added some paragraphs below, but that doesn't really work. The createImageBitmap() needs to be a coherent whole.

annevk · 2016-02-29T06:46:18Z

Yeah, something like that. Perhaps renaming "crop bitmap data to the source rectangle" to something more generic and adding these paragraphs as new steps at the end of that algorithm would be best.

xidachen · 2016-02-29T14:04:46Z

Yes, that makes more sense and is cleaner. I rename it to "crop bitmap data to the source rectangle with formatting", does that sound good?

annevk · 2016-02-29T14:28:30Z

I think that's fine. This looks a lot better already.

domenic · 2016-02-29T15:51:09Z

source

-  Promise&lt;ImageBitmap&gt; <span data-x="dom-createImageBitmap">createImageBitmap</span>(<span>ImageBitmapSource</span> image);
-  Promise&lt;ImageBitmap&gt; <span data-x="dom-createImageBitmap">createImageBitmap</span>(<span>ImageBitmapSource</span> image, long sx, long sy, long sw, long sh);
+  Promise&lt;ImageBitmap&gt; <span data-x="dom-createImageBitmap">createImageBitmap</span>(<span>ImageBitmapSource</span> image, optional <span>ImageBitmapOptions</span> options);
+  Promise&lt;ImageBitmap&gt; <span data-x="dom-createImageBitmap">createImageBitmap</span>(<span>ImageBitmapSource</span> image, long sx, long sy, long sw, long sh, optional <span>ImageBitmapOptions</span> options);


Just noticed that this overflows onto two lines. Could we move optional ImageBitmapOptions options onto a second line? Aligned with enough leading space so that it lines up with ImageBitmapSource on the above line.

No, we shouldn't wrap IDL.

Hmm I guess we don't do that in this spec. I was reading some CSSWG specs this morning that did. OK.

annevk · 2016-02-29T18:01:50Z

source

+    <p>If <var>image</var> is an <code>img</code> element or a <code>Blob</code> object, let
+    <var>val</var> be the value of the <dfn><code
+    data-x="dom-ImageBitmapOptions-colorspaceConversion">colorspaceConversion</code></dfn> member
+    of <var>options</var>, run these substeps:</p>


and then run these substeps*

domenic · 2016-02-29T21:04:06Z

source

+
+    <p>Let <var>val</var> be the value of <dfn><code
+    data-x="dom-ImageBitmapOptions-premultiplyAlpha">premultiplyAlpha</code></dfn> member of
+    <var>options</var>, run these substeps:</p>


"and then" run these substeps.

domenic · 2016-02-29T21:04:32Z

This LGTM with one nit that we can fix while merging if necessary. Will turn over to @annevk for his sign-off additionally.

This change allows for flipping images vertically (ignoring EXIF data), disabling colorspace conversion, and control over whether or not to premultiply by alpha. PR: #741

annevk · 2016-03-01T07:49:14Z

Thank you Xida! Great contribution. Landed as 49f9e04. (I changed the commit message, made @domenic's requested change, and made sure that the value inside Promise<> would be linked.)

xidachen · 2016-03-01T12:12:39Z

Thank you so much @domenic @annevk for reviewing this change and merging. I really appreciate your time.

domenic reviewed Feb 26, 2016
View reviewed changes

xidachen force-pushed the ImageBitmapSpecEditing branch from ca66e68 to e97c31e Compare February 26, 2016 15:11

domenic reviewed Feb 26, 2016
View reviewed changes

domenic self-assigned this Feb 26, 2016

domenic added the addition/proposal New features or enhancements label Feb 26, 2016

xidachen force-pushed the ImageBitmapSpecEditing branch 2 times, most recently from 807dda4 to eeb060e Compare February 28, 2016 02:12

annevk reviewed Feb 28, 2016
View reviewed changes

xidachen force-pushed the ImageBitmapSpecEditing branch from 567dd66 to 27cba12 Compare February 28, 2016 21:42

xidachen force-pushed the ImageBitmapSpecEditing branch from 27cba12 to c665107 Compare February 29, 2016 14:03

xidachen force-pushed the ImageBitmapSpecEditing branch 3 times, most recently from c546193 to 9530327 Compare February 29, 2016 15:45

domenic reviewed Feb 29, 2016
View reviewed changes

xidachen force-pushed the ImageBitmapSpecEditing branch from 9530327 to fd32bc9 Compare February 29, 2016 17:01

annevk reviewed Feb 29, 2016
View reviewed changes

Add ImageBitmapOptions to ImageBitmap's spec

38f377e

xidachen force-pushed the ImageBitmapSpecEditing branch from fd32bc9 to 38f377e Compare February 29, 2016 18:13

domenic reviewed Feb 29, 2016
View reviewed changes

domenic assigned annevk and unassigned domenic Feb 29, 2016

annevk closed this Mar 1, 2016

xidachen deleted the ImageBitmapSpecEditing branch March 2, 2016 13:33

annevk mentioned this pull request Oct 4, 2022

Behavior of createImageBitmap with { imageOrientation: "flipY" } is unclear #8118

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add ImageBitmapOptions to ImageBitmap's spec #741

Add ImageBitmapOptions to ImageBitmap's spec #741

xidachen commented Feb 25, 2016

domenic Feb 26, 2016

xidachen Feb 26, 2016

domenic commented Feb 26, 2016

domenic Feb 26, 2016

xidachen Feb 26, 2016

domenic commented Feb 26, 2016

domenic commented Feb 26, 2016

xidachen commented Feb 28, 2016

annevk commented Feb 28, 2016

annevk Feb 28, 2016

xidachen Feb 28, 2016

annevk commented Feb 28, 2016

annevk commented Feb 29, 2016

xidachen commented Feb 29, 2016

annevk commented Feb 29, 2016

domenic Feb 29, 2016

annevk Feb 29, 2016

domenic Feb 29, 2016

annevk Feb 29, 2016

xidachen Feb 29, 2016

domenic Feb 29, 2016

domenic commented Feb 29, 2016

annevk commented Mar 1, 2016

xidachen commented Mar 1, 2016

Add ImageBitmapOptions to ImageBitmap's spec #741

Add ImageBitmapOptions to ImageBitmap's spec #741

Conversation

xidachen commented Feb 25, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

domenic commented Feb 26, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

domenic commented Feb 26, 2016

domenic commented Feb 26, 2016

xidachen commented Feb 28, 2016

annevk commented Feb 28, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

annevk commented Feb 28, 2016

annevk commented Feb 29, 2016

xidachen commented Feb 29, 2016

annevk commented Feb 29, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

domenic commented Feb 29, 2016

annevk commented Mar 1, 2016

xidachen commented Mar 1, 2016