114854 - Horizontal lines in SVG circles test

Status: RESOLVED WORKSFORME

(Core :: SVG, defect)

Description

[Andrew Simmonds]

The first SVG test on http://skeeter-s.com/svg/new-svg.xml shows unwanted 1px
horizontal lines.

- These lines don't appear to be affecting the 100% blue !(aub)n(c) section at all.

- If I move a window over the lines they will disappear leaving me with a clean
image.

- I'm using GCC-3.0.2 maybe this has some weird affect on rounding.

An image will be attached to show what i'm experiencing.

Comment 1

[Andrew Simmonds]

Attachment: circles with random lines through them

Comment 2

[Bradley Baetz (:bbaetz)]

WFM. What bit depth are you using?

I'm using 2.96, though. Can you try compiling libart with -fno-strict-aliasing?
(/me guesses)

Comment 3

[Alex Fritze]

Ok so this seems to be occuring mainly at 72dpi, where we have 0.05 pixel/twips.
This means that in the conversion from dirty-rect in twips to the dirty-rect in
pixels, we often get values like 100.5, which to me seems very prone to roundoff
errors. Should it be 101 or 100?
Inflating the dirty rect passed to
nsSVGRenderingContext::nsSVGRenderingContext() fixes the problem. Patch to follow.
 

Comment 4

[Alex Fritze]

Attachment: proposed patch

Comment 5

[Bradley Baetz (:bbaetz)]

Where did 10 and 20 come from?

Comment 6

[Bradley Baetz (:bbaetz)]

Comment on attachment 61575 [details] [diff] [review]
proposed patch

You can't hard code this to 72dpi..

You can remove the <1 testing for mDiretyRect below, while you're at it.

Comment 7

[Alex Fritze]

Where did I get the 10 & 20 from?
I just pulled them out of my sleeve! 10 being about equal to half a pixel (at 
72dpi) is just a hand-waving argument. They are magic numbers, but they work. 
No harm in increasing the size of the dirty rect :-)
Initially, I thought that inflating by 1 twip should do the trick to pull us 
into the right direction when converting to pixels, but it sometimes didn't. So 
I tried to be pragmatic and considered that, on the other extreme, inflating by 
a full pixel should not be necessary, and picked some number: 
1 < magic number < a_typical_pixel_size. 
magic_number=a_typical_pixel_size/2 looked good. I don't know why we need this, 
but it works.

What value do you suggest we use? twipsPerPx/2? I deliberatly didn't do that. 
I have no evidence that we actually need to inflate by 1/2pixel. Until I fully 
understand what's going on here, twipsPerPx/2 would be a magical number just 
like 10 & 20, only one that chews up more cycles :-)
10 & 20 seem to fix our problem so why not be pragmatic about this and wait 
until someone tackles the real cause of the problem? We shouldn't have to do 
any inflating, be it by 1, twipsPerPx/2 or 20. I could possibly be convinced 
that 1 is plausible. Using twipsPerPx/2 would just obscure the fact that it is 
a magic number.

I have a feeling that somehow the *real* problem might be connected with us 
nudging the dirty rects in nsSVGOuterSVGFrame::InvalidateRegion(). That 'int 
twipsPerPx = (int)(GetTwipsPerPx()+0.5f);' on line 898 looks highly suspect :-)

Comment 8

[Bradley Baetz (:bbaetz)]

Hmm. OK, I guess. r=bbaetz; not sure if you need drivers@m.o approval to land it.

Yes, that code does look strange. Why is it there?

Comment 9

[Håkan Waara]

> I have a feeling that somehow the *real* problem might be connected with us 
> nudging the dirty rects in nsSVGOuterSVGFrame::InvalidateRegion(). That 'int 
> twipsPerPx = (int)(GetTwipsPerPx()+0.5f);' on line 898 looks highly suspect :-)

What would happen if we changed that? Somehow I get the feeling that was also
put there because of an ugly hack. :-)

Comment 10

[J. Cliff Dyer]

Attachment: Randomly scattered dots (small circles), misrendered

Having a stroke-width more than double the radius of the circle seems to cause
horizontal lines, but not always.

Comment 11

[J. Cliff Dyer]

Attachment: Counter example to above attachment

This is a counter example with circles at the same points, with the same
radius, but narrower stroke-widths.  None of these produce the horizontal line.

Comment 12

[Jonathan Watt [:jwatt]]

Mass reassign of SVG bugs that aren't currently being worked on by Alex to
[e-mail]. If you think someone should be assigned to your bug you can
join the #svg channel on mozilla.org's IRC server ( irc://irc.mozilla.org/svg )
where you can try to convince one of the SVG hackers to work on it. We aren't
always there, so if you don't get a response straight away please try again later. 

Comment 13

[Matt Sayler]

WFM 

(Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8) Gecko/20051025 Firefox/1.5)

Comment 14

[Leon Sha]

I can see this problem on the test case attached.
But I can not see this problem on this.
http://skeeter-s.com/svg/new-svg.xml
I am on solaris by the way.
Mozilla/5.0 (X11; U; SunOS sun4u; en-US; rv:1.9a1) Gecko/20051021 Firefox/1.6a1

Comment 15

[Jonathan Watt [:jwatt]]

The problem persists despite the switch to cairo. Still seeing it in FF1.5 and latest trunk on w32.

Comment 16

[Jeff Schiller]

WFM

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1) Gecko/20061023 SUSE/2.0-30 Firefox/2.0

and

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1) Gecko/20061010 Firefox/2.0