Flickering bug for World of Warcraft

[Bertrand Coconnier]

Hi,

After investigating a bit in the Wine code, it seems that the flickering 
bug of WoW in OpenGL mode is due to wglGetPbufferDCARB that returns a DC 
which type is OBJ_MEMDC while it should return a DC which type is 
OBJ_DC. And since in the current git tree, the DC type of a Pbuffer is 
OBJ_MEMDC then the code located in lines 1394-1395 of 
dlls/winex11.drv/opengl.c is executed : the Pbuffer is copied to the 
screen each time the GL context is made current to the Pbuffer hence the 
flickering (WoW bounds the GL context alternatively to the screen and to 
the Pbuffer). The expected behaviour is not to copy the Pbuffer content 
to the screen since WoW does it itself.

The easiest workaround for this bug would be to make 
X11DRV_wglGetPbufferDCARB to return the Pbuffer's object->hdc (this 
fixes the flickering bug). However, I have made some tests with Windows 
XP and it appears that wglGetPbufferDCARB returns a HDC different from 
the HDC passed when wglCreatePbufferARB was called. So such a patch 
would be a dirty hack not a real fix.

I am not familiar enough with Wine to submit a patch so I leave the 
gurus out there find an elegant solution.

Attached is the code that I used to perform a few tests about Pbuffers 
on Windows XP (code of interest is located from line 293 to line 298).

Regards,

Bertrand.

/*
 *		This Code Was Created By Jeff Molofee 2000
 *		A HUGE Thanks To Fredric Echols For Cleaning Up
 *		And Optimizing This Code, Making It More Flexible!
 *		If You've Found This Code Useful, Please Let Me Know.
 *		Visit My Site At nehe.gamedev.net
 */

#include <windows.h>		// Header File For Windows
#include <gl\gl.h>			// Header File For The OpenGL32 Library
#include <gl\glu.h>			// Header File For The GLu32 Library
#include <gl\glaux.h>		// Header File For The Glaux Library

HDC			hDC=NULL;		// Private GDI Device Context
HGLRC		hRC=NULL;		// Permanent Rendering Context
HWND		hWnd=NULL;		// Holds Our Window Handle
HINSTANCE	hInstance;		// Holds The Instance Of The Application

bool	keys[256];			// Array Used For The Keyboard Routine
bool	active=TRUE;		// Window Active Flag Set To TRUE By Default
bool	fullscreen=TRUE;	// Fullscreen Flag Set To Fullscreen Mode By Default

LRESULT	CALLBACK WndProc(HWND, UINT, WPARAM, LPARAM);	// Declaration For WndProc

DECLARE_HANDLE(HPBUFFERARB);

typedef HPBUFFERARB (APIENTRY * wglCreatePbufferARB_t)(HDC hDC,
    			    int iPixelFormat,
    			    int iWidth,
    			    int iHeight,
    			    const int *piAttribList);

typedef HDC (APIENTRY * wglGetPbufferDCARB_t)(HPBUFFERARB hPbuffer);

typedef int (APIENTRY * wglReleasePbufferDCARB_t)(HPBUFFERARB hPbuffer, HDC hDC);

typedef BOOL (APIENTRY * wglDestroyPbufferARB_t)(HPBUFFERARB hPbuffer);

wglCreatePbufferARB_t wglCreatePbufferARB = NULL;
wglGetPbufferDCARB_t wglGetPbufferDCARB = NULL;
wglReleasePbufferDCARB_t wglReleasePbufferDCARB = NULL;
wglDestroyPbufferARB_t wglDestroyPbufferARB = NULL;

GLvoid ReSizeGLScene(GLsizei width, GLsizei height)		// Resize And Initialize The GL Window
{
    if (height==0)										// Prevent A Divide By Zero By
    {
    	height=1;										// Making Height Equal One
    }

glViewport(0,0,width,height);						// Reset The Current Viewport

glMatrixMode(GL_PROJECTION);						// Select The Projection Matrix
    glLoadIdentity();									// Reset The Projection Matrix

// Calculate The Aspect Ratio Of The Window
    gluPerspective(45.0f,(GLfloat)width/(GLfloat)height,0.1f,100.0f);

glMatrixMode(GL_MODELVIEW);							// Select The Modelview Matrix
    glLoadIdentity();									// Reset The Modelview Matrix
}

int InitGL(GLvoid)										// All Setup For OpenGL Goes Here
{
    glShadeModel(GL_SMOOTH);							// Enable Smooth Shading
    glClearColor(0.0f, 0.0f, 0.0f, 0.5f);				// Black Background
    glClearDepth(1.0f);									// Depth Buffer Setup
    glEnable(GL_DEPTH_TEST);							// Enables Depth Testing
    glDepthFunc(GL_LEQUAL);								// The Type Of Depth Testing To Do
    glHint(GL_PERSPECTIVE_CORRECTION_HINT, GL_NICEST);	// Really Nice Perspective Calculations
    return TRUE;										// Initialization Went OK
}

int DrawGLScene(GLvoid)									// Here's Where We Do All The Drawing
{
    glClear(GL_COLOR_BUFFER_BIT | GL_DEPTH_BUFFER_BIT);	// Clear Screen And Depth Buffer
    glLoadIdentity();									// Reset The Current Modelview Matrix
    return TRUE;										// Everything Went OK
}

GLvoid KillGLWindow(GLvoid)								// Properly Kill The Window
{
    if (fullscreen)										// Are We In Fullscreen Mode?
    {
    	ChangeDisplaySettings(NULL,0);					// If So Switch Back To The Desktop
    	ShowCursor(TRUE);								// Show Mouse Pointer
    }

if (hRC)											// Do We Have A Rendering Context?
    {
    	if (!wglMakeCurrent(NULL,NULL))					// Are We Able To Release The DC And RC Contexts?
    	{
    		MessageBox(NULL,"Release Of DC And RC Failed.","SHUTDOWN ERROR",MB_OK | MB_ICONINFORMATION);
    	}

if (!wglDeleteContext(hRC))						// Are We Able To Delete The RC?
    	{
    		MessageBox(NULL,"Release Rendering Context Failed.","SHUTDOWN ERROR",MB_OK | MB_ICONINFORMATION);
    	}
    	hRC=NULL;										// Set RC To NULL
    }

if (hDC && !ReleaseDC(hWnd,hDC))					// Are We Able To Release The DC
    {
    	MessageBox(NULL,"Release Device Context Failed.","SHUTDOWN ERROR",MB_OK | MB_ICONINFORMATION);
    	hDC=NULL;										// Set DC To NULL
    }

if (hWnd && !DestroyWindow(hWnd))					// Are We Able To Destroy The Window?
    {
    	MessageBox(NULL,"Could Not Release hWnd.","SHUTDOWN ERROR",MB_OK | MB_ICONINFORMATION);
    	hWnd=NULL;										// Set hWnd To NULL
    }

if (!UnregisterClass("OpenGL",hInstance))			// Are We Able To Unregister Class
    {
    	MessageBox(NULL,"Could Not Unregister Class.","SHUTDOWN ERROR",MB_OK | MB_ICONINFORMATION);
    	hInstance=NULL;									// Set hInstance To NULL
    }
}

/*	This Code Creates Our OpenGL Window.  Parameters Are:					*
 *	title			- Title To Appear At The Top Of The Window				*
 *	width			- Width Of The GL Window Or Fullscreen Mode				*
 *	height			- Height Of The GL Window Or Fullscreen Mode			*
 *	bits			- Number Of Bits To Use For Color (8/16/24/32)			*
 *	fullscreenflag	- Use Fullscreen Mode (TRUE) Or Windowed Mode (FALSE)	*/

BOOL CreateGLWindow(char* title, int width, int height, int bits, bool fullscreenflag)
{
    GLuint		PixelFormat;			// Holds The Results After Searching For A Match
    WNDCLASS	wc;						// Windows Class Structure
    DWORD		dwExStyle;				// Window Extended Style
    DWORD		dwStyle;				// Window Style
    RECT		WindowRect;				// Grabs Rectangle Upper Left / Lower Right Values
    WindowRect.left=(long)0;			// Set Left Value To 0
    WindowRect.right=(long)width;		// Set Right Value To Requested Width
    WindowRect.top=(long)0;				// Set Top Value To 0
    WindowRect.bottom=(long)height;		// Set Bottom Value To Requested Height

fullscreen=fullscreenflag;			// Set The Global Fullscreen Flag

hInstance			= GetModuleHandle(NULL);				// Grab An Instance For Our Window
    wc.style			= CS_HREDRAW | CS_VREDRAW | CS_OWNDC;	// Redraw On Size, And Own DC For Window.
    wc.lpfnWndProc		= (WNDPROC) WndProc;					// WndProc Handles Messages
    wc.cbClsExtra		= 0;									// No Extra Window Data
    wc.cbWndExtra		= 0;									// No Extra Window Data
    wc.hInstance		= hInstance;							// Set The Instance
    wc.hIcon			= LoadIcon(NULL, IDI_WINLOGO);			// Load The Default Icon
    wc.hCursor			= LoadCursor(NULL, IDC_ARROW);			// Load The Arrow Pointer
    wc.hbrBackground	= NULL;									// No Background Required For GL
    wc.lpszMenuName		= NULL;									// We Don't Want A Menu
    wc.lpszClassName	= "OpenGL";								// Set The Class Name

HPBUFFERARB pBuffer = NULL;
    #define WGL_PBUFFER_LARGEST_ARB 0x2033
    int attrib[] = {WGL_PBUFFER_LARGEST_ARB, 0, 0};
    BOOL success;
    HDC pBufferHDC;
    DWORD type1 = 0;
    DWORD type2 = 0;
    int result = 0xdeadbeef;

if (!RegisterClass(&wc))									// Attempt To Register The Window Class
    {
    	MessageBox(NULL,"Failed To Register The Window Class.","ERROR",MB_OK|MB_ICONEXCLAMATION);
    	return FALSE;											// Return FALSE
    }
    
    if (fullscreen)												// Attempt Fullscreen Mode?
    {
    	DEVMODE dmScreenSettings;								// Device Mode
    	memset(&dmScreenSettings,0,sizeof(dmScreenSettings));	// Makes Sure Memory's Cleared
    	dmScreenSettings.dmSize=sizeof(dmScreenSettings);		// Size Of The Devmode Structure
    	dmScreenSettings.dmPelsWidth	= width;				// Selected Screen Width
    	dmScreenSettings.dmPelsHeight	= height;				// Selected Screen Height
    	dmScreenSettings.dmBitsPerPel	= bits;					// Selected Bits Per Pixel
    	dmScreenSettings.dmFields=DM_BITSPERPEL|DM_PELSWIDTH|DM_PELSHEIGHT;

// Try To Set Selected Mode And Get Results.  NOTE: CDS_FULLSCREEN Gets Rid Of Start Bar.
    	if (ChangeDisplaySettings(&dmScreenSettings,CDS_FULLSCREEN)!=DISP_CHANGE_SUCCESSFUL)
    	{
    		// If The Mode Fails, Offer Two Options.  Quit Or Use Windowed Mode.
    		if (MessageBox(NULL,"The Requested Fullscreen Mode Is Not Supported By\nYour Video Card. Use Windowed Mode Instead?","NeHe GL",MB_YESNO|MB_ICONEXCLAMATION)==IDYES)
    		{
    			fullscreen=FALSE;		// Windowed Mode Selected.  Fullscreen = FALSE
    		}
    		else
    		{
    			// Pop Up A Message Box Letting User Know The Program Is Closing.
    			MessageBox(NULL,"Program Will Now Close.","ERROR",MB_OK|MB_ICONSTOP);
    			return FALSE;									// Return FALSE
    		}
    	}
    }

if (fullscreen)												// Are We Still In Fullscreen Mode?
    {
    	dwExStyle=WS_EX_APPWINDOW;								// Window Extended Style
    	dwStyle=WS_POPUP;										// Windows Style
    	ShowCursor(FALSE);										// Hide Mouse Pointer
    }
    else
    {
    	dwExStyle=WS_EX_APPWINDOW | WS_EX_WINDOWEDGE;			// Window Extended Style
    	dwStyle=WS_OVERLAPPEDWINDOW;							// Windows Style
    }

AdjustWindowRectEx(&WindowRect, dwStyle, FALSE, dwExStyle);		// Adjust Window To True Requested Size

// Create The Window
    if (!(hWnd=CreateWindowEx(	dwExStyle,							// Extended Style For The Window
    							"OpenGL",							// Class Name
    							title,								// Window Title
    							dwStyle |							// Defined Window Style
    							WS_CLIPSIBLINGS |					// Required Window Style
    							WS_CLIPCHILDREN,					// Required Window Style
    							0, 0,								// Window Position
    							WindowRect.right-WindowRect.left,	// Calculate Window Width
    							WindowRect.bottom-WindowRect.top,	// Calculate Window Height
    							NULL,								// No Parent Window
    							NULL,								// No Menu
    							hInstance,							// Instance
    							NULL)))								// Dont Pass Anything To WM_CREATE
    {
    	KillGLWindow();								// Reset The Display
    	MessageBox(NULL,"Window Creation Error.","ERROR",MB_OK|MB_ICONEXCLAMATION);
    	return FALSE;								// Return FALSE
    }

static	PIXELFORMATDESCRIPTOR pfd=				// pfd Tells Windows How We Want Things To Be
    {
    	sizeof(PIXELFORMATDESCRIPTOR),				// Size Of This Pixel Format Descriptor
    	1,											// Version Number
    	PFD_DRAW_TO_WINDOW |						// Format Must Support Window
    	PFD_SUPPORT_OPENGL |						// Format Must Support OpenGL
    	PFD_DOUBLEBUFFER,							// Must Support Double Buffering
    	PFD_TYPE_RGBA,								// Request An RGBA Format
    	bits,										// Select Our Color Depth
    	0, 0, 0, 0, 0, 0,							// Color Bits Ignored
    	0,											// No Alpha Buffer
    	0,											// Shift Bit Ignored
    	0,											// No Accumulation Buffer
    	0, 0, 0, 0,									// Accumulation Bits Ignored
    	16,											// 16Bit Z-Buffer (Depth Buffer)  
    	0,											// No Stencil Buffer
    	0,											// No Auxiliary Buffer
    	PFD_MAIN_PLANE,								// Main Drawing Layer
    	0,											// Reserved
    	0, 0, 0										// Layer Masks Ignored
    };
    
    if (!(hDC=GetDC(hWnd)))							// Did We Get A Device Context?
    {
    	KillGLWindow();								// Reset The Display
    	MessageBox(NULL,"Can't Create A GL Device Context.","ERROR",MB_OK|MB_ICONEXCLAMATION);
    	return FALSE;								// Return FALSE
    }

if (!(PixelFormat=ChoosePixelFormat(hDC,&pfd)))	// Did Windows Find A Matching Pixel Format?
    {
    	KillGLWindow();								// Reset The Display
    	MessageBox(NULL,"Can't Find A Suitable PixelFormat.","ERROR",MB_OK|MB_ICONEXCLAMATION);
    	return FALSE;								// Return FALSE
    }

if(!SetPixelFormat(hDC,PixelFormat,&pfd))		// Are We Able To Set The Pixel Format?
    {
    	KillGLWindow();								// Reset The Display
    	MessageBox(NULL,"Can't Set The PixelFormat.","ERROR",MB_OK|MB_ICONEXCLAMATION);
    	return FALSE;								// Return FALSE
    }

if (!(hRC=wglCreateContext(hDC)))				// Are We Able To Get A Rendering Context?
    {
    	KillGLWindow();								// Reset The Display
    	MessageBox(NULL,"Can't Create A GL Rendering Context.","ERROR",MB_OK|MB_ICONEXCLAMATION);
    	return FALSE;								// Return FALSE
    }

if(!wglMakeCurrent(hDC,hRC))					// Try To Activate The Rendering Context
    {
    	KillGLWindow();								// Reset The Display
    	MessageBox(NULL,"Can't Activate The GL Rendering Context.","ERROR",MB_OK|MB_ICONEXCLAMATION);
    	return FALSE;								// Return FALSE
    }

wglCreatePbufferARB = (wglCreatePbufferARB_t)wglGetProcAddress("wglCreatePbufferARB");
    wglGetPbufferDCARB = (wglGetPbufferDCARB_t)wglGetProcAddress("wglGetPbufferDCARB");
    wglReleasePbufferDCARB = (wglReleasePbufferDCARB_t)wglGetProcAddress("wglReleasePbufferDCARB");
    wglDestroyPbufferARB = (wglDestroyPbufferARB_t)wglGetProcAddress("wglDestroyPbufferARB");

pBuffer = wglCreatePbufferARB(hDC, PixelFormat, width, height, attrib);
    pBufferHDC = wglGetPbufferDCARB(pBuffer);
    type1 = GetObjectType(hDC);
    type2 = GetObjectType(pBufferHDC);
    result = wglReleasePbufferDCARB(pBuffer, pBufferHDC);
    success = wglDestroyPbufferARB(pBuffer);

ShowWindow(hWnd,SW_SHOW);						// Show The Window
    SetForegroundWindow(hWnd);						// Slightly Higher Priority
    SetFocus(hWnd);									// Sets Keyboard Focus To The Window
    ReSizeGLScene(width, height);					// Set Up Our Perspective GL Screen

if (!InitGL())									// Initialize Our Newly Created GL Window
    {
    	KillGLWindow();								// Reset The Display
    	MessageBox(NULL,"Initialization Failed.","ERROR",MB_OK|MB_ICONEXCLAMATION);
    	return FALSE;								// Return FALSE
    }

return TRUE;									// Success
}

LRESULT CALLBACK WndProc(	HWND	hWnd,			// Handle For This Window
    						UINT	uMsg,			// Message For This Window
    						WPARAM	wParam,			// Additional Message Information
    						LPARAM	lParam)			// Additional Message Information
{
    switch (uMsg)									// Check For Windows Messages
    {
    	case WM_ACTIVATE:							// Watch For Window Activate Message
    	{
    		if (!HIWORD(wParam))					// Check Minimization State
    		{
    			active=TRUE;						// Program Is Active
    		}
    		else
    		{
    			active=FALSE;						// Program Is No Longer Active
    		}

return 0;								// Return To The Message Loop
    	}

case WM_SYSCOMMAND:							// Intercept System Commands
    	{
    		switch (wParam)							// Check System Calls
    		{
    			case SC_SCREENSAVE:					// Screensaver Trying To Start?
    			case SC_MONITORPOWER:				// Monitor Trying To Enter Powersave?
    			return 0;							// Prevent From Happening
    		}
    		break;									// Exit
    	}

case WM_CLOSE:								// Did We Receive A Close Message?
    	{
    		PostQuitMessage(0);						// Send A Quit Message
    		return 0;								// Jump Back
    	}

case WM_KEYDOWN:							// Is A Key Being Held Down?
    	{
    		keys[wParam] = TRUE;					// If So, Mark It As TRUE
    		return 0;								// Jump Back
    	}

case WM_KEYUP:								// Has A Key Been Released?
    	{
    		keys[wParam] = FALSE;					// If So, Mark It As FALSE
    		return 0;								// Jump Back
    	}

case WM_SIZE:								// Resize The OpenGL Window
    	{
    		ReSizeGLScene(LOWORD(lParam),HIWORD(lParam));  // LoWord=Width, HiWord=Height
    		return 0;								// Jump Back
    	}
    }

// Pass All Unhandled Messages To DefWindowProc
    return DefWindowProc(hWnd,uMsg,wParam,lParam);
}

int WINAPI WinMain(	HINSTANCE	hInstance,			// Instance
    				HINSTANCE	hPrevInstance,		// Previous Instance
    				LPSTR		lpCmdLine,			// Command Line Parameters
    				int			nCmdShow)			// Window Show State
{
    MSG		msg;									// Windows Message Structure
    BOOL	done=FALSE;								// Bool Variable To Exit Loop

// Ask The User Which Screen Mode They Prefer
    if (MessageBox(NULL,"Would You Like To Run In Fullscreen Mode?", "Start FullScreen?",MB_YESNO|MB_ICONQUESTION)==IDNO)
    {
    	fullscreen=FALSE;							// Windowed Mode
    }

// Create Our OpenGL Window
    if (!CreateGLWindow("NeHe's OpenGL Framework",640,480,16,fullscreen))
    {
    	return 0;									// Quit If Window Was Not Created
    }

while(!done)									// Loop That Runs While done=FALSE
    {
    	if (PeekMessage(&msg,NULL,0,0,PM_REMOVE))	// Is There A Message Waiting?
    	{
    		if (msg.message==WM_QUIT)				// Have We Received A Quit Message?
    		{
    			done=TRUE;							// If So done=TRUE
    		}
    		else									// If Not, Deal With Window Messages
    		{
    			TranslateMessage(&msg);				// Translate The Message
    			DispatchMessage(&msg);				// Dispatch The Message
    		}
    	}
    	else										// If There Are No Messages
    	{
    		// Draw The Scene.  Watch For ESC Key And Quit Messages From DrawGLScene()
    		if (active)								// Program Active?
    		{
    			if (keys[VK_ESCAPE])				// Was ESC Pressed?
    			{
    				done=TRUE;						// ESC Signalled A Quit
    			}
    			else								// Not Time To Quit, Update Screen
    			{
    				DrawGLScene();					// Draw The Scene
    				SwapBuffers(hDC);				// Swap Buffers (Double Buffering)
    			}
    		}

if (keys[VK_F1])						// Is F1 Being Pressed?
    		{
    			keys[VK_F1]=FALSE;					// If So Make Key FALSE
    			KillGLWindow();						// Kill Our Current Window
    			fullscreen=!fullscreen;				// Toggle Fullscreen / Windowed Mode
    			// Recreate Our OpenGL Window
    			if (!CreateGLWindow("NeHe's OpenGL Framework",640,480,16,fullscreen))
    			{
    				return 0;						// Quit If Window Was Not Created
    			}
    		}
    	}
    }

// Shutdown
    KillGLWindow();									// Kill The Window
    return (msg.wParam);							// Exit The Program
}

[Jesse Allen]

On 11/3/06, Bertrand Coconnier  wrote:
> Hi,
>
> After investigating a bit in the Wine code, it seems that the flickering
> bug of WoW in OpenGL mode is due to wglGetPbufferDCARB that returns a DC
> which type is OBJ_MEMDC while it should return a DC which type is
> OBJ_DC. And since in the current git tree, the DC type of a Pbuffer is
> OBJ_MEMDC then the code located in lines 1394-1395 of
> dlls/winex11.drv/opengl.c is executed : the Pbuffer is copied to the
> screen each time the GL context is made current to the Pbuffer hence the
> flickering (WoW bounds the GL context alternatively to the screen and to
> the Pbuffer). The expected behaviour is not to copy the Pbuffer content
> to the screen since WoW does it itself.
>
> The easiest workaround for this bug would be to make
> X11DRV_wglGetPbufferDCARB to return the Pbuffer's object->hdc (this
> fixes the flickering bug). However, I have made some tests with Windows
> XP and it appears that wglGetPbufferDCARB returns a HDC different from
> the HDC passed when wglCreatePbufferARB was called. So such a patch
> would be a dirty hack not a real fix.

http://oss.sgi.com/projects/ogl-sample/registry/ARB/wgl_pbuffer.txt

"To create a device context for the pbuffer, call

HDC wglGetPbufferDCARB(HPBUFFERARB hPbuffer);

where <hPbuffer> is a handle returned from a previous call to
wglCreatePbufferARB.  A device context is returned by
wglGetPbufferDCARB which can be used to associate a rendering
context with the pbuffer.  Any rendering context created with
a wglCreateContext that is "compatible" with the <iPixelFormat> may
be used to render into the pbuffer."

So yes, it's supposed to return a new one.

So I can understand better, what are these set to when you run your
program on both windows and linux?

type1 = GetObjectType(hDC);
    type2 = GetObjectType(pBufferHDC);

Jesse

[Bertrand Coconnier]

Jesse Allen wrote :
> On 11/3/06, Bertrand Coconnier  wrote:
> > Hi,
> >
> > After investigating a bit in the Wine code, it seems that the flickering
> > bug of WoW in OpenGL mode is due to wglGetPbufferDCARB that returns a DC
> > which type is OBJ_MEMDC while it should return a DC which type is
> > OBJ_DC. And since in the current git tree, the DC type of a Pbuffer is
> > OBJ_MEMDC then the code located in lines 1394-1395 of
> > dlls/winex11.drv/opengl.c is executed : the Pbuffer is copied to the
> > screen each time the GL context is made current to the Pbuffer hence the
> > flickering (WoW bounds the GL context alternatively to the screen and to
> > the Pbuffer). The expected behaviour is not to copy the Pbuffer content
> > to the screen since WoW does it itself.
> >
> > The easiest workaround for this bug would be to make
> > X11DRV_wglGetPbufferDCARB to return the Pbuffer's object->hdc (this
> > fixes the flickering bug). However, I have made some tests with Windows
> > XP and it appears that wglGetPbufferDCARB returns a HDC different from
> > the HDC passed when wglCreatePbufferARB was called. So such a patch
> > would be a dirty hack not a real fix.
>
> http://oss.sgi.com/projects/ogl-sample/registry/ARB/wgl_pbuffer.txt
>
> "To create a device context for the pbuffer, call
>
> HDC wglGetPbufferDCARB(HPBUFFERARB hPbuffer);
>
> where <hPbuffer> is a handle returned from a previous call to
> wglCreatePbufferARB.  A device context is returned by
> wglGetPbufferDCARB which can be used to associate a rendering
> context with the pbuffer.  Any rendering context created with
> a wglCreateContext that is "compatible" with the <iPixelFormat> may
> be used to render into the pbuffer."
>
> So yes, it's supposed to return a new one.
>
> So I can understand better, what are these set to when you run your
> program on both windows and linux?
> type1 = GetObjectType(hDC);
> type2 = GetObjectType(pBufferHDC);
>
> Jesse

Under Windows XP : 	type1 == OBJ_DC
    		type2 == OBJ_DC

Under Wine : 		type1 == OBJ_DC
    		type2 == OBJ_MEMDC /* should be OBJ_DC since pBuffers are supposed to be 
located in Video Memory or whatever its name */

And since device contexts of Wine's pBuffers are OBJ_MEMDC, the following code 
in X11DRV_wglMakeCurrent is executed (line 1392 of dlls/winex11.drv/opengl.c) :

if(ret && type == OBJ_MEMDC)
         {
             ctx->do_escape = TRUE;
             pglDrawBuffer(GL_FRONT_LEFT);
         }

the content of the Pbuffer's DC is copied to the color buffer by Wine while it 
should not (there is no reason for some memory data to be copied to the color 
buffer when a GL context is made current to a Pbuffer).

Bertrand.

[Roderick Colenbrander]

> Hi,
>
> After investigating a bit in the Wine code, it seems that the flickering 
> bug of WoW in OpenGL mode is due to wglGetPbufferDCARB that returns a DC 
> which type is OBJ_MEMDC while it should return a DC which type is 
> OBJ_DC. And since in the current git tree, the DC type of a Pbuffer is 
> OBJ_MEMDC then the code located in lines 1394-1395 of 
> dlls/winex11.drv/opengl.c is executed : the Pbuffer is copied to the 
> screen each time the GL context is made current to the Pbuffer hence the 
> flickering (WoW bounds the GL context alternatively to the screen and to 
> the Pbuffer). The expected behaviour is not to copy the Pbuffer content 
> to the screen since WoW does it itself.
>
> The easiest workaround for this bug would be to make 
> X11DRV_wglGetPbufferDCARB to return the Pbuffer's object->hdc (this 
> fixes the flickering bug). However, I have made some tests with Windows 
> XP and it appears that wglGetPbufferDCARB returns a HDC different from 
> the HDC passed when wglCreatePbufferARB was called. So such a patch 
> would be a dirty hack not a real fix.
>
> I am not familiar enough with Wine to submit a patch so I leave the 
> gurus out there find an elegant solution.
>
> Attached is the code that I used to perform a few tests about Pbuffers 
> on Windows XP (code of interest is located from line 293 to line 298).
>
> Regards,
>
> Bertrand.

It is correct that the return of OBJ_MEMDC causes the flickering as it turns on the line glDrawBuffer. One of the reasons I started to rewrite opengl32 was because of this. Right now most code is inside winex11.drv in which we have much more control over things.

The easiest hack I thought about some months ago is to just set a flag in the device returned wglGetPbufferDCARB though it would be tricky to get a X11DRV_PDEVICE in a 'legal' (AJ-proof) way.

There's also a chance that a hack like this isn't needed at all. Originally Huw Davies added the glDrawBuffer code together with X11DRV_SYNC_PIXMAP code to winex11.drv/opengl32 in order to allow windowed opengl rendering using DIBs. The glDrawBuffer line was used for emulating single buffering.

Recently Ulrich Czekalla worked on a better fix for the windowed opengl problem using glScissors. For this patch various calls like glViewport, glScissor and others need to be patched. It might be possible to get rid of the glDrawBuffer / x11 pixmap rendering code but Huw would need to answer that as he knows for what it was needed and it fully works.

I really hope it can be removed when the scissors code is in, it would simplify the code and make my work easier.

Regards,
Roderick

[Bertrand Coconnier]

Roderick Colenbrander wrote :
> > Hi,
> >
> > After investigating a bit in the Wine code, it seems that the flickering 
> > bug of WoW in OpenGL mode is due to wglGetPbufferDCARB that returns a DC 
> > which type is OBJ_MEMDC while it should return a DC which type is 
> > OBJ_DC. And since in the current git tree, the DC type of a Pbuffer is 
> > OBJ_MEMDC then the code located in lines 1394-1395 of 
> > dlls/winex11.drv/opengl.c is executed : the Pbuffer is copied to the 
> > screen each time the GL context is made current to the Pbuffer hence the 
> > flickering (WoW bounds the GL context alternatively to the screen and to 
> > the Pbuffer). The expected behaviour is not to copy the Pbuffer content 
> > to the screen since WoW does it itself.
> >
> > The easiest workaround for this bug would be to make 
> > X11DRV_wglGetPbufferDCARB to return the Pbuffer's object->hdc (this 
> > fixes the flickering bug). However, I have made some tests with Windows 
> > XP and it appears that wglGetPbufferDCARB returns a HDC different from 
> > the HDC passed when wglCreatePbufferARB was called. So such a patch 
> > would be a dirty hack not a real fix.
> >
> > I am not familiar enough with Wine to submit a patch so I leave the 
> > gurus out there find an elegant solution.
> >
> > Attached is the code that I used to perform a few tests about Pbuffers 
> > on Windows XP (code of interest is located from line 293 to line 298).
> >
> > Regards,
> >
> > Bertrand.
>
> It is correct that the return of OBJ_MEMDC causes the flickering as it turns on the line glDrawBuffer. One of the reasons I started to rewrite opengl32 was because of this. Right now most code is inside winex11.drv in which we have much more control over things.
>
> The easiest hack I thought about some months ago is to just set a flag in the device returned wglGetPbufferDCARB though it would be tricky to get a X11DRV_PDEVICE in a 'legal' (AJ-proof) way.
>
> There's also a chance that a hack like this isn't needed at all. Originally Huw Davies added the glDrawBuffer code together with X11DRV_SYNC_PIXMAP code to winex11.drv/opengl32 in order to allow windowed opengl rendering using DIBs. The glDrawBuffer line was used for emulating single buffering.
>
> Recently Ulrich Czekalla worked on a better fix for the windowed opengl problem using glScissors. For this patch various calls like glViewport, glScissor and others need to be patched. It might be possible to get rid of the glDrawBuffer / x11 pixmap rendering code but Huw would need to answer that as he knows for what it was needed and it fully works.
>
> I really hope it can be removed when the scissors code is in, it would simplify the code and make my work easier.
>
> Regards,
> Roderick

OK such a patch would remove the flickering bug for WoW but what about the wrong 
type of the Pbuffer's DC ? Obviously, Windows XP attaches a DC which type is 
OBJ_DC to the Pbuffers. I don't know if some apps rely on the assumption that 
GetObjectType(wglGetPbufferDCARB(...)) == OBJ_DC, but if some do they might 
behave wrongly with Wine.

Regards,

Bertrand.

[Roderick Colenbrander]

> Roderick Colenbrander wrote :
> > > Hi,
> > >
> > > After investigating a bit in the Wine code, it seems that the
>
> flickering
> > > bug of WoW in OpenGL mode is due to wglGetPbufferDCARB that returns a
>
> DC
> > > which type is OBJ_MEMDC while it should return a DC which type is 
> > > OBJ_DC. And since in the current git tree, the DC type of a Pbuffer is 
> > > OBJ_MEMDC then the code located in lines 1394-1395 of 
> > > dlls/winex11.drv/opengl.c is executed : the Pbuffer is copied to the 
> > > screen each time the GL context is made current to the Pbuffer hence
>
> the
> > > flickering (WoW bounds the GL context alternatively to the screen and
>
> to
> > > the Pbuffer). The expected behaviour is not to copy the Pbuffer content
> > > to the screen since WoW does it itself.
> > >
> > > The easiest workaround for this bug would be to make 
> > > X11DRV_wglGetPbufferDCARB to return the Pbuffer's object->hdc (this 
> > > fixes the flickering bug). However, I have made some tests with Windows
> > > XP and it appears that wglGetPbufferDCARB returns a HDC different from 
> > > the HDC passed when wglCreatePbufferARB was called. So such a patch 
> > > would be a dirty hack not a real fix.
> > >
> > > I am not familiar enough with Wine to submit a patch so I leave the 
> > > gurus out there find an elegant solution.
> > >
> > > Attached is the code that I used to perform a few tests about Pbuffers 
> > > on Windows XP (code of interest is located from line 293 to line 298).
> > >
> > > Regards,
> > >
> > > Bertrand.
> >
> > It is correct that the return of OBJ_MEMDC causes the flickering as it
>
> turns on the line glDrawBuffer. One of the reasons I started to rewrite
> opengl32 was because of this. Right now most code is inside winex11.drv in
> which we have much more control over things.
> > The easiest hack I thought about some months ago is to just set a flag
>
> in the device returned wglGetPbufferDCARB though it would be tricky to get a
> X11DRV_PDEVICE in a 'legal' (AJ-proof) way.
> > There's also a chance that a hack like this isn't needed at all.
>
> Originally Huw Davies added the glDrawBuffer code together with X11DRV_SYNC_PIXMAP
> code to winex11.drv/opengl32 in order to allow windowed opengl rendering
> using DIBs. The glDrawBuffer line was used for emulating single buffering.
> > Recently Ulrich Czekalla worked on a better fix for the windowed opengl
>
> problem using glScissors. For this patch various calls like glViewport,
> glScissor and others need to be patched. It might be possible to get rid of
> the glDrawBuffer / x11 pixmap rendering code but Huw would need to answer
> that as he knows for what it was needed and it fully works.
> > I really hope it can be removed when the scissors code is in, it would
>
> simplify the code and make my work easier.
> > Regards,
> > Roderick
>
> OK such a patch would remove the flickering bug for WoW but what about the
> wrong 
> type of the Pbuffer's DC ? Obviously, Windows XP attaches a DC which type
> is 
> OBJ_DC to the Pbuffers. I don't know if some apps rely on the assumption
> that 
> GetObjectType(wglGetPbufferDCARB(...)) == OBJ_DC, but if some do they
> might 
> behave wrongly with Wine.
>
> Regards,
>
> Bertrand.

I didn't have time yet to reply to the other email. This should be investigated more thoroughly. Second as this would be a 'serious' bug in Wine we need to convince Alexandre that this behavior is correct. For this purpose we need a test case. If you check the wine source you'll see that most dlls contain test cases using certain APIs. These testcases are used on wine and different windows versions to test the behavior of a function. Since the behavior of GetObjectType for this case is badly documented it should be added. Most likely it should be added to gdi32 and then dynamicly load opengl32.dll (you likely need to provide the function prototypes yourself if they aren't in wingdi.h as we don't have wine opengl headers yet).

If you could write such a test case in order to convince AJ that would be very usefull.

Regards,
Roderick

[Roderick Colenbrander]

> > Roderick Colenbrander wrote :
> > > > Hi,
> > > >
> > > > After investigating a bit in the Wine code, it seems that the
> >
> > flickering
> > > > bug of WoW in OpenGL mode is due to wglGetPbufferDCARB that returns a
> >
> > DC
> > > > which type is OBJ_MEMDC while it should return a DC which type is 
> > > > OBJ_DC. And since in the current git tree, the DC type of a Pbuffer
>
> is
> > > > OBJ_MEMDC then the code located in lines 1394-1395 of 
> > > > dlls/winex11.drv/opengl.c is executed : the Pbuffer is copied to the 
> > > > screen each time the GL context is made current to the Pbuffer hence
> >
> > the
> > > > flickering (WoW bounds the GL context alternatively to the screen and
> >
> > to
> > > > the Pbuffer). The expected behaviour is not to copy the Pbuffer
>
> content
> > > > to the screen since WoW does it itself.
> > > >
> > > > The easiest workaround for this bug would be to make 
> > > > X11DRV_wglGetPbufferDCARB to return the Pbuffer's object->hdc (this 
> > > > fixes the flickering bug). However, I have made some tests with
>
> Windows
> > > > XP and it appears that wglGetPbufferDCARB returns a HDC different
>
> from
> > > > the HDC passed when wglCreatePbufferARB was called. So such a patch 
> > > > would be a dirty hack not a real fix.
> > > >
> > > > I am not familiar enough with Wine to submit a patch so I leave the 
> > > > gurus out there find an elegant solution.
> > > >
> > > > Attached is the code that I used to perform a few tests about
>
> Pbuffers
> > > > on Windows XP (code of interest is located from line 293 to line
>
> 298).
> > > > Regards,
> > > >
> > > > Bertrand.
> > >
> > > It is correct that the return of OBJ_MEMDC causes the flickering as it
> >
> > turns on the line glDrawBuffer. One of the reasons I started to rewrite
> > opengl32 was because of this. Right now most code is inside winex11.drv
>
> in
> > which we have much more control over things.
> > > The easiest hack I thought about some months ago is to just set a flag
> >
> > in the device returned wglGetPbufferDCARB though it would be tricky to
>
> get a
> > X11DRV_PDEVICE in a 'legal' (AJ-proof) way.
> > > There's also a chance that a hack like this isn't needed at all.
> >
> > Originally Huw Davies added the glDrawBuffer code together with
>
> X11DRV_SYNC_PIXMAP
> > code to winex11.drv/opengl32 in order to allow windowed opengl rendering
> > using DIBs. The glDrawBuffer line was used for emulating single
>
> buffering.
> > > Recently Ulrich Czekalla worked on a better fix for the windowed
>
> opengl
> > problem using glScissors. For this patch various calls like glViewport,
> > glScissor and others need to be patched. It might be possible to get rid
>
> of
> > the glDrawBuffer / x11 pixmap rendering code but Huw would need to
>
> answer
> > that as he knows for what it was needed and it fully works.
> > > I really hope it can be removed when the scissors code is in, it would
> >
> > simplify the code and make my work easier.
> > > Regards,
> > > Roderick
> >
> > OK such a patch would remove the flickering bug for WoW but what about
>
> the
> > wrong 
> > type of the Pbuffer's DC ? Obviously, Windows XP attaches a DC which
>
> type
> > is 
> > OBJ_DC to the Pbuffers. I don't know if some apps rely on the assumption
> > that 
> > GetObjectType(wglGetPbufferDCARB(...)) == OBJ_DC, but if some do they
> > might 
> > behave wrongly with Wine.
> >
> > Regards,
> >
> > Bertrand.
>
> I didn't have time yet to reply to the other email. This should be
> investigated more thoroughly. Second as this would be a 'serious' bug in Wine we
> need to convince Alexandre that this behavior is correct. For this purpose
> we need a test case. If you check the wine source you'll see that most dlls
> contain test cases using certain APIs. These testcases are used on wine and
> different windows versions to test the behavior of a function. Since the
> behavior of GetObjectType for this case is badly documented it should be
> added. Most likely it should be added to gdi32 and then dynamicly load
> opengl32.dll (you likely need to provide the function prototypes yourself if they
> aren't in wingdi.h as we don't have wine opengl headers yet).
>
> If you could write such a test case in order to convince AJ that would be
> very usefull.
>
> Regards,
> Roderick

Or perhaps a testcase isn't needed at all. I think the use of CreateCompatibleDC in wglGetPbufferDCARB is incorrect. According to MSDN this function creates a memory device context. Perhaps something like this works:
HDC hdc = CreateDC(...);
int format_orig = GetPixelFormat(hdc_orig);
SetPixelFormat(hdc, format_orig);
return hdc;

[Bertrand Coconnier]

Roderick Colenbrander wrote :
> Or perhaps a testcase isn't needed at all. I think the use of CreateCompatibleDC in wglGetPbufferDCARB is incorrect. According to MSDN this function creates a memory device context. Perhaps something like this works:
> HDC hdc = CreateDC(...);
> int format_orig = GetPixelFormat(hdc_orig);
> SetPixelFormat(hdc, format_orig);
> return hdc;

Thanks to your hints I could make the attached patch. It fixes the issue on my 
side : wglGetPbufferDCARB returns a DC which type is OBJ_DC and WoW does not 
flicker any more.

Regards,

Bertrand.

diff --git a/dlls/winex11.drv/opengl.c b/dlls/winex11.drv/opengl.c
index 962962f..b529f12 100644
--- a/dlls/winex11.drv/opengl.c
+++ b/dlls/winex11.drv/opengl.c
@@ -1869,16 +1869,18 @@ static HDC WINAPI X11DRV_wglGetPbufferDC
 {
     Wine_GLPBuffer* object = (Wine_GLPBuffer*) hPbuffer;
     HDC hDC;
+    int iPixelFormat;
+    PIXELFORMATDESCRIPTOR  pfd;
     if (NULL == object) {
         SetLastError(ERROR_INVALID_HANDLE);
         return NULL;
     }
-    hDC = CreateCompatibleDC(object->hdc);

-    /* The function wglGetPbufferDCARB returns a DC to which the pbuffer can be connected.
-     * We only support one onscreen rendering format (the one from the main visual), so use that. */
-    SetPixelFormat(hDC, 1, NULL);
-    set_drawable(hDC, object->drawable); /* works ?? */
+    hDC = CreateDCA("DISPLAY", NULL, NULL, NULL);
+    iPixelFormat = GetPixelFormat(object->hdc);
+    DescribePixelFormat(hDC, iPixelFormat, sizeof(PIXELFORMATDESCRIPTOR), &pfd); 
+    SetPixelFormat(hDC, iPixelFormat, &pfd);
+
     TRACE("(%p)->(%p)\n", hPbuffer, hDC);
     return hDC;
 }

[Roderick Colenbrander]

> Roderick Colenbrander wrote :
> > Or perhaps a testcase isn't needed at all. I think the use of
>
> CreateCompatibleDC in wglGetPbufferDCARB is incorrect. According to MSDN this
> function creates a memory device context. Perhaps something like this works:
> > HDC hdc = CreateDC(...);
> > int format_orig = GetPixelFormat(hdc_orig);
> > SetPixelFormat(hdc, format_orig);
> > return hdc;
>
> Thanks to your hints I could make the attached patch. It fixes the issue
> on my 
> side : wglGetPbufferDCARB returns a DC which type is OBJ_DC and WoW does
> not 
> flicker any more.
>
> Regards,
>
> Bertrand.

@@ -1869,16 +1869,18 @@ static HDC WINAPI X11DRV_wglGetPbufferDC
 {
     Wine_GLPBuffer* object = (Wine_GLPBuffer*) hPbuffer;
     HDC hDC;
+    int iPixelFormat;
+    PIXELFORMATDESCRIPTOR  pfd;
     if (NULL == object) {
         SetLastError(ERROR_INVALID_HANDLE);
         return NULL;
     }
-    hDC = CreateCompatibleDC(object->hdc);

-    /* The function wglGetPbufferDCARB returns a DC to which the pbuffer can be connected.
-     * We only support one onscreen rendering format (the one from the main visual), so use that. */
-    SetPixelFormat(hDC, 1, NULL);
-    set_drawable(hDC, object->drawable); /* works ?? */
+    hDC = CreateDCA("DISPLAY", NULL, NULL, NULL);
+    iPixelFormat = GetPixelFormat(object->hdc);
+    DescribePixelFormat(hDC, iPixelFormat, sizeof(PIXELFORMATDESCRIPTOR), &pfd);
+    SetPixelFormat(hDC, iPixelFormat, &pfd);
+
     TRACE("(%p)->(%p)\n", hPbuffer, hDC);
     return hDC;
 }

I'm not sure if this is correct for all programs. Some more testing needs to be done in other programs that use pbuffers. The set_drawable line must have a purpose (though I like to get rid of it or atleast implement it without ExtEscape). And I'm also not sure if we need to retrieve the pixelformat of the 'parent hdc' as it is allways 1 though retrieving it is nicer.

Roderick

[Bertrand Coconnier]

Roderick Colenbrander wrote :
> > Roderick Colenbrander wrote :
> > > Or perhaps a testcase isn't needed at all. I think the use of
> >
> > CreateCompatibleDC in wglGetPbufferDCARB is incorrect. According to MSDN this
> > function creates a memory device context. Perhaps something like this works:
> > > HDC hdc = CreateDC(...);
> > > int format_orig = GetPixelFormat(hdc_orig);
> > > SetPixelFormat(hdc, format_orig);
> > > return hdc;
> >
> > Thanks to your hints I could make the attached patch. It fixes the issue
> > on my 
> > side : wglGetPbufferDCARB returns a DC which type is OBJ_DC and WoW does
> > not 
> > flicker any more.
> >
> > Regards,
> >
> > Bertrand.
>
> @@ -1869,16 +1869,18 @@ static HDC WINAPI X11DRV_wglGetPbufferDC
>  {
>      Wine_GLPBuffer* object = (Wine_GLPBuffer*) hPbuffer;
>      HDC hDC;
> - int iPixelFormat;
> - PIXELFORMATDESCRIPTOR  pfd;
> if (NULL == object) {
>     SetLastError(ERROR_INVALID_HANDLE);
>     return NULL;
> }
> -
>
> hDC = CreateCompatibleDC(object->hdc);
> -
>
> /* The function wglGetPbufferDCARB returns a DC to which the pbuffer can be connected.
> -
> * We only support one onscreen rendering format (the one from the main visual), so use that. */
> -
>
> SetPixelFormat(hDC, 1, NULL);
> -
>
> set_drawable(hDC, object->drawable); /* works ?? */
> - hDC = CreateDCA("DISPLAY", NULL, NULL, NULL);
> - iPixelFormat = GetPixelFormat(object->hdc);
> - DescribePixelFormat(hDC, iPixelFormat, sizeof(PIXELFORMATDESCRIPTOR), &pfd);
> - SetPixelFormat(hDC, iPixelFormat, &pfd);
> - TRACE("(%p)->(%p)\n", hPbuffer, hDC);
>    return hDC;
>
> }
>
> I'm not sure if this is correct for all programs. Some more testing needs to be done in other programs that use pbuffers. The set_drawable line must have a purpose (though I like to get rid of it or atleast implement it without ExtEscape). And I'm also not sure if we need to retrieve the pixelformat of the 'parent hdc' as it is allways 1 though retrieving it is nicer.
>
> Roderick

The patch fixes the bug whether the set_drawable is included or not. So I guess 
it does not hurt. I also dug in the wine-patches mailing list and it appeared 
that you included this line in your original patch (see 
http://www.winehq.org/pipermail/wine-patches/2006-September/030335.html) but 
infortunately you did not give any detail at the time about the reason of its 
inclusion. Yet it seems logical that the DC returned by wglGetPbufferDCARB is 
linked to the Pbuffer drawable rather than the window drawable.

About the retrival of the pixel format of the 'parent hdc', I agree that it is a 
bit overkill but once again it does not hurt and it may prevent bugs to occur if 
one day Wine will use more than one pixel format.

So what about this patch ?

@@ -1869,16 +1869,18 @@ static HDC WINAPI X11DRV_wglGetPbufferDC
  {
      Wine_GLPBuffer* object = (Wine_GLPBuffer*) hPbuffer;
      HDC hDC;
+    int iPixelFormat;
+    PIXELFORMATDESCRIPTOR  pfd;
      if (NULL == object) {
          SetLastError(ERROR_INVALID_HANDLE);
          return NULL;
      }
-    hDC = CreateCompatibleDC(object->hdc);

-    /* The function wglGetPbufferDCARB returns a DC to which the pbuffer can be 
connected.
-     * We only support one onscreen rendering format (the one from the main 
visual), so use that. */
-    SetPixelFormat(hDC, 1, NULL);
+    hDC = CreateDCA("DISPLAY", NULL, NULL, NULL);
+    iPixelFormat = GetPixelFormat(object->hdc);
+    DescribePixelFormat(hDC, iPixelFormat, sizeof(PIXELFORMATDESCRIPTOR), &pfd);
+    SetPixelFormat(hDC, iPixelFormat, &pfd);
      set_drawable(hDC, object->drawable); /* works ?? */
+
      TRACE("(%p)->(%p)\n", hPbuffer, hDC);
      return hDC;
  }

About testing, I don't have an app other than WoW that uses OpenGL (not to 
mention Pbuffers) so if some people out there could give this patch a try...

Bertrand.

[Roderick Colenbrander]

> Roderick Colenbrander wrote :
> > > Roderick Colenbrander wrote :
> > > > Or perhaps a testcase isn't needed at all. I think the use of
> > >
> > > CreateCompatibleDC in wglGetPbufferDCARB is incorrect. According to
>
> MSDN this
> > > function creates a memory device context. Perhaps something like this
>
> works:
> > > > HDC hdc = CreateDC(...);
> > > > int format_orig = GetPixelFormat(hdc_orig);
> > > > SetPixelFormat(hdc, format_orig);
> > > > return hdc;
> > >
> > > Thanks to your hints I could make the attached patch. It fixes the
>
> issue
> > > on my 
> > > side : wglGetPbufferDCARB returns a DC which type is OBJ_DC and WoW
>
> does
> > > not 
> > > flicker any more.
> > >
> > > Regards,
> > >
> > > Bertrand.
> >
> > @@ -1869,16 +1869,18 @@ static HDC WINAPI X11DRV_wglGetPbufferDC
> >  {
> >      Wine_GLPBuffer* object = (Wine_GLPBuffer*) hPbuffer;
> >      HDC hDC;
> > - int iPixelFormat;
> > - PIXELFORMATDESCRIPTOR  pfd;
> > if (NULL == object) {
> >     SetLastError(ERROR_INVALID_HANDLE);
> >     return NULL;
> > }
> > -
> >
> > hDC = CreateCompatibleDC(object->hdc);
> > -
> >
> > /* The function wglGetPbufferDCARB returns a DC to which the
>
> pbuffer can be connected.
> > -
> > * We only support one onscreen rendering format (the one from the
>
> main visual), so use that. */
> > - SetPixelFormat(hDC, 1, NULL);
> > - set_drawable(hDC, object->drawable); /* works ?? */
> > - hDC = CreateDCA("DISPLAY", NULL, NULL, NULL);
> > - iPixelFormat = GetPixelFormat(object->hdc);
> > - DescribePixelFormat(hDC, iPixelFormat,
>
> sizeof(PIXELFORMATDESCRIPTOR), &pfd);
> > - SetPixelFormat(hDC, iPixelFormat, &pfd);
> > - TRACE("(%p)->(%p)\n", hPbuffer, hDC);
> >    return hDC;
> >
> > }
> >
> > I'm not sure if this is correct for all programs. Some more testing
>
> needs to be done in other programs that use pbuffers. The set_drawable line
> must have a purpose (though I like to get rid of it or atleast implement it
> without ExtEscape). And I'm also not sure if we need to retrieve the
> pixelformat of the 'parent hdc' as it is allways 1 though retrieving it is nicer.
> > Roderick
>
> The patch fixes the bug whether the set_drawable is included or not. So I
> guess 
> it does not hurt. I also dug in the wine-patches mailing list and it
> appeared 
> that you included this line in your original patch (see 
> http://www.winehq.org/pipermail/wine-patches/2006-September/030335.html)
> but 
> infortunately you did not give any detail at the time about the reason of
> its 
> inclusion. Yet it seems logical that the DC returned by wglGetPbufferDCARB
> is 
> linked to the Pbuffer drawable rather than the window drawable.
>
> About the retrival of the pixel format of the 'parent hdc', I agree that
> it is a 
> bit overkill but once again it does not hurt and it may prevent bugs to
> occur if 
> one day Wine will use more than one pixel format.
>
> So what about this patch ?
>
> @@ -1869,16 +1869,18 @@ static HDC WINAPI X11DRV_wglGetPbufferDC
>   {
>       Wine_GLPBuffer* object = (Wine_GLPBuffer*) hPbuffer;
>       HDC hDC;
> - int iPixelFormat;
> - PIXELFORMATDESCRIPTOR  pfd;
>  if (NULL == object) {
>      SetLastError(ERROR_INVALID_HANDLE);
>      return NULL;
>  }
> -
>
> hDC = CreateCompatibleDC(object->hdc);
> -
>
> /* The function wglGetPbufferDCARB returns a DC to which the pbuffer
>
> can be 
> connected.
> -
> * We only support one onscreen rendering format (the one from the
>
> main 
> visual), so use that. */
> - SetPixelFormat(hDC, 1, NULL);
> - hDC = CreateDCA("DISPLAY", NULL, NULL, NULL);
> - iPixelFormat = GetPixelFormat(object->hdc);
> - DescribePixelFormat(hDC, iPixelFormat, sizeof(PIXELFORMATDESCRIPTOR),
>
> &pfd);
> - SetPixelFormat(hDC, iPixelFormat, &pfd);
>  set_drawable(hDC, object->drawable); /* works ?? */
> -
> TRACE("(%p)->(%p)\n", hPbuffer, hDC);
> return hDC;
> }
>
> About testing, I don't have an app other than WoW that uses OpenGL (not to
> mention Pbuffers) so if some people out there could give this patch a
> try...
>
> Bertrand.

The main issue I still have with the patch which isn't something you have done wrong is set_drawable. Before I started moving the original opengl32 code to winex11.drv (I didn't write the WGL code), there was a need to get access to X11 data inside opengl32. For this purpose 'ExtEscape' got used. This isn't a nice mechanism and Alexandre wants to get rid of it as much as possible.

The set_drawable call uses ExtEscape too. I think I'm going to move a part of wglGetCurrentDCARB to gdi32.dll in which we can do CreateDCA. Then we can pass the HDC and pbuffer to the other part of wglGetCurrentDCARB which will be in winex11.drv. The winex11.drv version can then just set the drawable in a X11DRV_PDEVICE (x11drv version of a hdc).

Regards,
Roderick

[Roderick Colenbrander]

> > Roderick Colenbrander wrote :
> > > > Roderick Colenbrander wrote :
> > > > > Or perhaps a testcase isn't needed at all. I think the use of
> > > >
> > > > CreateCompatibleDC in wglGetPbufferDCARB is incorrect. According to
> >
> > MSDN this
> > > > function creates a memory device context. Perhaps something like this
> >
> > works:
> > > > > HDC hdc = CreateDC(...);
> > > > > int format_orig = GetPixelFormat(hdc_orig);
> > > > > SetPixelFormat(hdc, format_orig);
> > > > > return hdc;
> > > >
> > > > Thanks to your hints I could make the attached patch. It fixes the
> >
> > issue
> > > > on my 
> > > > side : wglGetPbufferDCARB returns a DC which type is OBJ_DC and WoW
> >
> > does
> > > > not 
> > > > flicker any more.
> > > >
> > > > Regards,
> > > >
> > > > Bertrand.
> > >
> > > @@ -1869,16 +1869,18 @@ static HDC WINAPI X11DRV_wglGetPbufferDC
> > >  {
> > >      Wine_GLPBuffer* object = (Wine_GLPBuffer*) hPbuffer;
> > >      HDC hDC;
> > > - int iPixelFormat;
> > > - PIXELFORMATDESCRIPTOR  pfd;
> > > if (NULL == object) {
> > >     SetLastError(ERROR_INVALID_HANDLE);
> > >     return NULL;
> > > }
> > > -
> > >
> > > hDC = CreateCompatibleDC(object->hdc);
> > > -
> > >
> > > /* The function wglGetPbufferDCARB returns a DC to which the
> >
> > pbuffer can be connected.
> > > -
> > > * We only support one onscreen rendering format (the one from
>
> the
> > main visual), so use that. */
> > > - SetPixelFormat(hDC, 1, NULL);
> > > - set_drawable(hDC, object->drawable); /* works ?? */
> > > - hDC = CreateDCA("DISPLAY", NULL, NULL, NULL);
> > > - iPixelFormat = GetPixelFormat(object->hdc);
> > > - DescribePixelFormat(hDC, iPixelFormat,
> >
> > sizeof(PIXELFORMATDESCRIPTOR), &pfd);
> > > - SetPixelFormat(hDC, iPixelFormat, &pfd);
> > > - TRACE("(%p)->(%p)\n", hPbuffer, hDC);
> > >    return hDC;
> > >
> > > }
> > >
> > > I'm not sure if this is correct for all programs. Some more testing
> >
> > needs to be done in other programs that use pbuffers. The set_drawable
>
> line
> > must have a purpose (though I like to get rid of it or atleast implement
>
> it
> > without ExtEscape). And I'm also not sure if we need to retrieve the
> > pixelformat of the 'parent hdc' as it is allways 1 though retrieving it
>
> is nicer.
> > > Roderick
> >
> > The patch fixes the bug whether the set_drawable is included or not. So
>
> I
> > guess 
> > it does not hurt. I also dug in the wine-patches mailing list and it
> > appeared 
> > that you included this line in your original patch (see 
> > http://www.winehq.org/pipermail/wine-patches/2006-September/030335.html)
> > but 
> > infortunately you did not give any detail at the time about the reason
>
> of
> > its 
> > inclusion. Yet it seems logical that the DC returned by
>
> wglGetPbufferDCARB
> > is 
> > linked to the Pbuffer drawable rather than the window drawable.
> >
> > About the retrival of the pixel format of the 'parent hdc', I agree that
> > it is a 
> > bit overkill but once again it does not hurt and it may prevent bugs to
> > occur if 
> > one day Wine will use more than one pixel format.
> >
> > So what about this patch ?
> >
> > @@ -1869,16 +1869,18 @@ static HDC WINAPI X11DRV_wglGetPbufferDC
> >   {
> >       Wine_GLPBuffer* object = (Wine_GLPBuffer*) hPbuffer;
> >       HDC hDC;
> > - int iPixelFormat;
> > - PIXELFORMATDESCRIPTOR  pfd;
> >  if (NULL == object) {
> >      SetLastError(ERROR_INVALID_HANDLE);
> >      return NULL;
> >  }
> > -
> >
> > hDC = CreateCompatibleDC(object->hdc);
> > -
> >
> > /* The function wglGetPbufferDCARB returns a DC to which the
>
> pbuffer
> > can be 
> > connected.
> > -
> > * We only support one onscreen rendering format (the one from the
> >
> > main 
> > visual), so use that. */
> > - SetPixelFormat(hDC, 1, NULL);
> > - hDC = CreateDCA("DISPLAY", NULL, NULL, NULL);
> > - iPixelFormat = GetPixelFormat(object->hdc);
> > - DescribePixelFormat(hDC, iPixelFormat,
>
> sizeof(PIXELFORMATDESCRIPTOR),
> > &pfd);
> > - SetPixelFormat(hDC, iPixelFormat, &pfd);
> >  set_drawable(hDC, object->drawable); /* works ?? */
> > -
> > TRACE("(%p)->(%p)\n", hPbuffer, hDC);
> > return hDC;
> > }
> >
> > About testing, I don't have an app other than WoW that uses OpenGL (not
>
> to
> > mention Pbuffers) so if some people out there could give this patch a
> > try...
> >
> > Bertrand.
>
> The main issue I still have with the patch which isn't something you have
> done wrong is set_drawable. Before I started moving the original opengl32
> code to winex11.drv (I didn't write the WGL code), there was a need to get
> access to X11 data inside opengl32. For this purpose 'ExtEscape' got used.
> This isn't a nice mechanism and Alexandre wants to get rid of it as much as
> possible.
>
> The set_drawable call uses ExtEscape too. I think I'm going to move a part
> of wglGetCurrentDCARB to gdi32.dll in which we can do CreateDCA. Then we
> can pass the HDC and pbuffer to the other part of wglGetCurrentDCARB which
> will be in winex11.drv. The winex11.drv version can then just set the
> drawable in a X11DRV_PDEVICE (x11drv version of a hdc).
>
> Regards,
> Roderick

Could you check if the attached patch works? It does more or less what I explained in the last email. If it works I'll clean it up as right now it is a bit hacky. I hope to find some apps to test it on.

Regards,
Roderick

[Bertrand Coconnier]

Roderick Colenbrander wrote :
> Could you check if the attached patch works? It does more or less what I explained in the last email. If it works I'll clean it up as right now it is a bit hacky. I hope to find some apps to test it on.
>
> Regards,
> Roderick

Your patch fixes everything : WoW does not flicker any more, the DC type of the 
Pbuffer is OBJ_DC and the Pbuffer's DC is not the same than the window's DC.

Regards,

Bertrand.