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

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[Daniel Holbert [:dholbert]]

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.

Comment 1

[Daniel Holbert [:dholbert]]

Attachment: patch 1: rename SVG-as-an-image reftest directory ("hg mv image as-image" & update reftest.list)

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.

Comment 2

[Daniel Holbert [:dholbert]]

Attachment: patch 2: create reftest directory for <svg:image> with more tests

This patch (re)creates the /svg/image/ reftest directory, now with tests for <svg:image>.

Comment 3

[Daniel Holbert [:dholbert]]

Attachment: patch 3: typedef GraphicsFilter in nsLayoutUtils

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)

Comment 4

[Daniel Holbert [:dholbert]]

Attachment: patch 4: Tweak DrawSingleUnscaledImage - add GraphicsFilter, and make DirtyRect optional

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)

Comment 5

[Daniel Holbert [:dholbert]]

Attachment: patch 5: Switch to use DrawSingleUnscaledImage

...and this patch makes the actual change to use DrawSingleUnscaledImage.

Comment 6

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

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?

Comment 7

[Daniel Holbert [:dholbert]]

(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)

Comment 8

[Daniel Holbert [:dholbert]]

Landed patch 1 & patch 2 (which are tests-only):
  http://hg.mozilla.org/mozilla-central/rev/c6fa2ae426d4
  http://hg.mozilla.org/mozilla-central/rev/d1518f80fce2

Comment 9

[Daniel Holbert [:dholbert]]

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+

Comment 10

[Daniel Holbert [:dholbert]]

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

Comment 11

[Daniel Holbert [:dholbert]]

Attachment: followup: remove reftest-wait from this bug's tests (diff -w)

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.)

Comment 12

[Daniel Holbert [:dholbert]]

(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.)

Comment 13

[Daniel Holbert [:dholbert]]

Landed followup to remove reftest-wait:
  http://hg.mozilla.org/mozilla-central/rev/f4eb46861b64

Comment 14

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

You could have removed the |document.documentElement.removeAttribute("class");| lines too, FWIW.

Comment 15

[Daniel Holbert [:dholbert]]

d'oh, of course. Thanks, I'll fix that later today. :)

Comment 16

[Daniel Holbert [:dholbert]]

Fixed - thanks again for catching that:
 http://hg.mozilla.org/mozilla-central/rev/031a062400ff