Patch for Weston Running on a Secondary X Screen

Derek Foreman derekf at osg.samsung.com
Tue May 26 08:58:35 PDT 2015


On 26/05/15 04:42 AM, Pekka Paalanen wrote:
> On Fri, 22 May 2015 11:33:59 +0200
> Marko <bgz.marko at gmail.com> wrote:
> 
>> Hello!
>>
>>
>> I had a problem with Weston failing to run when launched in a secondary 
>> screen (such as DISPLAY=:1.1) on a multi X screen configuration. 
>> Attached is the patch that should fix the issue. I reported this as a bug in
>>
>> https://bugs.freedesktop.org/show_bug.cgi?id=90532
>>
>> and Derek suggested to post the patch here on wayland mailing list for a 
>> review.
> 
> Hi,
> 
> the idea of the patch looks good as far as I understand.
> 
> It's a bit hard to review and apply because it is an attachment without
> a proper commit message as produced by git-format-patch.
> 
> +static xcb_screen_t * 
> +x11_compositor_get_default_screen(struct x11_compositor *c)
> +{
> +    xcb_screen_iterator_t iter;
> +
> +    int screen_nbr = XDefaultScreen(c->dpy);
> +    iter = xcb_setup_roots_iterator(xcb_get_setup(c->conn));
> +    for (; iter.rem; --screen_nbr, xcb_screen_next (&iter))
> +        if (screen_nbr == 0)
> +            return iter.data;
> +
> +	return xcb_setup_roots_iterator(xcb_get_setup(c->conn)).data;
> +}
> 
> Indentation seems off and you are using spaces instead of tabs.
> 
> I can't really review the xcb API usage, I'm not familiar with it.

The xcb looks good to me - I implemented the same fix in parallel and
the code was almost identical.  :)

I find the countdown of screen_nbr to be a little counter intuitive.

something like:

for (i = 0; iter.rem; xcb_screen_next(&iter), i++)
	if (i == screen_nbr)
		return iter.data;

seems clearer to me.  But as written I think it's functionally correct.




More information about the wayland-devel mailing list