SimulaVR / Simula

Linux VR Desktop
MIT License
2.91k stars 87 forks source link

VR controllers don't register clicks and motion properly #77

Closed georgewsinger closed 4 years ago

georgewsinger commented 5 years ago

The table below describes the default behavior, as well as the behavior when different things are changed:

Code modification. Simula's Behavior
1 No changes. Pointer motion sent to all surfaces; clicking nor dragging works.
2 Pointer button DOWN force calls pointer button UP. Clicking works, but not dragging.
2 Pointer button DOWN force calls pointer button UP (with defocus in between). Causes "double-clicking" to effectively click links; dragging fails.
3 Clearing the pointer's focus right before a button press. Doesn't work (don't remember pathological behavior).
4 Pointer forced to defocus on every motion event. Dragging works (but you can't let go); clicking doesn't work (since you can't let go).
5 Pointer defocused only when controller points at new surface. Clicking nor dragging still doesn't work. (???)
6 Pointer refocused on both button press and unpress (Ludvig suggestion). Inputs work (hard to aim though), but dragging does not.
7 Use weston_pointer_send_motion instead of pointer_send_motion. Fails, but depends on WestonPointerMotionMask; usually motion just stops registering altogether; see comments.
georgewsinger commented 5 years ago

Simula Events. The following table shows how input events are currently being routed through Simula.

Module Input Event Received Ultimate Function Handler
Simula.hs VR Controller Buttons processClickEvent
SimulaController.hs VR Controller Motion + VR Grip processClickEvent
WestonSurfaceSprite.hs Mouse Motion + Buttons (used in pancake mode) processClickEvent
georgewsinger commented 5 years ago

Possibly relevant to fixing motion: Simula currently isn't sending enter/leave events.

/** Send wl_surface.enter/leave events
 *
 * \param surface The surface.
 * \param head A head of the entered/left output.
 * \param enter True if entered.
 * \param left True if left.
 *
 * Send the enter/leave events for all protocol objects bound to the given
 * output by the client owning the surface.
 */
static void
weston_surface_send_enter_leave(struct weston_surface *surface,
                struct weston_head *head,
                bool enter,
                bool leave)
{
    struct wl_resource *wloutput;
    struct wl_client *client;

    assert(enter != leave);

    client = wl_resource_get_client(surface->resource);
    wl_resource_for_each(wloutput, &head->resource_list) {
        if (wl_resource_get_client(wloutput) != client)
            continue;

        if (enter)
            wl_surface_send_enter(surface->resource, wloutput);
        if (leave)
            wl_surface_send_leave(surface->resource, wloutput);
    }
}
georgewsinger commented 5 years ago

Another idea before I forget: print the pointer's list of resources (i.e., weston_pointer->focus_client->pointer_resources) in both Simula and weston, and compare. This will verify whether or not Simula's pointer is indeed accumulating surfaces.

georgewsinger commented 5 years ago

Unfortunately, libweston3 predates weston_surface_set_enter_leave, though I discovered that for libweston3, the function weston_pointer_set_focus wraps wl_pointer_send_enter and wl_pointer_send_leave:

WL_EXPORT void
weston_pointer_set_focus(struct weston_pointer *pointer,
             struct weston_view *view,
             wl_fixed_t sx, wl_fixed_t sy)
{
    struct weston_pointer_client *pointer_client;
    struct weston_keyboard *kbd = weston_seat_get_keyboard(pointer->seat);
    struct wl_resource *resource;
    struct wl_resource *surface_resource;
    struct wl_display *display = pointer->seat->compositor->wl_display;
    uint32_t serial;
    struct wl_list *focus_resource_list;
    int refocus = 0;

    if ((!pointer->focus && view) ||
        (pointer->focus && !view) ||
        (pointer->focus && pointer->focus->surface != view->surface) ||
        pointer->sx != sx || pointer->sy != sy)
        refocus = 1;

    if (pointer->focus_client && refocus) {
        focus_resource_list = &pointer->focus_client->pointer_resources;
        if (!wl_list_empty(focus_resource_list)) {
            serial = wl_display_next_serial(display);
            surface_resource = pointer->focus->surface->resource;
            wl_resource_for_each(resource, focus_resource_list) {
                wl_pointer_send_leave(resource, serial,
                              surface_resource);
                pointer_send_frame(resource);
            }
        }

        pointer->focus_client = NULL;
    }

    pointer_client = find_pointer_client_for_view(pointer, view);
    if (pointer_client && refocus) {
        struct wl_client *surface_client = pointer_client->client;

        serial = wl_display_next_serial(display);

        if (kbd && kbd->focus != view->surface)
            send_modifiers_to_client_in_list(surface_client,
                             &kbd->resource_list,
                             serial,
                             kbd);

        pointer->focus_client = pointer_client;

        focus_resource_list = &pointer->focus_client->pointer_resources;
        wl_resource_for_each(resource, focus_resource_list) {
            wl_pointer_send_enter(resource,
                          serial,
                          view->surface->resource,
                          sx, sy);
            pointer_send_frame(resource);
        }

        pointer->focus_serial = serial;
    }

    wl_list_remove(&pointer->focus_view_listener.link);
    wl_list_init(&pointer->focus_view_listener.link);
    wl_list_remove(&pointer->focus_resource_listener.link);
    wl_list_init(&pointer->focus_resource_listener.link);
    if (view)
        wl_signal_add(&view->destroy_signal, &pointer->focus_view_listener);
    if (view && view->surface->resource)
        wl_resource_add_destroy_listener(view->surface->resource,
                         &pointer->focus_resource_listener);

    pointer->focus = view;
    pointer->focus_view_listener.notify = pointer_focus_view_destroyed;
    pointer->sx = sx;
    pointer->sy = sy;

    assert(view || sx == wl_fixed_from_int(-1000000));
    assert(view || sy == wl_fixed_from_int(-1000000));

    wl_signal_emit(&pointer->focus_signal, pointer);
}

Is Simula misusing this function?

georgewsinger commented 5 years ago

Or is it worth it for Simula to upgrade to weston5?

georgewsinger commented 5 years ago

Reading a function from wlroots makes me think this might be another issue: when we move GodotSurfaceSprite's around in Simula, are we updating weston where the views are (I presume not, and that weston just thinks all of our surfaces are sitting static in their initial launch positions)? Perhaps this is causing subtle issues?

static void process_cursor_move(struct tinywl_server *server, uint32_t time) {
    /* Move the grabbed view to the new position. */
    server->grabbed_view->x = server->cursor->x - server->grab_x;
    server->grabbed_view->y = server->cursor->y - server->grab_y;
}
KaneTW commented 5 years ago

Weston shouldn't need to know the global position, but maybe there's some reliance on it. Time to dive into weston again.

georgewsinger commented 5 years ago

A small observation in case it matters: Simula is (perhaps unintentionally) using an old helper function from simula-wayland/cbits/util.c:

void pointer_send_motion(struct weston_pointer *pointer, uint32_t time, wl_fixed_t sx, wl_fixed_t sy)
{
    struct wl_list *resource_list;
    struct wl_resource *resource;

  printf("Hello from util.pointer_send_motion\n"); //This test confirms that this is the function being called from WestonSurfaceSprite.hs

    if (sx == pointer->sx && sy == pointer->sy) return;
    pointer->grab->interface->focus(pointer->grab);
    wl_signal_emit(&pointer->motion_signal, pointer);

    if (!pointer->focus_client)
        return;

    resource_list = &pointer->focus_client->pointer_resources;
    wl_resource_for_each(resource, resource_list)
        wl_pointer_send_motion(resource, time, sx, sy);

}

Contrast this with weston's input.c (which isn't even exported to Simula):

static void
pointer_send_motion(struct weston_pointer *pointer, uint32_t time,
            wl_fixed_t sx, wl_fixed_t sy)
{
    struct wl_list *resource_list;
    struct wl_resource *resource;

    if (!pointer->focus_client)
        return;

    resource_list = &pointer->focus_client->pointer_resources;
    wl_resource_for_each(resource, resource_list)
        wl_pointer_send_motion(resource, time, sx, sy);
}

EDIT: I just verified it makes no difference which of these two functions Simula uses: inputs still fail (and in particular motion continues to be improperly sent to all Simula surfaces).

georgewsinger commented 5 years ago

Possibility: weston_views should be grabbed on button down events (such that pointer motion, etc is confined to the view), and then ungrabbed on button up events.