Re: [PATCH v2] winex11: Use GLX_OML_sync_control to synchronise buffer swaps with X11DRV_FLUSH_GL_DRAWABLE requests.

[Jacek Caban]

Hi Henri,

On 23.03.2017 23:58, Henri Verbeet wrote:
> diff --git a/dlls/winex11.drv/init.c b/dlls/winex11.drv/init.c
> index af2fd92..7efd603 100644
> --- a/dlls/winex11.drv/init.c
> +++ b/dlls/winex11.drv/init.c
> @@ -375,9 +375,6 @@ static INT X11DRV_ExtEscape( PHYSDEV dev, INT escape, INT in_count, LPCVOID in_d
>                      RECT rect = physDev->dc_rect;
>
> OffsetRect( &rect, -physDev->dc_rect.left, -physDev->dc_rect.top );
> -                    /* The GL drawable may be lagged behind if we don't flush first, so
> -                     * flush the display make sure we copy up-to-date data */
> -                    XFlush( gdi_display );

Shouldn't this be called if GLX_OML_sync_control is not available? It
seems to me that it should either depend on extension availability or be
a separated patch.


Thanks,

Jacek

[PATCH v2] winex11: Use GLX_OML_sync_control to synchronise buffer swaps with X11DRV_FLUSH_GL_DRAWABLE requests.

[Henri Verbeet]

On 24 March 2017 at 13:02, Jacek Caban <jacek(a)codeweavers.com> wrote:
> On 23.03.2017 23:58, Henri Verbeet wrote:
> > diff --git a/dlls/winex11.drv/init.c b/dlls/winex11.drv/init.c
> > index af2fd92..7efd603 100644
> > --- a/dlls/winex11.drv/init.c
> > +++ b/dlls/winex11.drv/init.c
> > @@ -375,9 +375,6 @@ static INT X11DRV_ExtEscape( PHYSDEV dev, INT escape, INT in_count, LPCVOID in_d
> >                      RECT rect = physDev->dc_rect;
> >
> > OffsetRect( &rect, -physDev->dc_rect.left, -physDev->dc_rect.top );
> > -                    /* The GL drawable may be lagged behind if we don't flush first, so
> > -                     * flush the display make sure we copy up-to-date data */
> > -                    XFlush( gdi_display );
>
> Shouldn't this be called if GLX_OML_sync_control is not available? It
> seems to me that it should either depend on extension availability or be
> a separated patch.

It doesn't work (on any current driver), and as far as I'm aware it
never did. In that regard it could be a separate commit, yes. I think
the change is closely enough related to the other changes and the
resulting patch is trivial enough to justify including it, but I don't
feel particularly strongly about that.