dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
From: Zack Rusin <zackr@vmware.com>
To: Simon Ser <contact@emersion.fr>
Cc: Martin Krastev <krastevm@vmware.com>,
	David Airlie <airlied@linux.ie>,
	"dri-devel@lists.freedesktop.org"
	<dri-devel@lists.freedesktop.org>,
	Gurchetan Singh <gurchetansingh@chromium.org>,
	Hans de Goede <hdegoede@redhat.com>,
	Pekka Paalanen <ppaalanen@gmail.com>,
	Gerd Hoffmann <kraxel@redhat.com>,
	Thomas Zimmermann <tzimmermann@suse.de>,
	wayland-devel <wayland-devel@lists.freedesktop.org>,
	Maaz Mombasawala <mombasawalam@vmware.com>
Subject: Re: [PATCH 0/6] drm: Add mouse cursor hotspot support to atomic KMS
Date: Sun, 5 Jun 2022 15:47:29 +0000	[thread overview]
Message-ID: <40CAE3E3-9C74-4DC3-B0FD-8F00C7C29291@vmware.com> (raw)
In-Reply-To: <VdFbe3wmVv8sSzgW8rsWsOaP3vTRyGGs86yIrHzf95_hCzy2ZNc90dY1TNUcVCo6057K1hsY0y_kVoFRfZ23hgUKMIpSoRAh5MAh5RmBUZI=@emersion.fr>



> On Jun 5, 2022, at 3:30 AM, Simon Ser <contact@emersion.fr> wrote:
> 
> ⚠ External Email
> 
> On Friday, June 3rd, 2022 at 20:31, Zack Rusin <zackr@vmware.com> wrote:
> 
>>>>>>> My proposal was:
>>>>>>> 
>>>>>>> - Introduce DRM_CLIENT_CAP_CURSOR_PLANE_NO_POSITION (or a better name). Only
>>>>>>> user-space which supports the hotspot props will enable it.
>>>>>>> - By default, don't expose a cursor plane, because current user-space doesn't
>>>>>>> support it (Weston might put a window in the cursor plane, and nobody can
>>>>>>> report hotspot).
>>>>>>> - If the KMS client enables the cap, advertise the cursor
>>>>>>> plane, and make it so the plane doesn't have the CRTC_X/CRTC_Y properties
>>>>>>> since the driver will pick the position.
>>>>>>> 
>>>>>>> That way both old and new user-space are fixed.
>>>>>> 
>>>>>> I don’t think I see how that fixes anything. In particular I don’t see a way
>>>>>> of fixing the old user space at all. We require hotspot info, old user space
>>>>>> doesn’t set it because there’s no way of setting it on atomic kms.
>>>>> 
>>>>> Old atomic user-space is fixed by removing the cursor plane. Then old
>>>>> atomic user-space will fallback to drawing the cursor itself, e.g. via
>>>>> OpenGL.
>>>> 
>>>> But it’s not fixed, because the driver is still on a deny-list and
>>>> nothing implements this. You’re saying you could potentially “fix” by
>>>> implementing client side cursor handling in all kms clients? That’s a
>>>> hard sell if the user space can just put the virtualized driver on
>>>> deny-lists and fallback to use old kms which supports hotspots.
>>> 
>>> What deny-list are you referring to?
>>> 
>>> All compositors I know of implement a fallback when no cursor plane is
>>> usable.
>> 
>> I put the links in the first email in the
>> series:
>> https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.gnome.org%2FGNOME%2Fmutter%2F-%2Fblob%2Fmain%2Fsrc%2Fbackends%2Fnative%2Fmeta-kms-impl-device-atomic.c%23L1188&amp;data=05%7C01%7Czackr%40vmware.com%7C2b0ab6c67e7d4bfb618a08da46c5394f%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637900110157712044%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=wggJaNScF0ziIG%2BfSdSUKBVZGoNjtm4Q8amS7SbJa%2FY%3D&amp;reserved=0
>> https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Finvent.kde.org%2Fplasma%2Fkwin%2F-%2Fblob%2Fmaster%2Fsrc%2Fbackends%2Fdrm%2Fdrm_gpu.cpp%23L156&amp;data=05%7C01%7Czackr%40vmware.com%7C2b0ab6c67e7d4bfb618a08da46c5394f%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637900110157712044%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=%2BCTEJ9XtlI9kuKJJZVMNodtkZSkRIv8RN9jSRAM1pqM%3D&amp;reserved=0
> 
> I was not aware of these lists. Having to work-around drivers in user-space is
> really not great.
> 
> But regardless, that doesn't really change anything. With my proposal:
> 
> - Old user-space with hacky deny-lists (Mutter, KWin) still goes through the
>  legacy uAPI.
> - Old user-space without the hacky deny-lists (Weston, wlroots) uses software
>  cursors and is not broken anymore.
> - New user-space can report hotspot and is not broken.

Yea, that doesn’t work, but I think below I stopped where the issue is.

>> Also, let me point this out because it also seems to be a fundamental
>> misunderstanding, user space implementing software cursor doesn’t fix
>> anything. Just leaves everything broken in different ways. The reason
>> virtualized drivers went away from software cursors is because it makes it
>> impossible to make it work with a bunch of interesting and desirable
>> scenarios, e.g. unity mode where the guest might have a terminal and browser
>> open and then the virtual machine software creates windows out of those, so
>> you don’t have the entire desktop of the guest but instead native looking
>> windows with the apps. In that case guest has no way of knowing when the
>> cursor leaves the window, so to make seemless cursors work you’d need to
>> implement irc or backdoors that send that information from the host to the
>> guest, then have virtualized drivers create some sort of uevent, to send to
>> the userspace to let it know to hide the cursor because it actually left the
>> window. That’s a trivial scenario and there’s a lot more with mice that are
>> remoted themselves, guests with singular or maybe many apps exported,
>> possibly overlapping on the host but all within a desktop in the guest, etc.
> 
> Are you saying that the current broken behavior is better than software
> cursors?

They’re broken in very different ways. You’re asking me which bugs do I prefer. Ultimately the current lack of hotspots leaves mouse unusable but I don’t see any reason to trade that for another set of bugs instead of just fixing it (either via fallback to legacy or using the new hotspot api).

>>>>> New user-space supports setting the hotspot prop, and is aware that it can't
>>>>> set the cursor plane position, so the cursor plane can be exposed again when
>>>>> the cap is enabled.
>>>> 
>>>> But we still use cursor plane position. Hotspots are offsets from
>>>> cursor plane positions. Both are required.
>>> 
>>> No, VM drivers don't need and disregard the cursor position AFAIK. They
>>> replace it with the host cursor's position.
>> 
>> Iirc they all use it.
> 
> What do they use it for?
> 
> The whole point of this discussion is to be able to display the guest's cursor
> image on the host's cursor, so that latency is reduced?

Ah, I think I see now where the confusion is coming from. No, it’s definitely not. This has nothing to do with latency. By default position of a mouse cursor doesn’t tell us where the point that is actually used as an activation of a click is, e.g. a mouse cursor image with an arrow pointing to the top-left and a mouse cursor image pointing to the bottom right - both at the same position, as you can imagine it will become impossible to use the desktop if the click defaults to the top left, especially as the number of cursor images increases, you need information about which point within the cursor image activates the click, that’s the hotspot. You need to know the position of the image and you need to know which point within that image is responsible for actually activating the events.

So this has nothing to do with latency, this is about mouse cursor clicks/drags simply having wrong coordinates and mouse being effectively impossible to use on anything that doesn’t have the same workarounds as gnome-shell/kwin. So if you have user space code which hasn’t implemented the same workarounds gnome-shell/kwin that means that is has never been used with virtualized drivers because people tend to notice pretty quickly that when they click on a button/link a completely different button/link in a different part of the screen is activated...

z


  reply	other threads:[~2022-06-05 15:47 UTC|newest]

Thread overview: 64+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-02 15:42 [PATCH 0/6] drm: Add mouse cursor hotspot support to atomic KMS Zack Rusin
2022-06-02 15:42 ` [PATCH 1/6] drm/atomic: Add support for mouse hotspots Zack Rusin
2022-06-02 15:42 ` [PATCH 2/6] drm/vmwgfx: Create mouse hotspot properties on cursor planes Zack Rusin
2022-06-03 13:11   ` Martin Krastev (VMware)
2022-06-02 15:42 ` [PATCH 3/6] drm/qxl: " Zack Rusin
2022-06-02 22:05   ` kernel test robot
2022-06-02 23:26   ` kernel test robot
2022-06-02 15:42 ` [PATCH 4/6] drm/vboxvideo: " Zack Rusin
2022-06-02 15:42 ` [PATCH 5/6] drm/virtio: " Zack Rusin
2022-06-02 15:42 ` [PATCH 6/6] drm: Remove legacy cursor hotspot code Zack Rusin
2022-06-03 10:28 ` [PATCH 0/6] drm: Add mouse cursor hotspot support to atomic KMS Gerd Hoffmann
2022-06-03 14:43   ` Zack Rusin
2022-06-03 14:14 ` Simon Ser
2022-06-03 14:27   ` Zack Rusin
2022-06-03 14:32     ` Simon Ser
2022-06-03 14:38       ` Zack Rusin
2022-06-03 14:56         ` Simon Ser
2022-06-03 15:17           ` Zack Rusin
2022-06-03 15:22             ` Simon Ser
2022-06-03 15:32               ` Zack Rusin
2022-06-03 15:49                 ` Simon Ser
2022-06-03 18:31                   ` Zack Rusin
2022-06-05  7:30                     ` Simon Ser
2022-06-05 15:47                       ` Zack Rusin [this message]
2022-06-05 16:26                         ` Simon Ser
2022-06-05 18:16                           ` Zack Rusin
2022-06-06  8:11                             ` Simon Ser
2022-06-04 21:19               ` Hans de Goede
2022-06-05  7:34                 ` Simon Ser
2022-06-07 11:25             ` Gerd Hoffmann
2022-06-06  9:13         ` Pekka Paalanen
2022-06-07  8:07   ` Pekka Paalanen
2022-06-07 14:30     ` Gerd Hoffmann
2022-06-08  7:53       ` Pekka Paalanen
2022-06-08 14:52         ` Gerd Hoffmann
2022-06-07 17:50     ` Zack Rusin
2022-06-08  7:53       ` Pekka Paalanen
2022-06-09 19:39         ` Zack Rusin
2022-06-10  7:49           ` Pekka Paalanen
2022-06-10  8:22             ` Jonas Ådahl
2022-06-10  8:54           ` Simon Ser
2022-06-10  9:01             ` Daniel Vetter
2022-06-10  8:41   ` Daniel Vetter
2022-06-10  8:56     ` Pekka Paalanen
2022-06-10  8:59     ` Daniel Vetter
2022-06-10 12:03       ` Gerd Hoffmann
2022-06-10 14:24       ` Zack Rusin
2022-06-13  7:33         ` Pekka Paalanen
2022-06-13 13:14           ` Zack Rusin
2022-06-13 14:25             ` Pekka Paalanen
2022-06-13 14:54               ` Zack Rusin
2022-06-14  7:36                 ` Pekka Paalanen
2022-06-14 14:40                   ` Zack Rusin
2022-06-14 14:54                     ` Daniel Stone
2023-06-09 15:20       ` Albert Esteve
2023-06-21  7:10         ` Javier Martinez Canillas
2023-06-22  4:29           ` Zack Rusin
2023-06-22  6:20             ` Javier Martinez Canillas
2022-06-10  9:15     ` Simon Ser
2022-06-10  9:49       ` Daniel Vetter
2022-06-10 12:36         ` Gerd Hoffmann
2022-06-10 12:53           ` Simon Ser
2022-06-11 15:34             ` Hans de Goede
2022-06-13  7:45               ` Pekka Paalanen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=40CAE3E3-9C74-4DC3-B0FD-8F00C7C29291@vmware.com \
    --to=zackr@vmware.com \
    --cc=airlied@linux.ie \
    --cc=contact@emersion.fr \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=gurchetansingh@chromium.org \
    --cc=hdegoede@redhat.com \
    --cc=krastevm@vmware.com \
    --cc=kraxel@redhat.com \
    --cc=mombasawalam@vmware.com \
    --cc=ppaalanen@gmail.com \
    --cc=tzimmermann@suse.de \
    --cc=wayland-devel@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).