Use DrawSingleUnscaledImage to paint <svg:image>, with raster-type images

RESOLVED FIXED in mozilla2.0b7

Status

()

defect
RESOLVED FIXED
9 years ago
9 years ago

People

(Reporter: dholbert, Assigned: dholbert)

Tracking

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

Firefox Tracking Flags

(blocking2.0 betaN+)

Details

Attachments

(6 attachments)

Filing this bug on a sub-part of bug 272288 -- namely, replacing nsSVGImageFrame::PaintSVG's call to "imgIContainer::GetFrame" with (a wrapper for) imgIContainer::Draw.
btw, in writing the patch here, I ran into a number of bugs that weren't uncovered by any reftests -- so I'm including some new reftests for <svg:image> functionality.

The first patch moves the existing /svg/image/ reftest directory (which tests svg as an image) to /svg/as-image/, so that we can then then create /svg/image/ for <svg:image> tests.
Assignee: nobody → dholbert
Status: NEW → ASSIGNED
This patch (re)creates the /svg/image/ reftest directory, now with tests for <svg:image>.
This patch just adds a GraphicsFilter typedef to nsLayoutUtils, so that we don't have to use the overly-long "gfxPattern::GraphicsFilter" typename everywhere. 

(This patch is here because my next patch adds another use of the GraphicsFilter typename in nsLayoutUtils)
This patch makes two tweaks to DrawSingleUnscaledImage to make it a little more generally useful -- #1, it removes the hard-coded GraphicsFilter, and #2, it makes aDirtyRect optional (since nsSVGImageFrame::PaintSVG doesn't necessarily have a dirty rect to pass down)
...and this patch makes the actual change to use DrawSingleUnscaledImage.
Attachment #477770 - Attachment description: patch 1: rename SVG-as-an-image reftest directory → patch 1: rename SVG-as-an-image reftest directory ("hg mv image as-image" & update reftest.list)
Attachment #477770 - Flags: review?(jwatt)
Attachment #477772 - Flags: review?(jwatt)
Attachment #477774 - Flags: review?(roc)
Attachment #477776 - Flags: review?(roc)
Attachment #477777 - Flags: review?(roc)
Attachment #477777 - Flags: review?(jwatt)
Comment on attachment 477777 [details] [diff] [review]
patch 5: Switch to use DrawSingleUnscaledImage

So after this, if you just remove the check for TYPE_VECTOR, SVG images work?
Attachment #477777 - Flags: review?(roc) → review+
(In reply to comment #6)
> So after this, if you just remove the check for TYPE_VECTOR, SVG images work?

"Yes", if they have non-percent height & width attributes on the <svg> element, so that imgIContainer's GetHeight & GetWidth method succeeds.  (DrawSingleUnscaledImage assumes that GetHeight and GetWidth succeed).

That might make DrawSingleUnscaledImage seem like bad choice here (as opposed to the more flexible DrawSingleImage).  However, per bug
272288 comment 36, I think we'll have some SVG-specific stuff that will likely merit a different Draw() clause for raster images vs. SVG images.  (i.e. we'll have a simple, straightforward DrawSingleUnscaledImage call for raster images, vs. a bit more setup and a more customized DrawSingleImage call for SVG images)
Attachment #477777 - Flags: review?(jwatt) → review+
Attachment #477770 - Flags: review?(jwatt) → review+
Attachment #477772 - Flags: review?(jwatt) → review+
Whiteboard: [needs landing]
Attachment #477770 - Attachment description: patch 1: rename SVG-as-an-image reftest directory ("hg mv image as-image" & update reftest.list) → [landed] patch 1: rename SVG-as-an-image reftest directory ("hg mv image as-image" & update reftest.list)
Attachment #477772 - Attachment description: patch 2: create reftest directory for <svg:image> with more tests → [landed] patch 2: create reftest directory for <svg:image> with more tests
Requesting blocking, to be able to land remaining (code-changing) patches here. 

Justification: This bug paves the way for bug 272288, which has blocking2.0=betaN+
blocking2.0: --- → ?
Landed remaining patches:
  http://hg.mozilla.org/mozilla-central/rev/9f566f515e92
  http://hg.mozilla.org/mozilla-central/rev/a29efb7cb08d
  http://hg.mozilla.org/mozilla-central/rev/8576b55d017e
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Whiteboard: [needs landing]
Target Milestone: --- → mozilla2.0b8
Attachment #477770 - Attachment description: [landed] patch 1: rename SVG-as-an-image reftest directory ("hg mv image as-image" & update reftest.list) → patch 1: rename SVG-as-an-image reftest directory ("hg mv image as-image" & update reftest.list)
Attachment #477772 - Attachment description: [landed] patch 2: create reftest directory for <svg:image> with more tests → patch 2: create reftest directory for <svg:image> with more tests
Depends on: 605420
Depends on: 606418
No longer depends on: 606418
Depends on: 606360
Target Milestone: mozilla2.0b8 → mozilla2.0b7
Depends on: 614272
The second patch on this bug added some tests with reftest-wait, but it turns out that's unnecessary (and potentially broken).

Here's a followup fix for those.  The version in my patch queue reduces indentation, too -- this here is the "diff -w" version for improved readability.

(This is along the lines of roc's "Part 2" fix for svg-as-an-image tests, in bug 617152.)
Attachment #495998 - Flags: review?(roc)
(FWIW, I ran this through TryServer already, and it's gotten through at least one run (opt|debug) per platform, with these tests passing everywhere so far.)
Landed followup to remove reftest-wait:
  http://hg.mozilla.org/mozilla-central/rev/f4eb46861b64
You could have removed the |document.documentElement.removeAttribute("class");| lines too, FWIW.
d'oh, of course. Thanks, I'll fix that later today. :)
Fixed - thanks again for catching that:
 http://hg.mozilla.org/mozilla-central/rev/031a062400ff
You need to log in before you can comment on or make changes to this bug.