Re: [weston] xkbcommon library is not optional.

2015-10-16 Thread Bryce Harrington
On Fri, Oct 16, 2015 at 12:10:57PM +0300, Pekka Paalanen wrote:
> On Fri, 16 Oct 2015 10:27:56 +0200
> Hardening  wrote:
> 
> > Le 16/10/2015 03:28, Bryce Harrington a écrit :
> > > On Tue, Oct 13, 2015 at 01:59:13PM +0200, Joaquim Duran wrote:
> > >> Hello,
> > >>
> > >> When configuring the Weston project, it is possible to disable (don't
> > >> include) the library libxkbcommon. To compile Weston successfully,
> > >> even if the option --disable-xkbcommon is specified, the library must
> > >> be installed because the file src/compositor.h requires it.
> > > 
> > > Joaquim, good find.
> > > 
> > > From the comments in configure.ac:
> > > 
> > >   AS_HELP_STRING([--disable-xkbcommon], [Disable libxkbcommon
> > >   support: This is only useful in environments
> > >   where you do not have a hardware keyboard. If
> > >   libxkbcommon support is disabled clients will not
> > >   be sent a keymap and and must know how to
> > >   interpret the keycode sent for any key event.]),,
> > > 
> > > So it sounds like this is a special case that is intended to work.
> > > 
> > > The header include was from commit 855028fe three years ago, while the
> > > --disable-xkbcommon was added by 382ff46f just two years ago.  That
> > > makes me think that your situation was simply overlooked.
> > > 
> > > If that's true, then presumably the fix would involve peppering
> > > compositor.* and other files with tests like,
> > > 
> > > #ifdef ENABLE_XKBCOMMON
> > > ...
> > > #endif
> > > 
> > > For example it looks like the weston_xkb_info struct would need to
> > > either be removed or stubbed out, and users of it be disabled.  There
> > > are also some weston calls that use xkb_keymap, xkb_rule_names,
> > > etc. structures for params that'd need addressed.
> > > 
> > > On the face of it, seems like it could be a fair amount of work, and a
> > > bit invasive, but maybe there's some tricks to make it simpler
> > > (typedeffing the unsupported struct args and so on.)
> > > 
> > > Alternatively, --disable-xkbcommon could be dropped.  However I get the
> > > sense it actually solves a real world need, and functional regression is
> > > rarely a good idea.
> > > 
> > 
> > I'm to fix the bug. Anyway it's almost sure that there's no real usage
> > of this flag as otherwise weston does not compile...
> 
> I would be suprised if it didn't work at the time it was added. I
> suspect we broke it later, and no-one noticed.

That was my initial assumption too, but reviewing the patches and the
order things landed, I didn't find any evidence that this is the case.
Is it possible it simply did not get tested on a system with xkbcommon
removed?

If it did work originally and subsequently broke, I'd be +1 on dropping
it and avoiding the clutter in the code.  It would help if we knew if
anyone was actually using/needing the option.
 
> I recall some discussions about some obscure embedded platform where
> xkbcommon, even when it's tiny, was deemed as waste of space,
> especially along with the xkeyboard-config database (even though you
> don't even need the whole database). Or maybe it was about wasting
> memory. Can't recall.
> 
> Hrm, now that I actually test --disable-xkbcommon, Weston *does* build.
> All I get during build is:
> 
>   CC   src/weston-input.o
> /home/pq/git/weston/src/input.c: In function ‘weston_seat_init_keyboard’:
> /home/pq/git/weston/src/input.c:2226:1: warning: label ‘err’ defined but not 
> used [-Wunused-label]
> /home/pq/git/weston/src/input.c: In function ‘weston_keyboard_reset_state’:
> /home/pq/git/weston/src/input.c:2238:20: warning: unused variable ‘state’ 
> [-Wunused-variable]
> /home/pq/git/weston/src/input.c: At top level:
> /home/pq/git/weston/src/input.c:555:1: warning: ‘weston_xkb_info_destroy’ 
> used but never defined [enabled by default]
> /home/pq/git/weston/src/input.c:989:1: warning: ‘run_modifier_bindings’ 
> defined but not used [-Wunused-function]
> 
> And no other warning or error.
> 
> I do not get a build failure, because libxkbcommon is installed in my
> system, and so I don't need additional -I flags to find its headers.
> When we build modules (backends, plugins, anything), missing symbols
> are not fatal because the parent executable is providing some we must
> not error on.
> 
> At least x11-backend.so will fail to load doe to missing
> xkbcommon symbols. Makes me wonder if there is some other backend that
> doesn't fail so.
> 
> So it's possible someone is or was actually getting away with this,
> having xkbcommon installed for the build but not at runtime.
> 
> We can remove --disable-xkbcommon, but be prepared having to revert and
> fix that properly later.

It would be worth attempting to contact the original author before going
down that road, just in case.

Bryce

> Thanks,
> pq

> -BEGIN PGP SIGNATURE-
> Version: GnuPG v2
> 
> 

Re: [PATCH weston] tests: Adding simple waycheck validation tool.

2015-10-16 Thread Jon A. Cruz
On 10/15/2015 01:51 AM, Pekka Paalanen wrote:
> I think it's better to land this stuff in pieces than massage a
> humongous replace-the-world patch series, if we can tell the pieces are
> going in the right direction. The bits here are mostly just following
> the existing weston-test-client-helper.c (which the commit message
> forgot to mention).
>
> But yeah, probably makes sense to see how hooking even this stuff up to
> 'make check' would happen before landing this. That will be one of the
> most interesting things here. This patch as is does not add anything to
> 'make check' which is a little awkward for a series adding new test
> code.
>
> As for writing tests in the mean time, people should just ignore the
> new framework for now, and write tests as if it didn't exist as long as
> it doesn't support what the old framework does.
>
> This way we can incrementally advance on both fronts at the same time.

So, the immediate question would now be as to how I should stage things.
Should the changes that will go into 'make check' be a separate patch
set or should it be an additional patch in this set?

I'd say that running it as a separate set might make juggling git a
little easier on me. But not to a degree to outweigh what would make
reviewing things easier.

-- 
Jon A. Cruz - Senior Open Source Developer
Samsung Open Source Group
j...@osg.samsung.com
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [weston] xkbcommon library is not optional.

2015-10-16 Thread Joaquim Duran
Disable xkbcommon library could be useful in devices based on
touchscreen, like smartphones, but if you have USB connectors, you
could connect an external keyboard. if xkbcommon is a small library,
just change the documentation.

Joaquim Duran

2015-10-16 10:27 GMT+02:00 Hardening :
> Le 16/10/2015 03:28, Bryce Harrington a écrit :
>> On Tue, Oct 13, 2015 at 01:59:13PM +0200, Joaquim Duran wrote:
>>> Hello,
>>>
>>> When configuring the Weston project, it is possible to disable (don't
>>> include) the library libxkbcommon. To compile Weston successfully,
>>> even if the option --disable-xkbcommon is specified, the library must
>>> be installed because the file src/compositor.h requires it.
>>
>> Joaquim, good find.
>>
>> From the comments in configure.ac:
>>
>>   AS_HELP_STRING([--disable-xkbcommon], [Disable libxkbcommon
>>   support: This is only useful in environments
>>   where you do not have a hardware keyboard. If
>>   libxkbcommon support is disabled clients will not
>>   be sent a keymap and and must know how to
>>   interpret the keycode sent for any key event.]),,
>>
>> So it sounds like this is a special case that is intended to work.
>>
>> The header include was from commit 855028fe three years ago, while the
>> --disable-xkbcommon was added by 382ff46f just two years ago.  That
>> makes me think that your situation was simply overlooked.
>>
>> If that's true, then presumably the fix would involve peppering
>> compositor.* and other files with tests like,
>>
>> #ifdef ENABLE_XKBCOMMON
>> ...
>> #endif
>>
>> For example it looks like the weston_xkb_info struct would need to
>> either be removed or stubbed out, and users of it be disabled.  There
>> are also some weston calls that use xkb_keymap, xkb_rule_names,
>> etc. structures for params that'd need addressed.
>>
>> On the face of it, seems like it could be a fair amount of work, and a
>> bit invasive, but maybe there's some tricks to make it simpler
>> (typedeffing the unsupported struct args and so on.)
>>
>> Alternatively, --disable-xkbcommon could be dropped.  However I get the
>> sense it actually solves a real world need, and functional regression is
>> rarely a good idea.
>>
>
> I'm to fix the bug. Anyway it's almost sure that there's no real usage
> of this flag as otherwise weston does not compile...
>
> Best regards
>
>
> --
> David FORT
> website: http://www.hardening-consulting.com/
>
> ___
> wayland-devel mailing list
> wayland-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [weston] xkbcommon library is not optional.

2015-10-16 Thread Hardening
Le 16/10/2015 03:28, Bryce Harrington a écrit :
> On Tue, Oct 13, 2015 at 01:59:13PM +0200, Joaquim Duran wrote:
>> Hello,
>>
>> When configuring the Weston project, it is possible to disable (don't
>> include) the library libxkbcommon. To compile Weston successfully,
>> even if the option --disable-xkbcommon is specified, the library must
>> be installed because the file src/compositor.h requires it.
> 
> Joaquim, good find.
> 
> From the comments in configure.ac:
> 
>   AS_HELP_STRING([--disable-xkbcommon], [Disable libxkbcommon
>   support: This is only useful in environments
>   where you do not have a hardware keyboard. If
>   libxkbcommon support is disabled clients will not
>   be sent a keymap and and must know how to
>   interpret the keycode sent for any key event.]),,
> 
> So it sounds like this is a special case that is intended to work.
> 
> The header include was from commit 855028fe three years ago, while the
> --disable-xkbcommon was added by 382ff46f just two years ago.  That
> makes me think that your situation was simply overlooked.
> 
> If that's true, then presumably the fix would involve peppering
> compositor.* and other files with tests like,
> 
> #ifdef ENABLE_XKBCOMMON
> ...
> #endif
> 
> For example it looks like the weston_xkb_info struct would need to
> either be removed or stubbed out, and users of it be disabled.  There
> are also some weston calls that use xkb_keymap, xkb_rule_names,
> etc. structures for params that'd need addressed.
> 
> On the face of it, seems like it could be a fair amount of work, and a
> bit invasive, but maybe there's some tricks to make it simpler
> (typedeffing the unsupported struct args and so on.)
> 
> Alternatively, --disable-xkbcommon could be dropped.  However I get the
> sense it actually solves a real world need, and functional regression is
> rarely a good idea.
> 

I'm to fix the bug. Anyway it's almost sure that there's no real usage
of this flag as otherwise weston does not compile...

Best regards


-- 
David FORT
website: http://www.hardening-consulting.com/

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [weston] xkbcommon library is not optional.

2015-10-16 Thread Joaquim Duran
The database of keymaps are not included in xkbcommon library, you
should download from
http://www.freedesktop.org/wiki/Software/XKeyboardConfig and then just
install all keymaps that you need.

Joaquim Duran


2015-10-16 11:31 GMT+02:00 Arnaud Vrac :
> On Fri, Oct 16, 2015 at 11:10 AM, Pekka Paalanen 
> wrote:
>>
>> On Fri, 16 Oct 2015 10:27:56 +0200
>> Hardening  wrote:
>>
>> > Le 16/10/2015 03:28, Bryce Harrington a écrit :
>> > > On Tue, Oct 13, 2015 at 01:59:13PM +0200, Joaquim Duran wrote:
>> > >> Hello,
>> > >>
>> > >> When configuring the Weston project, it is possible to disable (don't
>> > >> include) the library libxkbcommon. To compile Weston successfully,
>> > >> even if the option --disable-xkbcommon is specified, the library must
>> > >> be installed because the file src/compositor.h requires it.
>> > >
>> > > Joaquim, good find.
>> > >
>> > > From the comments in configure.ac:
>> > >
>> > >   AS_HELP_STRING([--disable-xkbcommon], [Disable libxkbcommon
>> > >   support: This is only useful in environments
>> > >   where you do not have a hardware keyboard. If
>> > >   libxkbcommon support is disabled clients will not
>> > >   be sent a keymap and and must know how to
>> > >   interpret the keycode sent for any key event.]),,
>> > >
>> > > So it sounds like this is a special case that is intended to work.
>> > >
>> > > The header include was from commit 855028fe three years ago, while the
>> > > --disable-xkbcommon was added by 382ff46f just two years ago.  That
>> > > makes me think that your situation was simply overlooked.
>> > >
>> > > If that's true, then presumably the fix would involve peppering
>> > > compositor.* and other files with tests like,
>> > >
>> > > #ifdef ENABLE_XKBCOMMON
>> > > ...
>> > > #endif
>> > >
>> > > For example it looks like the weston_xkb_info struct would need to
>> > > either be removed or stubbed out, and users of it be disabled.  There
>> > > are also some weston calls that use xkb_keymap, xkb_rule_names,
>> > > etc. structures for params that'd need addressed.
>> > >
>> > > On the face of it, seems like it could be a fair amount of work, and a
>> > > bit invasive, but maybe there's some tricks to make it simpler
>> > > (typedeffing the unsupported struct args and so on.)
>> > >
>> > > Alternatively, --disable-xkbcommon could be dropped.  However I get
>> > > the
>> > > sense it actually solves a real world need, and functional regression
>> > > is
>> > > rarely a good idea.
>> > >
>> >
>> > I'm to fix the bug. Anyway it's almost sure that there's no real usage
>> > of this flag as otherwise weston does not compile...
>>
>> I would be suprised if it didn't work at the time it was added. I
>> suspect we broke it later, and no-one noticed.
>>
>> I recall some discussions about some obscure embedded platform where
>> xkbcommon, even when it's tiny, was deemed as waste of space,
>> especially along with the xkeyboard-config database (even though you
>> don't even need the whole database). Or maybe it was about wasting
>> memory. Can't recall.
>
>
> If you know of a way to strip some parts of the database, I'm interested.
> I've tried only keep evdev mappings and a few layouts, but going at it it
> seemed like a very hard task so I just gave up.
>
>>
>>
>> Hrm, now that I actually test --disable-xkbcommon, Weston *does* build.
>> All I get during build is:
>>
>>   CC   src/weston-input.o
>> /home/pq/git/weston/src/input.c: In function ‘weston_seat_init_keyboard’:
>> /home/pq/git/weston/src/input.c:2226:1: warning: label ‘err’ defined but
>> not used [-Wunused-label]
>> /home/pq/git/weston/src/input.c: In function
>> ‘weston_keyboard_reset_state’:
>> /home/pq/git/weston/src/input.c:2238:20: warning: unused variable ‘state’
>> [-Wunused-variable]
>> /home/pq/git/weston/src/input.c: At top level:
>> /home/pq/git/weston/src/input.c:555:1: warning: ‘weston_xkb_info_destroy’
>> used but never defined [enabled by default]
>> /home/pq/git/weston/src/input.c:989:1: warning: ‘run_modifier_bindings’
>> defined but not used [-Wunused-function]
>>
>> And no other warning or error.
>>
>> I do not get a build failure, because libxkbcommon is installed in my
>> system, and so I don't need additional -I flags to find its headers.
>> When we build modules (backends, plugins, anything), missing symbols
>> are not fatal because the parent executable is providing some we must
>> not error on.
>>
>> At least x11-backend.so will fail to load doe to missing
>> xkbcommon symbols. Makes me wonder if there is some other backend that
>> doesn't fail so.
>>
>> So it's possible someone is or was actually getting away with this,
>> having xkbcommon installed for the build but not at runtime.
>>
>> We can remove --disable-xkbcommon, but be prepared having to revert and
>> fix that properly later.
>>
>>
>> Thanks,
>> pq
>
>
>
> --
> Arnaud
>
> 

Re: [RFC] weston: implement inert objects for keyboard/pointer/touch

2015-10-16 Thread Hardening
Le 16/10/2015 11:53, David FORT a écrit :
> This patch implements inert objects for wl_keyboard, wl_pointer and wl_touch.
> The target case is when the server has just send a capability event about a
> disappearing object, and the client binds the corresponding object. We bind an
> inert object: an object does nothing when it is requested. If the client 
> behave
> correctly, this object should be released when the capability event is 
> received
> and treated (calling the corresponding release request).
> As a consequence, we can rely on seat->[keyboard|pointer|touch]_state to know
> if the seat has the corresponding object.
> Weston doesn't really handle multiple mice for one wl_pointer, so I have 
> removed
> the corresponding code and tests (it was quite a weston_test hack BTW).
> Finally I have fixed a wrong behaviour: the capabilities event's 
> documentation states
> that the capabilities should be sent when a new capability is set on the 
> seat. So
> attaching a second mouse to an existing wl_pointer should not broadcast seat
> capabilities (and the same for keyboard and touch).
> ---
>  desktop-shell/exposay.c  |  16 ++--
>  desktop-shell/shell.c|  21 +++--
>  src/compositor-wayland.c |  10 +-
>  src/compositor-x11.c |  12 ++-
>  src/compositor.h |   3 -
>  src/input.c  | 235 
> ++-
>  src/libinput-device.c|   2 +-
>  src/screen-share.c   |   2 +
>  src/text-backend.c   |  44 +
>  src/zoom.c   |   7 +-
>  tests/devices-test.c |  20 
>  tests/weston-test.c  |  18 ++--
>  xwayland/dnd.c   |   3 +-
>  13 files changed, 216 insertions(+), 177 deletions(-)
> 
> diff --git a/desktop-shell/exposay.c b/desktop-shell/exposay.c
> index 08b86a3..d6919b3 100644
> --- a/desktop-shell/exposay.c
> +++ b/desktop-shell/exposay.c
> @@ -574,21 +574,23 @@ exposay_transition_active(struct desktop_shell *shell)
>   bool animate = false;
>  
>   shell->exposay.workspace = get_current_workspace(shell);
> - shell->exposay.focus_prev = get_default_view(keyboard->focus);
> - shell->exposay.focus_current = get_default_view(keyboard->focus);
> + if (keyboard) {
> + shell->exposay.focus_prev = get_default_view(keyboard->focus);
> + shell->exposay.focus_current = 
> get_default_view(keyboard->focus);
> + }
>   shell->exposay.clicked = NULL;
>   wl_list_init(>exposay.surface_list);
>  
>   lower_fullscreen_layer(shell, NULL);
>   shell->exposay.grab_kbd.interface = _kbd_grab;
> - weston_keyboard_start_grab(keyboard,
> ->exposay.grab_kbd);
> - weston_keyboard_set_focus(keyboard, NULL);
> + if (keyboard) {
> + weston_keyboard_start_grab(keyboard, >exposay.grab_kbd);
> + weston_keyboard_set_focus(keyboard, NULL);
> + }
>  
>   shell->exposay.grab_ptr.interface = _ptr_grab;
>   if (pointer) {
> - weston_pointer_start_grab(pointer,
> -   >exposay.grab_ptr);
> + weston_pointer_start_grab(pointer, >exposay.grab_ptr);
>   weston_pointer_clear_focus(pointer);
>   }
>   wl_list_for_each(shell_output, >output_list, link) {
> diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
> index 3eb3f5c..cdddf09 100644
> --- a/desktop-shell/shell.c
> +++ b/desktop-shell/shell.c
> @@ -380,7 +380,8 @@ shell_grab_start(struct shell_grab *grab,
>   struct desktop_shell *shell = shsurf->shell;
>   struct weston_touch *touch = weston_seat_get_touch(pointer->seat);
>  
> - popup_grab_end(pointer);
> + if (pointer)
> + popup_grab_end(pointer);
>   if (touch)
>   touch_popup_grab_end(touch);
>  
> @@ -391,11 +392,13 @@ shell_grab_start(struct shell_grab *grab,
> >shsurf_destroy_listener);
>  
>   shsurf->grabbed = 1;
> - weston_pointer_start_grab(pointer, >grab);
> + if (pointer)
> + weston_pointer_start_grab(pointer, >grab);
>   if (shell->child.desktop_shell) {
>   desktop_shell_send_grab_cursor(shell->child.desktop_shell,
>  cursor);
> - weston_pointer_set_focus(pointer,
> + if (pointer)
> + weston_pointer_set_focus(pointer,
>get_default_view(shell->grab_surface),
>wl_fixed_from_int(0),
>wl_fixed_from_int(0));
> @@ -925,12 +928,10 @@ restore_focus_state(struct desktop_shell *shell, struct 
> workspace *ws)
>   wl_list_insert(>compositor->seat_list,
>  >seat->link);
>  
> - if (!keyboard)
> - continue;
> -
>   surface = state->keyboard_focus;
>  
> - weston_keyboard_set_focus(keyboard, surface);
> +  

Re: [weston] xkbcommon library is not optional.

2015-10-16 Thread Hardening
Le 16/10/2015 10:34, Joaquim Duran a écrit :
> Disable xkbcommon library could be useful in devices based on
> touchscreen, like smartphones, but if you have USB connectors, you
> could connect an external keyboard. if xkbcommon is a small library,
> just change the documentation.
> 
> Joaquim Duran
> 


Don't get me wrong, I'm not saying it's not useful. I'm saying that
probably nobody's using it as otherwise it doesn't compile.


> 2015-10-16 10:27 GMT+02:00 Hardening :
>> Le 16/10/2015 03:28, Bryce Harrington a écrit :
>>> On Tue, Oct 13, 2015 at 01:59:13PM +0200, Joaquim Duran wrote:
 Hello,

 When configuring the Weston project, it is possible to disable (don't
 include) the library libxkbcommon. To compile Weston successfully,
 even if the option --disable-xkbcommon is specified, the library must
 be installed because the file src/compositor.h requires it.
>>>
>>> Joaquim, good find.
>>>
>>> From the comments in configure.ac:
>>>
>>>   AS_HELP_STRING([--disable-xkbcommon], [Disable libxkbcommon
>>>   support: This is only useful in environments
>>>   where you do not have a hardware keyboard. If
>>>   libxkbcommon support is disabled clients will not
>>>   be sent a keymap and and must know how to
>>>   interpret the keycode sent for any key event.]),,
>>>
>>> So it sounds like this is a special case that is intended to work.
>>>
>>> The header include was from commit 855028fe three years ago, while the
>>> --disable-xkbcommon was added by 382ff46f just two years ago.  That
>>> makes me think that your situation was simply overlooked.
>>>
>>> If that's true, then presumably the fix would involve peppering
>>> compositor.* and other files with tests like,
>>>
>>> #ifdef ENABLE_XKBCOMMON
>>> ...
>>> #endif
>>>
>>> For example it looks like the weston_xkb_info struct would need to
>>> either be removed or stubbed out, and users of it be disabled.  There
>>> are also some weston calls that use xkb_keymap, xkb_rule_names,
>>> etc. structures for params that'd need addressed.
>>>
>>> On the face of it, seems like it could be a fair amount of work, and a
>>> bit invasive, but maybe there's some tricks to make it simpler
>>> (typedeffing the unsupported struct args and so on.)
>>>
>>> Alternatively, --disable-xkbcommon could be dropped.  However I get the
>>> sense it actually solves a real world need, and functional regression is
>>> rarely a good idea.
>>>
>>
>> I'm to fix the bug. Anyway it's almost sure that there's no real usage
>> of this flag as otherwise weston does not compile...
>>
>> Best regards
>>
>>
>> --
>> David FORT
>> website: http://www.hardening-consulting.com/
>>
>> ___
>> wayland-devel mailing list
>> wayland-devel@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/wayland-devel


-- 
David FORT
website: http://www.hardening-consulting.com/

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel


IVI System Black screen after boot up with weston

2015-10-16 Thread RayBloodworth
Hi,  everyone
I have built Tizen-IVI image(tizen-ivi-image-dev) with single seat using 
yocto and run it on MCIMX6Q-SDP:
tizen-distro : tizen-ivi branchmeta-fsl-arm/meta-fsl-arm-extra: dizzy 
(kernel is upgraded to 3.14.28, imx-gpu-viv is upgraded to 5.0.11)
Then I configed boot params to lvds video(default is hdmi) output.
After booting the system, tty1 turn to black screen.
According to multi-user case, there should be a weston(A) running with 
genivi user and another one(B) with app user. A should use fbdev-backend and B 
should use wayland-backend.
Using pstree I got this:root@imx6qsabresd:~# pstreesystemd-+-ambd---{gdbus} 
   |-amd|-avahi-daemon---avahi-daemon|-bluetoothd
|-connmand|-dbus-daemon|-ofonod|-sshd---sh---pstree 
   |-system_server|-systemd-journal|-systemd-logind
|-systemd-udevd---systemd-udevd
|-tlm-+-tlm-sessiond-+-genivi-sessionweston-launch---weston---{weston}  
  | |  |-{gdbus}| |  `-{gmain}| 
|-tlm-sessiond-+-user-session---weston-+-weston-desktop-| | 
 |   `-{weston}| |  
|-{gdbus}| |  `-{gmain}| |-{gdbus}| 
`-{gmain}`-wpa_supplicant
It's looks like OK.
And I checked the genivi-weston log:
Date: 1970-01-01 UTC[00:06:14.141] weston 1.6.0   
http://wayland.freedesktop.org/   Bug reports to: 
https://bugs.freedesktop.org/enter_bug.cgi?product=Wayland=weston=1.6.0
   Build: tizen_3.0_ivi_release-dirty Fixed weston transfromation 
problem (2015-01-08 18:51:08 +0200)[00:06:14.142] OS: Linux, 
3.14.28-1.0.0_ga+g91cf351, #1 SMP PREEMPT Sun Oct 11 02:05:20 CST 2015, 
armv7l[00:06:14.147] Using config file 
'/etc/xdg/weston/weston-genivi.ini'[00:06:14.150] Loading module 
'/usr/lib/weston/fbdev-backend.so'[00:06:14.267] initializing fbdev 
backend[00:06:14.281] compositor->use_gal2d=0[00:06:14.289] Loading module 
'/usr/lib/weston/gl-renderer.so'[00:06:14.356] Creating fbdev output. /dev/fb0 
x=0 y=0[00:06:14.356] Opening fbdev frame buffer.[00:06:14.356] Calculating 
pixman format from:- type: 0 (aux: 0)- visual: 
2- bpp: 16 (grayscale: 0)- red: offset: 11, 
length: 5, MSB: 0- green: offset: 5, length: 6, MSB: 0  
  - blue: offset: 0, length: 5, MSB: 0- transp: offset: 0, 
length: 0, MSB: 0[00:06:14.425] EGL version: 1.4[00:06:14.425] EGL vendor: 
Vivante Corporation[00:06:14.425] EGL client APIs: OpenGL_ES 
OpenVG[00:06:14.425] EGL extensions: EGL_KHR_reusable_sync EGL_KHR_fence_sync   
EGL_KHR_image EGL_KHR_image_base EGL_KHR_image_pixmap   
EGL_KHR_gl_texture_2D_image EGL_KHR_gl_texture_cubemap_image   
EGL_KHR_gl_renderbuffer_image EGL_KHR_lock_surface   
EGL_KHR_create_context EGL_EXT_create_context_robustness   
EGL_EXT_buffer_age EGL_EXT_protected_surface   
EGL_WL_bind_wayland_display[00:06:14.425] GL version: OpenGL ES 3.0 
V5.0.11.p4.25762[00:06:14.425] GLSL version: OpenGL ES GLSL ES 
3.00[00:06:14.425] GL vendor: Vivante Corporation[00:06:14.425] GL renderer: 
Vivante GC2000[00:06:14.425] GL extensions: GL_OES_vertex_type_10_10_10_2   
GL_OES_vertex_half_float GL_OES_element_index_uint   
GL_OES_mapbuffer GL_OES_vertex_array_object   
GL_OES_compressed_ETC1_RGB8_texture   
GL_OES_compressed_paletted_texture GL_OES_texture_npot   
GL_OES_rgb8_rgba8 GL_OES_depth_texture   
GL_OES_depth_texture_cube_map GL_OES_depth24 GL_OES_depth32   
GL_OES_packed_depth_stencil GL_OES_fbo_render_mipmap   
GL_OES_get_program_binary GL_OES_fragment_precision_high   
GL_OES_standard_derivatives GL_OES_EGL_image GL_OES_EGL_sync   
GL_EXT_texture_type_2_10_10_10_REV   
GL_EXT_texture_filter_anisotropic   GL_EXT_texture_format_BGRA 
GL_EXT_read_format_bgra   GL_EXT_multi_draw_arrays 
GL_EXT_frag_depth   GL_EXT_discard_framebuffer GL_EXT_blend_minmax  
 GL_EXT_multisampled_render_to_texture GL_EXT_robustness
   GL_VIV_tex_direct[00:06:14.426] GL ES 2 renderer features:   
read-back format: BGRA   wl_shm sub-image to texture: no
   EGL Wayland extension: yes[00:06:14.426] Chosen EGL config details:  
 RGBA bits: 8 8 8 0   swap interval range: 0 - 10[00:06:14.426] 
fbdev output 1024×768 px   guessing 63 Hz and 96 dpi[00:06:14.428] 
input device 'gpio-keys.21', /dev/input/event7 is a keyboard[00:06:14.429] 
input device 'DELL Dell USB Entry Keyboard', /dev/input/event5 is a 
keyboard[00:06:14.430] not using input device 

Re: [weston] xkbcommon library is not optional.

2015-10-16 Thread Arnaud Vrac
On Fri, Oct 16, 2015 at 11:10 AM, Pekka Paalanen 
wrote:

> On Fri, 16 Oct 2015 10:27:56 +0200
> Hardening  wrote:
>
> > Le 16/10/2015 03:28, Bryce Harrington a écrit :
> > > On Tue, Oct 13, 2015 at 01:59:13PM +0200, Joaquim Duran wrote:
> > >> Hello,
> > >>
> > >> When configuring the Weston project, it is possible to disable (don't
> > >> include) the library libxkbcommon. To compile Weston successfully,
> > >> even if the option --disable-xkbcommon is specified, the library must
> > >> be installed because the file src/compositor.h requires it.
> > >
> > > Joaquim, good find.
> > >
> > > From the comments in configure.ac:
> > >
> > >   AS_HELP_STRING([--disable-xkbcommon], [Disable libxkbcommon
> > >   support: This is only useful in environments
> > >   where you do not have a hardware keyboard. If
> > >   libxkbcommon support is disabled clients will not
> > >   be sent a keymap and and must know how to
> > >   interpret the keycode sent for any key event.]),,
> > >
> > > So it sounds like this is a special case that is intended to work.
> > >
> > > The header include was from commit 855028fe three years ago, while the
> > > --disable-xkbcommon was added by 382ff46f just two years ago.  That
> > > makes me think that your situation was simply overlooked.
> > >
> > > If that's true, then presumably the fix would involve peppering
> > > compositor.* and other files with tests like,
> > >
> > > #ifdef ENABLE_XKBCOMMON
> > > ...
> > > #endif
> > >
> > > For example it looks like the weston_xkb_info struct would need to
> > > either be removed or stubbed out, and users of it be disabled.  There
> > > are also some weston calls that use xkb_keymap, xkb_rule_names,
> > > etc. structures for params that'd need addressed.
> > >
> > > On the face of it, seems like it could be a fair amount of work, and a
> > > bit invasive, but maybe there's some tricks to make it simpler
> > > (typedeffing the unsupported struct args and so on.)
> > >
> > > Alternatively, --disable-xkbcommon could be dropped.  However I get the
> > > sense it actually solves a real world need, and functional regression
> is
> > > rarely a good idea.
> > >
> >
> > I'm to fix the bug. Anyway it's almost sure that there's no real usage
> > of this flag as otherwise weston does not compile...
>
> I would be suprised if it didn't work at the time it was added. I
> suspect we broke it later, and no-one noticed.
>
> I recall some discussions about some obscure embedded platform where
> xkbcommon, even when it's tiny, was deemed as waste of space,
> especially along with the xkeyboard-config database (even though you
> don't even need the whole database). Or maybe it was about wasting
> memory. Can't recall.
>

If you know of a way to strip some parts of the database, I'm interested.
I've tried only keep evdev mappings and a few layouts, but going at it it
seemed like a very hard task so I just gave up.


>
> Hrm, now that I actually test --disable-xkbcommon, Weston *does* build.
> All I get during build is:
>
>   CC   src/weston-input.o
> /home/pq/git/weston/src/input.c: In function ‘weston_seat_init_keyboard’:
> /home/pq/git/weston/src/input.c:2226:1: warning: label ‘err’ defined but
> not used [-Wunused-label]
> /home/pq/git/weston/src/input.c: In function ‘weston_keyboard_reset_state’:
> /home/pq/git/weston/src/input.c:2238:20: warning: unused variable ‘state’
> [-Wunused-variable]
> /home/pq/git/weston/src/input.c: At top level:
> /home/pq/git/weston/src/input.c:555:1: warning: ‘weston_xkb_info_destroy’
> used but never defined [enabled by default]
> /home/pq/git/weston/src/input.c:989:1: warning: ‘run_modifier_bindings’
> defined but not used [-Wunused-function]
>
> And no other warning or error.
>
> I do not get a build failure, because libxkbcommon is installed in my
> system, and so I don't need additional -I flags to find its headers.
> When we build modules (backends, plugins, anything), missing symbols
> are not fatal because the parent executable is providing some we must
> not error on.
>
> At least x11-backend.so will fail to load doe to missing
> xkbcommon symbols. Makes me wonder if there is some other backend that
> doesn't fail so.
>
> So it's possible someone is or was actually getting away with this,
> having xkbcommon installed for the build but not at runtime.
>
> We can remove --disable-xkbcommon, but be prepared having to revert and
> fix that properly later.
>
>
> Thanks,
> pq



-- 
Arnaud
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel


[RFC] weston: implement inert objects for keyboard/pointer/touch

2015-10-16 Thread David FORT
This patch implements inert objects for wl_keyboard, wl_pointer and wl_touch.
The target case is when the server has just send a capability event about a
disappearing object, and the client binds the corresponding object. We bind an
inert object: an object does nothing when it is requested. If the client behave
correctly, this object should be released when the capability event is received
and treated (calling the corresponding release request).
As a consequence, we can rely on seat->[keyboard|pointer|touch]_state to know
if the seat has the corresponding object.
Weston doesn't really handle multiple mice for one wl_pointer, so I have removed
the corresponding code and tests (it was quite a weston_test hack BTW).
Finally I have fixed a wrong behaviour: the capabilities event's documentation 
states
that the capabilities should be sent when a new capability is set on the seat. 
So
attaching a second mouse to an existing wl_pointer should not broadcast seat
capabilities (and the same for keyboard and touch).
---
 desktop-shell/exposay.c  |  16 ++--
 desktop-shell/shell.c|  21 +++--
 src/compositor-wayland.c |  10 +-
 src/compositor-x11.c |  12 ++-
 src/compositor.h |   3 -
 src/input.c  | 235 ++-
 src/libinput-device.c|   2 +-
 src/screen-share.c   |   2 +
 src/text-backend.c   |  44 +
 src/zoom.c   |   7 +-
 tests/devices-test.c |  20 
 tests/weston-test.c  |  18 ++--
 xwayland/dnd.c   |   3 +-
 13 files changed, 216 insertions(+), 177 deletions(-)

diff --git a/desktop-shell/exposay.c b/desktop-shell/exposay.c
index 08b86a3..d6919b3 100644
--- a/desktop-shell/exposay.c
+++ b/desktop-shell/exposay.c
@@ -574,21 +574,23 @@ exposay_transition_active(struct desktop_shell *shell)
bool animate = false;
 
shell->exposay.workspace = get_current_workspace(shell);
-   shell->exposay.focus_prev = get_default_view(keyboard->focus);
-   shell->exposay.focus_current = get_default_view(keyboard->focus);
+   if (keyboard) {
+   shell->exposay.focus_prev = get_default_view(keyboard->focus);
+   shell->exposay.focus_current = 
get_default_view(keyboard->focus);
+   }
shell->exposay.clicked = NULL;
wl_list_init(>exposay.surface_list);
 
lower_fullscreen_layer(shell, NULL);
shell->exposay.grab_kbd.interface = _kbd_grab;
-   weston_keyboard_start_grab(keyboard,
-  >exposay.grab_kbd);
-   weston_keyboard_set_focus(keyboard, NULL);
+   if (keyboard) {
+   weston_keyboard_start_grab(keyboard, >exposay.grab_kbd);
+   weston_keyboard_set_focus(keyboard, NULL);
+   }
 
shell->exposay.grab_ptr.interface = _ptr_grab;
if (pointer) {
-   weston_pointer_start_grab(pointer,
- >exposay.grab_ptr);
+   weston_pointer_start_grab(pointer, >exposay.grab_ptr);
weston_pointer_clear_focus(pointer);
}
wl_list_for_each(shell_output, >output_list, link) {
diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
index 3eb3f5c..cdddf09 100644
--- a/desktop-shell/shell.c
+++ b/desktop-shell/shell.c
@@ -380,7 +380,8 @@ shell_grab_start(struct shell_grab *grab,
struct desktop_shell *shell = shsurf->shell;
struct weston_touch *touch = weston_seat_get_touch(pointer->seat);
 
-   popup_grab_end(pointer);
+   if (pointer)
+   popup_grab_end(pointer);
if (touch)
touch_popup_grab_end(touch);
 
@@ -391,11 +392,13 @@ shell_grab_start(struct shell_grab *grab,
  >shsurf_destroy_listener);
 
shsurf->grabbed = 1;
-   weston_pointer_start_grab(pointer, >grab);
+   if (pointer)
+   weston_pointer_start_grab(pointer, >grab);
if (shell->child.desktop_shell) {
desktop_shell_send_grab_cursor(shell->child.desktop_shell,
   cursor);
-   weston_pointer_set_focus(pointer,
+   if (pointer)
+   weston_pointer_set_focus(pointer,
 get_default_view(shell->grab_surface),
 wl_fixed_from_int(0),
 wl_fixed_from_int(0));
@@ -925,12 +928,10 @@ restore_focus_state(struct desktop_shell *shell, struct 
workspace *ws)
wl_list_insert(>compositor->seat_list,
   >seat->link);
 
-   if (!keyboard)
-   continue;
-
surface = state->keyboard_focus;
 
-   weston_keyboard_set_focus(keyboard, surface);
+   if (keyboard)
+   weston_keyboard_set_focus(keyboard, surface);
}
 
/* For any remaining seats that we don't have a focus 

Re: [weston] xkbcommon library is not optional.

2015-10-16 Thread Pekka Paalanen
On Fri, 16 Oct 2015 10:27:56 +0200
Hardening  wrote:

> Le 16/10/2015 03:28, Bryce Harrington a écrit :
> > On Tue, Oct 13, 2015 at 01:59:13PM +0200, Joaquim Duran wrote:
> >> Hello,
> >>
> >> When configuring the Weston project, it is possible to disable (don't
> >> include) the library libxkbcommon. To compile Weston successfully,
> >> even if the option --disable-xkbcommon is specified, the library must
> >> be installed because the file src/compositor.h requires it.
> > 
> > Joaquim, good find.
> > 
> > From the comments in configure.ac:
> > 
> >   AS_HELP_STRING([--disable-xkbcommon], [Disable libxkbcommon
> >   support: This is only useful in environments
> >   where you do not have a hardware keyboard. If
> >   libxkbcommon support is disabled clients will not
> >   be sent a keymap and and must know how to
> >   interpret the keycode sent for any key event.]),,
> > 
> > So it sounds like this is a special case that is intended to work.
> > 
> > The header include was from commit 855028fe three years ago, while the
> > --disable-xkbcommon was added by 382ff46f just two years ago.  That
> > makes me think that your situation was simply overlooked.
> > 
> > If that's true, then presumably the fix would involve peppering
> > compositor.* and other files with tests like,
> > 
> > #ifdef ENABLE_XKBCOMMON
> > ...
> > #endif
> > 
> > For example it looks like the weston_xkb_info struct would need to
> > either be removed or stubbed out, and users of it be disabled.  There
> > are also some weston calls that use xkb_keymap, xkb_rule_names,
> > etc. structures for params that'd need addressed.
> > 
> > On the face of it, seems like it could be a fair amount of work, and a
> > bit invasive, but maybe there's some tricks to make it simpler
> > (typedeffing the unsupported struct args and so on.)
> > 
> > Alternatively, --disable-xkbcommon could be dropped.  However I get the
> > sense it actually solves a real world need, and functional regression is
> > rarely a good idea.
> > 
> 
> I'm to fix the bug. Anyway it's almost sure that there's no real usage
> of this flag as otherwise weston does not compile...

I would be suprised if it didn't work at the time it was added. I
suspect we broke it later, and no-one noticed.

I recall some discussions about some obscure embedded platform where
xkbcommon, even when it's tiny, was deemed as waste of space,
especially along with the xkeyboard-config database (even though you
don't even need the whole database). Or maybe it was about wasting
memory. Can't recall.

Hrm, now that I actually test --disable-xkbcommon, Weston *does* build.
All I get during build is:

  CC   src/weston-input.o
/home/pq/git/weston/src/input.c: In function ‘weston_seat_init_keyboard’:
/home/pq/git/weston/src/input.c:2226:1: warning: label ‘err’ defined but not 
used [-Wunused-label]
/home/pq/git/weston/src/input.c: In function ‘weston_keyboard_reset_state’:
/home/pq/git/weston/src/input.c:2238:20: warning: unused variable ‘state’ 
[-Wunused-variable]
/home/pq/git/weston/src/input.c: At top level:
/home/pq/git/weston/src/input.c:555:1: warning: ‘weston_xkb_info_destroy’ used 
but never defined [enabled by default]
/home/pq/git/weston/src/input.c:989:1: warning: ‘run_modifier_bindings’ defined 
but not used [-Wunused-function]

And no other warning or error.

I do not get a build failure, because libxkbcommon is installed in my
system, and so I don't need additional -I flags to find its headers.
When we build modules (backends, plugins, anything), missing symbols
are not fatal because the parent executable is providing some we must
not error on.

At least x11-backend.so will fail to load doe to missing
xkbcommon symbols. Makes me wonder if there is some other backend that
doesn't fail so.

So it's possible someone is or was actually getting away with this,
having xkbcommon installed for the build but not at runtime.

We can remove --disable-xkbcommon, but be prepared having to revert and
fix that properly later.


Thanks,
pq


pgpxj8DH6Fq4W.pgp
Description: OpenPGP digital signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH wayland v2] Remove protocol/wayland.dtd

2015-10-16 Thread Pekka Paalanen
On Fri, 16 Oct 2015 12:29:11 +1000
Peter Hutterer  wrote:

> On Fri, Oct 09, 2015 at 01:16:49PM +0200, Nils Chr. Brause wrote:
> > Hi,
> > 
> > Reviewed-by: Nils Christopher Brause 
> > 
> > I ran distcheck and it worked. :)
> 
> a bit late, but I would like to register my disagreement with this patch :)
> 
> Having the DTD is a much simpler and less bug-prone description of what the
> protocol should look like. Having the scanner define the protocol means the
> protocol is whatever the current version of the scanner supports, which is
> not a good contract.

Hi Peter,

can't argue with that. It's just that the DTD was unused since
6292fe2af6a45decb7fd39090e74dd87bc4e22b2, Feb 2014.

> I'd argue for reverting this and fixing any mismatch if there is one. And
> using the DTD to validate before the scanner even runs.

We talked in IRC about this, and came to the conclusion that maybe we
could have wayland-scanner invoke a validity checker against a DTD or
an XSD.

If the original objection to a DTD was because it required manually
writing a lint phase in every project build system using the XML files,
then having wayland-scanner invoke the check automatically solves that.

Then it becomes a question of how to do that and what tools or
libraries to rely on. Running an external lint program from scanner.c
could be a start.

Migrating scanner.c from expat to some validating XML parser library
would be a big enough change that I would like to see some scanner
tests written before that.

I think this summarises our IRC discussion for the mailing list.


Thanks,
pq

> > On Fri, Oct 9, 2015 at 10:01 AM, Auke Booij  wrote:
> > > Yeah, that was a pretty embarrassing mistake by me, for such a simple
> > > patch. Thanks to Bryce for catching it.
> > >
> > > On 8 October 2015 at 15:05, Pekka Paalanen  wrote:
> > >> On Thu,  8 Oct 2015 14:35:34 +0100
> > >> Auke Booij  wrote:
> > >>
> > >>> The wayland scanner defines the protocol. The DTD specification is not 
> > >>> used.
> > >>> ---
> > >>>  Makefile.am  |  4 ++--
> > >>>  protocol/wayland.dtd | 29 -
> > >>>  2 files changed, 2 insertions(+), 31 deletions(-)
> > >>>  delete mode 100644 protocol/wayland.dtd
> > >>
> > >> Reviewed-by: Pekka Paalanen 


pgpBEC3Fe8UAz.pgp
Description: OpenPGP digital signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH wayland 1/3 v2] shm: Deprecate wl_shm_buffer_create()

2015-10-16 Thread Pekka Paalanen
On Sun, 4 Oct 2015 13:46:03 +0300
Giulio Camuffo  wrote:

> 2015-06-26 19:34 GMT+03:00 Derek Foreman :
> > From irc:
> >  it creates a wl_buffer object in a way that no client can ever
> >  access the storage.
> >
> > So, let's replace it with abort(); and mark it with WL_DEPRECATED
> > in the header.
> >
> > Signed-off-by: Derek Foreman 
> > ---
> >
> > This time using WL_DEPRECATED
> >
> >  src/wayland-server-core.h |  2 +-
> >  src/wayland-shm.c | 29 +
> >  2 files changed, 2 insertions(+), 29 deletions(-)
> >
> > diff --git a/src/wayland-server-core.h b/src/wayland-server-core.h
> > index e605432..dc87168 100644
> > --- a/src/wayland-server-core.h
> > +++ b/src/wayland-server-core.h
> > @@ -397,7 +397,7 @@ wl_display_add_shm_format(struct wl_display *display, 
> > uint32_t format);
> >  struct wl_shm_buffer *
> >  wl_shm_buffer_create(struct wl_client *client,
> >  uint32_t id, int32_t width, int32_t height,
> > -int32_t stride, uint32_t format);
> > +int32_t stride, uint32_t format) WL_DEPRECATED;
> >
> >  void wl_log_set_handler_server(wl_log_func_t handler);
> >
> > diff --git a/src/wayland-shm.c b/src/wayland-shm.c
> > index 5c419fa..a9a0b76 100644
> > --- a/src/wayland-shm.c
> > +++ b/src/wayland-shm.c
> > @@ -319,34 +319,7 @@ wl_shm_buffer_create(struct wl_client *client,
> >  uint32_t id, int32_t width, int32_t height,
> >  int32_t stride, uint32_t format)
> >  {
> > -   struct wl_shm_buffer *buffer;
> > -
> > -   if (!format_is_supported(client, format))
> > -   return NULL;
> > -
> > -   buffer = malloc(sizeof *buffer + stride * height);
> > -   if (buffer == NULL)
> > -   return NULL;
> > -
> > -   buffer->width = width;
> > -   buffer->height = height;
> > -   buffer->format = format;
> > -   buffer->stride = stride;
> > -   buffer->offset = 0;
> > -   buffer->pool = NULL;
> > -
> > -   buffer->resource =
> > -   wl_resource_create(client, _buffer_interface, 1, id);
> > -   if (buffer->resource == NULL) {
> > -   free(buffer);
> > -   return NULL;
> > -   }
> > -
> > -   wl_resource_set_implementation(buffer->resource,
> > -  _buffer_interface,
> > -  buffer, destroy_buffer);
> > -
> > -   return buffer;
> > +   abort();
> 
> 
> If we abort() here we make the function unusable and fatal and i'd
> argue that it may be better to just remove it instead, because no
> function is better than a pretend function that will kill you without
> arguing.

We can't remove it, because that is a major ABI break. Not when it can
be called without a crash at least. I suspect we'll have to keep the
stub forever.

> Maybe it's better to make it return NULL instead?

Yeah, that would be good enough.

NULL or abort(),
Reviewed-by: Pekka Paalanen 

Patch 2 needs a rewrite if you change anything here, but consider it
Acked-by me.

Patch 3 is Acked-by me too, I don't care too much if it's abort(),
assert(), return NULL or all of them. If we hit a NULL pool, there is
guaranteed a bug somewhere, so hopefully that would be caught ASAP
somehow.


Thanks,
pq


pgpO71CQ7SRrM.pgp
Description: OpenPGP digital signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH v2 wayland] shm: Add shm_buffer ref and shm_pool unref functions

2015-10-16 Thread Pekka Paalanen
On Mon,  5 Oct 2015 13:12:12 -0500
Derek Foreman  wrote:

> Sometimes the compositor wants to make sure a shm pool doesn't disappear
> out from under it.
> 
> For example, in Enlightenment, rendering happens in a separate thread
> while the main thread can still dispatch events.  If a client is destroyed
> during rendering, all its resources are cleaned up and its shm pools are
> unmapped.  This causes the rendering thread to segfault.
> 
> This patch adds a way for the compositor to increment the refcount of the
> shm pool so it can't disappear, and decrement it when it's finished.
> 
> The ref/unref are asymmetrical (ref returns the pool) because it's
> possible the buffer itself will be gone when you need to unref the pool.
> 
> Signed-off-by: Derek Foreman 
> ---
> 
> Changes from v1:
>  renamed functions to wl_shm_buffer_ref_pool and wl_shm_pool_unref
>  added doxy
> 
>  src/wayland-server-core.h |  7 +++
>  src/wayland-shm.c | 36 
>  2 files changed, 43 insertions(+)
> 
> diff --git a/src/wayland-server-core.h b/src/wayland-server-core.h
> index e605432..4c2bdfe 100644
> --- a/src/wayland-server-core.h
> +++ b/src/wayland-server-core.h
> @@ -362,6 +362,7 @@ wl_resource_get_destroy_listener(struct wl_resource 
> *resource,
>resource = tmp,
> \
>tmp = wl_resource_from_link(wl_resource_get_link(resource)->next))
>  
> +struct wl_shm_pool;
>  struct wl_shm_buffer;
>  
>  void
> @@ -388,6 +389,12 @@ wl_shm_buffer_get_width(struct wl_shm_buffer *buffer);
>  int32_t
>  wl_shm_buffer_get_height(struct wl_shm_buffer *buffer);
>  
> +struct wl_shm_pool *
> +wl_shm_buffer_ref_pool(struct wl_shm_buffer *buffer);
> +
> +void
> +wl_shm_pool_unref(struct wl_shm_pool *pool);
> +
>  int
>  wl_display_init_shm(struct wl_display *display);
>  
> diff --git a/src/wayland-shm.c b/src/wayland-shm.c
> index 5c419fa..4f9d9d0 100644
> --- a/src/wayland-shm.c
> +++ b/src/wayland-shm.c
> @@ -412,6 +412,42 @@ wl_shm_buffer_get_height(struct wl_shm_buffer *buffer)
>   return buffer->height;
>  }
>  
> +/** Get a reference to a shm_buffer's shm_pool
> + *
> + * \param buffer The buffer object
> + *
> + * Returns a pointer to a buffer's shm_pool and increases the
> + * shm_pool refcount.
> + *
> + * This refcount keeps the shm pool from being destroyed on client
> + * disconnect, so the compositor must call wl_shm_pool_unref on it
> + * when finished.

It's not the disconnect directly, but cleaning up all wl_resources
after a disconnect causes the side-effect of the pool refcount dropping
to zero. Not sure that's an important detail to mention here, though.
The client can also do this without a disconnect, by just destroying
all relevant wl_proxies.

> + *
> + * \memberof wl_shm_buffer

See also: wl_shm_pool_unref.

> + */
> +WL_EXPORT struct wl_shm_pool *
> +wl_shm_buffer_ref_pool(struct wl_shm_buffer *buffer)
> +{
> + assert(buffer->pool->refcount);
> +
> + buffer->pool->refcount++;
> + return buffer->pool;
> +}
> +
> +/** Unreference a shm_pool
> + *
> + * \param buffer The pool object
> + *
> + * Drops a reference to a wl_shm_pool object.
> + *
> + * \memberof wl_shm_buffer

wl_shm_buffer?

I think we already have an empty wl_shm_pool section in the docs,
wouldn't this belong there, with a see also wl_shm_buffer_ref_pool()?

If Giulio or anyone still needs to get the fd of a shm buffer or
anything, that would be naturally done by taking a ref for the
wl_shm_pool, and then asking the pool for its fd and size.

> + */
> +WL_EXPORT void
> +wl_shm_pool_unref(struct wl_shm_pool *pool)
> +{
> + shm_pool_unref(pool);
> +}
> +
>  static void
>  reraise_sigbus(void)
>  {

Anyway, documentation bikeshedding apart, this patch looks good to me,
so:

Reviewed-by: Pekka Paalanen 

I did not test this in any way.


Thanks,
pq


pgpv_PsYMFutk.pgp
Description: OpenPGP digital signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH wayland 1/3 v2] shm: Deprecate wl_shm_buffer_create()

2015-10-16 Thread Giulio Camuffo
2015-10-16 16:46 GMT+03:00 Pekka Paalanen :
> On Sun, 4 Oct 2015 13:46:03 +0300
> Giulio Camuffo  wrote:
>
>> 2015-06-26 19:34 GMT+03:00 Derek Foreman :
>> > From irc:
>> >  it creates a wl_buffer object in a way that no client can ever
>> >  access the storage.
>> >
>> > So, let's replace it with abort(); and mark it with WL_DEPRECATED
>> > in the header.
>> >
>> > Signed-off-by: Derek Foreman 
>> > ---
>> >
>> > This time using WL_DEPRECATED
>> >
>> >  src/wayland-server-core.h |  2 +-
>> >  src/wayland-shm.c | 29 +
>> >  2 files changed, 2 insertions(+), 29 deletions(-)
>> >
>> > diff --git a/src/wayland-server-core.h b/src/wayland-server-core.h
>> > index e605432..dc87168 100644
>> > --- a/src/wayland-server-core.h
>> > +++ b/src/wayland-server-core.h
>> > @@ -397,7 +397,7 @@ wl_display_add_shm_format(struct wl_display *display, 
>> > uint32_t format);
>> >  struct wl_shm_buffer *
>> >  wl_shm_buffer_create(struct wl_client *client,
>> >  uint32_t id, int32_t width, int32_t height,
>> > -int32_t stride, uint32_t format);
>> > +int32_t stride, uint32_t format) WL_DEPRECATED;
>> >
>> >  void wl_log_set_handler_server(wl_log_func_t handler);
>> >
>> > diff --git a/src/wayland-shm.c b/src/wayland-shm.c
>> > index 5c419fa..a9a0b76 100644
>> > --- a/src/wayland-shm.c
>> > +++ b/src/wayland-shm.c
>> > @@ -319,34 +319,7 @@ wl_shm_buffer_create(struct wl_client *client,
>> >  uint32_t id, int32_t width, int32_t height,
>> >  int32_t stride, uint32_t format)
>> >  {
>> > -   struct wl_shm_buffer *buffer;
>> > -
>> > -   if (!format_is_supported(client, format))
>> > -   return NULL;
>> > -
>> > -   buffer = malloc(sizeof *buffer + stride * height);
>> > -   if (buffer == NULL)
>> > -   return NULL;
>> > -
>> > -   buffer->width = width;
>> > -   buffer->height = height;
>> > -   buffer->format = format;
>> > -   buffer->stride = stride;
>> > -   buffer->offset = 0;
>> > -   buffer->pool = NULL;
>> > -
>> > -   buffer->resource =
>> > -   wl_resource_create(client, _buffer_interface, 1, id);
>> > -   if (buffer->resource == NULL) {
>> > -   free(buffer);
>> > -   return NULL;
>> > -   }
>> > -
>> > -   wl_resource_set_implementation(buffer->resource,
>> > -  _buffer_interface,
>> > -  buffer, destroy_buffer);
>> > -
>> > -   return buffer;
>> > +   abort();
>>
>>
>> If we abort() here we make the function unusable and fatal and i'd
>> argue that it may be better to just remove it instead, because no
>> function is better than a pretend function that will kill you without
>> arguing.
>
> We can't remove it, because that is a major ABI break. Not when it can
> be called without a crash at least. I suspect we'll have to keep the
> stub forever.

Imho making it abort() is an API break as well, in a sense, and a
worse one, because it's at runtime instead of at compile time. The
symbol is still there but any attempt at using it will punish you
hard, so in a way it's like if it wasn't there, but you don't have the
compiler complaining hard enough when you use it.


--
Giulio

>
>> Maybe it's better to make it return NULL instead?
>
> Yeah, that would be good enough.
>
> NULL or abort(),
> Reviewed-by: Pekka Paalanen 
>
> Patch 2 needs a rewrite if you change anything here, but consider it
> Acked-by me.
>
> Patch 3 is Acked-by me too, I don't care too much if it's abort(),
> assert(), return NULL or all of them. If we hit a NULL pool, there is
> guaranteed a bug somewhere, so hopefully that would be caught ASAP
> somehow.
>
>
> Thanks,
> pq
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [RFC] weston: implement inert objects for keyboard/pointer/touch

2015-10-16 Thread Hardening
Le 16/10/2015 11:53, David FORT a écrit :
> This patch implements inert objects for wl_keyboard, wl_pointer and wl_touch.
> The target case is when the server has just send a capability event about a
> disappearing object, and the client binds the corresponding object. We bind an
> inert object: an object does nothing when it is requested. If the client 
> behave
> correctly, this object should be released when the capability event is 
> received
> and treated (calling the corresponding release request).
> As a consequence, we can rely on seat->[keyboard|pointer|touch]_state to know
> if the seat has the corresponding object.
> Weston doesn't really handle multiple mice for one wl_pointer, so I have 
> removed
> the corresponding code and tests (it was quite a weston_test hack BTW).
> Finally I have fixed a wrong behaviour: the capabilities event's 
> documentation states
> that the capabilities should be sent when a new capability is set on the 
> seat. So
> attaching a second mouse to an existing wl_pointer should not broadcast seat
> capabilities (and the same for keyboard and touch).
> ---
>  desktop-shell/exposay.c  |  16 ++--
>  desktop-shell/shell.c|  21 +++--
>  src/compositor-wayland.c |  10 +-
>  src/compositor-x11.c |  12 ++-
>  src/compositor.h |   3 -
>  src/input.c  | 235 
> ++-
>  src/libinput-device.c|   2 +-
>  src/screen-share.c   |   2 +
>  src/text-backend.c   |  44 +
>  src/zoom.c   |   7 +-
>  tests/devices-test.c |  20 
>  tests/weston-test.c  |  18 ++--
>  xwayland/dnd.c   |   3 +-
>  13 files changed, 216 insertions(+), 177 deletions(-)
> 
> diff --git a/desktop-shell/exposay.c b/desktop-shell/exposay.c
> index 08b86a3..d6919b3 100644
> --- a/desktop-shell/exposay.c
> +++ b/desktop-shell/exposay.c
> @@ -574,21 +574,23 @@ exposay_transition_active(struct desktop_shell *shell)
>   bool animate = false;
>  
>   shell->exposay.workspace = get_current_workspace(shell);
> - shell->exposay.focus_prev = get_default_view(keyboard->focus);
> - shell->exposay.focus_current = get_default_view(keyboard->focus);
> + if (keyboard) {
> + shell->exposay.focus_prev = get_default_view(keyboard->focus);
> + shell->exposay.focus_current = 
> get_default_view(keyboard->focus);
> + }
>   shell->exposay.clicked = NULL;
>   wl_list_init(>exposay.surface_list);
>  
>   lower_fullscreen_layer(shell, NULL);
>   shell->exposay.grab_kbd.interface = _kbd_grab;
> - weston_keyboard_start_grab(keyboard,
> ->exposay.grab_kbd);
> - weston_keyboard_set_focus(keyboard, NULL);
> + if (keyboard) {
> + weston_keyboard_start_grab(keyboard, >exposay.grab_kbd);
> + weston_keyboard_set_focus(keyboard, NULL);
> + }
>  
>   shell->exposay.grab_ptr.interface = _ptr_grab;
>   if (pointer) {
> - weston_pointer_start_grab(pointer,
> -   >exposay.grab_ptr);
> + weston_pointer_start_grab(pointer, >exposay.grab_ptr);
>   weston_pointer_clear_focus(pointer);
>   }
>   wl_list_for_each(shell_output, >output_list, link) {
> diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
> index 3eb3f5c..cdddf09 100644
> --- a/desktop-shell/shell.c
> +++ b/desktop-shell/shell.c
> @@ -380,7 +380,8 @@ shell_grab_start(struct shell_grab *grab,
>   struct desktop_shell *shell = shsurf->shell;
>   struct weston_touch *touch = weston_seat_get_touch(pointer->seat);
>  
> - popup_grab_end(pointer);
> + if (pointer)
> + popup_grab_end(pointer);
>   if (touch)
>   touch_popup_grab_end(touch);
>  
> @@ -391,11 +392,13 @@ shell_grab_start(struct shell_grab *grab,
> >shsurf_destroy_listener);
>  
>   shsurf->grabbed = 1;
> - weston_pointer_start_grab(pointer, >grab);
> + if (pointer)
> + weston_pointer_start_grab(pointer, >grab);
>   if (shell->child.desktop_shell) {
>   desktop_shell_send_grab_cursor(shell->child.desktop_shell,
>  cursor);
> - weston_pointer_set_focus(pointer,
> + if (pointer)
> + weston_pointer_set_focus(pointer,
>get_default_view(shell->grab_surface),
>wl_fixed_from_int(0),
>wl_fixed_from_int(0));
> @@ -925,12 +928,10 @@ restore_focus_state(struct desktop_shell *shell, struct 
> workspace *ws)
>   wl_list_insert(>compositor->seat_list,
>  >seat->link);
>  
> - if (!keyboard)
> - continue;
> -
>   surface = state->keyboard_focus;
>  
> - weston_keyboard_set_focus(keyboard, surface);
> +  

Re: [PATCH wayland 1/3 v2] shm: Deprecate wl_shm_buffer_create()

2015-10-16 Thread Derek Foreman
On 16/10/15 09:04 AM, Giulio Camuffo wrote:
> 2015-10-16 16:46 GMT+03:00 Pekka Paalanen :
>> On Sun, 4 Oct 2015 13:46:03 +0300
>> Giulio Camuffo  wrote:
>>
>>> 2015-06-26 19:34 GMT+03:00 Derek Foreman :
 From irc:
  it creates a wl_buffer object in a way that no client can ever
  access the storage.

 So, let's replace it with abort(); and mark it with WL_DEPRECATED
 in the header.

 Signed-off-by: Derek Foreman 
 ---

 This time using WL_DEPRECATED

  src/wayland-server-core.h |  2 +-
  src/wayland-shm.c | 29 +
  2 files changed, 2 insertions(+), 29 deletions(-)

 diff --git a/src/wayland-server-core.h b/src/wayland-server-core.h
 index e605432..dc87168 100644
 --- a/src/wayland-server-core.h
 +++ b/src/wayland-server-core.h
 @@ -397,7 +397,7 @@ wl_display_add_shm_format(struct wl_display *display, 
 uint32_t format);
  struct wl_shm_buffer *
  wl_shm_buffer_create(struct wl_client *client,
  uint32_t id, int32_t width, int32_t height,
 -int32_t stride, uint32_t format);
 +int32_t stride, uint32_t format) WL_DEPRECATED;

  void wl_log_set_handler_server(wl_log_func_t handler);

 diff --git a/src/wayland-shm.c b/src/wayland-shm.c
 index 5c419fa..a9a0b76 100644
 --- a/src/wayland-shm.c
 +++ b/src/wayland-shm.c
 @@ -319,34 +319,7 @@ wl_shm_buffer_create(struct wl_client *client,
  uint32_t id, int32_t width, int32_t height,
  int32_t stride, uint32_t format)
  {
 -   struct wl_shm_buffer *buffer;
 -
 -   if (!format_is_supported(client, format))
 -   return NULL;
 -
 -   buffer = malloc(sizeof *buffer + stride * height);
 -   if (buffer == NULL)
 -   return NULL;
 -
 -   buffer->width = width;
 -   buffer->height = height;
 -   buffer->format = format;
 -   buffer->stride = stride;
 -   buffer->offset = 0;
 -   buffer->pool = NULL;
 -
 -   buffer->resource =
 -   wl_resource_create(client, _buffer_interface, 1, id);
 -   if (buffer->resource == NULL) {
 -   free(buffer);
 -   return NULL;
 -   }
 -
 -   wl_resource_set_implementation(buffer->resource,
 -  _buffer_interface,
 -  buffer, destroy_buffer);
 -
 -   return buffer;
 +   abort();
>>>
>>>
>>> If we abort() here we make the function unusable and fatal and i'd
>>> argue that it may be better to just remove it instead, because no
>>> function is better than a pretend function that will kill you without
>>> arguing.
>>
>> We can't remove it, because that is a major ABI break. Not when it can
>> be called without a crash at least. I suspect we'll have to keep the
>> stub forever.
> 
> Imho making it abort() is an API break as well, in a sense, and a
> worse one, because it's at runtime instead of at compile time. The
> symbol is still there but any attempt at using it will punish you
> hard, so in a way it's like if it wasn't there, but you don't have the
> compiler complaining hard enough when you use it.

Trying really hard to find a reason to disagree...  It's theoretically
possible for a library to test for the presence of the function (dlopen,
assign a function pointer) but never actually use it.

I think EFL does some clever tricks similar to this for some libraries.

So a symbol that used to be present disappearing could be a break, even
if the function is never actually called.

Maybe someone else has a more likely example of how removing this
utterly broken function would break a functional program. :)

> 
> --
> Giulio
> 
>>
>>> Maybe it's better to make it return NULL instead?
>>
>> Yeah, that would be good enough.
>>
>> NULL or abort(),
>> Reviewed-by: Pekka Paalanen 
>>
>> Patch 2 needs a rewrite if you change anything here, but consider it
>> Acked-by me.
>>
>> Patch 3 is Acked-by me too, I don't care too much if it's abort(),
>> assert(), return NULL or all of them. If we hit a NULL pool, there is
>> guaranteed a bug somewhere, so hopefully that would be caught ASAP
>> somehow.
>>
>>
>> Thanks,
>> pq

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH wayland v4] protocol: Add DnD actions

2015-10-16 Thread Daniel Stone
Hi,

On Wednesday, 30 September 2015, Carlos Garnacho  wrote:

> These 2 requests have been added:
>
> - wl_data_source.set_actions: Notifies the compositor of the available
>   actions on the data source.
> - wl_data_offer.set_actions: Notifies the compositor of the available
>   actions on the destination side, plus the preferred action.
>
> Out of the data from these requests, the compositor can determine the
> action
> both parts agree on (and let the user play a role through eg. keyboard
> modifiers). The chosen option will be notified to both parties
> through the following two requests:
>
> - wl_data_source.action
> - wl_data_offer.action
>
> In addition, the destination side can peek the source side actions through
> wl_data_offer.source_actions.
>
> Compared to the XDND protocol, there's two notable changes:
>
> - XDND lets the source suggest an action, whereas wl_data_device lets
>   the destination prefer a given action. The difference is subtle here,
>   it comes off as convenience because it is the drag destination which
>   receives the motion events (unlike in X) and can perform action updates.
>
>   The drag destination seems also in a better position to update the
>   preferred action based on things like the data being transferred, the
>   place being dropped, and whether the drag is client-local.
>
> - That same source-side preferred action is used in XDND to convey the
>   modifier-induced action to the drag destination, which would then ack
>   it, or reply with another action that's accepted (or none), this makes
>   the XdndPosition/XdndStatus messaging very verbose, and synchronous
>   because the drag source always needs to know the latest status/action
>   for every position+action sent.
>
>   Here it's the compositor which takes care of modifiers and matching
>   available/accepted actions, this allows for the signaling to happen
>   only whenever the actions/modifiers change for real.
>
> Roughly based on previous work by Giulio Camuffo  >
>

Mike had a look at this from EFL - CCing him.

Cheers,
Daniel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH wayland] protocol: Improve data source notification around DnD progress

2015-10-16 Thread Daniel Stone
[and again, with the actual CC this time ...]

On Friday, 16 October 2015, Daniel Stone  wrote:

> Hi,
>
> On Wednesday, 30 September 2015, Carlos Garnacho  > wrote:
>
>> Currently, there's no means for the DnD origin to know whether the
>> destination is actually finished with the DnD transaction, short of
>> finalizing it after the first transfer finishes, or leaking it forever.
>>
>> But this poses other interoperation problems, drag destinations might
>> be requesting several mimetypes at once, might be just poking to find
>> out the most suitable format, might want to defer the action to a popup,
>> might be poking contents early before the selection was dropped...
>>
>> In addition, data_source.cancelled is suitable for the situations where
>> the DnD operation fails (not on a drop target, no matching mimetypes,
>> etc..), but seems undocumented for that use (and unused in weston's DnD).
>>
>> In order to improve the situation, the drag source should be notified
>> of all stages of DnD. In addition to documenting the "cancelled" event
>> for DnD purposes, The following 2 events have been added:
>>
>> - wl_data_source.drop_performed: Happens when the operation has been
>>   physically finished (eg. the button is released), it could be the right
>>   place to reset the pointer cursor back and undo any other state
>> resulting
>>   from the initial button press.
>> - wl_data_source.drag_finished: Happens when the destination side destroys
>>   the wl_data_offer, at this point the source can just forget all data
>>   related to the DnD selection as well, plus optionally deleting the data
>>   on move operations.
>>
>> Signed-off-by: Carlos Garnacho 
>
>
>  Mike had a look at this from EFL - CCing him.
>
> Cheers,
> Daniel
>
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH wayland] protocol: Improve data source notification around DnD progress

2015-10-16 Thread Mike Blumenkrantz
On Fri, 16 Oct 2015 16:53:28 +0100
Daniel Stone  wrote:

> Hi,
> 
> On Wednesday, 30 September 2015, Carlos Garnacho 
> wrote:
> 
> > Currently, there's no means for the DnD origin to know whether the
> > destination is actually finished with the DnD transaction, short of
> > finalizing it after the first transfer finishes, or leaking it
> > forever.
> >
> > But this poses other interoperation problems, drag destinations
> > might be requesting several mimetypes at once, might be just poking
> > to find out the most suitable format, might want to defer the
> > action to a popup, might be poking contents early before the
> > selection was dropped...
> >
> > In addition, data_source.cancelled is suitable for the situations
> > where the DnD operation fails (not on a drop target, no matching
> > mimetypes, etc..), but seems undocumented for that use (and unused
> > in weston's DnD).
> >
> > In order to improve the situation, the drag source should be
> > notified of all stages of DnD. In addition to documenting the
> > "cancelled" event for DnD purposes, The following 2 events have
> > been added:
> >
> > - wl_data_source.drop_performed: Happens when the operation has been
> >   physically finished (eg. the button is released), it could be the
> > right place to reset the pointer cursor back and undo any other
> > state resulting from the initial button press.
> > - wl_data_source.drag_finished: Happens when the destination side
> > destroys the wl_data_offer, at this point the source can just
> > forget all data related to the DnD selection as well, plus
> > optionally deleting the data on move operations.
> >
> > Signed-off-by: Carlos Garnacho >
> 
> 
>  Mike had a look at this from EFL - CCing him.
> 
> Cheers,
> Daniel

Hi,

Having reviewed this, and also having fully implemented the current DND
protocol, I think that this is a good idea and would be a worthwhile
addition. I have no suggestions for modifications or trivial
bikeshedding.

Regards,
Mike
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH wayland] protocol: Improve data source notification around DnD progress

2015-10-16 Thread Daniel Stone
Hi,

On Wednesday, 30 September 2015, Carlos Garnacho  wrote:

> Currently, there's no means for the DnD origin to know whether the
> destination is actually finished with the DnD transaction, short of
> finalizing it after the first transfer finishes, or leaking it forever.
>
> But this poses other interoperation problems, drag destinations might
> be requesting several mimetypes at once, might be just poking to find
> out the most suitable format, might want to defer the action to a popup,
> might be poking contents early before the selection was dropped...
>
> In addition, data_source.cancelled is suitable for the situations where
> the DnD operation fails (not on a drop target, no matching mimetypes,
> etc..), but seems undocumented for that use (and unused in weston's DnD).
>
> In order to improve the situation, the drag source should be notified
> of all stages of DnD. In addition to documenting the "cancelled" event
> for DnD purposes, The following 2 events have been added:
>
> - wl_data_source.drop_performed: Happens when the operation has been
>   physically finished (eg. the button is released), it could be the right
>   place to reset the pointer cursor back and undo any other state resulting
>   from the initial button press.
> - wl_data_source.drag_finished: Happens when the destination side destroys
>   the wl_data_offer, at this point the source can just forget all data
>   related to the DnD selection as well, plus optionally deleting the data
>   on move operations.
>
> Signed-off-by: Carlos Garnacho >


 Mike had a look at this from EFL - CCing him.

Cheers,
Daniel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH wayland v4] protocol: Add DnD actions

2015-10-16 Thread Mike Blumenkrantz
On Fri, 16 Oct 2015 16:52:44 +0100
Daniel Stone  wrote:

> Hi,
> 
> On Wednesday, 30 September 2015, Carlos Garnacho 
> wrote:
> 
> > These 2 requests have been added:
> >
> > - wl_data_source.set_actions: Notifies the compositor of the
> > available actions on the data source.
> > - wl_data_offer.set_actions: Notifies the compositor of the
> > available actions on the destination side, plus the preferred
> > action.
> >
> > Out of the data from these requests, the compositor can determine
> > the action
> > both parts agree on (and let the user play a role through eg.
> > keyboard modifiers). The chosen option will be notified to both
> > parties through the following two requests:
> >
> > - wl_data_source.action
> > - wl_data_offer.action
> >
> > In addition, the destination side can peek the source side actions
> > through wl_data_offer.source_actions.
> >
> > Compared to the XDND protocol, there's two notable changes:
> >
> > - XDND lets the source suggest an action, whereas wl_data_device
> > lets the destination prefer a given action. The difference is
> > subtle here, it comes off as convenience because it is the drag
> > destination which receives the motion events (unlike in X) and can
> > perform action updates.
> >
> >   The drag destination seems also in a better position to update the
> >   preferred action based on things like the data being transferred,
> > the place being dropped, and whether the drag is client-local.
> >
> > - That same source-side preferred action is used in XDND to convey
> > the modifier-induced action to the drag destination, which would
> > then ack it, or reply with another action that's accepted (or
> > none), this makes the XdndPosition/XdndStatus messaging very
> > verbose, and synchronous because the drag source always needs to
> > know the latest status/action for every position+action sent.
> >
> >   Here it's the compositor which takes care of modifiers and
> > matching available/accepted actions, this allows for the signaling
> > to happen only whenever the actions/modifiers change for real.
> >
> > Roughly based on previous work by Giulio Camuffo
> > >
> >
> 
> Mike had a look at this from EFL - CCing him.
> 
> Cheers,
> Daniel

Hi,

Having reviewed this, and also having fully implemented the current DND
protocol, I think that this is a good idea and would be a worthwhile
addition. I have no suggestions for modifications or trivial
bikeshedding.

Regards,
Mike
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH wayland] protocol: Improve data source notification around DnD progress

2015-10-16 Thread Daniel Stone
[and again, with the actual CC this time ...]

On Friday, 16 October 2015, Daniel Stone  wrote:

> Hi,
>
> On Wednesday, 30 September 2015, Carlos Garnacho  > wrote:
>
>> Currently, there's no means for the DnD origin to know whether the
>> destination is actually finished with the DnD transaction, short of
>> finalizing it after the first transfer finishes, or leaking it forever.
>>
>> But this poses other interoperation problems, drag destinations might
>> be requesting several mimetypes at once, might be just poking to find
>> out the most suitable format, might want to defer the action to a popup,
>> might be poking contents early before the selection was dropped...
>>
>> In addition, data_source.cancelled is suitable for the situations where
>> the DnD operation fails (not on a drop target, no matching mimetypes,
>> etc..), but seems undocumented for that use (and unused in weston's DnD).
>>
>> In order to improve the situation, the drag source should be notified
>> of all stages of DnD. In addition to documenting the "cancelled" event
>> for DnD purposes, The following 2 events have been added:
>>
>> - wl_data_source.drop_performed: Happens when the operation has been
>>   physically finished (eg. the button is released), it could be the right
>>   place to reset the pointer cursor back and undo any other state
>> resulting
>>   from the initial button press.
>> - wl_data_source.drag_finished: Happens when the destination side destroys
>>   the wl_data_offer, at this point the source can just forget all data
>>   related to the DnD selection as well, plus optionally deleting the data
>>   on move operations.
>>
>> Signed-off-by: Carlos Garnacho 
>
>
>  Mike had a look at this from EFL - CCing him.
>
> Cheers,
> Daniel
>
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH wayland] protocol: Improve data source notification around DnD progress

2015-10-16 Thread Bill Spitzak
On Thu, Oct 1, 2015 at 12:57 PM, Carlos Garnacho  wrote:

> On Thu, Oct 1, 2015 at 8:48 PM, Bill Spitzak  wrote:
> >
> >
> > On Wed, Sep 30, 2015 at 1:45 PM, Carlos Garnacho 
> wrote:
> >>
> >>
> >> - wl_data_source.drop_performed: Happens when the operation has been
> >>   physically finished (eg. the button is released), it could be the
> right
> >>   place to reset the pointer cursor back and undo any other state
> >> resulting
> >>   from the initial button press.
> >
> >
> > This should not be necessary. When the grab is lost, whatever client is
> > under the pointer should get an enter event and should then set the
> cursor
> > to whatever it wants. The dnd source should certainly not set the cursor
> in
> > this case because it may set it to the wrong one, resulting in a blink of
> > this wrong cursor (ie it will quickly change between the drag cursor, the
> > cursor the source thought it should be, and the actual cursor for that
> > screen location).
>
> Remember, toolkits preserve some state. The drag source is in control
> of the pointer cursor as long as the DnD operation holds, so it should
> reset its internal current cursor to the regular one for the next time
> the pointer enters the surface. And that's the bare minimals to be
> done there, in X11-land toolkits usually implemented DnD and its
> cursor changes through a grab, at least in the case of GTK+ this is
> carried on, and brings in a lot more side effects than just an
> extraneous cursor, so really can't wait until the next
> wl_pointer.enter.
>
>   Carlos
>

I think you misunderstood. The *destination* (if the cursor is still
pointing at it) is what sets the cursor. It should get an enter event the
moment the compositor finishes the state where the source can send drag
events, and can update the cursor. If the user moves the mouse fast enough
perhaps some client other than the destination would get the enter event,
and it should set the cursor.

Absolutely no way should the source try to do anything to the cursor. All
it might do is set it to the wrong image. It has no idea what cursor the
client being currently pointed at wants.
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH wayland v4] protocol: Add DnD actions

2015-10-16 Thread Bill Spitzak
I wish to formally state that I very seriously believe this proposal is
wrong. The action must be decided by the destination client, not the way
this is trying to do it.

On Fri, Oct 16, 2015 at 8:59 AM, Mike Blumenkrantz 
wrote:

> On Fri, 16 Oct 2015 16:52:44 +0100
> Daniel Stone  wrote:
>
> > Hi,
> >
> > On Wednesday, 30 September 2015, Carlos Garnacho 
> > wrote:
> >
> > > These 2 requests have been added:
> > >
> > > - wl_data_source.set_actions: Notifies the compositor of the
> > > available actions on the data source.
> > > - wl_data_offer.set_actions: Notifies the compositor of the
> > > available actions on the destination side, plus the preferred
> > > action.
> > >
> > > Out of the data from these requests, the compositor can determine
> > > the action
> > > both parts agree on (and let the user play a role through eg.
> > > keyboard modifiers). The chosen option will be notified to both
> > > parties through the following two requests:
> > >
> > > - wl_data_source.action
> > > - wl_data_offer.action
> > >
> > > In addition, the destination side can peek the source side actions
> > > through wl_data_offer.source_actions.
> > >
> > > Compared to the XDND protocol, there's two notable changes:
> > >
> > > - XDND lets the source suggest an action, whereas wl_data_device
> > > lets the destination prefer a given action. The difference is
> > > subtle here, it comes off as convenience because it is the drag
> > > destination which receives the motion events (unlike in X) and can
> > > perform action updates.
> > >
> > >   The drag destination seems also in a better position to update the
> > >   preferred action based on things like the data being transferred,
> > > the place being dropped, and whether the drag is client-local.
> > >
> > > - That same source-side preferred action is used in XDND to convey
> > > the modifier-induced action to the drag destination, which would
> > > then ack it, or reply with another action that's accepted (or
> > > none), this makes the XdndPosition/XdndStatus messaging very
> > > verbose, and synchronous because the drag source always needs to
> > > know the latest status/action for every position+action sent.
> > >
> > >   Here it's the compositor which takes care of modifiers and
> > > matching available/accepted actions, this allows for the signaling
> > > to happen only whenever the actions/modifiers change for real.
> > >
> > > Roughly based on previous work by Giulio Camuffo
> > > >
> > >
> >
> > Mike had a look at this from EFL - CCing him.
> >
> > Cheers,
> > Daniel
>
> Hi,
>
> Having reviewed this, and also having fully implemented the current DND
> protocol, I think that this is a good idea and would be a worthwhile
> addition. I have no suggestions for modifications or trivial
> bikeshedding.
>
> Regards,
> Mike
> ___
> wayland-devel mailing list
> wayland-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
>
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH] Extending wayland protocol for helping a wayland client to identify the event source of device (pointer/keyboard)

2015-10-16 Thread Peter Hutterer
On Fri, Oct 16, 2015 at 06:58:32PM -0700, Bill Spitzak wrote:
> On Mon, Oct 12, 2015 at 10:01 PM, Peter Hutterer 
> wrote:
> 
> At least based on this example, I think this is fixing the wrong problem.
> > knowing which device sends a key is fairly meaningless unless you have
> > direct access to the devices anyway (which you probably don't, at least not
> > in the default setups).
> 
> 
> Nonsense. A client may very well want to do something different for the X
> key on device 1 than the X key on device 2. It does not need any low-level
> device access to do that, in fact it is not going to send or query anything
> from any device, just receive wayland events.
> 
> This is used to differentiate the tablet pen and eraser and puck and the
> normal mouse. The program does absolutely nothing special, it gets position
> events, but ignores the device, then they all move the cursor and have the
> same effect. But a painting program that figures out which device is the
> eraser will certainly act differently for that one.

what does this example have to do with the original example that you
conveniently dropped?
tablets are different, which is why we have a separate interface for it and
per-device handling. it's not the same as the keyboard.

> > And there are plenty of devices that have
> > meaningless names made from the pid/vid hex codes or even worse - "USB
> > Keyboard".
> >
> 
> Yea that is a problem, but not as big as you think, as long as all clients
> see the same name assignments, and all the devices have a different name.

and the last requirement is not guaranteed. yes, maybe in some ideal setup
you can assume this, but not in the real world where e.g. every gaming mouse
gives you multiple event nodes with the same pid/vid and the same name.
 
> > the source of your problem is IMO that you're treating the remote control
> > like a keyboard when it isn't one. This is what we've been trying to solve
> > with the buttonset interface in libinput (still WIP and needs a wayland
> > protocol extension). Those devices are merely sets of buttons and require
> > their own focus control and behaviour, separate to (and more
> > domain-specific
> > than) keyboards. But it moves the issue into its own separate corner where
> > it can be handled correctly, rather than papering over the issues.
> >
> 
> Buttonset interface is not going to solve anything. What if the user has
> two button devices, both with the same button?

I'm glad that you're convinced the buttonset interface isn't going to solve
anything, clearly without having looked at it or being part of the
design/discussion around it. Otherwise you'd know that unlike the
pointer/keyboard/touch interface it's designed as a per-device interface.

*plonk*

Cheers,
   Peter
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel