Patch coming up.

The Makefile changes are to allow linking to work properly. The gfxWindowsFonts.cpp changes fixes a compile error. The gfxPattern changes fix a link error; apparently MingW doesn't like having the constructor inline for cross-DLL classes. The cairo-win32-surface.c change fixes a crash; GDB says it crashes in RtlpWaitForCriticalSection in ntdll.dll, although it also says the stack appears to be corrupt.

Comment on attachment 199127 [details] [diff] [review] Patch edit cairo-platform.h and do the right bits to disable using the mutexes; we can't link in DllMain since it causes all sorts of problems. We don't use our cairo in a multithreaded fashion, so there's no need for the mutexes. I also don't like the gfxPattern changes; if gcc can handle it on linux, why can't mingw? pavlov@mozilla.org is probably the right person to review future win32 stuff.

er, pavlov@pavlov.net I mean.

yeah, I don't like the pattern changes either. as for the makefile changes, bsmedberg is making some changes for them to get some issues sorted out with building static vs building shared. I don't really understand the need for the windows font stuff either. as for the mutexes, yeah, we should just disable them completely like vlad said.

I'll go ahead and try removing the mutexes completely. The font stuff doesn't have enough context; the issue is that j gets used later, and gcc doesn't like that (the whole scope thing). Is there a bug for the makefile stuff or is it just in the planning stages? I guess I'll see if the makefile stuff changes linking enough to make changing gfxPattern unnessary; it might depending on what the changes are. As a side note, won't leaving calls into Cairo in headers mean that all Thebes users will always have to link directly to Cairo? That doesn't seem intentional.

Hopefully this patch will be more acceptable; I believe I've addressed all the issues. I completely disabled the mutex code as suggested. Note that the configure change is an alternate solution to the gfxPattern issue. I suppose the makefile changes can wait if the relevant code is being rewritten. I redid the gfxWindowsFonts change, and although the new change is clearer, I'n not certain it will compile on MSVC (does MSVC support scoping of variables within for loops?).

pavlov@pavlov.net, would you mind taking a look at the revised patch?

the Makefile changes conflict with ones I've made to change the way we link/build/etc thebes. The reuse of j in the font code is dumb and i shouldn't be doing that anyways, so I'll just change us to use another variable. I think the configure change is the only one we really need so I'll check that in to our branch and merge it to the trunk next time we land.

What about the cairo mutex fix? You didn't mention that.

I just tried to compile Firefox with cairo/thebes under Visual Studio .NET 2005 for the first time, and encountered the following error. Is it one of the problems covered by this bug? c:/MozillaRoot/mozilla/gfx/thebes/src/gfxWindowsFonts.cpp(443) : error C2065: 'j' : undeclared identifier c:/MozillaRoot/mozilla/gfx/thebes/src/gfxWindowsFonts.cpp(444) : error C2228: left of '.uJustification' must have class/struct/union c:/MozillaRoot/mozilla/gfx/thebes/src/gfxWindowsFonts.cpp(452) : error C2228: left of '.uJustification' must have class/struct/union

I suppose this bug covers that, although it doesn't cover vc++ in general. pavlov@pavlov.net, could you take another look at that code now that all the Thebes stuff has landed? (http://lxr.mozilla.org/seamonkey/source/gfx/thebes/src/gfxWindowsFonts.cpp#443; the issue is with the reuse of j)

this should be fixed now. can anyone verify?

The configure.in change still needs to be checked in, and the Makefile changes are still needed as well. Are they OK? Or is there still some conflict with something you're doing?

the configure change is probably OK, but the thebes Makefile change is wrong. We want to statically link cairo and pixman in to the thebes library, not dynamically link htem as your patch would change it to do.

Okay, I think this is better. Turns out that without my screwup, the configure change is not needed. I'm not sure if the style is right, but I'm pretty sure the fix is. The only thing I'm not sure about is the widget fix, but there is no thebes lib in my build, only a dll.

Comment on attachment 212681 [details] [diff] [review] Patch v3 Never mind. I need to fingure out some way to get thebes.dll to export the cairo symbols. Suggestions?

I made an attempt at making this work by adding "MKSHLIB_FORCE_ALL += -Wl,-export-all-symbols" in the makefile in gfx/thebes/src; with that, it links, but it doesn't run (the dlls using thebes fail to load). I've stopped working on this for now, so if anyone wants to take this up, feel free. Adding dependency to bug 328499 so that nobody misses that both bugs exist.

This work is going on elsewhere, so no need for this bug (see https://bugzilla.mozilla.org/show_bug.cgi?id=328499#c43 for details).