446323 - Upgrade cairo to 1.6.4-348-g96c9e2a

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Attachment: upgrade patch

Upgrade trunk cairo to latest snapshot.

Comment 1

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

This bounced out because it caused crashes on the unit test tinderboxen and the leak test tinderbox.  The talos boxes were unaffected, and I can't reproduce the problem on my linux box here -- all tests pass fine.  What do I do?  Just a stack trace would be helpful, even.

Comment 2

[Michael Moy]

> MMX is already on under MSVC and gcc; with a cairo update I'm about to do 
> (bug 446323), we'll get SSE2 code as well.  The SSE2 code will be initially 
> disabled on both MSVC and gcc for us -- under MSVC until we get some patches 
> to fix stupidity in the MSVC compiler when dealing with intrinsics, and in 
> gcc until this bug is fixed.

Pretty impressive patch. The SIMD code style is different from what I'm used to (it uses more small functions instead of long strings of code) and I'm aware of VS2005 idiosyncrasies. Do you have a list of VS2005 issues with this code?

Comment 3

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

(In reply to comment #2)
> Pretty impressive patch. The SIMD code style is different from what I'm used to
> (it uses more small functions instead of long strings of code) and I'm aware of
> VS2005 idiosyncrasies. Do you have a list of VS2005 issues with this code?

Yep, take a look at http://lists.cairographics.org/archives/cairo/2008-July/014538.html -- we think we have it resolved, just need to double-check that gcc doesn't generate crappy code.  Those guys are pretty open to any feedback, though, so please send any along!

Comment 4

[Michael Moy]

Passing arguments by reference seems like it would make it a lot harder to generate optimized code.

I usually prefer using macros to function calls for small SIMD routines as it allows the compiler the greatest freedom for optimization while keeping the code readable. It also avoids the problem with stack variable and it can avoid problems with initializing vectors that don't need to be initialized.

I have a new Windows machine to set up and will try a build with your patch and the additional patch to get things to compile and will look at the generated code.

Comment 5

[Peter Weilbacher]

Vlad, I really wish you would be a bit more verbose when updating cairo. From this bug I cannot see when it was push or backed out or which bug(s) the update is supposed to fix. It's also a bit weird that you r+'ed your own patch without any comment...

Now, what should I go on OS/2 for which the update broke the build? (It still has _cairo_font_reset_static_data in cairo-os2-surface.c while all other instances of that function were changed to _cairo_font_face_reset_static_data.) I want to fix that upstream in cairo ASAP and have sent a mail to the cairo list for that purpose. Should I push the same fix to mozilla-central? (It just touches cairo-os2-surface.c.)

Comment 6

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Whoops, forgot to resolve this.

We follow cairo development -- so we'll keep taking cairo updates even if they don't directly fix any bugs that we might have.  The changelog is just the regular cairo changelog, at http://gitweb.freedesktop.org/?p=cairo;a=shortlog .

For patches, it's fine to push cairo fixes directly to mozilla-central as long as you also create a patch file for it in gfx/cairo and add info about it to gfx/cairo/README -- our version of cairo/pixman is upstream (whatever version is in gfx/cairo/README) + the patches in gfx/cairo.  Any patches that are not there are liable to get blown away during an upgrade; at the same time I usually go through the patches that we have and upstream any that are appropriate.  (Ideally, send the patch to the cairo list at the same time.)

Comment 7

[Michael Moy]

I wrote up a quick change to the SSE2 code to use #defines for the problem routines and added back SSE2 in the Makefile and this built and ran fine. From a subjective point of view, performance felt better. I didn't try the patch from the mailing list but I'll assume that it works and that there are no performance issues at -O2 as everything should be inlined. I didn't see any tests on this though nor analysis on the generated code. Is this something that's desirable for 3.1 or will this eventually get picked up on a future Cairo merge?