[PATCH v2 1/3] xwayland: Fix the addition and removal of outputs

[Dima Ryazanov]

Some more context here: currently, the output_handle_done function is not
really doing anything, but I haven't figured out yet how to fix it. These
patches fix a few trivial bugs, but there's more work to be done.

The call to RRScreenSizeNotify is a no-op: it expects the width/height of
xwl_screen->screen (not xwl_screen!) to be updated, and returns immediately
otherwise. However, if I update xwl_screen->screen->width and
xwl_screen->screen->height, then RRScreenSizeNotify will crash due to a
NULL pointer (probably because it's called too soon, and some of the dix
state is not initialized yet).

The only time output_handle_done is useful is during the xwayland
initialization: xwl_screen_init waits for xwl_screen->expecting_event to
become 0, then reads the width and height of the xwl_screen.

On Thu, May 21, 2015 at 9:23 AM, Jasper St. Pierre <jstpierre at mecheye.net>
wrote:

> Expecting anything atomic from X11 in the first place is a wrong
> assumption.
>
> On Thu, May 21, 2015 at 6:18 AM, Marek Chalupa <mchqwerty at gmail.com>
> wrote:
> > Hi,
> >
> > On Sat, May 16, 2015 at 7:38 AM, Dima Ryazanov <dima at gmail.com> wrote:
> >>
> >> Add the output to the list when it's created rather than when its
> >> properties
> >> change (as pointed out by Marek Chalupa).
> >> Remove the output from the list when it's destroyed.
> >>
> >> Signed-off-by: Dima Ryazanov <dima at gmail.com>
> >> ---
> >>  hw/xwayland/xwayland-output.c | 5 +++--
> >>  1 file changed, 3 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/hw/xwayland/xwayland-output.c
> b/hw/xwayland/xwayland-output.c
> >> index 155cbc1..9baf4eb 100644
> >> --- a/hw/xwayland/xwayland-output.c
> >> +++ b/hw/xwayland/xwayland-output.c
> >> @@ -120,8 +120,6 @@ output_handle_done(void *data, struct wl_output
> >> *wl_output)
> >>      struct xwl_screen *xwl_screen = xwl_output->xwl_screen;
> >>      int width, height;
> >>
> >> -    xorg_list_append(&xwl_output->link, &xwl_screen->output_list);
> >> -
> >
> >
> > As I pointed out in the other e-mail: I don't think this is right. The
> > append is here on purpose to make the output update atomic.
> > But maybe somebody more erudated should review it :)
> >
> >>
> >>      width = 0;
> >>      height = 0;
> >>      xorg_list_for_each_entry(xwl_output, &xwl_screen->output_list,
> link)
> >> {
> >> @@ -177,6 +175,8 @@ xwl_output_create(struct xwl_screen *xwl_screen,
> >> uint32_t id)
> >>      xwl_output->randr_crtc = RRCrtcCreate(xwl_screen->screen,
> >> xwl_output);
> >>      xwl_output->randr_output = RROutputCreate(xwl_screen->screen, name,
> >>                                                strlen(name),
> xwl_output);
> >> +    xorg_list_append(&xwl_output->link, &xwl_screen->output_list);
> >> +
> >>      RRCrtcGammaSetSize(xwl_output->randr_crtc, 256);
> >>      RROutputSetCrtcs(xwl_output->randr_output, &xwl_output->randr_crtc,
> >> 1);
> >>      RROutputSetConnection(xwl_output->randr_output, RR_Connected);
> >> @@ -190,6 +190,7 @@ xwl_output_destroy(struct xwl_output *xwl_output)
> >>      wl_output_destroy(xwl_output->output);
> >>      RRCrtcDestroy(xwl_output->randr_crtc);
> >>      RROutputDestroy(xwl_output->randr_output);
> >> +    xorg_list_del(&xwl_output->link);
> >>      free(xwl_output);
> >>  }
> >>
> >> --
> >> 2.4.0
> >>
> >
> >
> > _______________________________________________
> > xorg-devel at lists.x.org: X.Org development
> > Archives: http://lists.x.org/archives/xorg-devel
> > Info: http://lists.x.org/mailman/listinfo/xorg-devel
>
>
>
> --
>   Jasper
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.x.org/archives/xorg-devel/attachments/20150524/1e490f29/attachment.html>