I just made that change in my set of patches, and I think it fixes the problem.
On Thu, May 14, 2015 at 9:43 AM, Dima Ryazanov <[email protected]> wrote: > Actually, why not just move "xorg_list_append(&xwl_output->link, > &xwl_screen->output_list);" to xwl_output_create? > > I can't tell if there's a reason it's in xwl_output_done, or if it's just > an oversight. > > On Thu, May 14, 2015 at 9:37 AM, Dima Ryazanov <[email protected]> wrote: > >> Oh wow, I was playing around with outputs, and never realized >> output_handle_done >> was being called after any geometry change, not just after the output was >> created. >> >> On Thu, May 14, 2015 at 2:58 AM, Marek Chalupa <[email protected]> >> wrote: >> >>> output.done event can be sent even on some property change, not only >>> when announcing the output. Therefore we must check if we already have it >>> otherwise we may corrupt the list by adding it multiple times. >>> >>> This fixes bug when xwayland looped indefinitely in output.done handler >>> and that can be reproduced following these steps (under X without >>> multi-monitor setup): >>> 1) run weston --output-count=2 >>> 2) run xterm, move it so that half is on one output >>> and half on the other >>> 3) close second output, try run weston-terminal >>> >> >> (I can only repro it after closing the first output, not the second one; >> is that what you meant?) >> >> >>> weston sends updated outputs which trigger this bug. >>> >>> Signed-off-by: Marek Chalupa <[email protected]> >>> --- >>> hw/xwayland/xwayland-output.c | 25 +++++++++++++++++++------ >>> 1 file changed, 19 insertions(+), 6 deletions(-) >>> >>> diff --git a/hw/xwayland/xwayland-output.c >>> b/hw/xwayland/xwayland-output.c >>> index 155cbc1..0c96e87 100644 >>> --- a/hw/xwayland/xwayland-output.c >>> +++ b/hw/xwayland/xwayland-output.c >>> @@ -116,15 +116,28 @@ output_handle_mode(void *data, struct wl_output >>> *wl_output, uint32_t flags, >>> static void >>> output_handle_done(void *data, struct wl_output *wl_output) >>> { >>> - struct xwl_output *xwl_output = data; >>> + struct xwl_output *it, *xwl_output = data; >>> struct xwl_screen *xwl_screen = xwl_output->xwl_screen; >>> - int width, height; >>> + int width = 0, height = 0, has_this_output = 0; >>> + >>> + xorg_list_for_each_entry(it, &xwl_screen->output_list, link) { >>> + /* output done event is sent even when some property >>> + * of output is changed. That means that we may already >>> + * have this output. If it is true, we must not add it >>> + * into the output_list otherwise we'll corrupt it */ >>> + if (it == xwl_output) >>> + has_this_output = 1; >>> + >>> + if (width < it->x + it->width) >>> + width = it->x + it->width; >>> + if (height < it->y + it->height) >>> + height = it->y + it->height; >>> + } >>> >>> - xorg_list_append(&xwl_output->link, &xwl_screen->output_list); >>> + if (!has_this_output) { >>> + xorg_list_append(&xwl_output->link, &xwl_screen->output_list); >>> >> >> I think this line should also be moved here: >> xwl_screen->expecting_event--; >> >> (It might not matter since it's only used by xwl_screen_init - but the >> code seems to assume it would only be decremented once after the output is >> created.) >> >> - width = 0; >>> - height = 0; >>> - xorg_list_for_each_entry(xwl_output, &xwl_screen->output_list, >>> link) { >>> + /* we did not check this output for new screen size, do it now >>> */ >>> if (width < xwl_output->x + xwl_output->width) >>> width = xwl_output->x + xwl_output->width; >>> if (height < xwl_output->y + xwl_output->height) >>> -- >>> 2.1.0 >>> >>> _______________________________________________ >>> wayland-devel mailing list >>> [email protected] >>> http://lists.freedesktop.org/mailman/listinfo/wayland-devel >>> >> >> Just a thought... You could instead: >> - check if the output already exists >> - add it if necessary >> - update the width and height >> >> That way, the width/height calculation code won't be duplicated. (Though >> it will require iterating over output_list twice.) Anyways, it's up to you. >> >> >
_______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
