Add the OffscreenCanvas interface and 2d context #1876

junov · 2016-10-07T21:24:13Z

This change defines the OffscreenCanvas and
OffscreenCanvasRenderingContext2d interfaces, based on the feature
proposal found here: https://wiki.whatwg.org/wiki/OffscreenCanvas

junov · 2016-10-07T21:28:31Z

Please take a look @mephisto41, @kenrussell, @grorg

kenrussell

Very nice. Looks excellent to me overall. Two very minor points.

kenrussell · 2016-10-07T21:34:59Z

source


    <p>Returns null if the given context ID is not supported, if the canvas has already been
-    initialized with the other context type (e.g. trying to get a "<code
+    initialized with an other context type (e.g. trying to get a "<code


an other -> another?

kenrussell · 2016-10-07T21:38:57Z

source

+    context that is created from a <code>canvas</code> element. There is also a specification that defines a "<code data-x="canvas-context-webgl">webgl</code>" context. <ref spec=WEBGL></p>
+
+    <p>Returns null if the given context ID is not supported, if the canvas has already been
+    initialized with the other context type (e.g. trying to get a "<code


the other -> another

xidachen · 2016-10-12T02:05:03Z

source

+  unrestricted double quality = 1.0;
+};
+
+[Constructor(unsigned long width, unsigned long height), Exposed=(Window,Worker)]


Should we have [EnforceRange] for both width and height. I was really hoping to minimize ambiguity in the future.

That sounds reasonable to me. The modulo conversions dictated by WebIDL will not give anything useful here, so it might as well throw.

xidachen · 2016-10-12T02:09:30Z

source

+    <p>Returns an object that exposes an API for drawing on the offscreen canvas. The first
+    argument specifies the desired API, either "<code
+    data-x="offscreencanvas-context-2d">2d</code>", or "<code
+    data-x="canvas-context-webgl">webgl</code>". Subsequent arguments are handled by that


Could it be webgl2?

Now that I read more, I realized that we just use webgl to refer to both. So please ignore my previous comment in here.

webgl2 is still a valid context type that should have its own line in the getContext table. My thinking was that we could add it in a separate change, at the same time as we add it to HTMLCanvasElement's version of getContext.

annevk

A whole lot of style nits basically. I don't think I know enough to critique the algorithms. As for the style nits, please go through the entire diff for each of them, I didn't point them all out individually. As a heads up, probably requires another style review after these are addressed. Hope that's okay.

(Also, very exciting to see workers finally getting some graphics.)

annevk · 2016-10-10T10:49:53Z

source


+  <p>When its <span data-x="concept-canvas-context-mode">canvas context mode</span> is <span
+  data-x="concept-canvas-placeholder">placeholder</span>, a <code>canvas</code> element has no
+  rendering context. It serves as a placeholder for an OffscreenCanvas object, and the content of


Here it needs <code>.

annevk · 2016-10-12T08:41:07Z

source


  USVString <span data-x="dom-canvas-toDataURL">toDataURL</span>(optional DOMString type, optional any quality);
  void <span data-x="dom-canvas-toBlob">toBlob</span>(<span>BlobCallback</span> _callback, optional DOMString type, optional any quality);
+  OffscreenCanvas <span data-x="dom-canvas-transferControlToOffscreen">transferControlToOffscreen</span>();


Here OffScreenCanvas needs to be wrapped in <span>.

annevk · 2016-10-12T08:41:59Z

source

+
+   <dd>
+
+    <p>Returns a newly created OffscreenCanvas object that uses this canvas element as a


<code> for OffscreenCanvas and canvas.

You should basically go through the entire diff, since it seems to have happened quite often.

annevk · 2016-10-12T08:43:49Z

source

+   <li><p>Create a new OffscreenCanvas object with its width and height equal to the
+   values of the <code data-x="attr-canvas-width">width</code> and <code
+   data-x="attr-canvas-height">height</code> content attributes of this <code>canvas</code>
+   element.</p></li>


I would prefer it if you assign the create result to a variable and use that in the remainder of these steps.

annevk · 2016-10-12T08:45:37Z

source

+  attribute unsigned long width;
+  attribute unsigned long height;
+
+  OffscreenRenderingContext? <code data-x="offscreencanvas-getContext">getContext</code>(DOMString contextId, any... arguments);


Needs <span>. Can we improve upon getContext() here by making it take an enum and a more restricted set of arguments or is that a lost cause?

Can we improve upon getContext() here by making it take an enum and a more restricted set of arguments or is that a lost cause?

We could make contextId it an enum. This means that passing an unrecognized context Id will result in an exception being thrown instead of the function returning null, as is currently the case with . I am not sure how to weigh cleanliness of an enum vs. breaking the mold of HTMLCanvasElement.getContext.

For the "arguments" argument we are using dictionaries for all context types, but each context type has its own dictionary definition. For example the WebGL spec defines creation options that have no meaning for 2D contexts. I would like to use a dictionary for the IDL here but I am not sure what would be the right pattern for spec'ing this correctly. Perhaps we could have a union typedef? That's a bit weird though because the bindings cannot determine which dictionary type an associative array should be mapped to (it depends on contextId). Another option would be to have a shared dictionary type that contains all creation options used by all context types. But that would force different context types that have options with the same name to use the same types for those options. The shared dictionary approach is actually what we did for Blink's internal IDL. But how would that work in the spec? We could define a shared dictionary in this spec and put a partial dictionary definition in the WebGL spec in order to add WebGL-specific members to the shared dictionary. Does that sound reasonable?

That sounds fine. It's an implicit requirement that all context types have to have the same default set of values for the common entries in this dictionary. For example, the "alpha" dictionary entry used by the 2D and WebGL contexts here:

https://html.spec.whatwg.org/multipage/scripting.html#2dcontext:dom-canvas-getcontext
https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.2

has to be a boolean in both, and default to true. This is needed in order to avoid custom binding code for HTMLCanvasElement's getContext method. Web IDL doesn't allow specializing the behavior of the "arguments" dictionary based on which contextId was passed. (All of the custom binding code for this method was removed in Blink some time ago.)

Changing to a shared dictionary was previously discussed in #595 (comment) - maybe continue discussion there? @smaug---- did not seem to like the merged dictionary idea there.

annevk · 2016-10-12T08:51:29Z

source

+   data-x="offscreencanvas-context-mode">context mode</span> is not equal to <span
+   data-x="offscreencanvas-context-none">none</span>, throw an
+   <span>"<code>InvalidStateError</code>"</span> <code>DOMException</code>, and abort these
+   steps.</p></li>


No need for abort these steps. Throwing does that.

annevk · 2016-10-12T08:53:29Z

source

+
+  <p class="tablenote"><small>* Vendors may define experimental contexts using the syntax <code
+  data-x=""><var>vendorname</var>-<var>context</var></code>, for example, <code
+  data-x="">moz-3d</code>.</small></p>


If we still have this text we should probably remove it. I think the whole industry has accepted that prefixes are bad now.

annevk · 2016-10-12T08:54:22Z

source

+   data-x="offscreencanvas-transferToImageBitmap">transferToImageBitmap</code>()</dt>
+
+   <dd>
+    <p>Returns a newly created ImageBitmap object with the image in the OffscreenCanvas.


Lacks <code> for two objects (though again, I don't think I have noted them all). OffscreenCanvas should also be followed by " object" consistently.

annevk · 2016-10-12T08:54:52Z

source

+  <p>The <dfn><code
+  data-x="offscreencanvas-convertToBlob">convertToBlob(<var>options</var>)</code></dfn> method,
+  when invoked, must run the following steps:</p>
+  <ol>


Newline before <ol>.

annevk · 2016-10-12T08:56:02Z

source

+
+  <pre class="idl">interface <dfn>OffscreenCanvasRenderingContext2D</dfn> {
+  void commit();
+  readonly attribute OffscreenCanvas <dfn data-x="offscreencontext2d-canvas">canvas</dfn>;


Also the IDL below needs lots of <span> love.

Including the oddly specific language for <canvas>’s getContext() feature that was added for Opera’s extensions back in the day if I remember correctly. Noticed we still had this while reviewing #1876.

junov · 2016-10-12T14:06:28Z

Since mozilla's implementation is nearly complete, adding "do not merge yet" label until this PR is checked against that implementation and that any compatibility issues are either challenged and corrected this PR review, or resolved in the implementation.

junov · 2016-10-12T20:37:56Z

Just realized that OffscreenCanvas needs to be an EventTarget so that WebGL contexts can dispatch context lost/restored events. Any objections to that?

junov · 2016-10-14T15:05:08Z

I have received feedback from Brandon Jones (co-editor of the WebVR spec) and from the Google maps team regarding the lack of ability to resize canvases. Though this may not be a deal breaker for 2D canvases (you can just create a new canvas with the new size and discard the old one), for WebGL it is another story. Recreating canvases for the purpose of a resize is prohibitively expensive for WebGL applications because it means that all the resources (e.g. textures, shaders, etc.) would need to be recreated in a new context, which is not acceptable for many use cases. I intend to make that change in this PR.

annevk · 2016-10-14T15:08:24Z

@junov can we do resizing separately or is that specifically about OffscreenCanvas? This is already a pretty big PR, if we can divide things up that'd be nice.

junov · 2016-10-14T15:16:57Z

That is a good idea, there might be quite a bit of complexity in spec'ing the resizing behavior.
The current PR has text that specifically states that resizing is not allowed. I will at least remove that from this PR.

junov · 2016-11-11T19:42:54Z

Applied all the fixes requested above except for:

resizing: will do it in a follow-up PR
refactoring the getContext dictionary argument. As pointed-out by @zcorpan there are objections to this in another thread. Keeping this for future consideration.

Noteworthy: I did make getContext use an enum instead of a string for the context type.

domenic · 2016-11-11T19:45:23Z

Noteworthy: I did make getContext use an enum instead of a string for the context type.

That's a normative change that makes it start throwing when "asdf" is passed in.

junov · 2016-11-11T19:56:53Z

That's a normative change that makes it start throwing when "asdf" is passed in.

Correct. I did not touch the existing HTMLCanvasElement.getContext to avoid any compat issues.
I only did the enum thing for the new OffscreenCanvas.getContext.

domenic

Lots of nits, but I'm very excited for this!

domenic · 2016-11-15T00:21:00Z

source

  must run the steps in the cell of the following table whose column header describes the
  <code>canvas</code> element's <span data-x="concept-canvas-context-mode">canvas context
-  mode</span> and whose row header describes the method's first argument.</p>
+  mode</span> and whose row header describes the method's first argument. Except when the <span


I'd rather just update the table to have a new column? It seems like a nice property that all the different modes are represented by columns in the table.

domenic · 2016-11-15T00:21:32Z

source

+   data-x="dom-canvas-transferControlToOffscreen">transferControlToOffscreen</code>()</dt>
+
+   <dd>
+    <p>Returns a newly created <code>OffscreenCanvas</code> object that uses <b>this</b>


"the canvas, not "this canvas", in this context

domenic · 2016-11-15T00:22:05Z

source

+    <p>Returns a newly created <code>OffscreenCanvas</code> object that uses <b>this</b>
+    <code>canvas</code> element as a placeholder. Once a canvas element has become a placeholder
+    for an <code>OffscreenCanvas</code> object, its intrinsic size can no longer be changed, and it
+    can not have a rendering context. The content of the placeholder canvas is updated by calling


s/can not/cannot/

domenic · 2016-11-15T00:22:15Z

source

+
+   <dd>
+    <p>Returns a newly created <code>OffscreenCanvas</code> object that uses <b>this</b>
+    <code>canvas</code> element as a placeholder. Once a canvas element has become a placeholder


s/a canvas element/the canvas/

domenic · 2016-11-15T00:22:32Z

source

+    <code>canvas</code> element as a placeholder. Once a canvas element has become a placeholder
+    for an <code>OffscreenCanvas</code> object, its intrinsic size can no longer be changed, and it
+    can not have a rendering context. The content of the placeholder canvas is updated by calling
+    the <code data-x="offscreencontext-commit">commit</code> method of the


commit() as the link text

domenic · 2016-11-15T00:49:27Z

source

+
+  </ol>
+
+  <p>The <dfn><code data-x="offscreencontext2d-commit">commit</code></dfn> method, when invoked, must


domenic · 2016-11-15T00:50:50Z

source

+
+  <pre class="idl">interface <dfn>OffscreenCanvasRenderingContext2D</dfn> {
+  void commit();
+  readonly attribute <span>OffscreenCanvas</span> <dfn data-x="offscreencontext2d-canvas">canvas</dfn>;


This should be a <span> not a <dfn>; see below for how to define it.

domenic · 2016-11-15T00:51:52Z

source

+
+   <li><p>Create a new <code>OffscreenCanvasRenderingContext2D</code> object.</p></li>
+
+   <li><p>Initialize its <code data-x="offscreencontext2d-canvas">canvas</code> attribute to point to


It would be better to make "associated OffscreenCanvas" something that each OCRC2D has, like it has a bitmap and alpha flag. Then you define the canvas attribute, on getting, to return this OCRC2D's associated OffscreenCanvas, and in this algorithm, you set the associated OffscreenCanvas. This also makes it so that commit()'s algorithm can simply say "this OCRC2D's associated OffscreenCanvas" instead of "the OffscreenCanvas from which this context was created"

domenic · 2016-11-15T00:52:34Z

source

+  where the <code data-x="offscreencontext2d-commit">commit</code> method is invioked from a Worker
+  and the <span>event loop</span> of the <span
+  data-x="offscreencanvas-placeholder">placeholder</span> <code>canvas</code> element's
+  <span>browsing context</span> is busy. However, such shortcuts must not have any\


Remove stray \

domenic · 2016-11-15T00:53:02Z

source

+  <span>browsing context</span> is busy. However, such shortcuts must not have any\
+  script-observable side-effects. This means that the committed bitmap still needs to be sent to
+  the <span data-x="offscreencanvas-placeholder">placeholder</span> <code>canvas</code> element, in
+  case the element is used as a <code>CanvasImageSource</code>, or in case toDataURL or toBlob are


Link toDataURL() and toBlob(), and add parens

This cahnge defines the OffscreenCanvas and OffscreenCanvasRenderingContext2d interfaces, based on the feature proposal found here: https://wiki.whatwg.org/wiki/OffscreenCanvas

junov · 2016-11-15T19:07:48Z

Changes requested by @domenic: done.

domenic · 2016-11-15T19:48:45Z

Pushed some more review tweaks. The only remaining thing I noticed is, what if converting to a blob fails? That possibility doesn't seem to be accounted for. I was thinking of adding a sentence like "If the serialization process fails for any reason, reject result with ..." to the end of step 6. But I wasn't sure what a good DOMException would be. Maybe "EncodingError"? Or maybe it just never fails?

domenic · 2016-11-15T19:53:19Z

Oh. Are OffscreenCanvas's width and height supposed to be mutable? If so we need to define algorithms for their setters. Maybe that is the resizing thing mentioned above? Or maybe they should just be made readonly.

junov · 2016-11-15T20:01:18Z

For blob failures: should we reject with what ever exception was thrown during serialization?

Regarding the setters for width and height. That was brought up earlier in this thread. @annevk suggested we take care of defining the resizing behavior in a follow-up PR to avoid adding more weight to this one, which is already quite big.

domenic · 2016-11-15T20:02:51Z

For blob failures: should we reject with what ever exception was thrown during serialization?

Sure, but what exception would that be? I mean, the C++ code will throw an exception, but what JS-facing exception would result?

Regarding the setters for width and height. That was brought up earlier in this thread. @annevk suggested we take care of defining the resizing behavior in a follow-up PR to avoid adding more weight to this one, which is already quite big.

That seems reasonable. Can we mark them readonly for now then?

junov · 2016-11-15T20:05:05Z

tweaks lgtm

junov · 2016-11-15T20:13:39Z

Can we mark them readonly for now then?

sure.

Sure, but what exception would that be?

After taking a closer look, I don't think the encode can fail other than an out-of-memory error, which typically crashes the browser. For ArrayBuffers, we throw a RangeError exception when allocation fails do we want to offer that here?

domenic · 2016-11-15T20:15:42Z

If there's no reason it can fail besides OOM, I'm OK leaving that for a separate discussion/PR. We'd probably want to change toBlob() as well in some way (although it doesn't have an error channel so I'm not sure what we'd do there).

junov · 2016-11-15T20:53:13Z

Summoning @mephisto41

mephisto41 · 2016-11-16T02:31:23Z

Sorry for late reply. I think this is very good. I'm very excited to see this becomes spec. Thanks for it.

annevk · 2016-11-16T07:44:12Z

source

+  data-x="concept-canvas-placeholder">placeholder</span>, a <code>canvas</code> element has no
+  rendering context. It serves as a placeholder for an <code>OffscreenCanvas</code> object, and
+  the content of the <code>canvas</code> element is updated by calling the <code
+  data-x="offscreencontext-commit">commit()</code> method of the <code>OffscreenCanvas</code>


This should be data-x="dom-OffscreenContext-commit" (and below).

annevk · 2016-11-16T07:46:30Z

source

+
+[<span data-x="dom-OffscreenCanvas">Constructor</span>([EnforceRange] unsigned long long width, [EnforceRange] unsigned long long height), Exposed=(Window,Worker)]
+interface <dfn>OffscreenCanvas</dfn> : <span>EventTarget</span> {
+  readonly attribute unsigned long long <span data-x="offscreencanvas-width">width</span>;


dom-OffscreenCanvas-width etc.

junov · 2016-11-16T15:08:23Z

@annevk out of curiosity, what exactly does the "dom-" prefix mean? I assumed we used it for interfaces related to the dom, but that is clearly not the case.

junov · 2016-11-16T15:24:39Z

Done. I put the "dom-OffscreenCanvas" prefix for the x-ref labels of things that are symbols in the OffscreenCanvas interface.

annevk · 2016-11-16T15:32:38Z

@junov it's a bit of a historical convention at this point, but since it also dictates the IDs these definitions end up having it's nice to be consistent so that if you know the interface and its members you can URL hack your way to the definitions quite easily.

junov · 2016-11-16T16:18:00Z

\o/

xidachen · 2016-11-16T16:18:38Z

Yay!

junov added the topic: canvas label Oct 7, 2016

kenrussell reviewed Oct 7, 2016

View reviewed changes

domenic added the addition/proposal New features or enhancements label Oct 10, 2016

xidachen reviewed Oct 12, 2016

View reviewed changes

annevk requested changes Oct 12, 2016

View reviewed changes

annevk mentioned this pull request Oct 12, 2016

Remove old-fashioned vendor-specific extension advice #1889

Merged

junov added the do not merge yet Pull request must not be merged per rationale in comment label Oct 12, 2016

bhack mentioned this pull request Oct 15, 2016

OffscreenCanvas transcranial/keras-js#3

Closed

travisleithead mentioned this pull request Nov 1, 2016

Review OffscreenCanvas w3ctag/design-reviews#144

Closed

junov removed the do not merge yet Pull request must not be merged per rationale in comment label Nov 11, 2016

domenic requested changes Nov 15, 2016

View reviewed changes

junov added 6 commits November 15, 2016 09:41

Add the OffscreenCanvas interface and 2d context

3237fe6

This cahnge defines the OffscreenCanvas and OffscreenCanvasRenderingContext2d interfaces, based on the feature proposal found here: https://wiki.whatwg.org/wiki/OffscreenCanvas

Applying editorial changes suggested by @kenrussell

e442db0

Addressed annevk's feedback

d771542

Made OffscreenCanvas inherrit EventTarget

b3f481b

fixed markup errors

0fce37d

Addressed feedback by @domenic

c6a32f5

Added data-x for getContext alpha setting

c2d07b1

domenic added 2 commits November 15, 2016 14:45

More minor review tweaks

29f39c9

Cross-link the constructor too

1ee3cfa

domenic approved these changes Nov 15, 2016

View reviewed changes

Make width and height attributes readonly

ff43f3e

annevk reviewed Nov 16, 2016

View reviewed changes

Change some data-x labels to use "dom-OffscreenCanvas" prefix

d6743ab

domenic assigned annevk Nov 16, 2016

annevk approved these changes Nov 16, 2016

View reviewed changes

domenic merged commit aedc72d into whatwg:master Nov 16, 2016

xidachen mentioned this pull request Nov 22, 2016

Modify some tests under 2dcontext web-platform-tests/wpt#4234

Merged

junov deleted the oc branch July 7, 2022 16:05


		<dd>

		<p>Returns a newly created OffscreenCanvas object that uses this canvas element as a


		</ol>

		<p>The <dfn><code data-x="offscreencontext2d-commit">commit</code></dfn> method, when invoked, must


		<li><p>Create a new <code>OffscreenCanvasRenderingContext2D</code> object.</p></li>

		<li><p>Initialize its <code data-x="offscreencontext2d-canvas">canvas</code> attribute to point to

Add the OffscreenCanvas interface and 2d context #1876

Add the OffscreenCanvas interface and 2d context #1876

Uh oh!

Conversation

junov commented Oct 7, 2016

Uh oh!

junov commented Oct 7, 2016

Uh oh!

kenrussell left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

annevk left a comment

Choose a reason for hiding this comment

Uh oh!

annevk Oct 10, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

annevk Oct 12, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

annevk Oct 12, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

junov commented Oct 12, 2016

Uh oh!

junov commented Oct 12, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

junov commented Oct 14, 2016

Uh oh!

annevk commented Oct 14, 2016

Uh oh!

junov commented Oct 14, 2016

Uh oh!

junov commented Nov 11, 2016

annevk Oct 10, 2016 •

edited

Loading

annevk Oct 12, 2016 •

edited

Loading

annevk Oct 12, 2016 •

edited

Loading

junov commented Oct 12, 2016 •

edited

Loading