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

RESOLVED FIXED in mozilla2.0b7

Status

()

defect
RESOLVED FIXED
9 years ago
7 years ago

People

(Reporter: darxus-mozillabug, Assigned: dholbert)

Tracking

(Blocks 1 bug, {html5})

Trunk
mozilla2.0b7
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(blocking2.0 betaN+)

Details

()

Attachments

(3 attachments, 1 obsolete attachment)

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
Status: UNCONFIRMED → NEW
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

crazytimes
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
  nsLayoutUtils::DrawableFromElement()
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.
Posted 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:
 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)
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b8
Noted that some of the known-failing tests (mentioned in comment 10) actually pass on OS:
 http://hg.mozilla.org/mozilla-central/rev/3c4f52eb4d8e
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.