604723 - Wrong svg:image position

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[Danil Ivanov]

User-Agent:       Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b8pre) Gecko/20101015 Firefox/4.0b8pre
Build Identifier: 

<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 85 85">
  <image xmlns:xlink="http://www.w3.org/1999/xlink"
         xlink:href="firefox.png"
         x="0" y="0"
         width="100%" height="100%"/>
</svg>

Wrong svg:image position.

Reproducible: Always

Comment 1

[Boris Zbarsky [:bzbarsky]]

Can you please either point to a web page showing the problem, or attach the full SVG file and image needed to reproduce the issue?

Comment 2

[Danil Ivanov]

Attachment: Testcase with actual and expected results

Comment 3

[Boris Zbarsky [:bzbarsky]]

Ah, indeed.  Looks like a regression from 3.6.

Comment 4

[Danil Ivanov]

Regression window:
ok: 2010-10-11-03-mozilla-central
bad: 2010-10-12-03-mozilla-central

Comment 5

[Daniel Holbert [:dholbert] (PTO until 2/24)]

Attachment: reduced testcase 1 (interactive)

This testcase demonstrates the bug.  It looks like, for <svg> embedded in HTML, we set the "0,0" position to be the upper-left corner of the _host_ document, rather than the upper-left corner of the <svg> node.

Comment 6

[Boris Zbarsky [:bzbarsky]]

yeah, indeed:

http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=5a41a70eb631&tochange=2593c8c8af8b

It's gotta be bug 598798.

Comment 7

[Daniel Holbert [:dholbert] (PTO until 2/24)]

Attachment: reference case (using circle instead of image)

This testcase replaces the <image> element with a <circle>.  No bug here.

Comment 8

[Daniel Holbert [:dholbert] (PTO until 2/24)]

(so, specific to <image> painting --> definitely bug 598798.)

Comment 9

[Daniel Holbert [:dholbert] (PTO until 2/24)]

Attachment: fix

Trivial fix.  I should have used ctx->Multiply() instead of SetMatrix(), to match the behavior in nsSVGUtils::CompositeSurfaceMatrix() (the method we used to use for applying the matrix & drawing).  This fixes that and fixes the bug.

Comment 10

[Daniel Holbert [:dholbert] (PTO until 2/24)]

Comment on attachment 483590 [details] [diff] [review]
fix

(RE the svg/image/reftest.list change -- this patch just adds one line to that file, though I also increased indentation on other lines, so the reference cases would still line up nicely.)

Comment 12

[Daniel Holbert [:dholbert] (PTO until 2/24)]

Comment on attachment 483590 [details] [diff] [review]
fix

additional-r?'ing longsonr in case he gets to this first.

Comment 13

[Daniel Holbert [:dholbert] (PTO until 2/24)]

http://hg.mozilla.org/mozilla-central/rev/7d35f27d4897

Comment 14

[Biju]

(In reply to comment #11)
> *** Bug 605416 has been marked as a duplicate of this bug. ***

There are still some not got fixed, 
see Bug 606418