Hi, On Mon, Jul 16, 2018 at 10:47 AM Simon Ser <cont...@emersion.fr> wrote: > > On July 16, 2018 8:50 AM, Olivier Fourdan <ofour...@redhat.com> wrote: > > Hi, > > > > Thanks for your follow up on that! > > > > On Fri, Jul 13, 2018 at 9:51 PM Simon Ser <cont...@emersion.fr> wrote: > > > > > > From: emersion <cont...@emersion.fr> > > > > > > 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 <cont...@emersion.fr> > > > --- > > > > > > 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. > > Yes, except this branch handles the "don't do anything" case: width = width, > height = height. The other branch is when we actually need to rotate.
Yeap, agreed. > This variable could probably be renamed to something more sensible (like > `is_logical_size` and invert conditions?). Agreed, the variable name could be better, I've never been very good at picking good function and variable names :) > > > + 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 :) > > The problem is, `output_get_new_size` needs the logical size and we need the > mode size, so one is swapped and the other isn't. We could still factor those > in > a separate function, but I'm afraid this will make things more complicated > than > they already are. Right > > > + > > > + 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 > > So this is Reviewed-by: Olivier Fourdan <ofour...@redhat.com> Cheers, Olivier _______________________________________________ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel