Archived

This topic is now archived and is closed to further replies.

ANSI2000

Win32 Windows and classes again. Something wrong...

Recommended Posts

We talked about this a while back... I have managed to wrap the win32 code that wraps up the window creation within classes.. But i thought I should give it a try to clean it up a bit a or rearange things they way "I think" they should be.... Of course this failed miserably, can some one look and tell me what is wrong? I want to implement the constructor and create() function as I have them shown, of course this might not be possible... It doesnt seem that the WM_CREATE ever happens....
// g3dgui.h
#ifndef G3DGUI_H
#define G3DGUI_H

// Required include files.
#include <windows.h>
#include <windowsx.h>

namespace G3DGUI
{
	class Object
	{
	public:

		Object(HINSTANCE pInstance, HWND pParent) : instance(pInstance), parent(pParent) {}
		virtual ~Object() {}

		const HINSTANCE getInstance() const { return(instance); }
		const HWND getParent() const { return(parent); }
		const HWND getHandle() const { return(handle); }
		const char* getClassName() const { return(className); }
		const getX() const { return(x); }		
		const getY() const { return(y); }		
		const getWidth() const { return(width); }		
		const getHeight() const { return(height); }		
		const unsigned int getStyleEx() const { return(styleEx); }
		const unsigned int getStyle() const { return(style); }
		const char* getCaption() const { return(caption); }			

		void create(const char *pClassName, int pX, int pY, int pWidth, int pHeight, unsigned int pStyle, unsigned int pStyleEx, const char *pCaption)
		{
			setClassName(pClassName);
			setX(pX);
			setY(pY);
			setWidth(pWidth);
			setHeight(pHeight);
			setStyle(pStyle);
			setStyleEx(pStyleEx);
			setCaption(pCaption);

			windowClass.lpfnWndProc = defaultProc;
			windowClass.hInstance = getInstance();
			windowClass.lpszClassName = getClassName();
			windowClass.cbSize = sizeof(WNDCLASSEX);
			windowClass.cbClsExtra = 0;
			windowClass.cbWndExtra = 0;
			windowClass.style = 0;
			windowClass.hCursor = LoadCursor(0, IDC_ARROW);
			windowClass.hbrBackground = (HBRUSH)GetStockObject(BLACK_BRUSH);
			windowClass.hIcon = LoadIcon(0, IDI_APPLICATION);
			windowClass.hIconSm = LoadIcon(0, IDI_APPLICATION);
			windowClass.lpszMenuName = 0;

			registerClass();
			
			handle = CreateWindowEx(getStyleEx(), getClassName(), getCaption(), getStyle(), 
									getX(), getY(), getWidth(), getHeight(), getParent(), 
									NULL, getInstance(), (LPSTR)this);
			update();
			
			show();
		}
		
		void update() { UpdateWindow(getHandle()); }
		void show()	{ ShowWindow(getHandle(), SW_SHOW); }
		void hide()	{ ShowWindow(getHandle(), SW_HIDE); }
		void focus() { SetFocus(getHandle()); }

		static LRESULT CALLBACK defaultProc(HWND hWnd, UINT Msg, WPARAM wParam, LPARAM lParam)
		{
			Object *thisObject = (Object*)GetWindowLong(hWnd, GWL_USERDATA);
			
			if(!thisObject)
			{
				if(Msg == WM_CREATE)
				{
					MessageBox(NULL, "WM_CREATE", "WM_CREATE", 0);
					LPCREATESTRUCT lpcs = (LPCREATESTRUCT)lParam;
					thisObject = (Object*)lpcs->lpCreateParams;
					SetWindowLong(hWnd, GWL_USERDATA, (LONG)thisObject);
					return thisObject->winProc(Msg, wParam, lParam);
				}
				else
					return DefWindowProc(hWnd, Msg, wParam, lParam); // should never be called
			}
			else
			{
				MessageBox(NULL, "winProc", "winProc", 0);
				return thisObject->winProc(Msg, wParam, lParam);
			}
		}
	
		virtual LRESULT winProc(UINT Msg, WPARAM wParam, LPARAM lParam) = 0;

	protected:

		void setClassName(const char *pClassName) { className = pClassName; }
		void setX(int pX) { x = pX; }		
		void setY(int pY) { y = pY; }		
		void setWidth(int pWidth) { width = pWidth; }		
		void setHeight(int pHeight) { height = pHeight; }		
		void setStyle(unsigned int pStyle) { style |= pStyle; }
		void setStyleEx(unsigned int pStyleEx) { styleEx |= pStyleEx; }
		void setCaption(const char *pCaption) { caption = pCaption; }

		bool registerClass()
		{
			if(!RegisterClassEx(&windowClass))
				return(false);
			else
				return(true);
		}
		
		void unRegisterClass()
		{
			UnregisterClass(getClassName(), getInstance());
		}

	private:
		HINSTANCE instance;
		HWND parent;
		HWND handle;
		WNDCLASSEX windowClass;
		const char* className;
		const char*	caption;
		int x;
		int	y;
		int	width;
		int	height;
		unsigned int style;
		unsigned int styleEx;
	};
}

#endif

// g3dgui.cpp
//Required include files.

//Application include files.
#include "g3dgui.h"
// Nothing in here all of it is inlined in the g3dgui.h file.

//winmain.cpp
#include <g3dgui.h>

using namespace G3DGUI;

bool running = true;

class TestWindow : public Object
{	
public:
	
	TestWindow(HINSTANCE pInstance, HWND pParent) : Object(pInstance, pParent){}

protected:

	LRESULT winProc(UINT Msg, WPARAM wParam, LPARAM lParam);
};

LRESULT TestWindow::winProc(UINT Msg, WPARAM wParam, LPARAM lParam)
{
	switch (Msg)
	{
	case WM_CREATE:
		MessageBox(NULL, "dood", "doods", 0);
	case WM_DESTROY:
		PostQuitMessage(0);

		running = false;

		break;
	default:
		return DefWindowProc(getHandle(), Msg, wParam, lParam);
	}
	return 0;
}

int APIENTRY WinMain(HINSTANCE hInstance, HINSTANCE hPrevInstance, LPSTR lpCmdLine, int nCmdShow)
{
	TestWindow testWindow(hInstance, NULL);

	testWindow.create("My Window", 10, 10, 100, 100, WS_OVERLAPPEDWINDOW | WS_CLIPSIBLINGS | WS_CLIPCHILDREN, NULL, "My Window");

	while(running)
	{
		MSG msg;

		while(PeekMessage(&msg, NULL, 0, 0, PM_REMOVE))
		{
			TranslateMessage(&msg);
			DispatchMessage(&msg);
		}	
	}


	return(0);
}
 

Share this post


Link to post
Share on other sites
Try removing the condition around the WM_CREATE in Object::defaultProc - do you know that GWL_USERDATA is always going to be 0 at the WM_CREATE stage ?

It''s unnecessary anyway, you are only ever going to get a WM_CREATE once per window instance.

Have you tried stepping through the code with the debugger and seeing where it fails ?

HTH,
t0ne.



Slightly shrimpy smell == unsafe breadbin

Share this post


Link to post
Share on other sites
So, the CreateWindowEx call executes but returns NULL, yea ?
I would check then that the hInstance is valid and that you''ve correctly registered the WNDCLASS amd check your window styles are a correct combination. Also what does GetLastError return after the failure ?

A small point, although it probably works OK in this instance your setClassName and setCaption methods are unsafe. You should allocate some memory and store the contents of the buffers rather than the pointer to it (it could go out of scope, get modified elsewhere etc.).



Slightly shrimpy smell == unsafe breadbin

Share this post


Link to post
Share on other sites
The class does register correctly...

To check if the instance is correct I do what check for a null value?

But you should be able to pass an instance of null and it should work anyways

Share this post


Link to post
Share on other sites
Tone -> having a char* as paramter cant go out of scope : heres an example :

void setCaption(const char* cap)

the calling

wnd.setCaption("hello");

in this case "hello" is static and can never be modified.

Share this post


Link to post
Share on other sites
I think the very ''1st'' few messages that Windows send is not WM_CREATE. I think WM_GETMINMAXINFO or WM_NCCREATE came before WM_CREATE, and the WM_GETMINMAXINFO is the 1st.

I suggest that WM_NCCREATE would be a nice spot for SetWindowLong

Share this post


Link to post
Share on other sites
quote:
Original post by hotdot
Tone -> having a char* as paramter cant go out of scope : heres an example :

void setCaption(const char* cap)

the calling

wnd.setCaption("hello");

in this case "hello" is static and can never be modified.


In this particular case this is true. But how about:

char buffer[256];
strcpy(buffer, "Hello");

setCaption(buffer);

strcpy(buffer, "something else");

or if the above code was done inside a function, buffer would be created on the stack and lost when it went out of scope.




Slightly shrimpy smell == unsafe breadbin

Share this post


Link to post
Share on other sites
quote:
Original post by ANSI2000
The class does register correctly...

To check if the instance is correct I do what check for a null value?

But you should be able to pass an instance of null and it should work anyways


No, AFAIK you need a valid instance handle, and it (usually) needs to be the same hInstance that registers the WNDCLASS and creates the window. You can use a NULL hInstance for things like LoadIcon to load a system icon.

You can obtain one by calling GetModuleHandle(NULL) if you haven't got access to the one passed to your WinMain.



Slightly shrimpy smell == unsafe breadbin

Edited by - t0ne on February 21, 2002 1:37:54 PM

Share this post


Link to post
Share on other sites
quote:
Original post by DerekSaw
I think the very ''1st'' few messages that Windows send is not WM_CREATE. I think WM_GETMINMAXINFO or WM_NCCREATE came before WM_CREATE, and the WM_GETMINMAXINFO is the 1st.

I suggest that WM_NCCREATE would be a nice spot for SetWindowLong


That''s true, although doing this in WM_CREATE has worked fine for me for donkeys years. On the occaisions I''ve needed to access WM_GETMINMAXINFO I''ve tested the object instance is not NULL first.

Share this post


Link to post
Share on other sites
But if you look at the code I do use the hInstance passed from winmain....

And the windowclass does register it doesn''t fail.

I know what the problem is I think...

The problen can be fixed using a copy constructor.

I directly hardcoded the styleEx and style in the create method and it works.... When your passing the this pointer to the Windows user data. The class data is not beeing copied over...

I will create a copy constructor to initialize all the data and see what hapens....

Share this post


Link to post
Share on other sites
quote:
Original post by ANSI2000
I know what the problem is I think...

The problen can be fixed using a copy constructor.

I directly hardcoded the styleEx and style in the create method and it works.... When your passing the this pointer to the Windows user data. The class data is not beeing copied over...

I will create a copy constructor to initialize all the data and see what hapens....


That''s a red herring - you''re passing a pointer so no copy constructor is invoked.
I think I know what it is ...
You don''t seem to be initialising anything though, and when you set your window styles in the setStyle and setStyleEx methods you are effectively OR''ing them into garbage values. Initialise these variables to zero in the constructor an see if that helps.


Slightly shrimpy smell == unsafe breadbin

Share this post


Link to post
Share on other sites
This is a mistake a lot of people do when deeling with classes and function pointers.
you might want to look at THIS for info on how to create it safe.

- err, the last signiture sucked bigtime!

Share this post


Link to post
Share on other sites
Finally decided to try your revisions it out I see.
Only took, what... four months?

"Don''t be afraid to dream, for out of such fragile things come miracles."

Share this post


Link to post
Share on other sites
Hmm, first before I start let me say that this is not negative criticism, but just a few coments that I think would help. First name your clases something relevent! like hmmmm....say CWindow? Next I find that its easier to serperate your setting up the window structure and registering it and calling creatwindowex. So id suggest setting up your wndClass in your class constructor and then registering it there. If it failed set a falg, then in your create function first test if there was a failure, if not call create window. Ill give you the code a consrtuctor and a create function i use:

  
CBWindow::CBWindow(const HINSTANCE& hInstance,char* pszClassName, const WNDPROC WndProc )
: hInst(hInstance), bInitFail(false), pszCName(pszClassName)
{ wndClass.cbSize = sizeof(WNDCLASSEX);
wndClass.cbClsExtra = 0;
wndClass.cbWndExtra = 0;
wndClass.hbrBackground = (HBRUSH)COLOR_WINDOW;
wndClass.hInstance = hInstance;
wndClass.hCursor = LoadCursor(NULL,IDC_ARROW);
wndClass.hIcon = LoadIcon(NULL,IDI_APPLICATION);
wndClass.hIconSm = LoadIcon(NULL,IDI_APPLICATION);
wndClass.lpszClassName = pszClassName;
wndClass.lpszMenuName = NULL;
wndClass.style = CS_OWNDC | CS_VREDRAW | CS_HREDRAW;

if(WndProc)
{ wndClass.lpfnWndProc = WndProc;

}
else
{ wndClass.lpfnWndProc = &(this->WindowProc);
}

if(!RegisterClassEx(&wndClass))
{ MessageBox(NULL,"ERROR IN APPLICATION STARTUP","Error In Registering Window Class",MB_OK);
bInitFail = true;
}
}

void CBWindow::CreateWin(const DWORD& dwExStyle, char* pszWinName, const DWORD& dwStyle,
unsigned int ucXPos, unsigned int ucYPos, unsigned int ucXSize,
unsigned int ucYSize)
{ if(!bInitFail) // IF Registering Window Class Did Not Fail

{ hWnd = CreateWindowEx(dwExStyle, // Extened Window Style

pszCName, // Window Class Name

(LPCSTR)pszWinName, // Window Name

dwStyle, // Window Style

ucXPos, // Window X Pos

ucYPos, // Window Y Pos

ucXSize, // Window X Size

ucYSize, // Window Y Size

NULL, // Parent Handle

NULL, // Menu Handle

hInst, // Applicatino Instance

(void*)this); // LPVOID lpParam

}
if(!hWnd) // If Creating the Window Failed

{ bInitFail = true;
Message("Error Creating Window","ERROR IN APPLICATION INITIALIZATION");
hWnd = NULL;
}
else
{ SetWindowLong(hWnd,GWL_USERDATA,(long)this);

//SetProp(hWnd,"CConvWindowAtom",(HANDLE)this);


ShowWindow(hWnd,SW_SHOW);
UpdateWindow(hWnd);

}
}




"Computer games don't affect kids; I mean if Pac-Man affected us as kids, we'd all be running around in darkened rooms, munching magic pills and listening to repetitive electronic music."
- Kristian Wilson, Nintendo, Inc, 1989


Edited by - xds4lx on February 22, 2002 1:41:12 AM

Share this post


Link to post
Share on other sites
Yeah RedLeaf more like 2 months or so You know when you have a "real" job as a programmer home projects tend to get put off
Dont worry pixel by pixel I will finish my game

Thanks xDS4Lx, the problem was fixed, it was because I was oring my styles into garbage...

Share this post


Link to post
Share on other sites
quote:
Original post by xDS4Lx
So id suggest setting up your wndClass in your class constructor and then registering it there.

Two problems with this:
  1. You end up attempting to reregister the windowclass for every window (of the same type) that you create

  2. Even if the windowclass registration fails, the constructor is still successful. Constructors never fail.


[ GDNet Start Here | GDNet Search Tool | GDNet FAQ | MS RTFM [MSDN] | SGI STL Docs | Google! ]
Thanks to Kylotan for the idea!

Share this post


Link to post
Share on other sites
ANSI2000 has a point about inducing a constructor failure.

However, ANSI... I don''t think I mentioned this, but I originally intended this to be used for getting a window up and running with very little code, and condensed the abstraction of WNDCLASS with an actual instance of such.

If you wish to make multiple instances, or even a more complex windows-based GUI, you really should separate these abstractions as they should be. Something like Window vs. WindowDesc (or your own analogous term) comes to mind.

If this is the case, I definitely think the separation would be a much cleaner design, and I highly recommend reading the windows programming sections at this site.

For a simple one-window app however, I think you''re still in the clear.

"Don''t be afraid to dream, for out of such fragile things come miracles."

Share this post


Link to post
Share on other sites
Well for now I just wanted to rearange things a bit in the class the way I understand it. Eventually I will massage it around even more...

Ill check out the link...

Thanks.

Share this post


Link to post
Share on other sites