Hi, Thanks for your follow up on that!
On Fri, Jul 13, 2018 at 9:51 PM Simon Ser <[email protected]> wrote: > > From: emersion <[email protected]> > > The logical size is the size of the output in the global compositor > space. The mode width/height should be scaled as in the logical > size, but shouldn't be transformed. Thus we need to rotate back > the logical size to be able to use it as the mode width/height. > > This fixes issues with pointer input on transformed outputs. > > Signed-Off-By: Simon Ser <[email protected]> > --- > > Changes from v1 to v2: > - Fixed rotation when xdg-output isn't available > > I've made sure this works on both rootston (with xdg-output) and > Weston (without xdg-output). > > hw/xwayland/xwayland-output.c | 12 +++++++++++- > 1 file changed, 11 insertions(+), 1 deletion(-) > > diff --git a/hw/xwayland/xwayland-output.c b/hw/xwayland/xwayland-output.c > index 379062549..0d2ec7890 100644 > --- a/hw/xwayland/xwayland-output.c > +++ b/hw/xwayland/xwayland-output.c > @@ -213,6 +213,7 @@ apply_output_change(struct xwl_output *xwl_output) > { > struct xwl_screen *xwl_screen = xwl_output->xwl_screen; > struct xwl_output *it; > + int mode_width, mode_height; > int width = 0, height = 0, has_this_output = 0; > RRModePtr randr_mode; > Bool need_rotate; > @@ -224,7 +225,16 @@ apply_output_change(struct xwl_output *xwl_output) > /* xdg-output sends output size in compositor space. so already rotated > */ > need_rotate = (xwl_output->xdg_output == NULL); > > - randr_mode = xwayland_cvt(xwl_output->width, xwl_output->height, > + /* We need to rotate back the logical size for the mode */ > + if (need_rotate || xwl_output->rotation & (RR_Rotate_0 | RR_Rotate_180)) > { But is it `need_rotate` or `!need_rotate` here? `need_rotate` being `TRUE` means we don't have xdg-output available, in which case we shouldn't have to do this. Basically, we need to revert to the original width/height only if we have xdg-output (which has already applied the rotation), so I reckon it should be `!need_rotate` here. > + mode_width = xwl_output->width; > + mode_height = xwl_output->height; > + } else { > + mode_width = xwl_output->height; > + mode_height = xwl_output->width; > + } So if we use `(!need_rotate)`, this addition becomes completely similar to the code found in `output_get_new_size()` it might be interesting to move that to a small helper function, e.g. `output_get_transform_size()` that would return the swapped width/height depending on the output rotation, so we don't duplicate that small portion of code. Nit picking, I know :) > + > + randr_mode = xwayland_cvt(mode_width, mode_height, > xwl_output->refresh / 1000.0, 0, 0); > RROutputSetModes(xwl_output->randr_output, &randr_mode, 1, 1); > RRCrtcNotify(xwl_output->randr_crtc, randr_mode, > -- > 2.18.0 Also, this is something I noticed the hard way some time ago (completely unrelated to your patch, though), the width/height parameters for `output_get_new_size()` are inverted (`height, width` instead of `width, height` as usual) which is error prone (don't ask how I noticed...). I can't see a reason for this everywhere else in the code we use `width, height`, I reckon it would be great if we could fix that (in a separate patch) as well, while at it... Because things are already complicated enough that we don't need to add more confusion by changing the well established order of parameters just for the sake of it :) Cheers, Olivier _______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
