549019 - Build bustage with older Windows SDKs due to Direct2D landing

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Ryan VanderMeulen [:RyanVM]]

Using older versions of the Windows SDK (Vista SDK in my specific case), the trunk is currently unbuildable due to the landing of the Direct2D patch. Compiler can't find dwrite.h.

D2D/DWrite build support should be conditional on the presence of the Windows 7 SDK.

Comment 1

[Bas Schouten (:bas.schouten)]

The includes in the top of gfxWindowsPlatform.cpp should be wrapped in CAIRO_HAS_DWRITE_FONT. I'm preparing a patch for this.

Comment 2

[Bas Schouten (:bas.schouten)]

Attachment: Properly #ifdef includes in gfxWindowsPlatform

This patch should solve the issue.

Comment 3

[[e-mail]]

Don't you also need to ifdef the Makefile to stop compiling the gfxD* files?

Comment 4

[[e-mail]]

Attachment: Proposed patch

Tested using the Windows 2003 R2 Platform SDK.

Comment 5

[Bas Schouten (:bas.schouten)]

Comment on attachment 429316 [details] [diff] [review]
Proposed patch

>--- a/gfx/thebes/src/gfxFontUtils.cpp	Fri Feb 26 18:54:18 2010 -0500
>+++ b/gfx/thebes/src/gfxFontUtils.cpp	Sat Feb 27 12:01:40 2010 +0000
>@@ -590,7 +590,7 @@
>     }
>     
>     // append each font name to the list
>-    nsAutoString fontname;
>+    nsString fontname;
>     nsPromiseFlatString fonts(fontlistValue);
>     const PRUnichar *p, *p_end;
>     fonts.BeginReading(p);
This really doesn't belong in this patch.

Note that think this patch is correct, so I'll r+ it with the aforementioned change, but I'm not a reviewer for gfx so this is not enough for landing it. Requesting additional review.

Comment 6

[Bas Schouten (:bas.schouten)]

(In reply to comment #3)
> Don't you also need to ifdef the Makefile to stop compiling the gfxD* files?

You're absolutely right, this netbook doesn't build very well so I didn't have a chance to check yet :-).

Comment 7

[[e-mail]]

(In reply to comment #5)
>(From update of attachment 429316 [details] [diff] [review])
>>-    nsAutoString fontname;
>>+    nsString fontname;
>This really doesn't belong in this patch.
Sorry about that, it's left over from some bloat testing work (the autobuffer for that particular string never gets used).

Comment 8

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 429316 [details] [diff] [review]
Proposed patch

Looks ok to me.

Comment 9

[Bernd]

even if you have the windows 7 SDK installed but your cairo_config.h looks like the following:
....
#define CAIRO_HAS_PDF_SURFACE 1
#define CAIRO_HAS_WIN32_SURFACE 1
#define CAIRO_HAS_WIN32_FONT 1
#define CAIRO_HAS_PNG_FUNCTIONS 1
#endif

It does not compile. I am not sure, why on win xp sp3 with sdk win7 it behaves like this.

Comment 10

[[e-mail]]

Comment on attachment 429316 [details] [diff] [review]
Proposed patch

Pushed changeset 28d69119620b to mozilla-central.

Comment 12

[Bas Schouten (:bas.schouten)]

This was fixed.