Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add new canvas rendering context: ImageBitmapRenderingContext #1028

Closed
wants to merge 1 commit into from
Closed

Add new canvas rendering context: ImageBitmapRenderingContext #1028

wants to merge 1 commit into from

Conversation

junov
Copy link
Member

@junov junov commented Apr 11, 2016

This change introduces the ImageBitmapRenderingContext interface.
This is part of the larger OffscreenCanvas feature proposal found here:
https://wiki.whatwg.org/wiki/OffscreenCanvas
The documentation for the implementation by Mozilla, which is
experimental at this time, can be found here:
https://developer.mozilla.org/en-US/docs/Web/API/ImageBitmapRenderingContext

@junov
Copy link
Member Author

junov commented Apr 11, 2016

Summoning @mephisto41 who implemented this in Gecko. Other than the sizing behavior, is there anything in the PR that does not match the current implementation?

<th>"<dfn><code data-x="canvas-context-bitmaprenderer">bitmaprenderer</code></dfn>"
<td>

Follow the <span>image bitmap context creation algorithm</span> defined in the section
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not xlinking to anything. (Yes, it sucks that the build tools fail to notify for this.)

@domenic
Copy link
Member

domenic commented Apr 11, 2016

Really nice clean slice of OffscreenCanvas to introduce. Lots of editorial tweaks but overall solid.

Do you think it would benefit developers to have an example of this in use, in order to show how this renders the ImageBitmap into a canvas without doubling memory usage, as noted in https://www.w3.org/Bugs/Public/show_bug.cgi?id=25647 ? I would put it at the end, after the w-nodev section ends.

@mephisto41
Copy link

Sorry for late reply. I think the spec looks good.

@junov
Copy link
Member Author

junov commented Apr 12, 2016

New commit. Addressed all comments made up to now.

Do you think it would benefit developers to have an example of this in use, in order to show how this renders the ImageBitmap into a canvas without doubling memory usage, as noted in https://www.w3.org/Bugs/Public/show_bug.cgi?id=25647 ?

I was thinking of adding an example once the the spec for OffscreenCanvas is in place, to show the two interfaces working together with ImageBitmaps being passed from Worker to main thread for display. However, I agree with the more general underlying point you are making that we should give more context to developers, so I added a brief intro paragraph between the section title and the IDL.

@junov
Copy link
Member Author

junov commented Apr 12, 2016

New commit:

  • Fixed missing </p>
  • Added a note to instruct implementers to elide the composite over black step (when alpha flag is false) whenever possible.

@@ -59795,7 +59795,8 @@ callback <dfn>BlobCallback</dfn> = void (<span>Blob</span>? blob);</pre>
what kind of rendering context it is, a <code>canvas</code> also has a <dfn
data-x="concept-canvas-context-mode">canvas context mode</dfn>, which is initially <dfn
data-x="concept-canvas-none">none</dfn> but can be changed to either <dfn
data-x="concept-canvas-2d">2d</dfn> or <dfn
data-x="concept-canvas-2d">2d</dfn>, <dfn
data-x="concept-canvas-bitmaprenderer">bitmaprenderer</dfn> or <dfn
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: comma after bitmaprenderer</dfn>

<ol>

<li><p>If the algorithm was passed some arguments, let <var>arg</var> be the first such
argument. Otherwise, let <var>arg</var> be <i>undefined</i>.</p></li>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No <i> around undefined

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This sentence was copy-pasted from the 2d context creation algorithm. Will remove <i> there as well.

@domenic
Copy link
Member

domenic commented Apr 12, 2016

Looks good, those last few nits should do it from me. I'll merge after they're addressed. Nice intro paragraph.

This change introduces the ImageBitmapRenderingContext interface.
This is part of the larger OffscreenCanvas feature proposal found here:
https://wiki.whatwg.org/wiki/OffscreenCanvas
The documentation for the implementation by Mozilla, which is
experimental at this time, can be found here:
https://developer.mozilla.org/en-US/docs/Web/API/ImageBitmapRenderingContext
@junov
Copy link
Member Author

junov commented Apr 12, 2016

Done.

domenic pushed a commit that referenced this pull request Apr 12, 2016
This change introduces the ImageBitmapRenderingContext interface.
This is part of the larger OffscreenCanvas feature proposal found here:
https://wiki.whatwg.org/wiki/OffscreenCanvas
The documentation for the implementation by Mozilla, which is
experimental at this time, can be found here:
https://developer.mozilla.org/en-US/docs/Web/API/ImageBitmapRenderingContext

PR: #1028
@domenic
Copy link
Member

domenic commented Apr 12, 2016

Awesome!! Found a couple more un-<code>ed ImageBitmapRenderingContexts, and merged as 4c59797.

@domenic domenic closed this Apr 12, 2016
@junov
Copy link
Member Author

junov commented Apr 12, 2016

Yay! Thanks for the responsive reviewing!

annevk added a commit that referenced this pull request Jan 16, 2018
It needs to list ImageBitmapRenderingContext.

Missed in #1028.
annevk added a commit that referenced this pull request Jan 17, 2018
It needs to list ImageBitmapRenderingContext.

Missed in #1028.
alice pushed a commit to alice/html that referenced this pull request Jan 8, 2019
It needs to list ImageBitmapRenderingContext.

Missed in whatwg#1028.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

4 participants