Hi,

> I would prefer that you use the expect() macro when comparing integers in tests.

>

>If you don't expect a function (in this case GetDIBits, in patch 2) to

>fail when running a test, use ok() to check that it succeeded. It's OK

>if the test will crash when the ok() check fails.

>

>If GetDIBits really does fail sometimes, we need to understand why

>that happens and document it in the test.

>

>When writing comments about this, make sure you specify that you are

>talking about gdi32 device coordinates, which are different from

>gdiplus device coordinates.  

I have modified, please see the new patch file.

>+    status = GdipSetClipRectI(graphics, rect.left+width/2, rect.top+height/2,

>+            rect.right, rect.bottom, CombineModeReplace);

>

>This looks wrong to me. GdipSetClipRectI takes a width and height, not

>right and bottom.

The above statement is setting a clip range:

(rect.left+width/2, rect.top+height/2) - (rect.left+rect.right+width/2, rect.top+rect.bottom+height/2).

Because the rect is {100, 100, 180, 180}, so the clip range is: (140,140) - (320,320).

In fact GdipSetClipRectI can take any int value to specify a clip region.  

But according to your advice, I have changed to take a width and height .  

For helping review, I attached a BMP file which is the output of each test_GdipFillRectangle drawing on Memory DC.

Thank you.