On Tue, Sep 13, 2016 at 8:15 PM, Christopher Michael
<[email protected]> wrote:
> Hi Gustavo,
>
> Quite a few inlined comments below, so please read whole email :)

It was Dilly and Iscaro that wrote that RFC, not me :-)

they work with me and I reviewed the text, so some replies to you below.


> On 09/13/2016 06:00 PM, Bruno Dilly wrote:
>> Hi folks,
>>
>> We’re working on improve EFL to support multi-seat in the same application
>> window.
>>
>> For make it doable, we’re considering the following approaches:
>>
>>    -
>>
>>    When using Wayland (Weston compositor) just get seat information from it
>>    -
>>
>
> What about the Enlightenment wayland compositor ??

it seems to not assign multi seat to different devices, in the future
we should change that and allow E to play well with multi-seat as
well, not in this project scope -- where we'll test with Weston.



>>    _Evas_Public_Data should contain a hash indexed by seat of focused
>>    Evas_Objects. From “pd->focused” to “pd->focused[seat]”.
>>    -
>>
>>    A function similar to evas_object_top_at_pointer_get(const Evas *e)
>>    should be added. This new function will receive the Efl_Input_Device (the
>>    seat) as argument. The old function should return the top Evas_Object
>>    according to the default seat
>>    -
>
> I don't think using Efl_Input_Device as "the seat" is a good idea here.
> A single seat can have multiple input devices (pointer, keyboard, etc).
> Efl_Input_Device (to me) implies a single input device (ie: a single
> pointer, single keyboard, etc). Whereas a "seat" can have multiple
> devices attached...

Efl_Input_Device device_type == class, one being keyboard, another
being pointer and... one being SEAT :-)

In my understanding, the seat device_type should be the parent of
others (keyboard, pointer...).

We do have that code in current GIT, it just happens nobody uses that :-D


>>    evas_canvas_focus_get() should return the object focused by the default
>>    seat.
>
> IMO, this is not going to entirely work. In Wayland, you can have
> "pointer focus", "keyboard focus", "touch focus", etc, etc ... a single
> object focused by the "default" seat is not likely to cover everything.

it was targeted at legacy behavior, providing what we do have now...
where we do have a single focused object for the whole canvas.

OTOH, it was nice to know the focus is per device, not per seat... are
you sure about that?


> Pointer One on Seat One could be focused Object One, while Pointer Two
> on Seat One could be focusing a different object but on the same canvas.
> The function evas_canvas_focus_get should likely take some parameters
> here...perhaps the seat ? Perhaps the pointer ? Not 100% sure, just
> tossing out thoughts...

well, we'd store per seat... now we should store per device :-)

maybe we can do the query based on "if it's a seat, then consider the
focused object any object with focus set to one of its children".

Likewise, focus set with a seat should focus the object with all
inputs? If the seat provides 3 inputs, all of them would be focused to
the same object... sounds good?


>>    evas_canvas_pointer_canvas_xy_get() Should return the XY from the
>>    default seat.
>>    -
>>
>>    evas_canvas_seat_pointer_canvas_xy_get() - Same thing as
>>    evas_canvas_pointer_canvas_xy_get(), however it returns the XY based on 
>> the
>>    seat.
>>    -
>>
>>    evas_object_focus_set() - Will set/unset the focus only for the default
>>    seat.
>>    -
>>
>>    evas_object_focus_by_seat_set() should be added
>>    -
>>
>>    evas_event_feed_hold - Will act in the default seat
>>    -
>>
>
> Likely all these above functions should take a "seat" as a parameter ...
> Everything above is "coded" using the default seat ... What happens with
> other seats ???

this is legacy support... what to do in existing calls?


>>    Add evas_event_feed_hold_by_seat() - Should we support this ?
>
> Probably, yes...

what is the semantics? Stop processing the events of that given seat only?


>>    All evas_event_* functions will work on the top most seat.
>
> I think you may run into a problem in the future with this ... Have not
> thought it out entirely, but (imo) limiting functions to a single "top
> most" seat is likely to lead to problems in the future where there are
> multiple seats... ie: What about events coming from other seats ??

Read event processing code. Since there is no "device" in the feed
functions, likely to not break existing API... it was introduced (but
UNUSED IN CURRENT GIT! :-() evas_device_push()/pop, thus one is
expected to:

    evas_device_push(e, dev);
    evas_event_feed_...(e, ...); // this operates and generates events
with "dev" set to the last pushed (topmost).
    evas_device_pop(e, dev);

see the evas docs, they have this written, although no code uses that.


>>    In _Ecore_Wl2_Input - a Eina_Stringshare * will be added to the wl
>>    substruct. This Eina_Stringshare * will represent the seat name.
>>    -
>
> Why ??? Ecore_Wl2_Input structure already has input->wl.seat pointer ...
> Ecore_Wl2_Display already has an Eina_List of Ecore_Wl2_Seats wherein
> Ecore_Wl2_Seat already contains a stringshare 'name'....

Nice, I see Ecore_Wl2_Seat already have the ->name...  thanks


>>    For every wl_pointer/wl_keyboard/wl_seat it will be create an
>>    Efl_Input_Device. The wl_pointer and wl_keyboard will have the wl_seat as
>>    parent. After creation of these Efl_Input_Devices an ecore_event will be
>>    generated, so ecore_evas_input can grab it and properly add the devices
>>    into the canvas.
>>
>
> Please see above comment about "I don't think using Efl_Input_Device as
> "the seat" is a good idea here." ... It seems like above
> Efl_Input_Device is a "seat", where here it is an actual Input Device....

I'd like to hear what jpeg has to say about that, he created
Efl_Input_Device with seat as device type... AFAIU the idea is to
unify these multiple seats and all, thus Ecore_Wl2 to use that would
help avoid translations.



>> = Ecore =
>>
>>    -
>>
>>    Ecore_Events structs should contain the Efl_Input_Device that originated
>>    the event.
>>
>>
>>    -
>>
>>    These functions may be useful:
>>    -
>>
>>       Eina_List *ecore_input_get_seats() - Return all the available seats
>
> Would prefer verbs last (as is EFL style) .. ecore_input_seats_get...

oops, this one passed my review :-D


-- 
Gustavo Sverzut Barbieri
--------------------------------------
Mobile: +55 (16) 99354-9890

------------------------------------------------------------------------------
_______________________________________________
enlightenment-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel

Reply via email to