Re: [PATCH weston] clients: Add a weston-autorotater client and rotater protocol
On Mon, 8 Aug 2016 16:36:36 +0100 Emmanuel Gil Peyrot wrote: > On Mon, May 23, 2016 at 12:00:13PM +0300, Pekka Paalanen wrote: > > On Sat, 21 May 2016 03:43:04 +0100 > > Emmanuel Gil Peyrot wrote: > > > > > This client uses libiio to retrieve accelerometer values from the iio > > > subsystem on Linux (and maybe some other kernels), and automatically > > > rotate the output whenever orientation changed. > > > > > > I tested it with a mma8453 accelerometer, but everything needed should > > > be available in the configuration to make it work with any other iio > > > device. > > > > > > Signed-off-by: Emmanuel Gil Peyrot > > > --- > > > Makefile.am | 24 +++ > > > clients/autorotater.c | 472 > > > > > > configure.ac| 15 ++ > > > desktop-shell/shell.c | 1 + > > > protocol/weston-rotater.xml | 26 +++ > > > src/compositor.h| 3 + > > > src/rotater.c | 150 ++ > > > 7 files changed, 691 insertions(+) > > > create mode 100644 clients/autorotater.c > > > create mode 100644 protocol/weston-rotater.xml > > > create mode 100644 src/rotater.c > > > > Hi Emmanuel, > > Hi Pekka, > > > > > why is there a new client for this? Could it not be a plugin? > > > > Is IIO potentially so slow and blocking we cannot use it in the server > > process? > > > > Why the polling approach, cannot IIO deliver events? > > The actual issue here is driver support for event-based IIO is spotty > at best, as I found out while trying to implement that method. I > haven’t been able to find out programatically whether a driver will > behave as expected in the event-based API (which IIO people call “the > high-speed mode”), and implementing both methods with a blacklist > doesn’t seem sensible. Having a method working with every driver seems > to be the most sensible and generic way. Hi, ok, that's an important design choice worth mentioning. > > > > Should there be a way to set which outputs get rotated by a specific > > IIO device, rather than assuming there is just one device rotating all > > outputs at once? > > This has been added to the v2, an arbitrary number of autorotators can > now be used to drive an arbitrary number of weston_outputs. > > > > > Any reason why this is limited to desktop-shell rather than being a > > shell-agnostic feature? > > In the v2 I made it a Weston module, it now won’t be used without being > listed in the modules option of the [core] section. Sounds good! Thanks, pq pgp4E6ZnFX2DN.pgp Description: OpenPGP digital signature ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston v4 5/5] Switch to use safe_strtoint instead of strtol
On Tue, 9 Aug 2016 11:12:33 -0700
Bryce Harrington wrote:
> On Mon, Aug 08, 2016 at 02:55:14PM +0300, Pekka Paalanen wrote:
> > On Wed, 3 Aug 2016 17:40:52 -0700
> > Bryce Harrington wrote:
> >
> > > Reviewed-by: Eric Engestrom
> > > Signed-off-by: Bryce Harrington
> > > ---
> > > compositor/main.c | 7 +++
> > > compositor/systemd-notify.c | 9 +++--
> > > libweston/compositor.c | 9 +++--
> > > libweston/libbacklight.c| 11 +--
> > > shared/config-parser.c | 7 ++-
> > > shared/option-parser.c | 5 ++---
> > > xwayland/launcher.c | 7 +++
> > > 7 files changed, 21 insertions(+), 34 deletions(-)
> > >
> >
> > > diff --git a/compositor/systemd-notify.c b/compositor/systemd-notify.c
> > > index 6104124..49e51f4 100644
> > > --- a/compositor/systemd-notify.c
> > > +++ b/compositor/systemd-notify.c
> > > @@ -25,12 +25,13 @@
> > >
> > > #include "config.h"
> > >
> > > -#include
> > > #include
> > > #include
> > > #include
> > > #include
> > > +
> > > #include "shared/helpers.h"
> > > +#include "shared/string-helpers.h"
> > > #include "shared/zalloc.h"
> > > #include "compositor.h"
> > >
> > > @@ -116,7 +117,6 @@ WL_EXPORT int
> > > module_init(struct weston_compositor *compositor,
> > > int *argc, char *argv[])
> > > {
> > > - char *tail;
> > > char *watchdog_time_env;
> > > struct wl_event_loop *loop;
> > > long watchdog_time_conv;
> > > @@ -140,13 +140,10 @@ module_init(struct weston_compositor *compositor,
> > >* by systemd to transfer 'WatchdogSec' watchdog timeout
> > >* setting from service file.*/
> > > watchdog_time_env = getenv("WATCHDOG_USEC");
> > > -
> > > if (!watchdog_time_env)
> > > return 0;
> > >
> > > - errno = 0;
> > > - watchdog_time_conv = strtol(watchdog_time_env, &tail, 10);
> > > - if (errno != 0 || tail == watchdog_time_env || *tail != '\0')
> > > + if (!safe_strtoint(watchdog_time_env, &watchdog_time_conv))
> > > return 0;
> > >
> > > /* Convert 'WATCHDOG_USEC' to milliseconds and notify
> >
> > Hi,
> >
> > there's a new warning:
> >
> > /home/pq/git/weston/compositor/systemd-notify.c: In function ‘module_init’:
> > /home/pq/git/weston/compositor/systemd-notify.c:146:40: warning: passing
> > argument 2 of ‘safe_strtoint’ from incompatible pointer type
> > if (!safe_strtoint(watchdog_time_env, &watchdog_time_conv))
> > ^
> > In file included from /home/pq/git/weston/compositor/systemd-notify.c:34:0:
> > /home/pq/git/weston/shared/string-helpers.h:45:1: note: expected ‘int32_t
> > *’ but argument is of type ‘long int *’
> > safe_strtoint(const char *str, int32_t *value)
> > ^
> >
> > Please CC me if you want me to test a patch for this. I suppose int32_t
> > should be fine?
>
> attached
Hi Bryce,
the patch results in:
CC compositor/systemd_notify_la-systemd-notify.lo
/home/pq/git/weston/compositor/systemd-notify.c: In function ‘module_init’:
/home/pq/git/weston/compositor/systemd-notify.c:154:11: error:
‘watchdog_time_conf’ undeclared (first use in this function)
else if (watchdog_time_conf > INT_MAX)
^
/home/pq/git/weston/compositor/systemd-notify.c:154:11: note: each undeclared
identifier is reported only once for each function it appears in
/home/pq/git/weston/compositor/systemd-notify.c:154:32: error: ‘INT_MAX’
undeclared (first use in this function)
else if (watchdog_time_conf > INT_MAX)
^
Thanks,
pq
pgpkkIho5Hjin.pgp
Description: OpenPGP digital signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston 2/2] README: updates on libweston versioning
From: Pekka Paalanen WE have agreed to use the major as the ABI-version, so talk about major to avoid confusion. Remove unncessary or incorrect wording related to breaking ABI on minor bumps. Explain a little about the weston vs. libweston version numbers. Signed-off-by: Pekka Paalanen --- README | 34 ++ 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/README b/README index 37090d5..0275c09 100644 --- a/README +++ b/README @@ -36,7 +36,7 @@ input and output systems, so that people who just want to write a new "Wayland window manager" (WM) or a small desktop environment (DE) can focus on the WM part. -Libweston was first introduced in Weston 1.9, and is expected to +Libweston was first introduced in Weston 1.12, and is expected to continue evolving through many Weston releases before it achieves a stable API and feature completeness. @@ -45,23 +45,17 @@ API/ABI (in)stability and parallel installability - As libweston's API surface is huge, it is impossible to get it right -in one go. Therefore developers reserve the right to break the API/ABI -between every 1.x.0 Weston release (minor version bumps), just like -Weston's plugin API does. For git snapshots of the master branch, the -API/ABI can break any time without warning or version bump. - -Libweston API or ABI will not be broken between Weston's stable -releases 1.x.0 and 1.x.y, where y < 90. +in one go. Therefore developers reserve the right to break the API/ABI and bump +the major version to signify that. For git snapshots of the master branch, the +API/ABI can break any time without warning. To make things tolerable for libweston users despite API/ABI breakages, -libweston is designed to be perfectly parallel-installable. An -API/ABI-version is defined for libweston, and it is bumped for releases as -needed. Different non-backward compatible ABI versions of libweston can be -installed in parallel, so that external projects can easily depend on a -particular ABI-version. Thus they do not have to fight over which ABI-version -is installed in a user's system. This allows a user to install many -different compositors each requiring a different libweston ABI-version -without tricks or conflicts. +different libweston major versions are designed to be perfectly +parallel-installable. This way external projects can easily depend on a +particular API/ABI-version. Thus they do not have to fight over which +ABI-version is installed in a user's system. This allows a user to install many +different compositors each requiring a different libweston ABI-version without +tricks or conflicts. Note, that versions of Weston itself will not be parallel-installable, only libweston is. @@ -82,6 +76,14 @@ The document provides the full details, with the gist summed below: - Minor - new backward compatible features. - Patch - internal (implementation specific) fixes. +Weston and libweston have separate version numbers in configure.ac. All +releases are made by the Weston version number. Libweston version number +matches the Weston version number in all releases except maybe pre-releases. +Pre-releases have the micro/patch version 91 or greater. + +A pre-release is allowed to install a libweston version greater than the +Weston version. + Forward compatibility - -- 2.7.3 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston 1/2] releasing: how to handle libweston
From: Pekka Paalanen libweston has separate version numbering from weston because of development needs. During development, weston version is major.minor.90 which will never be a release version number. While developing, we may break the libweston backward-compatibility, in which case libweston_major_version will be bumped. This means that libweston_major_version > weston_major_version but only during the development period and for the pre-releases. When the official x.y.0 release is made, weston and libweston versions will get synchronized as explained in releasing.txt. The reason we do this is that e.g. during the weston 3.0.90 development period we must be able to install libweston-4.so because the development has broken the compatibility and so we cannot install it as libweston-3.so anymore. However, we cannot bump weston to 4.0.90, because then the official release would go backwards in numbers to 4.0.0. This also means that weston pre-releases major.minor.9x may install libweston-(major+1).so. There is also libweston-(major+1).pc file but it will give the weston version as the version number. IOW, pkg-config check for 'libweston-M < M.0.0' matches only the pre-releases of the libweston major version M. Hence, 'libweston-M >= M.0.0' cannot be satisfied by pre-releases. The weston and libweston version numbers MUST be identical in all releases except the pre-releases major.minor.9x. When the 1.11.91 pre-release is made, the rules imply that libweston version will be bumped from 0.0.0 to 1.11.91. The bumping will continue up to the 1.12.0 release. After the bump to 1.12.90, the libweston version may be bumped to 2.0.0. Then the rules imply that: - 1.12.9x pre-releases install libweston 2.0.0 - the next .0 release is 2.0.0 containing libweston 2.0.0 If the 1.12 stable branch will see additional releases, those will be numbered 1.12.1, 1.12.2, etc. with the libweston version being the same as the release version number. If we have release 2.0.91 without libweston major bump, then libweston version will match the release version, leading up to 2.1.0. Signed-off-by: Pekka Paalanen --- releasing.txt | 15 ++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/releasing.txt b/releasing.txt index 0aae23a..b30991f 100644 --- a/releasing.txt +++ b/releasing.txt @@ -10,7 +10,20 @@ To make a release of Weston and/or Wayland, follow these steps. release with any needed protocol updates. 2. Update the first three lines of configure.ac to the intended - version, commit. Then commit your changes: + version. + + For Weston's x.y.0 releases, if libweston_major_version is greater than + weston_major_version, bump the Weston version numbers (major, minor, + micro) to match the libweston version numbers (major, minor, patch). + + Additionally for all Weston releases, if libweston version + major.minor.patch is less than Weston version major.minor.micro, bump + libweston version numbers to match the Weston version numbers. + + Weston releases are made with the Weston version number, not with the + libweston version number. + + Then commit your changes: $ export RELEASE_NUMBER="x.y.z" $ export RELEASE_NAME="[alpha|beta|RC1|RC2|official|point]" -- 2.7.3 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Libweston release blockers (Re: [PATCH weston 1/2] releasing: how to handle libweston)
Hi all, please mind that this patch is a weston release blocker. We cannot make any release until we have agreed and documented how the weston vs. libweston versions work. I believe this is the last libweston release blocker. If you have any further concerns, now is the time. Thanks, pq On Wed, 10 Aug 2016 15:01:12 +0300 Pekka Paalanen wrote: > From: Pekka Paalanen > > libweston has separate version numbering from weston because of > development needs. > > During development, weston version is major.minor.90 which will never be > a release version number. While developing, we may break the libweston > backward-compatibility, in which case libweston_major_version will be > bumped. This means that libweston_major_version > weston_major_version > but only during the development period and for the pre-releases. When > the official x.y.0 release is made, weston and libweston versions will > get synchronized as explained in releasing.txt. > > The reason we do this is that e.g. during the weston 3.0.90 development > period we must be able to install libweston-4.so because the development > has broken the compatibility and so we cannot install it as libweston-3.so > anymore. However, we cannot bump weston to 4.0.90, because then the > official release would go backwards in numbers to 4.0.0. > > This also means that weston pre-releases major.minor.9x may install > libweston-(major+1).so. There is also libweston-(major+1).pc file but it > will give the weston version as the version number. IOW, pkg-config > check for 'libweston-M < M.0.0' matches only the pre-releases of the > libweston major version M. Hence, 'libweston-M >= M.0.0' cannot be > satisfied by pre-releases. > > The weston and libweston version numbers MUST be identical in all > releases except the pre-releases major.minor.9x. > > When the 1.11.91 pre-release is made, the rules imply that libweston > version will be bumped from 0.0.0 to 1.11.91. The bumping will continue > up to the 1.12.0 release. After the bump to 1.12.90, the libweston > version may be bumped to 2.0.0. Then the rules imply that: > - 1.12.9x pre-releases install libweston 2.0.0 > - the next .0 release is 2.0.0 containing libweston 2.0.0 > > If the 1.12 stable branch will see additional releases, those will be > numbered 1.12.1, 1.12.2, etc. with the libweston version being the same > as the release version number. > > If we have release 2.0.91 without libweston major bump, then libweston > version will match the release version, leading up to 2.1.0. > > Signed-off-by: Pekka Paalanen > --- > releasing.txt | 15 ++- > 1 file changed, 14 insertions(+), 1 deletion(-) > > diff --git a/releasing.txt b/releasing.txt > index 0aae23a..b30991f 100644 > --- a/releasing.txt > +++ b/releasing.txt > @@ -10,7 +10,20 @@ To make a release of Weston and/or Wayland, follow these > steps. >release with any needed protocol updates. > >2. Update the first three lines of configure.ac to the intended > - version, commit. Then commit your changes: > + version. > + > + For Weston's x.y.0 releases, if libweston_major_version is greater than > + weston_major_version, bump the Weston version numbers (major, minor, > + micro) to match the libweston version numbers (major, minor, patch). > + > + Additionally for all Weston releases, if libweston version > + major.minor.patch is less than Weston version major.minor.micro, bump > + libweston version numbers to match the Weston version numbers. > + > + Weston releases are made with the Weston version number, not with the > + libweston version number. > + > + Then commit your changes: > >$ export RELEASE_NUMBER="x.y.z" >$ export RELEASE_NAME="[alpha|beta|RC1|RC2|official|point]" pgplyAtUBMI1k.pgp Description: OpenPGP digital signature ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH 1/3 v3] wayland-server: Add API to control globals visibility
Hi Yong, > Despite the R-b's you received, and I hate to encourage you to do > a v4, I would like to suggest two things that are important: > - naming > - more comments/doc > > I'll send another email soon with some suggestions to save you time. Sure, I would need some more details and a precise review as to where you'd like to see changes. Cheers, Olivier ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston 1/2] input: Update keyboard serial on press and release
Hi all,
Small reminder, I don't think I got any review/reply of these two patches...
Cheers,
Olivier
- Original Message -
> Other compositors such as mutter update the keyboard serial for both key
> press and key release, unlike weston which updates it only on key press.
>
> When dealing with popup windows which require a match in serials, if the
> event that caused the popup to be shown is a key release, then the popup
> would be dismissed.
>
> This occurs when navigating gtk+ sub-menus using the keyboard.
>
> Signed-off-by: Olivier Fourdan
> Bugzilla: https://bugzilla.gnome.org/show_bug.cgi?id=768017
> ---
> libweston/input.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/libweston/input.c b/libweston/input.c
> index 8f46698..18c4c1f 100644
> --- a/libweston/input.c
> +++ b/libweston/input.c
> @@ -1661,9 +1661,8 @@ notify_key(struct weston_seat *seat, uint32_t time,
> uint32_t key,
> state);
> }
>
> + keyboard->grab_serial = wl_display_get_serial(compositor->wl_display);
> if (state == WL_KEYBOARD_KEY_STATE_PRESSED) {
> - keyboard->grab_serial =
> - wl_display_get_serial(compositor->wl_display);
> keyboard->grab_time = time;
> keyboard->grab_key = key;
> }
> --
> 2.7.4
>
> ___
> wayland-devel mailing list
> wayland-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/wayland-devel
>
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston 2/2] shell: Check for keyboard driven popup as well
... sorry, by two patches, I meant this one as well :)
Cheers,
Olivier
- Original Message -
> xdh-shell's xdg-popup takes a serial, if the serial doesn't match the
> compositor would dismiss the popup.
>
> Currently, weston checks for pointer and touch serials, but popups can
> also be triggered by keyboard shortcuts.
>
> If a user activates a menu without any previous pointer ineraction with
> the client, weston would dismiss the xdg-popup because the keyboard
> serial would not match the popup serial.
>
> Signed-off-by: Olivier Fourdan
> Bugzilla: https://bugzilla.gnome.org/show_bug.cgi?id=768017
> ---
> desktop-shell/shell.c | 14 +++---
> 1 file changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
> index c125d55..fdab394 100644
> --- a/desktop-shell/shell.c
> +++ b/desktop-shell/shell.c
> @@ -230,7 +230,7 @@ struct shell_seat {
> struct wl_list surfaces_list;
> struct wl_client *client;
> int32_t initial_up;
> - enum { POINTER, TOUCH } type;
> + enum { KEYBOARD, POINTER, TOUCH } type;
> } popup_grab;
> };
>
> @@ -3467,12 +3467,15 @@ add_popup_grab(struct shell_surface *shsurf,
> } else if (type == TOUCH) {
> shseat->popup_grab.touch_grab.interface =
> &touch_popup_grab_interface;
> + } else if (type == KEYBOARD) {
> + shseat->popup_grab.grab.interface =
> + &popup_grab_interface;
> }
>
> wl_list_insert(&shseat->popup_grab.surfaces_list,
> &shsurf->popup.grab_link);
>
> - if (type == POINTER) {
> + if (type == POINTER || type == KEYBOARD) {
> weston_pointer_start_grab(pointer,
> &shseat->popup_grab.grab);
> } else if (type == TOUCH) {
> @@ -3504,7 +3507,7 @@ remove_popup_grab(struct shell_surface *shsurf)
> wl_list_remove(&shsurf->popup.grab_link);
> wl_list_init(&shsurf->popup.grab_link);
> if (wl_list_empty(&shseat->popup_grab.surfaces_list)) {
> - if (shseat->popup_grab.type == POINTER) {
> + if (shseat->popup_grab.type == POINTER ||
> shseat->popup_grab.type ==
> KEYBOARD) {
>
> weston_pointer_end_grab(shseat->popup_grab.grab.pointer);
> shseat->popup_grab.grab.interface = NULL;
> } else if (shseat->popup_grab.type == TOUCH) {
> @@ -3521,6 +3524,7 @@ shell_map_popup(struct shell_surface *shsurf)
> struct weston_view *parent_view = get_default_view(shsurf->parent);
> struct weston_pointer *pointer = weston_seat_get_pointer(shseat->seat);
> struct weston_touch *touch = weston_seat_get_touch(shseat->seat);
> + struct weston_keyboard *keyboard =
> weston_seat_get_keyboard(shseat->seat);
>
> shsurf->surface->output = parent_view->output;
> shsurf->view->output = parent_view->output;
> @@ -3537,6 +3541,10 @@ shell_map_popup(struct shell_surface *shsurf)
> touch->grab_serial == shsurf->popup.serial) {
> if (add_popup_grab(shsurf, shseat, TOUCH) != 0)
> return -1;
> + } else if (keyboard &&
> +keyboard->grab_serial == shsurf->popup.serial) {
> + if (add_popup_grab(shsurf, shseat, KEYBOARD) != 0)
> + return -1;
> } else {
> shell_surface_send_popup_done(shsurf);
> shseat->popup_grab.client = NULL;
> --
> 2.7.4
>
> ___
> wayland-devel mailing list
> wayland-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/wayland-devel
>
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH wayland 2/2] scanner: Generate all SINCE_VERSION macros for everyone
On Tue, 9 Aug 2016 11:05:49 +0300
Pekka Paalanen wrote:
> On Mon, 8 Aug 2016 15:59:37 +0200
> Quentin Glidic wrote:
>
> > On 08/08/2016 15:45, Pekka Paalanen wrote:
> > > On Tue, 5 Jul 2016 20:41:50 +0200
> > > Quentin Glidic wrote:
> > >
> > >> From: Quentin Glidic
> > >>
> > >> Practical example: a client supporting version 2 of wl_output will wait
> > >> for the wl_output.done event before starting wl_output-related
> > >> operations. However, if the server only supports version 1, no event
> > >> will ever come, and it must fallback to use the wl_output.geometry event
> > >> alone.
> > >> Without this macro, it cannot check for that in a nice way.
> > >>
> > >> Signed-off-by: Quentin Glidic
> > >> ---
> > >>
> > >> I do not have a real world use-case for the request macro on the
> > >> server-side,
> > >> but I guess you could do the same: wait the for a "commit" request if
> > >> client is
> > >> new enough, otherwise use some older request as commit.
> > >>
> > >> Actually I think there was something like that somewhere, now that I
> > >> write that,
> > >> but I do not remember where exactly.
> > >>
> > >>
> > >> src/scanner.c | 2 ++
> > >> 1 file changed, 2 insertions(+)
> > >>
> > >> diff --git a/src/scanner.c b/src/scanner.c
> > >> index 4708cae..a4c1984 100644
> > >> --- a/src/scanner.c
> > >> +++ b/src/scanner.c
> > >> @@ -1544,10 +1544,12 @@ emit_header(struct protocol *protocol, enum side
> > >> side)
> > >> emit_structs(&i->request_list, i, side);
> > >> emit_opcodes(&i->event_list, i);
> > >> emit_opcode_versions(&i->event_list, i);
> > >> +emit_opcode_versions(&i->request_list, i);
> > >> emit_event_wrappers(&i->event_list, i);
> > >> } else {
> > >> emit_structs(&i->event_list, i, side);
> > >> emit_opcodes(&i->request_list, i);
> > >> +emit_opcode_versions(&i->event_list, i);
> > >> emit_opcode_versions(&i->request_list, i);
> > >> emit_stubs(&i->request_list, i);
> > >> }
> > >
> > > Hi,
> > >
> > > I have just one question about this. Users must be able to include both
> > > server and client headers in the same compilation unit. Wouldn't this
> > > cause the same thing to be #defined in two different headers? More
> > > importantly, are we sure it won't cause problems?
> >
> > At least with GCC, you can have twice the same #define (same name + same
> > value) without issue. I do not know about the C standard take on that
> > though.
>
> Hi,
>
> I would need more proof/opinions than just a "it worked for me".
>
> > If that would cause problems, it would be because a request and an event
> > have the same name, and come from different versions. But if a user must
> > be able to include both client and server headers, it is already an issue.
>
> I think we can assume that client and server side should use the same
> version of the XML files.
>
> How is it already an issue? Do we already #define something in both
> headers?
>
> At least we already seem to have a test in the Wayland test suite that
> includes both client and server protocol headers in the same
> compilation unit.
>
> > If you really want, I can make server-side define events + prefixed
> > requests and the other way around.
> >
> > server.h:
> > #define NAMESPACE_INTERFACE_SOMEEV_SINCE_VERSION 2
> > #define NAMESPACE_INTERFACE_REQUEST_SOMEREQ_SINCE_VERSION 3
> > client.h:
> > #define NAMESPACE_INTERFACE_EVENT_SOMEEV_SINCE_VERSION 2
> > #define NAMESPACE_INTERFACE_SOMEREQ_SINCE_VERSION 3
>
> I'd like to avoid that naming hassle if possible. I just want to know
> if it is possible.
Hi all,
after talking to my colleagues and noting that no Microsoft
compiler would ever be used to compile this code, my worries have
cleared.
GCC has this:
https://gcc.gnu.org/onlinedocs/cpp/Undefining-and-Redefining-Macros.html
I assume Clang would be similar.
We can add duplicate #defines just fine. In the unlikely case that it
will blow up something, we can fix the generator to emit
#ifndef/#define/#endif instead of just #define.
How's that for a contingency plan?
The patch is
Reviewed-by: Pekka Paalanen
Thanks,
pq
pgpxx7UhMLqnN.pgp
Description: OpenPGP digital signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH wayland 2/2] scanner: Generate all SINCE_VERSION macros for everyone
On 10/08/2016 14:36, Pekka Paalanen wrote:
On Tue, 9 Aug 2016 11:05:49 +0300
Pekka Paalanen wrote:
On Mon, 8 Aug 2016 15:59:37 +0200
Quentin Glidic wrote:
On 08/08/2016 15:45, Pekka Paalanen wrote:
On Tue, 5 Jul 2016 20:41:50 +0200
Quentin Glidic wrote:
From: Quentin Glidic
Practical example: a client supporting version 2 of wl_output will wait
for the wl_output.done event before starting wl_output-related
operations. However, if the server only supports version 1, no event
will ever come, and it must fallback to use the wl_output.geometry event
alone.
Without this macro, it cannot check for that in a nice way.
Signed-off-by: Quentin Glidic
---
I do not have a real world use-case for the request macro on the server-side,
but I guess you could do the same: wait the for a "commit" request if client is
new enough, otherwise use some older request as commit.
Actually I think there was something like that somewhere, now that I write that,
but I do not remember where exactly.
src/scanner.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/scanner.c b/src/scanner.c
index 4708cae..a4c1984 100644
--- a/src/scanner.c
+++ b/src/scanner.c
@@ -1544,10 +1544,12 @@ emit_header(struct protocol *protocol, enum side side)
emit_structs(&i->request_list, i, side);
emit_opcodes(&i->event_list, i);
emit_opcode_versions(&i->event_list, i);
+ emit_opcode_versions(&i->request_list, i);
emit_event_wrappers(&i->event_list, i);
} else {
emit_structs(&i->event_list, i, side);
emit_opcodes(&i->request_list, i);
+ emit_opcode_versions(&i->event_list, i);
emit_opcode_versions(&i->request_list, i);
emit_stubs(&i->request_list, i);
}
Hi,
I have just one question about this. Users must be able to include both
server and client headers in the same compilation unit. Wouldn't this
cause the same thing to be #defined in two different headers? More
importantly, are we sure it won't cause problems?
At least with GCC, you can have twice the same #define (same name + same
value) without issue. I do not know about the C standard take on that
though.
Hi,
I would need more proof/opinions than just a "it worked for me".
If that would cause problems, it would be because a request and an event
have the same name, and come from different versions. But if a user must
be able to include both client and server headers, it is already an issue.
I think we can assume that client and server side should use the same
version of the XML files.
How is it already an issue? Do we already #define something in both
headers?
The issue shows up for this kind of protocol:
Currently, with this protocol, including both client and server headers
will fail, because one #define will be 3 and the other 2.
This patch will break even when including one header.
I guess this kind of protocol is just considered bad design? :-)
At least we already seem to have a test in the Wayland test suite that
includes both client and server protocol headers in the same
compilation unit.
If you really want, I can make server-side define events + prefixed
requests and the other way around.
server.h:
#define NAMESPACE_INTERFACE_SOMEEV_SINCE_VERSION 2
#define NAMESPACE_INTERFACE_REQUEST_SOMEREQ_SINCE_VERSION 3
client.h:
#define NAMESPACE_INTERFACE_EVENT_SOMEEV_SINCE_VERSION 2
#define NAMESPACE_INTERFACE_SOMEREQ_SINCE_VERSION 3
I'd like to avoid that naming hassle if possible. I just want to know
if it is possible.
Hi all,
after talking to my colleagues and noting that no Microsoft
compiler would ever be used to compile this code, my worries have
cleared.
GCC has this:
https://gcc.gnu.org/onlinedocs/cpp/Undefining-and-Redefining-Macros.html
I assume Clang would be similar.
We can add duplicate #defines just fine. In the unlikely case that it
will blow up something, we can fix the generator to emit
#ifndef/#define/#endif instead of just #define.
How's that for a contingency plan?
Good enough for me.
The patch is
Reviewed-by: Pekka Paalanen
Thanks,
--
Quentin “Sardem FF7” Glidic
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH wayland 2/2] scanner: Generate all SINCE_VERSION macros for everyone
On Wed, 10 Aug 2016 14:47:06 +0200
Quentin Glidic wrote:
> On 10/08/2016 14:36, Pekka Paalanen wrote:
> > On Tue, 9 Aug 2016 11:05:49 +0300
> > Pekka Paalanen wrote:
> >
> >> On Mon, 8 Aug 2016 15:59:37 +0200
> >> Quentin Glidic wrote:
> >>
> >>> On 08/08/2016 15:45, Pekka Paalanen wrote:
> On Tue, 5 Jul 2016 20:41:50 +0200
> Quentin Glidic wrote:
>
> > From: Quentin Glidic
> >
> > Practical example: a client supporting version 2 of wl_output will wait
> > for the wl_output.done event before starting wl_output-related
> > operations. However, if the server only supports version 1, no event
> > will ever come, and it must fallback to use the wl_output.geometry event
> > alone.
> > Without this macro, it cannot check for that in a nice way.
> >
> > Signed-off-by: Quentin Glidic
> > ---
> >
> > I do not have a real world use-case for the request macro on the
> > server-side,
> > but I guess you could do the same: wait the for a "commit" request if
> > client is
> > new enough, otherwise use some older request as commit.
> >
> > Actually I think there was something like that somewhere, now that I
> > write that,
> > but I do not remember where exactly.
> >
> >
> > src/scanner.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/src/scanner.c b/src/scanner.c
> > index 4708cae..a4c1984 100644
> > --- a/src/scanner.c
> > +++ b/src/scanner.c
> > @@ -1544,10 +1544,12 @@ emit_header(struct protocol *protocol, enum
> > side side)
> > emit_structs(&i->request_list, i, side);
> > emit_opcodes(&i->event_list, i);
> > emit_opcode_versions(&i->event_list, i);
> > + emit_opcode_versions(&i->request_list, i);
> > emit_event_wrappers(&i->event_list, i);
> > } else {
> > emit_structs(&i->event_list, i, side);
> > emit_opcodes(&i->request_list, i);
> > + emit_opcode_versions(&i->event_list, i);
> > emit_opcode_versions(&i->request_list, i);
> > emit_stubs(&i->request_list, i);
> > }
>
> Hi,
>
> I have just one question about this. Users must be able to include both
> server and client headers in the same compilation unit. Wouldn't this
> cause the same thing to be #defined in two different headers? More
> importantly, are we sure it won't cause problems?
> >>>
> >>> At least with GCC, you can have twice the same #define (same name + same
> >>> value) without issue. I do not know about the C standard take on that
> >>> though.
> >>
> >> Hi,
> >>
> >> I would need more proof/opinions than just a "it worked for me".
> >>
> >>> If that would cause problems, it would be because a request and an event
> >>> have the same name, and come from different versions. But if a user must
> >>> be able to include both client and server headers, it is already an
> >>> issue.
> >>
> >> I think we can assume that client and server side should use the same
> >> version of the XML files.
> >>
> >> How is it already an issue? Do we already #define something in both
> >> headers?
>
> The issue shows up for this kind of protocol:
>
>
>
> Currently, with this protocol, including both client and server headers
> will fail, because one #define will be 3 and the other 2.
> This patch will break even when including one header.
>
> I guess this kind of protocol is just considered bad design? :-)
Indeed. Are there any known cases of this?
If not, let's go ahead and see if we can get away with it.
Thanks,
pq
>
>
> >> At least we already seem to have a test in the Wayland test suite that
> >> includes both client and server protocol headers in the same
> >> compilation unit.
> >>
> >>> If you really want, I can make server-side define events + prefixed
> >>> requests and the other way around.
> >>>
> >>> server.h:
> >>> #define NAMESPACE_INTERFACE_SOMEEV_SINCE_VERSION 2
> >>> #define NAMESPACE_INTERFACE_REQUEST_SOMEREQ_SINCE_VERSION 3
> >>> client.h:
> >>> #define NAMESPACE_INTERFACE_EVENT_SOMEEV_SINCE_VERSION 2
> >>> #define NAMESPACE_INTERFACE_SOMEREQ_SINCE_VERSION 3
> >>
> >> I'd like to avoid that naming hassle if possible. I just want to know
> >> if it is possible.
> >
> > Hi all,
> >
> > after talking to my colleagues and noting that no Microsoft
> > compiler would ever be used to compile this code, my worries have
> > cleared.
> >
> > GCC has this:
> > https://gcc.gnu.org/onlinedocs/cpp/Undefining-and-Redefining-Macros.html
> >
> > I assume Clang would be similar.
> >
> > We can add duplicate #defines just fine. In the unlikely case that it
> > will blow up something, we can fix the generator to emit
Re: [PATCH 5/7] fullscreen-shell: Add helpers for managing surfaces on zero outputs
On Fri, 29 Jul 2016 13:26:26 +0200
Armin Krezović wrote:
> This adds helper functions for managing fullscreen surfaces outside
> of fullscreen outputs.
>
> Signed-off-by: Armin Krezović
> ---
> fullscreen-shell/fullscreen-shell.c | 55
> +
> 1 file changed, 55 insertions(+)
>
> diff --git a/fullscreen-shell/fullscreen-shell.c
> b/fullscreen-shell/fullscreen-shell.c
> index 2ec2d02..3dbd0d9 100644
> --- a/fullscreen-shell/fullscreen-shell.c
> +++ b/fullscreen-shell/fullscreen-shell.c
> @@ -46,6 +46,16 @@ struct fullscreen_shell {
> struct wl_listener output_created_listener;
>
> struct wl_listener seat_created_listener;
> +
> + /* List of surfaces that need to be mapped after an output
> + * gets created.
> + *
> + * At the moment, only one surface can be created on an output.
> + *
> + * This is implemented as a list in case someone fixes the shell
> + * implementation to support more than one client.
> + */
> + struct wl_list unmapped_surfaces; /* struct fs_client_surface::link */
> };
>
> struct fs_output {
> @@ -83,6 +93,50 @@ struct pointer_focus_listener {
> struct wl_listener seat_destroyed;
> };
>
> +struct fs_client_surface {
> + struct weston_surface *surface;
> + enum zwp_fullscreen_shell_v1_present_method method;
> + struct wl_list link; /* struct fullscreen_shell::unmapped_surfaces */
> + struct wl_listener surface_destroyed;
> +};
> +
> +static void
> +remove_unmapped_surface(struct fs_client_surface *surf)
> +{
> + wl_list_remove(&surf->surface_destroyed.link);
> + wl_list_remove(&surf->link);
> + free(surf);
> +}
> +
> +static void
> +unmapped_surface_destroy_listener(struct wl_listener *listener, void *data)
> +{
> + struct fs_client_surface *surf;
> +
> + surf = container_of(listener, struct fs_client_surface,
> surface_destroyed);
> +
> + remove_unmapped_surface(surf);
> +}
> +
> +static void
> +add_unmapped_surface(struct fullscreen_shell *shell, struct weston_surface
> *surface,
> + enum zwp_fullscreen_shell_v1_present_method method)
> +{
> + struct fs_client_surface *surf;
> +
> + surf = zalloc(sizeof *surf);
> + if (!surf)
> + return;
> +
> + surf->surface = surface;
> + surf->method = method;
> +
> + wl_list_insert(shell->unmapped_surfaces.prev, &surf->link);
> +
> + surf->surface_destroyed.notify = unmapped_surface_destroy_listener;
> + wl_signal_add(&surface->destroy_signal, &surf->surface_destroyed);
> +}
> +
> static void
> pointer_focus_changed(struct wl_listener *listener, void *data)
> {
> @@ -831,6 +885,7 @@ module_init(struct weston_compositor *compositor,
> return -1;
>
> shell->compositor = compositor;
> + wl_list_init(&shell->unmapped_surfaces);
>
> shell->client_destroyed.notify = client_destroyed;
>
Hi Armin,
the code looks correct in this patch, but its purpose is impossible to
review by this patch alone, as there are no users of this code. The
commit message is terse, and the code comments don't quite match my
recollection on what this might have been needed for.
More comments in the review of the following patch.
Thanks,
pq
pgp9h7rOyNT7K.pgp
Description: OpenPGP digital signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston] libweston: Add move (without scale) animation
From: Quentin Glidic
Signed-off-by: Quentin Glidic
---
libweston/animation.c | 35 +--
libweston/compositor.h | 7 ++-
2 files changed, 35 insertions(+), 7 deletions(-)
diff --git a/libweston/animation.c b/libweston/animation.c
index 30b3e5d..75da957 100644
--- a/libweston/animation.c
+++ b/libweston/animation.c
@@ -418,7 +418,8 @@ weston_slide_run(struct weston_view *view, float start,
float stop,
struct weston_move_animation {
int dx;
int dy;
- int reverse;
+ bool reverse;
+ bool scale;
weston_view_animation_done_func_t done;
};
@@ -436,7 +437,9 @@ move_frame(struct weston_view_animation *animation)
(animation->stop - animation->start) *
progress;
weston_matrix_init(&animation->transform.matrix);
- weston_matrix_scale(&animation->transform.matrix, scale, scale, 1.0f);
+ if (move->scale)
+ weston_matrix_scale(&animation->transform.matrix, scale, scale,
+ 1.0f);
weston_matrix_translate(&animation->transform.matrix,
move->dx * progress, move->dy * progress,
0);
@@ -453,10 +456,11 @@ move_done(struct weston_view_animation *animation, void
*data)
free(move);
}
-WL_EXPORT struct weston_view_animation *
-weston_move_scale_run(struct weston_view *view, int dx, int dy,
- float start, float end, int reverse,
- weston_view_animation_done_func_t done, void *data)
+static struct weston_view_animation *
+weston_move_scale_run_internal(struct weston_view *view, int dx, int dy,
+ float start, float end, bool reverse, bool scale,
+ weston_view_animation_done_func_t done,
+ void *data)
{
struct weston_move_animation *move;
struct weston_view_animation *animation;
@@ -467,6 +471,7 @@ weston_move_scale_run(struct weston_view *view, int dx, int
dy,
move->dx = dx;
move->dy = dy;
move->reverse = reverse;
+ move->scale = scale;
move->done = done;
animation = weston_view_animation_create(view, start, end, move_frame,
@@ -484,3 +489,21 @@ weston_move_scale_run(struct weston_view *view, int dx,
int dy,
return animation;
}
+
+WL_EXPORT struct weston_view_animation *
+weston_move_scale_run(struct weston_view *view, int dx, int dy,
+ float start, float end, bool reverse,
+ weston_view_animation_done_func_t done, void *data)
+{
+ return weston_move_scale_run_internal(view, dx, dy, start, end, reverse,
+ true, done, data);
+}
+
+WL_EXPORT struct weston_view_animation *
+weston_move_run(struct weston_view *view, int dx, int dy,
+ float start, float end, bool reverse,
+ weston_view_animation_done_func_t done, void *data)
+{
+ return weston_move_scale_run_internal(view, dx, dy, start, end, reverse,
+ false, done, data);
+}
diff --git a/libweston/compositor.h b/libweston/compositor.h
index 0133084..b17c71c 100644
--- a/libweston/compositor.h
+++ b/libweston/compositor.h
@@ -1702,9 +1702,14 @@ weston_fade_run(struct weston_view *view,
struct weston_view_animation *
weston_move_scale_run(struct weston_view *view, int dx, int dy,
- float start, float end, int reverse,
+ float start, float end, bool reverse,
weston_view_animation_done_func_t done, void *data);
+struct weston_view_animation *
+weston_move_run(struct weston_view *view, int dx, int dy,
+ float start, float end, bool reverse,
+ weston_view_animation_done_func_t done, void *data);
+
void
weston_fade_update(struct weston_view_animation *fade, float target);
--
2.9.2
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH 7/7] fullscreen-shell: Keep fullscreen surface after all outputs are gone
On Fri, 29 Jul 2016 13:26:28 +0200
Armin Krezović wrote:
> Currently, when all outputs are gone (all views are gone), a surface
> is unmapped and destroyed, even though the client is still running.
>
> This utilizes the previously added code to remap the surface when a
> new output gets attached.
>
> Signed-off-by: Armin Krezović
> ---
> fullscreen-shell/fullscreen-shell.c | 12 +++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/fullscreen-shell/fullscreen-shell.c
> b/fullscreen-shell/fullscreen-shell.c
> index 71d609c..42b1662 100644
> --- a/fullscreen-shell/fullscreen-shell.c
> +++ b/fullscreen-shell/fullscreen-shell.c
> @@ -262,11 +262,21 @@ fs_output_clear_pending(struct fs_output *fsout);
> static void
> fs_output_destroy(struct fs_output *fsout)
> {
> - fs_output_set_surface(fsout, NULL, 0, 0, 0);
> fs_output_clear_pending(fsout);
>
> wl_list_remove(&fsout->link);
>
> + if (fsout->view) {
> + weston_view_destroy(fsout->view);
> + fsout->view = NULL;
> + }
> +
> + if (fsout->surface && wl_list_empty(&fsout->shell->output_list)) {
> + add_unmapped_surface(fsout->shell, fsout->surface,
> + fsout->method);
> + fsout->surface = NULL;
> + }
> +
> if (fsout->output)
> wl_list_remove(&fsout->output_destroyed.link);
> }
Hi Armin,
if you implement the things I talked about in patch 6's review, this
patch will not be necessary at all. The surface presented for the NULL
output will just stay in the list, so there is not need to put it
back.
Thanks,
pq
pgpcDzIrgUWU2.pgp
Description: OpenPGP digital signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH 6/7] fullscreen-shell: Remap surfaces when attaching a primary output
On Fri, 29 Jul 2016 13:26:27 +0200
Armin Krezović wrote:
> When no outputs are present, and no output resource was given,
> a fullscreen surface won't get configured because the default
> behavior when there is no output resource given is to create
> the same surface on all fullscreen outputs.
>
> This patch makes the shell reconfigure the surface as soon
> as an output gets plugged in and configured, so it can properly
> be displayed if it was started when no outputs were present.
>
> Signed-off-by: Armin Krezović
> ---
> fullscreen-shell/fullscreen-shell.c | 14 ++
> 1 file changed, 14 insertions(+)
Hi Armin,
the previous patch should have been squashed with this patch. The
review comments here are partially for the previous patch too.
>
> diff --git a/fullscreen-shell/fullscreen-shell.c
> b/fullscreen-shell/fullscreen-shell.c
> index 3dbd0d9..71d609c 100644
> --- a/fullscreen-shell/fullscreen-shell.c
> +++ b/fullscreen-shell/fullscreen-shell.c
> @@ -299,10 +299,15 @@ pending_surface_destroyed(struct wl_listener *listener,
> void *data)
> fsout->pending.surface = NULL;
> }
>
> +static void
> +configure_presented_surface(struct weston_surface *surface, int32_t sx,
> + int32_t sy);
> +
> static struct fs_output *
> fs_output_create(struct fullscreen_shell *shell, struct weston_output
> *output)
> {
> struct fs_output *fsout;
> + struct fs_client_surface *surf, *next;
>
> fsout = zalloc(sizeof *fsout);
> if (!fsout)
> @@ -325,6 +330,13 @@ fs_output_create(struct fullscreen_shell *shell, struct
> weston_output *output)
> weston_layer_entry_insert(&shell->layer.view_list,
> &fsout->black_view->layer_link);
> wl_list_init(&fsout->transform.link);
> +
> + wl_list_for_each_safe(surf, next, &shell->unmapped_surfaces, link) {
> + fs_output_set_surface(fsout, surf->surface, surf->method, 0, 0);
> + configure_presented_surface(surf->surface, 0, 0);
> + remove_unmapped_surface(surf);
> + }
I think this hunk needs an XXX comment too. Right now the code says:
when a new output is created, assing all not yet mapped surfaces to it.
It doesn't make sense because an output can only have one surface at a
time. The comment should explain why the code is saying strange things.
Or better, fix the code to make sense, e.g. search the list for one
particular surface intended for this particular output.
Hmm, no, that does not make sense either. There cannot be a surface
dedicated for this output, because the output has just been created.
The client has not had a chance to create and assign a surface.
So, this is to handle the surfaces assigned to no-particular-output
instead? For which there can be only one, as there can be only one
client.
In that case, the list should be documented to contain the surfaces
that were assigned to the NULL-output, i.e. no-particular-output. It's
not a list of just any unmapped surfaces, it is a list of surfaces
presented to no-particular-output. Also the list name should reflect
that.
Then I think it would make better sense, but this hunk of code still
needs the XXX comment.
The remove_unmapped_surface() call does not seem right either. The
surface should stay in the list until another surface is presented for
output NULL. If you look at...
> +
> return fsout;
> }
>
> @@ -750,6 +762,8 @@ fullscreen_shell_present_surface(struct wl_client *client,
> output = wl_resource_get_user_data(output_res);
> fsout = fs_output_for_output(output);
> fs_output_set_surface(fsout, surface, method, 0, 0);
> + } else if (wl_list_empty(&shell->output_list)) {
> + add_unmapped_surface(shell, surface, method);
> } else {
> wl_list_for_each(fsout, &shell->output_list, link)
> fs_output_set_surface(fsout, surface, method, 0, 0);
...this wl_list_for_each() loop, it seems it should be possible to set
the same surface to several outputs. Keeping the surface in the
"presented for the NULL output" list would allow additional outputs to
get a clone of the surface later too.
The condition on when to call add_unmapped_surface() looks wrong. It
should be called if and only if output_res is NULL.
Whether the output list is currently empty or not does not actually
matter at all. A surface presented for NULL output is what it is, and
should be kept in the surface list until replaced or destroyed. Every
time a new output gets plugged in, the surface list needs to be
processed so that the surface gets cloned on the new output.
I think the code you wrote works for the single output case, but it
does not generalize for more outputs, nor it handles replacing the
surface presented for the NULL output.
Thanks,
pq
pgpRS9mi9p2Rb.pgp
Description: OpenPGP digital signature
___
wayland-devel mailing list
wayland-devel@li
Re: [PATCH weston v3 2/6] Add API to retrieve the interface name of a wl_resource
On Aug 9, 2016, at 11:46 PM, Giulio Camuffo wrote:
>
> 2016-08-10 0:48 GMT+02:00 Yong Bakos :
>> Hi Giulio,
>> I missed these Wayland patches due to the weston subject
>> line, so forgive me for the late feedback. Suggestion inline
>> below.
>
> Oh indeed, sorry about that.
>
>>
>>> On Aug 9, 2016, at 3:46 AM, Giulio Camuffo wrote:
>>>
>>> Signed-off-by: Giulio Camuffo
>>> Reviewed-by: Jonas Ådahl
>>> ---
>>> src/wayland-server-core.h | 2 ++
>>> src/wayland-server.c | 12
>>> 2 files changed, 14 insertions(+)
>>>
>>> diff --git a/src/wayland-server-core.h b/src/wayland-server-core.h
>>> index 43e76fb..c0d25e9 100644
>>> --- a/src/wayland-server-core.h
>>> +++ b/src/wayland-server-core.h
>>> @@ -427,6 +427,8 @@ int
>>> wl_resource_instance_of(struct wl_resource *resource,
>>> const struct wl_interface *interface,
>>> const void *implementation);
>>> +const char *
>>> +wl_resource_get_class(struct wl_resource *resource);
>>
>> Why isn't this called wl_resource_get_interface_name,
>> since that is exactly what this function returns?
>
> Because we have wl_proxy_get_class(), and this is the same for wl_resource.
Ah, of course. Thank you.
Reviewed-by: Yong Bakos
yong
>>
>> Regards,
>> yong
>>
>>
>>> void
>>> wl_resource_add_destroy_listener(struct wl_resource *resource,
>>> diff --git a/src/wayland-server.c b/src/wayland-server.c
>>> index b44ec9c..e2212e2 100644
>>> --- a/src/wayland-server.c
>>> +++ b/src/wayland-server.c
>>> @@ -690,6 +690,18 @@ wl_resource_get_destroy_listener(struct wl_resource
>>> *resource,
>>> return wl_signal_get(&resource->destroy_signal, notify);
>>> }
>>>
>>> +/** Retrieve the interface name (class) of a resource object.
>>> + *
>>> + * \param resource The resource object
>>> + *
>>> + * \memberof wl_resource
>>> + */
>>> +WL_EXPORT const char *
>>> +wl_resource_get_class(struct wl_resource *resource)
>>> +{
>>> + return resource->object.interface->name;
>>> +}
>>> +
>>> WL_EXPORT void
>>> wl_client_add_destroy_listener(struct wl_client *client,
>>> struct wl_listener *listener)
>>> --
>>> 2.9.2
>>>
>>> ___
>>> wayland-devel mailing list
>>> wayland-devel@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/wayland-devel
>>
> ___
> wayland-devel mailing list
> wayland-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/wayland-devel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston v3 6/6] Add API to install protocol loggers on the server wl_display
Hi Giulio,
> On Aug 9, 2016, at 3:46 AM, Giulio Camuffo wrote:
>
> The new wl_display_add_protocol_logger allows to set a function as
> a logger, which will get called when a new request is received or an
> event is sent.
> This is akin to setting WAYLAND_DEBUG=1, but more powerful because it
> can be enabled at run time and allows to show the log e.g. in a UI view.
> A test is added for the new functionality.
>
> Signed-off-by: Giulio Camuffo
If this series goes to v4 I have some minor suggestions inline below,
including the test toward the end.
Otherwise, this is:
Reviewed-by: Yong Bakos
> ---
>
> v3: - added missing test file
>- renamed wl_protocol_logger_direction to wl_protocol_logger_type
>- renamed closure_log to log_closure
>- renamed wl_display_remove_protocol_logger to wl_protocol_logger_destroy
>- improved documentation
>
> Makefile.am | 5 +-
> src/wayland-server-core.h| 24 +++
> src/wayland-server.c | 103 --
> tests/protocol-logger-test.c | 146 +++
> 4 files changed, 271 insertions(+), 7 deletions(-)
> create mode 100644 tests/protocol-logger-test.c
>
> diff --git a/Makefile.am b/Makefile.am
> index e684a87..3eb6fd5 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -161,7 +161,8 @@ TESTS = \
> resources-test \
> message-test\
> headers-test\
> - compositor-introspection-test
> + compositor-introspection-test \
> + protocol-logger-test
>
> if ENABLE_CPP_TEST
> TESTS += cpp-compile-test
> @@ -220,6 +221,8 @@ message_test_SOURCES = tests/message-test.c
> message_test_LDADD = libtest-runner.la
> compositor_introspection_test_SOURCES = tests/compositor-introspection-test.c
> compositor_introspection_test_LDADD = libtest-runner.la
> +protocol_logger_test_SOURCES = tests/protocol-logger-test.c
> +protocol_logger_test_LDADD = libtest-runner.la
> headers_test_SOURCES = tests/headers-test.c \
> tests/headers-protocol-test.c \
> tests/headers-protocol-core-test.c
> diff --git a/src/wayland-server-core.h b/src/wayland-server-core.h
> index 56e8d80..8e118dc 100644
> --- a/src/wayland-server-core.h
> +++ b/src/wayland-server-core.h
> @@ -522,6 +522,30 @@ wl_shm_buffer_create(struct wl_client *client,
> void
> wl_log_set_handler_server(wl_log_func_t handler);
>
> +enum wl_protocol_logger_type {
> + WL_PROTOCOL_LOGGER_REQUEST,
> + WL_PROTOCOL_LOGGER_EVENT,
> +};
> +
> +struct wl_protocol_logger_message {
> + struct wl_resource *resource;
> + int message_opcode;
> + const struct wl_message *message;
> + int arguments_count;
> + const union wl_argument *arguments;
> +};
> +
> +typedef void (*wl_protocol_logger_func_t)(void *user_data,
> + enum wl_protocol_logger_type
> direction,
> + const struct
> wl_protocol_logger_message *message);
> +struct wl_protocol_logger;
Needs line breaks, or better yet just remove the forward declaration.
It's not needed since you're getting that for free with the return
type in the next line.
> +struct wl_protocol_logger *
> +wl_display_add_protocol_logger(struct wl_display *display,
> +wl_protocol_logger_func_t, void *user_data);
> +
> +void
> +wl_protocol_logger_destroy(struct wl_protocol_logger *logger);
> +
> #ifdef __cplusplus
> }
> #endif
> diff --git a/src/wayland-server.c b/src/wayland-server.c
> index fb44f13..2113cd3 100644
> --- a/src/wayland-server.c
> +++ b/src/wayland-server.c
> @@ -95,6 +95,7 @@ struct wl_display {
> struct wl_list global_list;
> struct wl_list socket_list;
> struct wl_list client_list;
> + struct wl_list protocol_loggers;
>
> struct wl_signal destroy_signal;
> struct wl_signal create_client_signal;
> @@ -123,8 +124,42 @@ struct wl_resource {
> wl_dispatcher_func_t dispatcher;
> };
>
> +struct wl_protocol_logger {
> + struct wl_list link;
> + wl_protocol_logger_func_t func;
> + void *user_data;
> +};
> +
> static int debug_server = 0;
>
Would be nice to see a comment header here, but we can add
after the merge.
> +static void
> +log_closure(struct wl_resource *resource,
> + struct wl_closure *closure, int send)
> +{
> + struct wl_object *object = &resource->object;
> + struct wl_display *display = resource->client->display;
> + struct wl_protocol_logger *protocol_logger;
> + struct wl_protocol_logger_message message;
> +
> + if (debug_server)
> + wl_closure_print(closure, object, send);
Are you sure it makes sense to couple these?
> +
> + if (!wl_list_empty(&display->protocol_loggers)) {
> + message.resource = resource;
> + message.message_opcode = closur
[PATCH weston v4 2/3] compositor: Implement idle inhibition
Signed-off-by: Bryce Harrington
---
Makefile.am| 4 ++-
configure.ac | 2 ++
libweston/compositor.c | 88 ++
3 files changed, 93 insertions(+), 1 deletion(-)
diff --git a/Makefile.am b/Makefile.am
index 32627f5..951f9ed 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -139,7 +139,9 @@ nodist_libweston_@LIBWESTON_MAJOR@_la_SOURCES =
\
protocol/relative-pointer-unstable-v1-protocol.c\
protocol/relative-pointer-unstable-v1-server-protocol.h \
protocol/pointer-constraints-unstable-v1-protocol.c \
- protocol/pointer-constraints-unstable-v1-server-protocol.h
+ protocol/pointer-constraints-unstable-v1-server-protocol.h \
+ protocol/idle-inhibit-unstable-v1-protocol.c\
+ protocol/idle-inhibit-unstable-v1-server-protocol.h
BUILT_SOURCES += $(nodist_libweston_@LIBWESTON_MAJOR@_la_SOURCES)
diff --git a/configure.ac b/configure.ac
index 74f931d..de26599 100644
--- a/configure.ac
+++ b/configure.ac
@@ -7,6 +7,8 @@ m4_define([libweston_major_version], [0])
m4_define([libweston_minor_version], [0])
m4_define([libweston_patch_version], [0])
+# Note: Inhibition patchset requires inhibition protocol in wayland-protocol
+
AC_PREREQ([2.64])
AC_INIT([weston],
[weston_version],
diff --git a/libweston/compositor.c b/libweston/compositor.c
index aa14504..e4ce273 100644
--- a/libweston/compositor.c
+++ b/libweston/compositor.c
@@ -51,6 +51,8 @@
#include
#include
+#include
+
#include "timeline.h"
#include "compositor.h"
@@ -4665,6 +4667,88 @@ compositor_bind(struct wl_client *client,
compositor, NULL);
}
+struct weston_idle_inhibitor {
+ struct weston_surface *surface;
+};
+
+static void
+destroy_idle_inhibitor(struct wl_resource *resource)
+{
+ struct weston_idle_inhibitor *inhibitor =
wl_resource_get_user_data(resource);
+
+ inhibitor->surface = NULL;
+ free(inhibitor);
+}
+
+static void
+idle_inhibitor_destroy(struct wl_client *client, struct wl_resource *resource)
+{
+ struct weston_idle_inhibitor *inhibitor =
wl_resource_get_user_data(resource);
+
+ assert(inhibitor);
+
+ inhibitor->surface->inhibit_idling = false;
+}
+
+static const struct zwp_idle_inhibitor_v1_interface idle_inhibitor_interface =
{
+ idle_inhibitor_destroy
+};
+
+static void
+idle_inhibit_manager_destroy(struct wl_client *client, struct wl_resource
*resource)
+{
+}
+
+static void
+idle_inhibit_manager_create_inhibitor(struct wl_client *client, struct
wl_resource *resource,
+ uint32_t id, struct wl_resource
*surface_resource)
+{
+ struct weston_surface *surface =
wl_resource_get_user_data(surface_resource);
+ struct weston_idle_inhibitor *inhibitor;
+ struct wl_resource *cr;
+
+ cr = wl_resource_create(client, &zwp_idle_inhibitor_v1_interface,
+ wl_resource_get_version(resource), id);
+ if (cr == NULL) {
+ wl_client_post_no_memory(client);
+ return;
+ }
+
+ inhibitor = zalloc(sizeof *inhibitor);
+ if (inhibitor == NULL) {
+ wl_client_post_no_memory(client);
+ return;
+ }
+
+ inhibitor->surface = surface;
+ inhibitor->surface->inhibit_idling = true;
+
+ wl_resource_set_implementation(cr, &idle_inhibitor_interface,
+ inhibitor, destroy_idle_inhibitor);
+}
+
+static const struct zwp_idle_inhibit_manager_v1_interface
idle_inhibit_manager_interface = {
+ idle_inhibit_manager_destroy,
+ idle_inhibit_manager_create_inhibitor
+};
+
+static void
+bind_idle_inhibit_manager(struct wl_client *client,
+ void *data, uint32_t version, uint32_t id)
+{
+ struct wl_resource *resource;
+
+ resource = wl_resource_create(client,
&zwp_idle_inhibit_manager_v1_interface,
+ version, id);
+ if (resource == NULL) {
+ wl_client_post_no_memory(client);
+ return;
+ }
+
+ wl_resource_set_implementation(resource,
&idle_inhibit_manager_interface,
+ NULL, NULL);
+}
+
WL_EXPORT int
weston_environment_get_fd(const char *env)
{
@@ -4760,6 +4844,10 @@ weston_compositor_create(struct wl_display *display,
void *user_data)
if (weston_input_init(ec) != 0)
goto fail;
+ if (!wl_global_create(ec->wl_display,
&zwp_idle_inhibit_manager_v1_interface, 1,
+ ec, bind_idle_inhibit_manager))
+ goto fail;
+
wl_list_init(&ec->view_list);
wl_list_init(&ec->plane_list);
wl_list_init(&ec->layer_list);
--
1.9.1
___
wayland-devel mailing list
wayland-devel@lists.
[PATCH weston v4 1/3] compositor: If the output is inhibited, don't idle it off
Adds a helper routine weston_output_inhibited_outputs() which returns a
mask of outputs that should inhibit screen idling.
Use this routine to check for inhibiting outputs for handling of idle
behaviors in core: In sleep mode, only halt repainting outputs that
don't have valid inhibits. Don't send these monitors DPMS off commands
either, if the system would otherwise be powering them down.
Signed-off-by: Bryce Harrington
---
libweston/compositor.c | 57 --
libweston/compositor.h | 10 +
2 files changed, 61 insertions(+), 6 deletions(-)
diff --git a/libweston/compositor.c b/libweston/compositor.c
index b17c76d..aa14504 100644
--- a/libweston/compositor.c
+++ b/libweston/compositor.c
@@ -485,6 +485,8 @@ weston_surface_create(struct weston_compositor *compositor)
wl_list_init(&surface->pointer_constraints);
+ surface->inhibit_idling = false;
+
return surface;
}
@@ -2320,15 +2322,42 @@ weston_output_schedule_repaint_reset(struct
weston_output *output)
TL_POINT("core_repaint_exit_loop", TLP_OUTPUT(output), TLP_END);
}
+/** Retrieves a mask of outputs that should inhibit screensaving
+ *
+ * \param compositor The compositor instance.
+ * \return An output mask indicating the ids of all inhibiting outputs
+ *
+ * Checks for surfaces whose clients have requested that they
+ * disable the screenserver and display powersaving. Note
+ * the output ids for these surfaces.
+ */
+WL_EXPORT uint32_t
+weston_compositor_inhibited_outputs(struct weston_compositor *compositor)
+{
+ struct weston_view *view;
+ uint32_t inhibited_outputs_mask = 0;
+
+ wl_list_for_each(view, &compositor->view_list, link) {
+ /* Does the view's surface inhibit this output? */
+ if (!view->surface->inhibit_idling)
+ continue;
+
+ inhibited_outputs_mask |= view->output_mask;
+ }
+ return inhibited_outputs_mask;
+}
+
static int
output_repaint_timer_handler(void *data)
{
struct weston_output *output = data;
struct weston_compositor *compositor = output->compositor;
+ uint32_t inhibited_outputs_mask =
weston_compositor_inhibited_outputs(compositor);
if (output->repaint_needed &&
- compositor->state != WESTON_COMPOSITOR_SLEEPING &&
compositor->state != WESTON_COMPOSITOR_OFFSCREEN &&
+ (compositor->state != WESTON_COMPOSITOR_SLEEPING
+|| inhibited_outputs_mask & (1 << output->id)) &&
weston_output_repaint(output) == 0)
return 0;
@@ -2450,9 +2479,15 @@ weston_output_schedule_repaint(struct weston_output
*output)
{
struct weston_compositor *compositor = output->compositor;
struct wl_event_loop *loop;
+ uint32_t inhibited_outputs_mask =
weston_compositor_inhibited_outputs(compositor);
- if (compositor->state == WESTON_COMPOSITOR_SLEEPING ||
- compositor->state == WESTON_COMPOSITOR_OFFSCREEN)
+ /* If we're offscreen, or if we're sleeping and the monitor
+* isn't currently being inhibited by an active surface, then
+* skip repainting.
+*/
+ if (compositor->state == WESTON_COMPOSITOR_OFFSCREEN ||
+ (compositor->state == WESTON_COMPOSITOR_SLEEPING &&
+!(inhibited_outputs_mask & (1 << output->id
return;
if (!output->repaint_needed)
@@ -3859,10 +3894,20 @@ weston_compositor_dpms(struct weston_compositor
*compositor,
enum dpms_enum state)
{
struct weston_output *output;
+ struct weston_view *view;
+ uint32_t inhibited_outputs_mask =
weston_compositor_inhibited_outputs(compositor);
-wl_list_for_each(output, &compositor->output_list, link)
- if (output->set_dpms)
- output->set_dpms(output, state);
+ wl_list_for_each(output, &compositor->output_list, link) {
+ if (!output->set_dpms)
+ continue;
+
+ /* If output is idle-inhibited, don't toggle to any DPMS state
except ON. */
+ if (state != WESTON_DPMS_ON &&
+ inhibited_outputs_mask & (1 << output->id))
+ continue;
+
+ output->set_dpms(output, state);
+ }
}
/** Restores the compositor to active status
diff --git a/libweston/compositor.h b/libweston/compositor.h
index 0133084..3a9a098 100644
--- a/libweston/compositor.h
+++ b/libweston/compositor.h
@@ -1140,6 +1140,14 @@ struct weston_surface {
/* An list of per seat pointer constraints. */
struct wl_list pointer_constraints;
+
+ /*
+* Indicates the surface prefers no screenblanking, screensaving,
+* or other automatic obscurement to kick in while the surface is
+* considered "active" by the shell.
+*/
+ bool inhibit_idling;
+
};
struct weston_s
[PATCH weston v4 3/3] desktop-shell: Enable per-output fade animations
Instead of creating a single global fade surface across all outputs,
create a separate surface for each output. This lets us individually
fade each output (or block fading if the output has an inhibiting
surface).
When there are client surfaces with valid idle inhibits present, fade
out only the outputs those surfaces are not visible on. Then fade those
out too, if the client terminates for some reason while the system is in
sleep/idle/offscreen mode.
This also fixes a potential issue if on multihead layout spanning a
desktop wider than 8096 (or higher than 8096), the fade animation may
not completely cover all surfaces.
This assumes the output geometry doesn't change larger during the course
of the fade animation.
Signed-off-by: Bryce Harrington
---
desktop-shell/shell.c | 167 ++
desktop-shell/shell.h | 14 ++---
2 files changed, 109 insertions(+), 72 deletions(-)
diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
index 64979cd..0aae142 100644
--- a/desktop-shell/shell.c
+++ b/desktop-shell/shell.c
@@ -3606,10 +3606,23 @@ destroy_shell_surface(struct shell_surface *shsurf)
}
static void
+shell_fade(struct desktop_shell *shell, enum fade_type type);
+
+static void
shell_destroy_shell_surface(struct wl_resource *resource)
{
struct shell_surface *shsurf = wl_resource_get_user_data(resource);
+ /* Re-queue the fade animation to be evaluated if client had been
inhibiting */
+ if (shsurf->surface->inhibit_idling &&
+ (shsurf->shell->compositor->state == WESTON_COMPOSITOR_IDLE
+|| shsurf->shell->compositor->state == WESTON_COMPOSITOR_OFFSCREEN
+|| shsurf->shell->compositor->state == WESTON_COMPOSITOR_SLEEPING))
+ {
+ shsurf->surface->inhibit_idling = false;
+ shell_fade(shsurf->shell, FADE_OUT);
+ }
+
if (!wl_list_empty(&shsurf->popup.grab_link))
remove_popup_grab(shsurf);
if (shsurf->resource)
@@ -4356,9 +4369,6 @@ xdg_shell_unversioned_dispatch(const void *implementation,
/***/
static void
-shell_fade(struct desktop_shell *shell, enum fade_type type);
-
-static void
configure_static_view(struct weston_view *ev, struct weston_layer *layer)
{
struct weston_view *v, *next;
@@ -5356,24 +5366,26 @@ static void
shell_fade_done(struct weston_view_animation *animation, void *data)
{
struct desktop_shell *shell = data;
+ struct shell_output *shell_output;
- shell->fade.animation = NULL;
-
- switch (shell->fade.type) {
- case FADE_IN:
- weston_surface_destroy(shell->fade.view->surface);
- shell->fade.view = NULL;
- break;
- case FADE_OUT:
- lock(shell);
- break;
- default:
- break;
+ wl_list_for_each(shell_output, &shell->output_list, link) {
+ shell_output->fade.animation = NULL;
+ switch (shell_output->fade.type) {
+ case FADE_IN:
+
weston_surface_destroy(shell_output->fade.view->surface);
+ shell_output->fade.view = NULL;
+ break;
+ case FADE_OUT:
+ lock(shell);
+ break;
+ default:
+ break;
+ }
}
}
static struct weston_view *
-shell_fade_create_surface(struct desktop_shell *shell)
+shell_fade_create_surface_for_output(struct desktop_shell *shell, struct
shell_output *shell_output)
{
struct weston_compositor *compositor = shell->compositor;
struct weston_surface *surface;
@@ -5389,8 +5401,8 @@ shell_fade_create_surface(struct desktop_shell *shell)
return NULL;
}
- weston_surface_set_size(surface, 8192, 8192);
- weston_view_set_position(view, 0, 0);
+ weston_surface_set_size(surface, shell_output->output->width,
shell_output->output->height);
+ weston_view_set_position(view, shell_output->output->x,
shell_output->output->y);
weston_surface_set_color(surface, 0.0, 0.0, 0.0, 1.0);
weston_layer_entry_insert(&compositor->fade_layer.view_list,
&view->layer_link);
@@ -5405,6 +5417,8 @@ static void
shell_fade(struct desktop_shell *shell, enum fade_type type)
{
float tint;
+ struct shell_output *shell_output;
+ uint32_t inhibit_mask =
weston_compositor_inhibited_outputs(shell->compositor);
switch (type) {
case FADE_IN:
@@ -5414,36 +5428,41 @@ shell_fade(struct desktop_shell *shell, enum fade_type
type)
tint = 1.0;
break;
default:
- weston_log("shell: invalid fade type\n");
return;
}
- shell->fade.type = type;
+ /* Create a separate fade surface for each output */
+ wl_
