589558 - (ietestcenter) Support SVG images for HTML canvas drawImage() method

Status: RESOLVED FIXED

(Core :: Graphics: Canvas2D, defect)

Description

[Darxus]

User-Agent:       Mozilla/5.0 (X11; Linux x86_64; rv:2.0b4pre) Gecko/20100817 Minefield/4.0b4pre
Build Identifier: 

Fails test.

Reproducible: Always

Comment 1

[Daniel Holbert [:dholbert]]

Per bug 276431 comment 160, this needs an implementation of imgIContainer::CopyFrame for VectorImage, which I intend to work on soon.

Also, note-to-self: keep bug 276431 comment 130 in mind. (We need to mark a canvas as write-only once it's had SVG content drawn onto it, so as not to leak information.)

Comment 2

[Daniel Holbert [:dholbert]]

Attachment: Patch 1: Implement VectorImage::GetFrame, which calls CopyFrame, which calls Draw

Comment 3

[Daniel Holbert [:dholbert]]

Attachment: Patch 2: Make canvas write-only after drawing SVG image

Comment 4

[Daniel Holbert [:dholbert]]

NOTE: This implementation of GetFrame & CopyFrame will only work for <svg> images that provide a pixel-valued height & width.  (That is to say: for an image whose <svg> node has a percent height or width, GetFrame & CopyFrame will return NS_ERROR_FAILURE.)

This makes sense at least in the context of canvas.drawImage() -- drawImage doesn't necessarily give us a size to draw into, and there's no percent-length-basis that would really make sense.

Comment 5

[Joe Drew (not getting mail)]

Comment on attachment 480283 [details] [diff] [review]
Patch 1: Implement VectorImage::GetFrame, which calls CopyFrame, which calls Draw

crazytimes

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

This is OK, but rendering through a temporary surface is suboptimal. Ideally canvas drawImage would call Draw() directly when possible.

Comment 7

[Daniel Holbert [:dholbert]]

So right now, nsCanvasRenderingContext2D::DrawImage uses nsLayoutUtils::SurfaceFromElement() to get a gfxASurface for our image, which it draws.

Perhaps instead, it should instead be calling a new method named something like
  nsLayoutUtils::DrawableFromElement()
to get a gfxDrawable that (internally) draws using our imgIContainer's Draw() method.

Comment 8

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Can we do that in a followup?  Seems like a decent chunk of work, though has advantages for other things probably.

Comment 9

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Sure.

Comment 10

[Daniel Holbert [:dholbert]]

Attachment: Patch 3: reftests

Here are some reftests for this.

The ones that involve scaling currently fail (i.e. have fuzzy edges) since we rasterize to a temporary surface (with size determined by <svg> height/width attributes), and then scale that rasterized surface up or down before painting.

I think this is a problem that we're basically stuck with in the current canvas architecture (using a temporary surface), but we can fix it as part of the followup work in switching to use Draw() discussed in comment 6 thru comment 9.

Comment 11

[Daniel Holbert [:dholbert]]

Attachment: Patch 3 v2: reftests

(tweaked tests to make things a bit more consistent between them)

Comment 12

[Daniel Holbert [:dholbert]]

Landed patches:
 http://hg.mozilla.org/mozilla-central/rev/e0e28d38dd59
 http://hg.mozilla.org/mozilla-central/rev/4c30e758923c
and tests:
 http://hg.mozilla.org/mozilla-central/rev/3e289ae794d6

Filed bug 603488 on using imgIContainer::Draw() instead of imgIContainer::GetFrame()/CopyFrame() to implement canvas's DrawImage method (per comment 6 thru comment 9)

Comment 13

[Daniel Holbert [:dholbert]]

Noted that some of the known-failing tests (mentioned in comment 10) actually pass on OS:
 http://hg.mozilla.org/mozilla-central/rev/3c4f52eb4d8e