Re: [5/5] winex11: Use an offscreen redirected window for child OpenGL rendering

[Alexandre Julliard]

Chris Robinson <chris.kcat(a)gmail.com> writes:
> +    if(w > 0 && h > 0) {
> +        XFlush(gdi_display);
> +        XCopyArea(gdi_display, physDev->gl_drawable, physDev->drawable,
> +                  physDev->gc, 0, 0, w, h, physDev->dc_rect.left,
> +                  physDev->dc_rect.top);

Why do you need an XFlush here?
> +    if(data->whole_window && vis->visualid == XVisualIDFromVisual(visual))
> +    {
> +        TRACE("Whole window available and visual match, rendering onscreen\n");
> +        goto done;
> +    }
> +
> +    if(!composite_gl)
> +        goto done;

There's no sense in having a config option for this since the
alternative doesn't work anyway, particularly since you removed the
scissor support.
> +        data->gl_drawable = XCreateWindow(gdi_display, root_window, -w, 0, w, h,
> +                                          0, vis->depth, InputOutput,
> +                                          vis->visual,
> +                                          CWColormap | CWOverrideRedirect,
> +                                          &attrib);

The window needs to be created on the thread display, and as a child
of the main window.
> +static void update_gl_drawable(struct x11drv_win_data *data)
> +{
> +    unsigned int border, depth;
> +    int x, y, w, h;
> +    Window root_win;
> +
> +    if(!data->gl_drawable)
> +        return;
> +
> +    wine_tsx11_lock();
> +    XGetGeometry(gdi_display, data->gl_drawable, &root_win, &x, &y,
> +                 (unsigned int*)&w, (unsigned int*)&h, &border, &depth);

You must not use XGetGeometry, this incurs a server round-trip.

-- 
Alexandre Julliard
julliard(a)winehq.org

[5/5] winex11: Use an offscreen redirected window for child OpenGL rendering

[Chris Robinson]

On Tuesday 25 September 2007 04:31:04 am Alexandre Julliard wrote:
> Chris Robinson <chris.kcat(a)gmail.com> writes:
> > +    if(w > 0 && h > 0) {
> > +        XFlush(gdi_display);
> > +        XCopyArea(gdi_display, physDev->gl_drawable, physDev->drawable,
> > +                  physDev->gc, 0, 0, w, h, physDev->dc_rect.left,
> > +                  physDev->dc_rect.top);
>
> Why do you need an XFlush here?

Given the nature of glFlush/glFinish, I think it would probably be correct 
behavior to make sure the X copy command is flushed by the time the GL 
functions finish, too. Why it ended up before the copy though, I don't know.
> > +    if(data->whole_window && vis->visualid ==
> > XVisualIDFromVisual(visual)) +    {
> > +        TRACE("Whole window available and visual match, rendering
> > onscreen\n"); +        goto done;
> > +    }
> > +
> > +    if(!composite_gl)
> > +        goto done;
>
> There's no sense in having a config option for this since the
> alternative doesn't work anyway, particularly since you removed the
> scissor support.

True. I guess I can use an option just for extra onscreen pixel formats being 
composited instead, in a seperate patch of course.
> > +        data->gl_drawable = XCreateWindow(gdi_display, root_window, -w,
> > 0, w, h, +                                          0, vis->depth,
> > InputOutput, +                                          vis->visual,
> > +                                          CWColormap |
> > CWOverrideRedirect, +                                          &attrib);
>
> The window needs to be created on the thread display, and as a child
> of the main window.

With the child being parented to the main window, a BadWindow error sometimes 
crops up on XDestroyWindow for the X11 child. I couldn't see why, but 
parenting it to the root_window works. Would that be due to using gdi_display 
for creating/destroying the child?
> > +static void update_gl_drawable(struct x11drv_win_data *data)
> > +{
> > +    unsigned int border, depth;
> > +    int x, y, w, h;
> > +    Window root_win;
> > +
> > +    if(!data->gl_drawable)
> > +        return;
> > +
> > +    wine_tsx11_lock();
> > +    XGetGeometry(gdi_display, data->gl_drawable, &root_win, &x, &y,
> > +                 (unsigned int*)&w, (unsigned int*)&h, &border, &depth);
>
> You must not use XGetGeometry, this incurs a server round-trip.

Is it safe to call XMoveResizeWindow even if the size didn't change, and have 
X make it a no-op (or at least be negligible for processing time)? Calling to 
resize the child window every time the hwnd simply moves (or whenever it 
thinks it resized/moved) can cause it to be done unnecessarilly often.

[5/5] winex11: Use an offscreen redirected window for child OpenGL rendering

[Alexandre Julliard]

Chris Robinson <chris.kcat(a)gmail.com> writes:
> With the child being parented to the main window, a BadWindow error sometimes 
> crops up on XDestroyWindow for the X11 child. I couldn't see why, but 
> parenting it to the root_window works. Would that be due to using gdi_display 
> for creating/destroying the child?

Maybe, but in any case making it a child of the root window is wrong.
> Is it safe to call XMoveResizeWindow even if the size didn't change, and have 
> X make it a no-op (or at least be negligible for processing time)? Calling to 
> resize the child window every time the hwnd simply moves (or whenever it 
> thinks it resized/moved) can cause it to be done unnecessarilly often.

No, you have all the info you need in SetWindowPos, there's no reason
to guess.

-- 
Alexandre Julliard
julliard(a)winehq.org