Re: [PATCH xserver] xwayland: don't use logical size for RRMode
Hi, On Tue, 10 Jul 2018 at 11:59, Simon Ser wrote: > Yes, that's intentional. When the physical size isn't available (e.g. when > using > projectors or virtual outputs) we just send zero. Do you think this is > harmful? According to the xrandr protocol definition: https://cgit.freedesktop.org/xorg/proto/randrproto/tree/randrproto.txt For the definition of RRGetOutputInfo: | 'widthInMillimeters' and 'heightInMillimeters' report the physical | size of the displayed area. If unknown, or not really fixed (e.g., | for a projector), these values are both zero. So I reckon using 0 for physical size is valid. > I like sending zero because it clearly says "this piece of information is not > relevant in the current situation" instead of faking data. Maybe Xwayland > should > fake it and use a default 96 DPI in this case? > I'm not aware of any X11 client that breaks because of this (yet). If zero for the physical size is a valid value (as stated by the protocol for RRGetOutputInfo), clients should be able to deal with that or else it's a client bug. 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
Re: [PATCH xserver] xwayland: don't use logical size for RRMode
On July 10, 2018 8:44 AM, Olivier Fourdan wrote: > Yeap, that's what I meant, xdg-output would advertise the logical size > (i.e. appropriately scaled), so we want to pass RR the values provided > by xdg-output here, not those from wl_output. > > Basically, we do not need to keep or reuse the mode_width/height as > with your initial patch, we just need to compensate the rotation as > you said (as for the rationale behind this bug, mutter doesn't use > rotations, so I didn't notice it). Right. Just sent another patch which does that. I just thought it was weird to use the logical size for the mode, but it makes sense since X11 doesn't support scaling. > [unrelated, as an aside, it looks like in your example, the compositor > might not be sending the geometry, so we end up with 0 in the physical > size] Yes, that's intentional. When the physical size isn't available (e.g. when using projectors or virtual outputs) we just send zero. Do you think this is harmful? I like sending zero because it clearly says "this piece of information is not relevant in the current situation" instead of faking data. Maybe Xwayland should fake it and use a default 96 DPI in this case? I'm not aware of any X11 client that breaks because of this (yet). > 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
Re: [PATCH xserver] xwayland: don't use logical size for RRMode
On July 9, 2018 3:42 PM, Olivier Fourdan wrote: > So, can you please elaborate exactly what this is supposed to fix and > how to reproduce the issue? I've configured my compositor with a 90 degree output rotation. Here are the values sent by my compositor, as received by xwayland: output_handle_mode 1600 850 xdg_output_handle_logical_size 850 1600 As far as I can tell this is correct. Without this patch, if I open for instance Firefox, pointer events are correctly handled if the pointer is in the top half of the screen, but when the pointer moves in the bottom part the Y axis coordinate gets stuck. Here's xrandr's output: Screen 0: minimum 320 x 200, current 850 x 1600, maximum 8192 x 8192 XWAYLAND0 connected 1600x848+0+0 left (normal left inverted right x axis y axis) 0mm x 0mm 848x1600 59.95*+ This patch fixes this bug. Here's xrandr's output: Screen 0: minimum 320 x 200, current 850 x 1600, maximum 8192 x 8192 XWAYLAND0 connected 850x1600+0+0 left (normal left inverted right x axis y axis) 0mm x 0mm 1600x850 59.92*+ Notice the difference between the logical geometry (850x1600+0+0) and the current mode (1600x850). I just noticed this patch breaks HiDPI outputs (xrandr advertises a 850x1600+0+0 logical geometry). We might want to use the logical size for RRMode, but compensate transformations. What do you think? Thanks, Simon ___ 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
Re: [PATCH xserver] xwayland: don't use logical size for RRMode
Hi,
On Mon, 9 Jul 2018 at 09:17, Simon Ser wrote:
>
> The logical size is the size of the output in the global compositor
> space. The mode width/height shouldn't be transformed and scaled.
>
> This fixes issues with pointer input on transformed outputs.
>
> Signed-Off-By: Simon Ser
> ---
> hw/xwayland/xwayland-output.c | 9 ++---
> hw/xwayland/xwayland.h| 3 ++-
> 2 files changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/hw/xwayland/xwayland-output.c b/hw/xwayland/xwayland-output.c
> index 379062549..44fd5c26a 100644
> --- a/hw/xwayland/xwayland-output.c
> +++ b/hw/xwayland/xwayland-output.c
> @@ -113,12 +113,15 @@ output_handle_mode(void *data, struct wl_output
> *wl_output, uint32_t flags,
> if (!(flags & WL_OUTPUT_MODE_CURRENT))
> return;
>
> +xwl_output->mode_width = width;
> +xwl_output->mode_height = height;
> +xwl_output->mode_refresh = refresh;
So, with that patch mode_width/height reflects the wl_output size.
> +
> /* Apply the change from wl_output only if xdg-output is not supported */
> if (!xwl_output->xdg_output) {
> xwl_output->width = width;
> xwl_output->height = height;
> }
> -xwl_output->refresh = refresh;
> }
>
> static inline void
> @@ -224,8 +227,8 @@ 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,
> - xwl_output->refresh / 1000.0, 0, 0);
> +randr_mode = xwayland_cvt(xwl_output->mode_width,
> xwl_output->mode_height,
> + xwl_output->mode_refresh / 1000.0, 0, 0);
And we'd advertise wl_ouput size as RR size? That's not how it's meant
to work, we want RR to reflect the xdg-output (logical) size.
> RROutputSetModes(xwl_output->randr_output, _mode, 1, 1);
> RRCrtcNotify(xwl_output->randr_crtc, randr_mode,
> xwl_output->x, xwl_output->y,
> diff --git a/hw/xwayland/xwayland.h b/hw/xwayland/xwayland.h
> index d70ad54bf..66daf58de 100644
> --- a/hw/xwayland/xwayland.h
> +++ b/hw/xwayland/xwayland.h
> @@ -368,7 +368,8 @@ struct xwl_output {
> struct xwl_screen *xwl_screen;
> RROutputPtr randr_output;
> RRCrtcPtr randr_crtc;
> -int32_t x, y, width, height, refresh;
> +int32_t x, y, width, height;
> +int32_t mode_width, mode_height, mode_refresh;
> Rotation rotation;
> Bool wl_output_done;
> Bool xdg_output_done;
> --
> 2.18.0
So, can you please elaborate exactly what this is supposed to fix and
how to reproduce the issue?
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
[PATCH xserver] xwayland: don't use logical size for RRMode
The logical size is the size of the output in the global compositor
space. The mode width/height shouldn't be transformed and scaled.
This fixes issues with pointer input on transformed outputs.
Signed-Off-By: Simon Ser
---
hw/xwayland/xwayland-output.c | 9 ++---
hw/xwayland/xwayland.h| 3 ++-
2 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/hw/xwayland/xwayland-output.c b/hw/xwayland/xwayland-output.c
index 379062549..44fd5c26a 100644
--- a/hw/xwayland/xwayland-output.c
+++ b/hw/xwayland/xwayland-output.c
@@ -113,12 +113,15 @@ output_handle_mode(void *data, struct wl_output
*wl_output, uint32_t flags,
if (!(flags & WL_OUTPUT_MODE_CURRENT))
return;
+xwl_output->mode_width = width;
+xwl_output->mode_height = height;
+xwl_output->mode_refresh = refresh;
+
/* Apply the change from wl_output only if xdg-output is not supported */
if (!xwl_output->xdg_output) {
xwl_output->width = width;
xwl_output->height = height;
}
-xwl_output->refresh = refresh;
}
static inline void
@@ -224,8 +227,8 @@ 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,
- xwl_output->refresh / 1000.0, 0, 0);
+randr_mode = xwayland_cvt(xwl_output->mode_width, xwl_output->mode_height,
+ xwl_output->mode_refresh / 1000.0, 0, 0);
RROutputSetModes(xwl_output->randr_output, _mode, 1, 1);
RRCrtcNotify(xwl_output->randr_crtc, randr_mode,
xwl_output->x, xwl_output->y,
diff --git a/hw/xwayland/xwayland.h b/hw/xwayland/xwayland.h
index d70ad54bf..66daf58de 100644
--- a/hw/xwayland/xwayland.h
+++ b/hw/xwayland/xwayland.h
@@ -368,7 +368,8 @@ struct xwl_output {
struct xwl_screen *xwl_screen;
RROutputPtr randr_output;
RRCrtcPtr randr_crtc;
-int32_t x, y, width, height, refresh;
+int32_t x, y, width, height;
+int32_t mode_width, mode_height, mode_refresh;
Rotation rotation;
Bool wl_output_done;
Bool xdg_output_done;
--
2.18.0
___
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
