GDI+: headers and one test

[Evan Stade]

Changelog:
*Added public GDI+ headers in include/
*Added private GDI+ headers in dlls/gdiplus
*Added small implementation of GDI+ pens in dlls/gdiplus/pen.c
*Added (first) GDI+ test

dlls/gdiplus/Makefile.in         |    3 +
 dlls/gdiplus/gdip_defines.h      |   43 +++++++++++++++++++
 dlls/gdiplus/gdip_enumerations.h |   57 ++++++++++++++++++++++++++
 dlls/gdiplus/gdiplus.c           |   27 ++++++++++++
 dlls/gdiplus/gdiplus.spec        |    8 ++--
 dlls/gdiplus/gdiplus_private.h   |   43 +++++++++++++++++++
 dlls/gdiplus/pen.c               |   84
++++++++++++++++++++++++++++++++++++++
 dlls/gdiplus/pen.h               |   30 ++++++++++++++
 dlls/gdiplus/tests/Makefile.in   |   13 ++++++
 dlls/gdiplus/tests/pen.c         |   44 ++++++++++++++++++++
 include/Gdiplus.h                |   10 +++++
 include/GdiplusEnums.h           |   39 ++++++++++++++++++
 include/GdiplusFlat.h            |   23 ++++++++++
 include/GdiplusGpStubs.h         |   10 +++++
 include/GdiplusInit.h            |   26 ++++++++++++
 include/GdiplusTypes.h           |    7 +++
 16 files changed, 462 insertions(+), 5 deletions(-)

-Evan Stade

headers and one test

[Dmitry Timoshkov]

"Evan Stade"  wrote:
> +WINE_DEFAULT_DEBUG_CHANNEL(gdip);
> -
>
> +COLORREF ARGB2COLORREF(ARGB color)
> +{

If this is a module local function it should be static.
> +GpStatus WINGDIPAPI GdipCreatePen1(ARGB color, FLOAT width, GpUnit unit,
> - GpPen **pen)
>
> +{
> - LOGBRUSH lb;
> - GpPen *gp_pen;
> -
> - gp_pen = (GpPen*) malloc(sizeof(GpPen));

Do not use malloc in Wine, use HeapAlloc and friends.
> - if(!pen){ return OutOfMemory; }

if (!pen) return OutOfMemory;

looks more naturally IMO without redundant braces, here and in other places.
> +START_TEST(pen)
> +{
> - GdiplusStartupInput gdiplusStartupInput;
> - ULONG_PTR gdiplusToken;
> -
> - gdiplusStartupInput.GdiplusVersion=1;
> - gdiplusStartupInput.DebugEventCallback=NULL;
> - gdiplusStartupInput.SuppressBackgroundThread=0;
> - gdiplusStartupInput.SuppressExternalCodecs=0;
> -
> - GdiplusStartup(&gdiplusToken, &gdiplusStartupInput, NULL);
> -
> - test_constructor_destructor();
> -
> - GdiplusShutdown(gdiplusToken);

It's better to check whether GdiplusStartup/GdiplusShutdown fail.
> --- /dev/null
> +++ b/include/GdiplusEnums.h
> @@ -0,0 +1,39 @@
> +#ifndef _GP_ENUMERATIONS_H_
> +#define _GP_ENUMERATIONS_H_
> -
>
> +typedef enum {
> - UnitWorld = 0,  /* seemingly equivalent to notion of "logical unit" */
> - UnitDisplay = 1,
> - UnitPixel = 2,
> - UnitPoint = 3,
> - UnitInch = 4,
> - UnitDocument = 5,
> - UnitMillimeter = 6
>
> +} Unit;
> -
>
> +typedef enum {
> - Ok = 0,
> - GenericError = 1,
> - InvalidParameter = 2,
> - OutOfMemory = 3,
> - ObjectBusy = 4,
> - InsufficientBuffer = 5,
> - NotImplemented = 6,
> - Win32Error = 7,
> - WrongState = 8,
> - Aborted = 9,
> - FileNotFound = 10,
> - ValueOverflow = 11,
> - AccessDenied = 12,
> - UnknownImageFormat = 13,
> - FontFamilyNotFound = 14,
> - FontStyleNotFound = 15,
> - NotTrueTypeFont = 16,
> - UnsupportedGdiplusVersion = 17,
> - GdiplusNotInitialized = 18,
> - PropertyNotFound = 19,
> - PropertyNotSupported = 20,
> - ProfileNotFound = 21
>
> +} Status;

Why are you duplicating dlls/gdiplus/gdip_enumerations.h? Besides my version
of PSDK doesn't have Status in GdiplusEnums.h, they are in GdiPlusTypes.h.

Otherwise the patch looks good.

[Francois Gouget]

On Tue, 29 May 2007, Evan Stade wrote:
[...]
> include/Gdiplus.h                |   10 +++++
> include/GdiplusEnums.h           |   39 ++++++++++++++++++
> include/GdiplusFlat.h            |   23 ++++++++++
> include/GdiplusGpStubs.h         |   10 +++++
> include/GdiplusInit.h            |   26 ++++++++++++
> include/GdiplusTypes.h           |    7 +++

All filenames in Wine should be lowercase.

--- /dev/null
+++ b/include/Gdiplus.h
[...]
+
+#ifndef __cplusplus

Why check for __cplusplus here? The PSDK contains no such check, at 
least in this file.

--- /dev/null
+++ b/include/GdiplusEnums.h
[...]
+#ifndef _GP_ENUMERATIONS_H_
+#define _GP_ENUMERATIONS_H_

The standard format for Wine headers is __WINE_HEADERNAME_H, unless we 
need to keep the original macro name for compatibility with the PSDK. 
But here you're following neither conventions (the other headers would 
need to be rechecked but they seem to keep the PSDK names).

--- /dev/null
+++ b/include/GdiplusFlat.h
[...]
+GpStatus WINGDIPAPI GdipCreateFromHDC(HDC hdc, GpGraphics **graphics);
+GpStatus WINGDIPAPI GdipDeleteGraphics(GpGraphics *graphics);

Wine headers generally don't keep the parameter names in the headers as 
they are not needed.

--- /dev/null
+++ b/include/GdiplusTypes.h
[...]
+typedef enum {
[...]
+} Status;

Hmm, this is not the same as the PSDK. The PSDK only defines 'enum 
Status { ... }' which, in theory, should only allow one to use 'enum 
Status' and not 'Status'. Yet the PSDK headers seem to make use of 
'Status'. This might be different in C++ though... 
So on the whole it's probably ok to use a typedef. I just prefer to 
bring this up so others can chime in.

[Alexandre Julliard]

Francois Gouget  writes:
> Hmm, this is not the same as the PSDK. The PSDK only defines 'enum 
> Status { ... }' which, in theory, should only allow one to use 'enum 
> Status' and not 'Status'. Yet the PSDK headers seem to make use of 
> 'Status'. This might be different in C++ though... 
> So on the whole it's probably ok to use a typedef. I just prefer to 
> bring this up so others can chime in.

I think it's better without the typedef, in C we can simply use 'enum
Status'.

[Chris Robinson]

On Thursday 31 May 2007 06:59:09 am Francois Gouget wrote:
> --- /dev/null
> +++ b/include/GdiplusTypes.h
> [...]
> +typedef enum {
> [...]
> +} Status;
>
> Hmm, this is not the same as the PSDK. The PSDK only defines 'enum
> Status { ... }' which, in theory, should only allow one to use 'enum
> Status' and not 'Status'. Yet the PSDK headers seem to make use of
> 'Status'. This might be different in C++ though...
> So on the whole it's probably ok to use a typedef. I just prefer to
> bring this up so others can chime in.

C++ auto-typedefs structs/classes, enums, and the like.
enum Status { };
in C++ would be equivilant to:
typedef enum Status { } Status;
in C. And yes, both the enum and typedef can have the same name.. the compiler 
is smart enough to tell the difference between 'enum Status' and 'Status' 
being used as types. Both can be used interchangeabley, too.

[Steven Edwards]

On 5/31/07, Francois Gouget  wrote:
> The standard format for Wine headers is __WINE_HEADERNAME_H, unless we
> need to keep the original macro name for compatibility with the PSDK.
> But here you're following neither conventions (the other headers would
> need to be rechecked but they seem to keep the PSDK names).

This is something I've never understand. Why don't we use the same
sentinel names for all of the Wine headers as the PSDK headers. You
really can't mix and match the headers very well anyway. I always had
to muck with include paths anyway if I wanted to use the PSDK headers
with Wine because your still always going to need to use
wine/include/wine for debug.h, port.h, list.h etc so using the same
name for SDK headers won't cause a conflict anywhere.