Re: WineX11.drv: Add an X11 window for the client area [attempt3]

[Alexandre Julliard]

"Roderick Colenbrander"  writes:
> - if (data->client_window)
> - {
> -
>    int w = data->client_rect.right - data->client_rect.left;
> -
>    int h = data->client_rect.bottom - data->client_rect.top;
> -
> -
>    TRACE("Updating client window 0x%lx to %dx%x,%dx%d\n", data->client_window,
> -
>          data->client_rect.left, data->client_rect.top, w, h);
> -
> -
>    if(w>0 && h>0) 
> -
>    {
> -
>        wine_tsx11_lock();
> -
>        XMoveResizeWindow(display, data->client_window, data->client_rect.left,
> -
>                          data->client_rect.top, w, h);
> -
>        wine_tsx11_unlock();

You should do that only when the size has actually changed.
> +static Window create_client_window( Display *display, struct x11drv_win_data *data)
> +{
> - XSetWindowAttributes attr;
> -
> - attr.event_mask = ExposureMask;
> - attr.bit_gravity = NorthWestGravity;
> - attr.backing_store = NotUseful;

Using NorthWestGravity is good but you still need to fix the valid rects
calculation accordingly.
> +static void destroy_client_window( Display *display, struct x11drv_win_data *data )
> +{
> - if (!data->client_window) return;
> -
> - wine_tsx11_lock();
> -
> - XDeleteContext( display, data->client_window, winContext );
> - XDestroyWindow( display, data->client_window );
> - data->client_window = 0;
> - XFlush(display);
> -
> - wine_tsx11_unlock();

The XFlush shouldn't be needed.
> @@ -1569,7 +1648,19 @@ void X11DRV_SetParent( HWND hwnd, HWND parent, HWND old_parent )
>          if (old_parent == GetDesktopWindow())
>          {
>              /* destroy the old X windows */
> -
>        destroy_whole_window( display, data );
> -
> -
>        /* Don't destroy the X11 window twice in order to prevent a BadWindow error, if the client window is the whole window */
> -
>        if(data->whole_window == data->client_window)
> -
>        {
> -
>            destroy_whole_window( display, data );
> -
>            data->client_window = 0;
> -
>            RemovePropA( data->hwnd, client_window_prop );
> -
>        }

The test is useless, if you are changing the parent of the desktop
window something is very wrong...

WineX11.drv: Add an X11 window for the client area [attempt3]

[Roderick Colenbrander]

> "Roderick Colenbrander"  writes:
> > - if (data->client_window)
> > - {
> > -
> >    int w = data->client_rect.right - data->client_rect.left;
> > -
> >    int h = data->client_rect.bottom - data->client_rect.top;
> > -
> > -
> >    TRACE("Updating client window 0x%lx to %dx%x,%dx%d\n",
>
> data->client_window,
> > -
> >          data->client_rect.left, data->client_rect.top, w, h);
> > -
> > -
> >    if(w>0 && h>0) 
> > -
> >    {
> > -
> >        wine_tsx11_lock();
> > -
> >        XMoveResizeWindow(display, data->client_window,
>
> data->client_rect.left,
> > -
> >                          data->client_rect.top, w, h);
> > -
> >        wine_tsx11_unlock();
>
> You should do that only when the size has actually changed.

Is it enough to check if the dimensions changed or should I also care about the position? (I don't think it changes)
> > +static Window create_client_window( Display *display, struct
>
> x11drv_win_data *data)
> > +{
> > - XSetWindowAttributes attr;
> > -
> > - attr.event_mask = ExposureMask;
> > - attr.bit_gravity = NorthWestGravity;
> > - attr.backing_store = NotUseful;
>
> Using NorthWestGravity is good but you still need to fix the valid rects
> calculation accordingly.

What's the problem in X11DRV_SetWindowPos? I have checked valid_rects there but the contents of the two rects stored in it look ok to me. They seem to contain the current and previous location of the whole window (the location of where the window starts ignoring the border added by the WM). What should be done in this part of the code?
> > +static void destroy_client_window( Display *display, struct
>
> x11drv_win_data *data )
> > +{
> > - if (!data->client_window) return;
> > -
> > - wine_tsx11_lock();
> > -
> > - XDeleteContext( display, data->client_window, winContext );
> > - XDestroyWindow( display, data->client_window );
> > - data->client_window = 0;
> > - XFlush(display);
> > -
> > - wine_tsx11_unlock();
>
> The XFlush shouldn't be needed.

When destroying the client window when code to recreate the client window is there, this was needed. I guess because events hadn't arrived yet for the destroyed window. But I can remove it for now as perhaps something else was wrong.

Thanks,
Roderick

WineX11.drv: Add an X11 window for the client area [attempt3]

[Alexandre Julliard]

"Roderick Colenbrander"  writes:
> Is it enough to check if the dimensions changed or should I also care
> about the position? (I don't think it changes)

Position can definitely change too.
> What's the problem in X11DRV_SetWindowPos? I have checked valid_rects
> there but the contents of the two rects stored in it look ok to
> me. They seem to contain the current and previous location of the
> whole window (the location of where the window starts ignoring the
> border added by the WM). What should be done in this part of the code?

Since the client window is moved with NorthwestGravity, the valid rects
have to be computed relative to the client, not the whole window. The
client can move relative to the whole window.