Scrolling the attached testcase on Mac, I see that some parts of the gradient aren't repainted. Michael, can you reproduce this on Linux?

If we can't reproduce this on Linux or Windows, I'm calling this a Cairo/Quartz bug.

Fwiw, I see this on OS X, but not on Ubuntu904 running in a VM

WFM on Ubuntu 9.04.

Could not reproduce on Windows.

Click on "Redraw". Every single time the element becomes visible, a small band at the bottom of the element is not painted. It only happens with repeating radial gradients --- not linear gradients, not non-repeating radial gradients. It's something to do with the dirty area though, since if you force repainting of the whole window, it paints OK. So I suspect a cairo/quartz bug with repeating radial gradients being clipped.

Hmm, but we actually fall back to pixman for repeating radial gradients: http://mxr.mozilla.org/mozilla-central/source/gfx/cairo/cairo/src/cairo-quartz-surface.c#1169 I don't understand why we have to, but anyway, it seems this bug must be in cairo somewhere.

We seem to be consistently chopping 18 rows off the bottom of the rendered gradient. There must be something wrong in the fallback path; _cairo_quartz_setup_fallback_source is only used for repeating radial gradients. However, the image data looks correct to me before it goes into _cairo_surface_to_cgimage, the extra lines are all there. I'm stumped for now.

Hmm. The 18 is the height of the title bar. I think the problem is here: // get the Y flip right -- the CTM will always have a Y flip in place clipBox.origin.y = surface->extents.height - (clipBox.origin.y + clipBox.size.height); Here (in a modified testcase) clipBox starts at x=0,y=0,width=200,height=200. The CTM (inverse) is a=1,b=0,c=0,d=-1,tx=0,ty=612. surface->extents is x=0,y=0,width=1038,height=594. So we compute a clipBox of x=0,y=-18,width=200,height=200. I'm still not 100% sure what's going on here but clearly surface->extents being different from the 612 in the CTM is a problem.

Actually, 18 is the height of the status bar.

This fixes it. The CTM we keep in our CGContext is always a mapping from cairo "device space" to CGContext's native space (i.e., a flip). The comment "// the clipBox is in userspace, so:" is misleading; it's actually already in cairo device space, so we shouldn't be doing anything at all with it here. I also happened to notice that in _cairo_quartz_surface_paint with action DO_SHADING we aren't saving and restoring the CGContext state properly. Unfortunately we can't really test this; I think drawing to a canvas won't exercise the problem here.

I filed bug 508227 on avoiding fallback for repeated radial gradients. I think there will be degenerate cases where we still have to fall back, though.

Comment on attachment 392459 [details] [diff] [review] fix Maybe Jeff can review this if he gets to it first?

Guys, we really need review here. We're not really able to talk about our gradient support until it's not completely broken on Mac.

Comment on attachment 392459 [details] [diff] [review] fix Vlad said that he thought that some of this code might be for 10.4 but he couldn't remember anything. He didn't put a comment in, so we agreed to delete it. As long as we pass tests this looks fine to me.

I tested this on 10.4, it works. Landed: http://hg.mozilla.org/mozilla-central/rev/cc6bebbd93bb

Does this need to be added to a list of local patches we have to cairo? And does it need to be moved upstream?

Yes and yes.

We also need to get this on branch...

verified with: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9.2a2pre) Gecko/20090911 Namoroka/3.6a2pre Probably should this to litmus FFT

removing in-litmus flag, it no longer exists