protocol: Add support for side mouse buttons

Side mouse buttons currently do not exist in the protocol, causing them to be inexplicably ignored by VMs in virt-manager and such. This lays the groundwork for fixing that issue.

I have also already implemented this in spice-server, spice-gtk and the Linux vd-agent. I plan on submitting MRs for those projects as well if this is accepted.

Nothing wrong changing this file but it's autogenerated from spice-common/spice.proto. Change there can be done later too.

My mistake. I can submit a MR there if we agree to go forward with this.

changed this line in version 2 of the diff

I think it would be great to keep this VD_AGENT_xBUTTON_ list in sync with SPICE_BUTTON_MASK_ one. There is a code in the server that could use this sync to support any future button too.

@kpouget, what do you think? Recently you had a PoC about this.

yes, in my POC I had to define SPICE_MOUSE_BUTTON_PREV/SPICE_MOUSE_BUTTON_NEXT (side/extra)

you can have a look at what I shared last month:

https://gitlab.freedesktop.org/kpouget/spice/-/commits/exp/mouse-bt https://gitlab.freedesktop.org/kpouget/spice-gtk/-/commits/exp/mouse-bt https://gitlab.freedesktop.org/kpouget/spice-protocol/-/commits/exp/mouse-bt https://gitlab.freedesktop.org/kpouget/vd_agent/-/tree/exp/mouse-bt

but the code in spice-gtk relied on undocumented GTK event IDs, @SimonPilkington do you have a solution for that?

Unfortunately no. Per https://gitlab.gnome.org/GNOME/gtk/-/issues/515 there seems to be no interest from GTK in fixing this.

My current implementation looks like this:

static int button_gdk_to_spice(guint gdk)
{
    static const int map[] = {
        [ 1 ] = SPICE_MOUSE_BUTTON_LEFT,
        [ 2 ] = SPICE_MOUSE_BUTTON_MIDDLE,
        [ 3 ] = SPICE_MOUSE_BUTTON_RIGHT,
        [ 4 ] = SPICE_MOUSE_BUTTON_UP,
        [ 5 ] = SPICE_MOUSE_BUTTON_DOWN,
        [ 8 ] = SPICE_MOUSE_BUTTON_SIDE,
        [ 9 ] = SPICE_MOUSE_BUTTON_EXTRA
    };

    if (gdk < SPICE_N_ELEMENTS(map)) {
        return map [ gdk ];
    }
    return 0;
}

static int button_mask_gdk_to_spice(int gdk)
{
    int spice = 0;

    if (gdk & GDK_BUTTON1_MASK)
        spice |= SPICE_MOUSE_BUTTON_MASK_LEFT;
    if (gdk & GDK_BUTTON2_MASK)
        spice |= SPICE_MOUSE_BUTTON_MASK_MIDDLE;
    if (gdk & GDK_BUTTON3_MASK)
        spice |= SPICE_MOUSE_BUTTON_MASK_RIGHT;
    if (gdk & (1 << 15))
        spice |= SPICE_MOUSE_BUTTON_MASK_SIDE;
    if (gdk & (1 << 16))
        spice |= SPICE_MOUSE_BUTTON_MASK_EXTRA;
    return spice;
}

I have learned that this is problematic. In button_mask_gdk_to_spice() GDK sets the (1 << 15) and (1 << 16) bits on Wayland sessions, but not on X11 (or Xwayland).

Since the server seems to rely entirely on these to let vdagent know what button is pressed, my current implementation doesn't work on X11. As button_gdk_to_spice() still works fine on X11, this could potentially be solved by keeping track of modifiers ourselves instead of relying on GDK.

@kpouget Can you confirm your implementation also has this problem? It appears mostly identical to mine, so I would assume it does, but I'd like to be sure I didn't make a mistake somewhere.

@SimonPilkington yes, I think we had the same issue, it was working well on my workstation with Wayland, and it didn't work in the environment of the guy who ask for the feature. I didn't try to understand why it was failing for him, but Wayland vs Xorg must be the explanation!

Thanks for confirming. I was able to get around it using my idea above. Tested only with Xwayland but I don't see why it wouldn't work on a full Xorg session.

The problem here is not the patches for the protocol but to fill and use these bits reliably. I think GDK or other layer do not reliably set additional bits so it could be that the bits you are testing are not the one you expect (so empty or referring to other buttons). Same issues for both guest and client side. For instance I remember that GTK on X11 copy some bytes ("detail" field) from X11 but bit outside left/right/middle buttons have different meaning based on the device so you can't assume that bit 3 have a specific meaning.

Hey, is there anything I could do to help this move forward ?

Added a small change.

Personally I would merge this on protocol. The protocol part is the less problematic, the more complicated is guest/client.

@SimonPilkington, do you agree on my change? Could you squash it in?

Done. LGTM.

The problem with the inconsistency of GDK's button handling between Xorg and wayland remains, but can be worked around as outlined above. In button_gdk_to_spice() the side buttons seem to consistently be 8 and 9.

added 1 commit

• 4a5c43e1 - Use same order for SPICE protocol and agent protocol

Compare with previous version

added 1 commit

• a24e5fa6 - protocol: Add support for side mouse buttons

Compare with previous version

mentioned in issue spice#45 (closed)

Merged this change. Maybe future general discussions on this subject should be done on spice#45 (closed).

closed

mentioned in merge request spice-gtk!96 (merged)

mentioned in merge request spice/linux/vd_agent!40 (merged)