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

Should createImageBitmap support vector formats? #923

Closed
jakearchibald opened this issue Mar 22, 2016 · 28 comments
Closed

Should createImageBitmap support vector formats? #923

jakearchibald opened this issue Mar 22, 2016 · 28 comments
Labels
addition/proposal New features or enhancements

Comments

@jakearchibald
Copy link
Contributor

Step 3 of https://html.spec.whatwg.org/multipage/webappapis.html#dom-createimagebitmap for <img> specifically rejects on vector data, but it doesn't reject for blobs. There's a vague "If the image data is not in a supported file format" step for blobs, is this supposed to reject on vectors?

See https://jakearchibald.github.io/svgomg/ - as a developer, I'd like to control the rasterisation of SVG data using createImageBitmap to allow for smooth zooming and panning but lazy re-rendering at the target dimension (and cropped area).

@annevk
Copy link
Member

annevk commented Mar 22, 2016

See #921.

@junov
Copy link
Member

junov commented Mar 22, 2016

I think #921 is a short term fix to clarify the spec, while this issue is an actual feature request.
To support the creation of ImageBitmaps from SVG media, we need to define either or both of:
a) A sizing algorithm to determine the scale of the vector graphics.
b) A version of createImageBitmap that takes size parameters (not to be confused with crop) to determine SVG rasterization scale explicitly

It should be noted that the sizing algorithm that is used when drawing SVG with 2D canvas's drawImage() cannot be use here because createImageBitmap is not tied to a canvas element that can be used as a reference for sizing.

domenic pushed a commit that referenced this issue Mar 22, 2016
This change makes it explicit that creating an ImageBitmap from a Blob
only supports *bitmap* file formats. This behaviour is consistent with
creating an ImageBitmap from an img element, which explicitly does not
support vector graphics.

Note: Supporting vector graphics would require defining a sizing
algorithm to determine the scale of image, or to have an alternate API
that has additional size parameters. See feature request in #923.

PR: #921
@domenic domenic added the addition/proposal New features or enhancements label Mar 22, 2016
@jakearchibald
Copy link
Contributor Author

a) A sizing algorithm to determine the scale of the vector graphics.
b) A version of createImageBitmap that takes size parameters (not to be confused with crop) to determine SVG rasterization scale explicitly

I agree we should have this, but if an SVG has width & height attributes set then we don't need either.

@jakearchibald
Copy link
Contributor Author

#865 proposes the size params

@junov
Copy link
Member

junov commented Mar 22, 2016

In the options proposal (https://wiki.whatwg.org/wiki/ImageBitmap_Options) we had resizedWidth/resizedHeight. Perhaps we should just drop the "resized" prefix, which does not make much sense in the case of SVG. The width and height option would apply to both svg and bitmap scaling. When calling createImageBitmap with an svg source, I suppose the behavior would be to throw an exception if width/height are not specified? Also, svg images do have an intrinsic aspect ratio, so we could get away with specifying just width or height, and the other dimension could be inferred. In the case where both width and height are specified, we'd need an additional option to specify how differences in aspect ratio are handled. Something like the fill, contain and cover modes of the object-fit css property.

WDYT?

@junov
Copy link
Member

junov commented Mar 22, 2016

@jakearchibald: regarding your comment about using the width & height attributes, I assuming you are referring to the image element's, computed size? I would like to avoid depending on that unless absolutely necessary. Two reasons : It is problematic for elements that are not attached to the active DOM because their style and layout is not evaluated, and it would make createImageBitmap dependent on style computation (may induce style recalc).

@jakearchibald
Copy link
Contributor Author

It's not style dependant, it's width and height attributes on the SVG element itself. This is used to determine the <img> size if it isn't otherwise set.

@jakearchibald
Copy link
Contributor Author

If the SVG has no defined size and no width/height set in createImageBitmap, happy with rejecting.

Also agree with width/height rather than "resize".

@junov
Copy link
Member

junov commented Mar 23, 2016

So you were referring to the width and height content attributes then? That seems logical, except that the spec says that those attributes "must be omitted if the resource in question does not have both an intrinsic width and an intrinsic height".

See: https://html.spec.whatwg.org/multipage/embedded-content.html#attr-dim-width

@jakearchibald
Copy link
Contributor Author

I'm guessing there must be something that overrides that. See http://jsbin.com/lijana/edit?html,output - the IMG here has no size specified, but it gets its dimensions from the SVG element's width and height attributes.

@jakearchibald
Copy link
Contributor Author

https://svgwg.org/svg2-draft/coords.html#IntrinsicSizing - intrinsic sizing of SVG

@junov
Copy link
Member

junov commented Mar 23, 2016

This is interesting. In your jsbin, I tried adding width="100" to the img tag, and it worked, it shrunk the car. So it seems that svg image are treated as having an intrinsic size for the purposes of this section of the spec: https://html.spec.whatwg.org/multipage/embedded-content.html#attr-dim-width

This is uncomfortably inconsistent with the behavior of CanvasRenderingContext2D.drawImage
Summoning @doomdavve and @progers

@progers
Copy link

progers commented Mar 23, 2016

@doomdavve, you're our only hope.

@tabatkins
Copy link
Contributor

Yeah, the intrinsic sizes of an are computed from the width/height/aspectRatio attributes on its root. This gets fed into the sizing algorithms, exactly like the intrinsic sizes of raster images.

In the case that those don't exist, CSS has a well-defined algorithm that all image-y things share to determine the concrete width/height of the resource: https://drafts.csswg.org/css-images/#concrete-size-resolution. You just need to define the default object size for this case and invoke the algorithm; the output is a width and height for you to use.

From HTML:

The two attributes must be omitted if the resource in question does not have both an intrinsic width and an intrinsic height.

Wtf is this talking about. This makes absolutely no sense. I'll file a separate bug to have this line removed, it's nonsense.

@doomdavve
Copy link

I agree with all what Tab writes. And just to tie it together, https://drafts.csswg.org/css-images/#concrete-size-resolution is what CanvasRenderingContext2D.drawImage() uses to determine the width and height, with no specified size and the canvas size as the default object size.

https://html.spec.whatwg.org/multipage/scripting.html#drawing-images

I'm a bit confused about this discussion. @jakearchibald seems to be talking about the width and height attributes of the <svg>, the intrinsic size, while @junov seems to be talking about the presentation attributes of <img> which do not affect the intrinsic size. In practice, they instead map to the CSS width and height properties.

This is interesting. In your jsbin, I tried adding width="100" to the img tag, and it worked, it shrunk the car. So it seems that svg image are treated as having an intrinsic size for the purposes of this section of the spec: https://html.spec.whatwg.org/multipage/embedded-content.html#attr-dim-width

Yes, adding width="100", a presentation attribute for <img>, changes the specified width and CSS uses intrinsic aspect ratio to compute the used height.

This is uncomfortably inconsistent with the behavior of CanvasRenderingContext2D.drawImage

@junov Could you clarify the inconsistency? CanvasRenderingContext2D.drawImage() doesn't have any specified size constraints but should otherwise be quite analogous to the <img> case, AFAIK.

@junov
Copy link
Member

junov commented Mar 24, 2016

Here is what I am confused about:
An <img> that refers to an svg file behaves as if the image has an intrinsic size, given by the width and height of the svg file's <svg> tag.
Use that same <img> as the image argument of a drawImage() call, and it behaves as if the image has no intrinsic size: the concrete size algorithm falls back to using the default object size, which is the size of the destination canvas + contains constraint.

@tabatkins
Copy link
Contributor

Why do you say that? The behavior of drawing an <img src=foo.svg> should be the same as <img src=foo.jpg> - the intrinsic dimensions are taken from the resource. What makes you think they're different?

@junov
Copy link
Member

junov commented Mar 24, 2016

@tabatkins
Copy link
Contributor

I don't feel like digging thru a patch right now, so I'll let you summarize what it does. Are you saying that that code treats all SVG images as having no intrinsic dimensions? If so, it's buggy. width/height/aspectRatio on the root <svg> define the image's intrinsic dimensions.

@junov
Copy link
Member

junov commented Mar 24, 2016

My understanding is that it considers svg images as having intrinsic aspect ratio but no intrinsic size. @doomdavve ?

@doomdavve
Copy link

It certainly considers the intrinsic size of SVG images. The thing that particular patch does is to pipe through the canvas size as default object size to use as fallback size in case the SVG image doesn't have an intrinsic size.

@junov
Copy link
Member

junov commented Mar 31, 2016

Okay, I got it. Thanks for enlightening me. So SVG images may or may not have an intrinsic size. Sorry about all the confused arguments on my part. So to cycle back to the desired behavior for createImageBitmap. If the source is an HTMLImageElement, an SVGImageElement or a Blob the currently spec'ed behavior is to reject the promise if the source is a vector graphics. Instead, the behavior could be to reject the promise if the source has no intrinsic dimensions. And once we add width/height parameters to the creation options dictionary, the the behavior could be to reject the promise only if the source has no intrinsic dimensions and the width and height parameters are unspecified. Does that make more sense?

@jakearchibald
Copy link
Contributor Author

Perfect! I'm happy with this as a first pass too, as I can easily manipulate the SVG string to add the width & height attrs.

@tabatkins
Copy link
Contributor

No reason to reject images with no intrinsic dimensions. First, if they're loaded in an <img>, we can use the element's size as their dimensions if such exists. Otherwise, we just run the sizing algorithm using the canvas size (or the width/height params if specified) as the default object size.

@jakearchibald
Copy link
Contributor Author

No reason to reject images with no intrinsic dimensions

What would the size be of an SVG blob with no width/height attrs?

const svgBlob = new Blob(['<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 10 10">…</svg>'], {
  type: 'image/svg+xml'
});

createImageBitmap(svgBlob);

@tabatkins
Copy link
Contributor

Ugh, sorry, forgot the thread of the conversation. This is about ImageBitmap, so we don't have a canvas yet.

Yeah, reject things without intrinsic width and height.

@junov
Copy link
Member

junov commented Mar 31, 2016

Great! I'll whip-up a PR.

annevk pushed a commit that referenced this issue Apr 1, 2016
This change fixes issue #923 by allowing ImageBitmap objects to
be created from SVG source images, as long as the image has an
intrinsic size. If the image does not have an intrinsic size,
we still reject the promise with an InvalidStateError, as before.

This change also removes CanvasRenderingContext2D from
ImageBtimapSource, which was an oversight in PR #790. Since there
is no longer a concept of a standalone context, this is no longer
needed.
@annevk
Copy link
Member

annevk commented Apr 1, 2016

This was fixed through #972. Thanks everyone!

@annevk annevk closed this as completed Apr 1, 2016
jungkees pushed a commit to jungkees/html that referenced this issue Apr 8, 2016
This change fixes issue whatwg#923 by allowing ImageBitmap objects to
be created from SVG source images, as long as the image has an
intrinsic size. If the image does not have an intrinsic size,
we still reject the promise with an InvalidStateError, as before.

This change also removes CanvasRenderingContext2D from
ImageBtimapSource, which was an oversight in PR whatwg#790. Since there
is no longer a concept of a standalone context, this is no longer
needed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
addition/proposal New features or enhancements
Development

No branches or pull requests

7 participants