[PATCH 1/2] xwayland: Update screen size on output removal

[Olivier Fourdan]

Hi Pekka,

> I see that the above is old code just moved and I don't really know

Yes, it is old code that I simply moved up in the source so I could reuse it without adding a forward declaration :-)

> how Xwayland works here, but that code looks strange to me. I don't
> know what useful meaning wl_output's x+width or y+height have wrt. size
> - all it gives you is the lower-right corner of the wl_output in some
> arbitrary coordinate system.

Yes, as you said, it seems it tries rather naively to compute the overall screen size by adding up the various output size/positions.

> I suppose an example highlights my worry:
> 
> Say there is a wl_output at x=0, y=0 and width=1600, height=1200. Then
> there is a new output hotplugged to the left of the first output. If
> the new output is 800x600, I would expect a Wayland compositor for the
> wl_output to set x=-800, y=0, width=800, height=600[*]. This then results
> in x+width = 0.
> 
> Could Xwayland have a problem with that?

I'll need to investigate that further, but I suspect it would, yes, indeed, that's a good point.

Although this is not necessarily related to the output removal, it's a more general, older problem (and Xwayland has quite a few problems with xrandr, like crashes on exit after an output has been removed, that we need to investigate as well).

> Or does Xwayland perhaps have a requirement for compositors to have
> no output space to the left and/or right of 0,0?

I don't know if there's such a written requirement, but reading the code as you did, I don't think it expect outputs to a have negative position.

> We have also entertained a thought to position the first output in
> Weston not to x=0, y=0 but somewhere totally different like x=-3000,
> y=2000 and see what would break, but I'm not sure if anyone tried it
> yet. In theory it should not make any difference.

That's a good idea, and worth a try.

> [*] Why? So it does not need to move all existing outputs, which would
> lead to sending geometry events for all outputs rather than just one,
> and perhaps updating the internal position of every wl_surface.

Thanks
Olivier