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

Combine createImageBitmap() / createPattern() / drawImage() checks #3399

Closed
annevk opened this issue Jan 24, 2018 · 4 comments
Closed

Combine createImageBitmap() / createPattern() / drawImage() checks #3399

annevk opened this issue Jan 24, 2018 · 4 comments
Labels
clarification Standard could be clearer topic: canvas

Comments

@annevk
Copy link
Member

annevk commented Jan 24, 2018

createPattern()/drawImage() have these checks:

  1. If image is an HTMLOrSVGImageElement object whose current request's state is broken, then throw an "InvalidStateError" DOMException.
  2. If image is an HTMLOrSVGImageElement object that is not fully decodable, or if image is an HTMLVideoElement object whose readyState attribute is either HAVE_NOTHING or HAVE_METADATA, then return bad.
  3. If image is an HTMLOrSVGImageElement object with an intrinsic width or intrinsic height (or both) equal to zero, then return bad.
  4. If image is an HTMLCanvasElement object with either a horizontal dimension or a vertical dimension equal to zero, then return bad.
  5. If image is an ImageBitmap object with its [[Detached]] internal slot value set to true, then throw an "InvalidStateError" DOMException.

createImageBitmap() does not have 1, but always throws for 2 (which is a superset). 3 however is seemingly incompatible as createImageBitmap() only throws if sw/sh are not given. 4/5 are compatible.

@junov thoughts? Does the implementation look as messy as the specification setup? (And also, there are many checks that createImageBitmap() has that are not present in the other two methods. Is that intentional?)

@annevk annevk added clarification Standard could be clearer topic: canvas labels Jan 24, 2018
@junov
Copy link
Member

junov commented Jan 24, 2018

One of the differences is that createImageBitmap is async and drawImage is not. So with creatImageBitmap there is an opportunity to handle objects that are not yet loaded (meaning 2. could be revised). In the case of creatImageBitmap, it is conceivable that 3 may not be verifyable synchronously.

The implementation is not that messy because it uses a virtual interfaces through which each object type knows how to check whether it is in a usable state.

@annevk
Copy link
Member Author

annevk commented Jan 25, 2018

Well, createImageBitmap() does all these checks synchronously, well before it goes "in parallel". Arguably we could change it, but I'm mostly curious if we could get to a place that's closer to how implementations work, with shared checks.

@junov
Copy link
Member

junov commented Jan 25, 2018

Right. I see two ways:

  1. We could have a shared "check whether a CanvasImageSource is valid" algorithm.
  2. A polymorphic design: We could state that all members of the CanvasImageSource union must have a [[getBitmap]] internal method, which returns a bitmap and may throw an exception if a bitmap could not be obtained.

I think 2) is more flexible. It it makes it easy to spec one-off behaviors if necessary:

if <var>image</var> is an XYZ:
        do something special
Otherwise:
         let <var>bitmap</var> be the result of calling <var>image</var>'s [[getBitmap]] internal method.

@annevk
Copy link
Member Author

annevk commented Jan 26, 2018

I ran a test with images/red-zerowidth.svg from web-platform-tests and it seems browsers throw anyway if the source image width (or height) is 0 so we can probably postpone 2 for now and just reuse the "usable" algorithm.

annevk added a commit to web-platform-tests/wpt that referenced this issue Jan 26, 2018
annevk added a commit that referenced this issue Jan 26, 2018
In particular, with createPattern() and drawImage().

Tests: web-platform-tests/wpt#9207.

Fixes #3399.
annevk added a commit to web-platform-tests/wpt that referenced this issue Jan 26, 2018
@annevk annevk closed this as completed in 7458477 Jan 29, 2018
alice pushed a commit to alice/html that referenced this issue Jan 8, 2019
In particular, with createPattern() and drawImage().

Tests: web-platform-tests/wpt#9207.

Fixes whatwg#3399.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clarification Standard could be clearer topic: canvas
Development

No branches or pull requests

2 participants