Closed Bug 589558 Opened 11 years ago Closed 11 years ago

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


(Core :: Canvas: 2D, defect)

Not set



Tracking Status
blocking2.0 --- betaN+


(Reporter: darxus-mozillabug, Assigned: dholbert)


(Blocks 1 open bug, )


(Keywords: html5)


(3 files, 1 obsolete file)

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

Fails test.

Reproducible: Always
Blocks: ietestcenter
Version: unspecified → Trunk
Component: General → Canvas: 2D
Depends on: 276431
Ever confirmed: true
Keywords: html5
OS: Linux → All
QA Contact: general → canvas.2d
Hardware: x86_64 → All
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.)
Assignee: nobody → dholbert
blocking2.0: --- → ?
blocking2.0: ? → -
Summary: (ietestcenter) HTML5 Canvas 15/15: drawImage() method draws SVG images on the canvas → (ietestcenter) Support SVG images for HTML canvas drawImage() method
blocking2.0: - → ?
Attachment #480283 - Attachment description: atch 1: Implement VectorImage::GetFrame, which calls CopyFrame, which calls Draw. → Patch 1: Implement VectorImage::GetFrame, which calls CopyFrame, which calls Draw
Attachment #480283 - Attachment is patch: true
Attachment #480283 - Attachment mime type: application/octet-stream → text/plain
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 on attachment 480283 [details] [diff] [review]
Patch 1: Implement VectorImage::GetFrame, which calls CopyFrame, which calls Draw

Attachment #480283 - Flags: review?(joe) → review+
blocking2.0: ? → betaN+
This is OK, but rendering through a temporary surface is suboptimal. Ideally canvas drawImage would call Draw() directly when possible.
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
to get a gfxDrawable that (internally) draws using our imgIContainer's Draw() method.
Can we do that in a followup?  Seems like a decent chunk of work, though has advantages for other things probably.
Attached patch Patch 3: reftests (obsolete) — Splinter Review
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.
(tweaked tests to make things a bit more consistent between them)
Attachment #482372 - Attachment is obsolete: true
Blocks: 603488
Landed patches:
and tests:

Filed bug 603488 on using imgIContainer::Draw() instead of imgIContainer::GetFrame()/CopyFrame() to implement canvas's DrawImage method (per comment 6 thru comment 9)
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b8
Noted that some of the known-failing tests (mentioned in comment 10) actually pass on OS:
Target Milestone: mozilla2.0b8 → mozilla2.0b7
Blocks: 598204
Blocks: 774311
You need to log in before you can comment on or make changes to this bug.