Flickering bug for World of Warcraft

[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
-- 
GMX DSL-Flatrate 0,- Euro* - Überall, wo DSL verfügbar ist!
NEU: Jetzt bis zu 16.000 kBit/s! http://www.gmx.net/de/go/dsl