441780 - SVG radialGradient won't be rendered correct when focus point is moved (outside the radius)

Status: VERIFIED FIXED

(Core :: SVG, defect)

Description

[Patrick R. Gansterer]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; de; rv:1.9) Gecko/2008052906 Firefox/3.0
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; de; rv:1.9) Gecko/2008052906 Firefox/3.0

Try the SVG.

Reproducible: Always

Steps to Reproduce:
Try the following SVG:
<svg xmlns="http://www.w3.org/2000/svg">
	<defs>
		<radialGradient id="gradient1" fx="0">
			<stop offset="0" style="stop-color:#FF0000;stop-opacity:1" />
			<stop offset="1" style="stop-color:#00FF00;stop-opacity:1" />
		</radialGradient>
		<radialGradient id="gradient2" r="0.4" fx="0.1">
			<stop offset="0" style="stop-color:#FF0000;stop-opacity:1" />
			<stop offset="1" style="stop-color:#00FF00;stop-opacity:1" />
		</radialGradient>
		<radialGradient id="gradient3" r="0.4" fx="0.2">
			<stop offset="0" style="stop-color:#FF0000;stop-opacity:1" />
			<stop offset="1" style="stop-color:#00FF00;stop-opacity:1" />
		</radialGradient>
		<radialGradient id="gradient4" fx="1">
			<stop offset="0" style="stop-color:#FF0000;stop-opacity:1" />
			<stop offset="1" style="stop-color:#00FF00;stop-opacity:1" />
		</radialGradient>
		<radialGradient id="gradient5" r="0.7" fx="1.3">
			<stop offset="0" style="stop-color:#FF0000;stop-opacity:1" />
			<stop offset="1" style="stop-color:#00FF00;stop-opacity:1" />
		</radialGradient>
		<radialGradient id="gradient6" r="0.6" fx="1.3">
			<stop offset="0" style="stop-color:#FF0000;stop-opacity:1" />
			<stop offset="1" style="stop-color:#00FF00;stop-opacity:1" />
		</radialGradient>
	</defs>
	<circle cx="100" cy="100" r="50" style="fill:url(#gradient1);" />
	<circle cx="100" cy="200" r="50" style="fill:url(#gradient2);" />
	<circle cx="100" cy="300" r="50" style="fill:url(#gradient3);" />
	<circle cx="200" cy="100" r="50" style="fill:url(#gradient4);" />
	<circle cx="200" cy="200" r="50" style="fill:url(#gradient5);" />
	<circle cx="200" cy="300" r="50" style="fill:url(#gradient6);" />
</svg>
Actual Results:  
Some gradients are handled incorrect.


In Firefox 2 the gradients rendered correctly.

Comment 1

[Robert Longson [:longsonr]]

Can you add the SVG as an attachment please? Also a screenshot of what you expect it to look like would be nice.

Comment 2

[Patrick R. Gansterer]

Attachment: The SVG as file

Comment 3

[Patrick R. Gansterer]

Attachment: Rendering in Firefox 2

Comment 4

[Patrick R. Gansterer]

Attachment: Rendering in Firefox 3

Comment 5

[Robert Longson [:longsonr]]

A regression range would help.

Comment 6

[Ria Klaassen (not reading all bugmail)]

Regression range is
http://bonsai.mozilla.org/cvsquery.cgi?module=PhoenixTinderbox&date=explicit&mindate=1191970380&maxdate=1191971339
with only Bug 384681 in it.

Comment 7

[Jonathan Watt [:jwatt]]

CC'ing Vlad on this regression from the 16.16 work.

Comment 8

[Robert Longson [:longsonr]]

Attachment: patch

Comment 9

[Robert Longson [:longsonr]]

Comment on attachment 352940 [details] [diff] [review]
patch

The key thing is that .99 < 127/128 which is the limit of 24.8 resolution.

Comment 11

[Jonathan Watt [:jwatt]]

Comment on attachment 352940 [details] [diff] [review]
patch

Nice! r=jwatt. Can you also note this in the comment? Perhaps append the following:


>     // The focal point (fFx and fFy) must be clamped to be *inside* - not on -
>
>     // the circumference of the gradient or we'll get rendering anomalies. We
>
>     // calculate the distance from the focal point to the gradient center and
>
>-    // make sure it is *less* than the gradient radius. 0.999 is used as the
>
>+    // make sure it is *less* than the gradient radius. 0.99 is used as the
>
>     // factor of the radius because it's close enough to 1 that we won't get a
>
>     // fringe at the edge of the gradient if we clamp, but not so close to 1
>
>     // that rounding error will give us the same results as using fR itself.


    // Also note that .99 < 127/128, which is the limit of the fractional part of
    // cairo's 24.8 fixed point representation.

Comment 12

[Robert Longson [:longsonr]]

Attachment: address review comment [Checkin: Comment 15 & 18]

Comment 13

[Robert Longson [:longsonr]]

Attachment: reftest [Checkin: Comment 15 & 18]

Comment 14

[Robert Longson [:longsonr]]

Comment on attachment 353029 [details] [diff] [review]
address review comment
[Checkin: Comment 15 & 18]

Simple low risk change that fixes a regression we've had since Firefox 2

Comment 15

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

Pushed 05bf7c30877e to trunk.

Comment 16

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

I agree we should have this for 3.1.

Comment 17

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

Comment on attachment 353029 [details] [diff] [review]
address review comment
[Checkin: Comment 15 & 18]

Since this blocks, we no longer need 1.9.1 approval

Comment 18

[Serge Gautherie (:sgautherie)]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/dc2948a5ba22

Comment 19

[Aakash Desai [:aakashd]]

verified FIXED on builds:

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.1pre) Gecko/20090720 Shiretoko/3.5.1pre ID:20090720042942

and

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2a1pre) Gecko/20090721 Minefield/3.6a1pre ID:20090721044139