GDI+ renders properly on Windows 7 but not XP

[Druzyek]

Sorry if this is double-posted. I don't see my original post after waiting 2 days.

I am using DrawCachedBitmap in a simple game. On Windows 7 it works fine but when I test it on Windows XP there is a problem. Any parts of the image I draw that have transparency are drawn at 0,0 and any part without transparency is drawn in the correct place. It does this both for bitmaps I load from resources and change in memory to be partially transparent and with bitmaps I create only in memory using rectangles and text. Here is the relevant code for drawing text and rectangles:

void Item::LoadToolTip()
{
//All these variables are members of Item
MaxWidth=0;
text=new wstringstream;
TempBmp=new Bitmap(TOOLTIP_BASE_WIDTH,TOOLTIP_BASE_HEIGHT,PixelFormat32bppPARGB);
TempGfx=new Graphics(TempBmp);
brush=new SolidBrush(TOOLTIP_BACK_COLOR);
font=new Font(_T(TOOLTIP_FONT_NAME),10,FontStyleBold);
TextRect=new RectF(0,TOOLTIP_TOP_BORDER,TOOLTIP_BASE_WIDTH,TOOLTIP_BASE_HEIGHT);
format=new StringFormat();

text->precision(2);
TempGfx->FillRectangle(brush,0,0,TOOLTIP_BASE_WIDTH,TOOLTIP_BASE_HEIGHT);
format->SetAlignment(StringAlignmentCenter);

brush->SetColor(TOOLTIP_COMMON_COLOR);

*text<<name.c_str();DrawToolTipText();

brush->SetColor(TOOLTIP_ATTRIBUTE_COLOR);
delete font;font=new Font(_T(TOOLTIP_FONT_NAME),8);

if (def!=0) {*text<<"Defense: "<<def;DrawToolTipText();}
if (dmg_high!=0) {*text<<"Damage: "<<dmg_low<<"-"<<dmg_high;DrawToolTipText();}

tooltip=new SpriteElement(&Globals->Buffer->graph);
tooltip->LoadBitmap(TempBmp,int(TOOLTIP_BASE_WIDTH-MaxWidth)/2-TOOLTIP_SIDE_BORDER,0,int(MaxWidth+TOOLTIP_SIDE_BORDER*2),int(TextRect->Y)+TOOLTIP_BOTTOM_BORDER);
}

void Item:rawToolTipText(string msg="")
{
if (msg!="")
{
text->str(_T(""));
*text<<msg.c_str();
}
if (text->str().length()==0) return;
TempGfx->DrawString(text->str().c_str(),-1,font,*TextRect,format,brush);
TempGfx->MeasureString(text->str().c_str(),-1,font,*TextRect,&compute);
TextRect->Y+=compute.Height;
if (compute.Width>MaxWidth) MaxWidth=compute.Width;
text->str(_T(""));
}

//SpriteElement contains a Bitmap, CachedBitmap, and a few other variables like x and y
//It also has some functions for drawing and processing images

void SpriteElement::LoadBitmap(Bitmap *Src, int fx, int fy, int fwidth, int fheight)
{
if ((fx<0)||(fy<0)) return;
if (UINT(fwidth) > Src->GetWidth() || UINT(height) > Src->GetHeight()) return;
RawBitmap=Src->Clone(fx,fy,fwidth,fheight,PixelFormat32bppPARGB);
}

//Buffer is type GraphicsPlane
void SpriteElement::ProcessImage()
{
width = RawBitmap->GetWidth();
height = RawBitmap->GetHeight();
image = new CachedBitmap(RawBitmap,&Globals->Buffer->graph);
}

class GraphicsPlane
{
public:
HDC hdc;
HBITMAP bmp;
HANDLE oldhandle;
Graphics graph;
HWND hWnd;
GraphicsPlane(HWND h,int bmpwidth=WindowWidth,int bmpheight=WindowHeight);
private:
HDC init(HWND nh,int bw,int bh);
}

HDC GraphicsPlane::init(HWND nh,int bw,int bh)
{
hdc = CreateCompatibleDC(GetDC(nh));
bmp = CreateCompatibleBitmap(GetDC(nh),bw,bh);
oldhandle = SelectObject(hdc,bmp);

return hdc;
}

GraphicsPlane::GraphicsPlane(HWND h,int bmpwidth,int bmpheight):graph(init(h,bmpwidth,bmpheight))
{
hWnd=h;
}

[Paul McKenzie]

Sorry if this is double-posted. I don't see my original post after waiting 2 days.

First, tell us what language you're using. If it's ANSI C++ (not Managed C++), you have memory and possibly resource leaks all over the place. I see no reason to be using "new" in the manner you're using it.

If it's Managed C++, please state this. Also, use code tags when posting code as your code is almost unreadable.

Regards,

Paul McKenzie

[Druzyek]

Hi Paul,
Thanks for your reply. My program is a Win32 project in Visual C++ 2010.

Sorry about not using code tags. I don't see them. Is it just CODE and /CODE in brackets?

As I said originally, I only posted the relevant parts. I commented out everything that checks pointers and deletes memory and I still have the same problem so I didn't paste any of the code that clean things up. The eight pointers at the beginning of Item::LoadToolTip are members instead of locals so that I can call Item:rawToolTipText without having to pass 8 arguments to it. If you see any memory or resource leaks other than that, I would really like to know.

[Paul McKenzie]

As I said originally, I only posted the relevant parts.

Your problem is a runtime issue. For others to solve it, we need to run your program and duplicate the error. For us to run the program, we need the code.

We also don't know when, where, or how any of that code is executed. Programs take a path of executtion, and we have no idea what that path of execution is. Just posting snippets of functions doesn't convey that information.

I commented out everything that checks pointers and deletes memory and I still have the same problem so I didn't paste any of the code that clean things up.

You're asking us to work with a subset of code that you believe is causing the problem. But what if it isn't the code you posted that is the root of the problem?

Second, mismanagement of pointers and dynamic memory can cause all sorts of trouble. So just saying "you don't need to see that part of the code" is not wise.

Looking at what you did post, here are the issues I am referring to when I say you're overusing operator new:

void Item::LoadToolTip()
{
   MaxWidth=0;
   //text=new wstringstream;  // why are you creating a stringstream dynamically???
    wstringstream text;

   text.precision(2);
   text<<name.c_str();DrawToolTipText();
}

That is only one example. A stringstream should be declared and used "locally", so trying to create one dynamically and hold onto it doesn't make sense and you're just making it easier to introduce a memory leak. Also, it seems you're trying to code C# or Java, not C++. Practically every variable you have is created dynamically, when it is unnecessary to do so for most of them (if not all of them).

Then you have this:

void Item:rawToolTipText(string msg="")
{
   if (msg!="")
  {
       text->str(_T(""));
      *text<<msg.c_str();
   }
   if (text->str().length()==0) return;
//..
}

Here is the same code, rewritten without using any streams:

void Item:rawToolTipText(const string& msg="")
{
   if ( msg.empty() )
      return;

   TempGfx->DrawString(msg.c_str(),-1,font,*TextRect,format,brush);
   TempGfx->MeasureString(msg.c_str(),-1,font,*TextRect,&compute);
   TextRect->Y+=compute.Height;
    if (compute.Width>MaxWidth) 
          MaxWidth=compute.Width;
}

Note the use of the reference parameter instead of passing by value, and the usage of the stream object is gone, as I saw no purpose in it.

But the bottom line is that we need to see the entire code, or you try to create a much smaller application that duplicates the problem.

Also, code tags are done this way:

[code]
Your code goes here:
[/code]


Regards,

Paul McKenzie

[Druzyek]

You are right that it doesn't make sense to allocate the text stream dynamically. I was playing around with different ideas when I was making the function and never changed the stream back to a regular variable which it should have been all along. Actually, I do need the stream. It is convenient to put whatever combination of text and numbers I want into the stream then pass the WCHAR string it generates to DrawString. Did you actually test the function you wrote? DrawString expects an array of WCHAR, not a simple char array like you are passing. As you probably know, mixing char and WCHAR can get complicated. Also, the reason I have the default msg paramater is just to make typing easier. If I want to draw a line of text with no numbers I can just pass the text as an argument and let the function put it into the stream rather than putting it into the stream first myself before calling the function. Maybe you can recommend a more convenient way for me to do it. Consider the next three lines. The first two are equivalent:

text<<"Item";DrawToolTipText();
DrawToolTipText("Item");
text<<"Average damage: "<<(weapon.dmg_high+weapon.dmg_low)/(2*weapon.speed)<<" damage per second";DrawToolTipText();

I was able to reproduce the error I am having. I made a new win32 project in Visual C++ 2010 Express and made the following changes:

Paste this at the top of WndProc right after "HDC hdc":

	static CachedBitmap *image;
	static ULONG_PTR gdiplusToken;
	static HDC hdc2;
	static HANDLE oldhandle;
	static HBITMAP bmp;
	static bool init=true;
	if (init)
	{
		GdiplusStartupInput gdiplusStartupInput;
		GdiplusStartup(&gdiplusToken, &gdiplusStartupInput, NULL);

		Bitmap *TempBmp=new Bitmap(100,100,PixelFormat32bppPARGB);
		Graphics TempGfx(TempBmp);
		SolidBrush *brush=new SolidBrush(0x800000FF);
		TempGfx.FillRectangle(brush,0,0,100,100);
		brush->SetColor(0xFF00FF00);
		TempGfx.FillRectangle(brush,25,25,50,50);
			
		hdc2 = CreateCompatibleDC(GetDC(hWnd));
		bmp = CreateCompatibleBitmap(GetDC(hWnd),300,300);
		oldhandle = SelectObject(hdc2,bmp);
			
		Graphics BuffGfx(hdc2);
		brush->SetColor(0xFFFF0000);
		BuffGfx.FillRectangle(brush,0,0,300,300);

		image=new CachedBitmap(TempBmp,&BuffGfx);
		BuffGfx.DrawCachedBitmap(image,100,100);
		
		delete brush;
		delete TempBmp;
		init=false;
	}

Draw the bitmap in the paint event:

	case WM_PAINT:
		hdc = BeginPaint(hWnd, &ps);
		BitBlt(hdc,0,0,300,300,hdc2,0,0,SRCCOPY);
		EndPaint(hWnd, &ps);
		break;

Don't forget to deinitialize GDI+, free the CachedBitmap, and call DeleteObj and DeleteDC when you exit.

This produces the same effect for me. On my computer with Windows 7 the blue square (which looks purple since it is half transparent on a red background) is drawn around the green rectangle as I expect. On XP it draws the blue rectangle at 0,0. As in my program DrawCachedBitmap is only called once.

[Paul McKenzie]

I was able to reproduce the error I am having. I made a new win32 project in Visual C++ 2010 Express and made the following changes:

Paste this at the top of WndProc right after "HDC hdc":

Please post the project (without the junk files such as .ncb, .obj, etc), not a code snippet and a description.

As to your code:

	static CachedBitmap *image;
	static ULONG_PTR gdiplusToken;
	static HDC hdc2;
	static HANDLE oldhandle;
	static HBITMAP bmp;
	static bool init=true;

Just to let you know, if your app is multithreaded, code like the above will not work reliably.

Second, you do no checking to see if any of those functions return an error. Instead, your code goes ahead and assumes those functions returned with a success code. You should be checking for errors and act accordingly.

Third:

    Bitmap *TempBmp=new Bitmap(100,100,PixelFormat32bppPARGB);
    SolidBrush *brush=new SolidBrush(0x800000FF);

This will be a resource leak if any of those functions after these lines throws an exception, or you modify your code to call functions that throw exceptions, or you modify your code to have another return point and you forget to call delete.

There is no need to create dynamically anyway:

Bitmap TempBmp(100,100,PixelFormat32bppPARGB);
SolidBrush brush(0x8000000FF);

Then if that block of code is exited for any reason, the bitmap and brush destructor will be called automatically. This makes the code, at least for those two items, exception safe.

Regards,

Paul McKenzie

[Druzyek]

Here is the source.

[Paul McKenzie]

Here is the source.

The fundamental question about your code is this:

You are calling all of the init code within the Window procedure, but what Window message is being sent when the init code is invoked? I don't know, and I bet neither do you. You just have init sitting there naked in the window procedure.

What I'm saying is that your init code may not work reliably, because you are relying on the state of the window to be OK for any graphics related processing. The init is called on the first message that the Windows OS sends to the window, and you have no idea what state the window is in at that point (or what the first message is). Maybe Windows 7 has a different architecture when it comes to creating windows than does XP, and the first message sent to an XP created window isn't for your use.

So in other words, your program is faulty -- it isn't guaranteed to work for XP or really any version of Windows. You are lucky it seems to work with Windows 7.

In general, you should never put code like that in a window proc without knowing what message is being sent. All code that I've ever written (and have seen written) in a window proc responds to a known message that is sent to the window (and I've been doing this for many years now).

Try this: instead of just placing init() inside of the window procedure, why not create a menu item, and when you select the menu item, then you draw your boxes using init. That way, you will need to capture a known message, and then the boxes will be drawn consistently. At the very least, if the boxes are still not drawn correctly, you eliminate the possibility of using the window prematurely before it's properly initialized.

Regards,

Paul McKenzie

[Druzyek]

Paul,
Thanks for looking at my code. No, it is not a fundamental question at all. I put everything there so it would be easier for you to copy and paste as an example. Of course everything is initialized elsewhere in my program. Try this version. It initializes fine on both systems but still shows the same problem.

[MastAvalons]

Hmm... I usually use deleaker or similar plugin for search GDI leaks in such cases... I'm lazy

[ovidiucucu]

I can presume it's a BitBlt issue so instead, try to "store" only the Bitmap object. Create CachedBitmap object then call DrawCachedBitmap directly in WM_PAINT message handler.

@MastAvalons: please stop posting off-topic!

[Druzyek]

ovidiucucu, thanks for your response. What you suggest seems to solve the problem but it won't work in my program. I want to draw lots of CachedBitmaps to the buffer in memory and then use BitBlt to draw the buffer all at once. If I call DrawCachedBitmap a lot of times in the WM_PAINT handler it probably won't look smooth. Anyway, it is good to know that we are narrowing the problem down.

[Druzyek]

Here is more information:

http://support.microsoft.com/kb/311221

It seems the problem was assuming the hdc never changes. By creating and deleting the hdc in the paint event instead of when the program loads I fixed the problem.

EDIT: I use an HBITMAP to do this. This is extremely slow. I'm still looking for a fast solution.

EDIT: Using CreateDIBSection instead of CreateCompatibleBitmap solves the problem. The program also runs about two times faster now! Problem solved as far as I'm concerned.