[ANNOUNCE] libinput 1.3.901
The first RC for libinput 1.4 is now available. The main feature merged in this cycle is mode support for graphics tablets. On many tablets, buttons, rings and strips can be used in virtualised modes, reflected by physical LEDs on the touchpad. For example, a ring may send scroll events when in mode 0 but zoom events in mode 1. A broader explanation of this feature is available here: http://who-t.blogspot.com.au/2016/07/libinput-and-graphics-tablet-mode.html Note that the backend implementation of pad modes is effectively nonexistent. We are currently working on the kernel patches to clean up the LED system for Wacom tablets, until this is in place all tablets will simply feature a single mode and group. We now support rotation on some relative devices. This is particularly useful for trackballs that may be permanently placed at a nonzero rotation angle. See libinput_device_config_rotation_set_angle() in the libinput documentation for more info. A feature that has found its way into the 1.3.x stable series is the re-introduction of touchpad hysteresis handling to stop pointer wobbles. Too many devices do not handle input coordinates reliably enough and playing whack-a-model with each laptop model was simply not good enough. The mouse and lens cursor tools on tablets are now accelerated like a normal 1000dpi mouse when used for relative coordinates. Styli's relative coordinates remain non-accelerated. Aside from that we had the usual fixes, and, like always, mostly in the touchpad code. As usual, the git shortlog is below. Peter Hutterer (78): evdev: de-duplicate the model property->model flag list Add tagging of trackballs configure.ac: move libunwind/addr2line test to inside the test conditions gestures: don't send swipe gestures when gestures are disabled Add support for relative device rotation (trackball only) Add configure.ac check for static_assert test: add a keyboard test device with all codes enabled evdev: the range between dpad and trigger-happy are keys, not buttons touchpad: only use negative pressure change check on Lenovo *50 and *60 series touchpad: disable cursor jump detection for Wacom tablets test: fix a couple of touchpad tests that triggered the cursor jump warning doc: fix a few typos test: skip manually setting uinput resolution if it's already set udev: add the Yoga 2 to the wobbly touchpads udev: mark ALPS touchpads fw version 300 as wobbly touchpads Drop the ALPS_RUSHMORE tag doc: update doc to explicitly state that the seat isn't referenced doc: update ref/unref behavior for the tablet tool tools: print the pad capabilities pad: ignore EV_MSC events doc: sort the svg files alphabetically doc: add the missing svg files to the Makefile.am doc: grammar fix in the Makefile tools: add missing space in libinput-list-devices output udev: mark the Logitech M570 as trackball tools: print the rotation angle in libinput-list-devices touchpad: fix link in error message - add missing '.html' tablet: up the reference count for the tool in the event touchpad: short-circuit the edge scroll handling when it's not enabled touchpad: warn if we have invalid touchpad ranges doc: ship the doc sources even when not building with docs doc: add a check to compare local files with those in the makefile tools: add missing space after listing touch/tablet capabilities tools: show tablet-pad capabilities in libinput-list-devices tools: print button/ring/strip info for pads touchpad: restore the hysteresis by default pad: group the button state into a private struct test: start the pad button tests at BTN_0 pad: change button map ordering evdev: check model flags for actual booleans touchpad: use the tp_libinput_context() helper evdev: add helper to get the libinput context from the evdev device tablet: add helper function to access the libinput context pad: add helper function to access the libinput context touchpad: don't warn about kernel jumps on semi-mt devices touchpad: split palm movement detection into a helper function touchpad: stop palm detection when a second finger is detected test: make the valgrind target depend on 'all' test: when creating an abs test device, force the abs->value to the mimimum test: add an apple magicmouse device Revert "test: add a test for the T450 dropped motion events" Revert "touchpad: only use negative pressure change check on Lenovo *50 and *60 series" Revert "touchpad: reset the motion history on significant negative pressure changes" touchpad: re-enable hysteresis by default for all devices util: add safe_atoi helper function doc: split the tablet pad events into their own doxygen module page doc: move the struct decl
Re: Weston versioning (Re: [PATCH weston 6/6] libweston: do not use weston version in libweston.pc)
On Monday 2016-07-11 16:44, Emil Velikov wrote:
>> Without pkgconfig supporting some new alias tag (hint, hint) to cover
>> such a case,
>No idea what such a "alias tag" is supposed to do/look like. Do you
>have examples ?
Proposed concept would be to make pkgconfig recognize
a new Alias directive:
gtk-2.0.pc:
Name: gtk-2.0
Version: 2
Alias: gtk
@OTHER_TYPICAL_FIELDS@
gtk-3.0.pc:
Name: foo-3.0
Version: 3
Alias: gtk
Result (recap):
A single PKG_CHECK_MODULESM([gtk], [gtk >= 2]) call may be used instead of
the usual two or more.
>> I predict that whoever uses libfoo-LotsANumbersHere
>> limits themselves to 3 or maybe 4 PKG_CHECK_MODULES calls, because it
>> is looking real silly.
>>
>If (and that's a _huge_ if) they support more than 3-4 versions of
>libfoo, then the configure.ac will be the least ugly thing.
Heh, true: The projects which do use foo-N.pc naming move about in
slow cycles (we had dbus-1, gtk-2, libnl-3 for a long time now), and
if they change, it can be assured they threw *everything* over the
fence.
Those who have just foo.pc are kind of the Linux kernel type: it
changes a bit here and there, and every now and then, and external
components with new failed builds are comparatively easily fixable.
This should not be overdiscussed; just pick something already. {1:
Either it will work out, and all is good, or it will not work out,
gets changed and then goto 1.} :p
^ That always worked out so far.
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston] config-parser: Catch negative numbers assigned to unsigned config values
strtoul() has a side effect that when given a string representing a
negative number, it returns a negated version as the value, and does not
flag an error. IOW, strtoul("-42", &val) sets val to 42. This could
potentially result in unintended surprise behaviors, such as if one were
to inadvertantly set a config param to -1 expecting that to disable it,
but with the result of setting the param to 1 instead.
Catch this by using strtol() and then manually check for the negative
value. This logic is modelled after Wayland's strtouint().
Note that this change unfortunately reduces the range of parseable
numbers from [0,UINT_MAX] to [0,INT_MAX]. The current users of
weston_config_section_get_uint() are anticipating numbers far smaller
than either of these limits, so the change is believed to have no impact
in practice.
Also add a test case for negative numbers that catches this error
condition.
Signed-off-by: Bryce Harrington
---
shared/config-parser.c | 12 +++-
tests/config-parser-test.c | 31 +++
2 files changed, 42 insertions(+), 1 deletion(-)
diff --git a/shared/config-parser.c b/shared/config-parser.c
index 151ae9c..247e880 100644
--- a/shared/config-parser.c
+++ b/shared/config-parser.c
@@ -186,6 +186,7 @@ weston_config_section_get_uint(struct weston_config_section
*section,
const char *key,
uint32_t *value, uint32_t default_value)
{
+ long int ret;
struct weston_config_entry *entry;
char *end;
@@ -197,13 +198,22 @@ weston_config_section_get_uint(struct
weston_config_section *section,
}
errno = 0;
- *value = strtoul(entry->value, &end, 0);
+ ret = strtol(entry->value, &end, 0);
if (errno != 0 || end == entry->value || *end != '\0') {
*value = default_value;
errno = EINVAL;
return -1;
}
+ /* check range */
+ if (ret < 0 || ret > INT_MAX) {
+ *value = default_value;
+ errno = ERANGE;
+ return -1;
+ }
+
+ *value = ret;
+
return 0;
}
diff --git a/tests/config-parser-test.c b/tests/config-parser-test.c
index 735da4e..f88e89b 100644
--- a/tests/config-parser-test.c
+++ b/tests/config-parser-test.c
@@ -117,6 +117,7 @@ static struct zuc_fixture config_test_t1 = {
"# more comments\n"
"number=5252\n"
"zero=0\n"
+ "negative=-42\n"
"flag=false\n"
"\n"
"[stuff]\n"
@@ -461,6 +462,36 @@ ZUC_TEST_F(config_test_t1, test019, data)
ZUC_ASSERT_EQ(0, errno);
}
+ZUC_TEST_F(config_test_t1, test020, data)
+{
+ int r;
+ int32_t n;
+ struct weston_config_section *section;
+ struct weston_config *config = data;
+
+ section = weston_config_get_section(config, "bar", NULL, NULL);
+ r = weston_config_section_get_int(section, "negative", &n, 600);
+
+ ZUC_ASSERT_EQ(0, r);
+ ZUC_ASSERT_EQ(-42, n);
+ ZUC_ASSERT_EQ(0, errno);
+}
+
+ZUC_TEST_F(config_test_t1, test021, data)
+{
+ int r;
+ uint32_t n;
+ struct weston_config_section *section;
+ struct weston_config *config = data;
+
+ section = weston_config_get_section(config, "bar", NULL, NULL);
+ r = weston_config_section_get_uint(section, "negative", &n, 600);
+
+ ZUC_ASSERT_EQ(-1, r);
+ ZUC_ASSERT_EQ(600, n);
+ ZUC_ASSERT_EQ(ERANGE, errno);
+}
+
ZUC_TEST_F(config_test_t2, doesnt_parse, data)
{
struct weston_config *config = data;
--
1.9.1
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston] rdp: Check for non-digits and errno in strtol call
Improve error checking for situations like RDP_FD=42foo, or where the provided number is out of range. Suggestion by Yong Bakos. Signed-off-by: Bryce Harrington --- libweston/compositor-rdp.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/libweston/compositor-rdp.c b/libweston/compositor-rdp.c index 79f0687..53c7124 100644 --- a/libweston/compositor-rdp.c +++ b/libweston/compositor-rdp.c @@ -1263,7 +1263,8 @@ rdp_backend_create(struct weston_compositor *compositor, } fd = strtoul(fd_str, &fd_tail, 10); - if (fd_tail == fd_str || rdp_peer_init(freerdp_peer_new(fd), b)) + if (errno != 0 || fd_tail == fd_str || *fd_tail != '\0' + || rdp_peer_init(freerdp_peer_new(fd), b)) goto err_output; } -- 1.9.1 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston] config-parser: Improve error checks for strtol/strtoul calls
On Mon, Jul 11, 2016 at 01:19:54PM -0700, Bryce Harrington wrote: > On Mon, Jul 11, 2016 at 09:39:00AM -0700, Bill Spitzak wrote: > > On Sun, Jul 10, 2016 at 4:28 PM, Peter Hutterer > > wrote: > > > > > On Thu, Jul 07, 2016 at 02:24:39PM -0700, Bryce Harrington wrote: > > > > On Thu, Jul 07, 2016 at 02:08:28PM -0700, Bryce Harrington wrote: > > > > > Check errno, which is set of over/underflow, out of range, etc. Also > > > > > check for empty strings (the usages covered in this patch already also > > > > > cover the case where there are non-digits present). Set errno to 0 > > > > > before making the strto*l call in case of pre-existing errors > > > > > (i.e. ENOTTY when running under the testsuite). > > > > > > > > > > This follows the error checking style used in Wayland > > > > > (c.f. wayland-client.c and scanner.c). > > > > > > > > > > In tests, also check errno, and add testcases for parsing '0'. > > > > > > > > > > Signed-off-by: Bryce Harrington > > > > > > > > If this approach looks ok to folks, there are other irregular uses of > > > > strto*l I'll clean up as well subsequently. > > > > > > it'd be best to have a safe_strtol helper function in a weston-util.h > > > header and just enforce use of that. because most of the time all we care > > > about is "has it parsed and what's the result", you can get that with a > > > bool safe_strtol(const char *string, long *result); > > > > > > or safe_atoi, or whatever you want to call it. > > > > > > > The problem is that it is a pain to have to pass a reference to the output > > variable, which discourages use and why everybody keeps calling strtol. A > > function that returns a value is much more usable and easy to read in the > > source code. > > > To report the error setting errno will work. Programs can check this after > > return. Returning a bool, or passing a pointer to a bool, really does not > > help, the program can ignore that just as easily as ignoring errno, and you > > have made it harder to call the function and thus discouraged it's use. > Hmm, I get your point, but checking bool error code returns is way more > common than checking for errno, and I would think less likely to be > ignored. As far as convenience goes... well error checking is annoying > in general, so it's a bit of a wash, but checking errno with a value > return is going to require slightly more typing than checking return > codes with a value passed by reference. Using errno also places a > requirement for including errno.h on the caller. a bool has a not-so-obvious side effect, it makes it clear in the signature that the return code is *not* the converted value. compare: bool magic_atoi(const char *str, int *value); int magic_atoi(const char *str, int *value); in the latter case, you may think that the value is also returned (there is precedence for that with e.g. time(). a bool is non-ambiguous here. and of course nothing stops you from setting errno as extra condition. really, what we need is a sensible version of something like this: typedef int negative_errno; negative_errno magic_atoi(...) but this is still C and we can't have nice things. And single-value structs are type-safe but a huge pain otherwise. Cheers, Peter > But to me, those points are both really minor. The real benefits to > relying on errno here would be: 1) the errors are more expressive > (e.g. ERANGE or EINVAL rather than just a simple true/false), and 2) > errno can be checked after making a sequence of conversion calls to > check if any of them failed, rather than checking the return code on > each individually. > > On the first point, most strtol/strtoul calls in Weston don't seem to > care much why it failed. For the second point, there is indeed a > sequence of calls in the config parser, where checking errno just once > after all calls were made might make for more concise code, but if there > *is* an error, then for debugging purposes I'm guessing folks would want > to know what line in the config file errored, in which case we'd be back > to needing to check errors after each conversion. > > So, considering all these points it seems like the trade-off favors > something more like what Peter suggests - pass the value by reference > and indicate success or failure with a bool return value. We could also > set errno in addition, in case the caller might want to know more > details about why the failure occurred, but since none of the callers > seem to care to that level of detail maybe it'd be better to just leave > errno unchanged, as Wayland's strtouint() does; a caller that *did* care > about the exact errno can always just do their own direct strtol call. > > > Have the function log errors to stderr if you want to make it "impossible" > > to ignore errors. > > Hmm, not sure on this. From the software's point of view, I would argue > that logging messages to stderr and continuing on with processing is > indeed ignoring the errors. Also, for ge
Re: [PATCH weston] config-parser: Improve error checks for strtol/strtoul calls
On Mon, Jul 11, 2016 at 09:39:00AM -0700, Bill Spitzak wrote: > On Sun, Jul 10, 2016 at 4:28 PM, Peter Hutterer > wrote: > > > On Thu, Jul 07, 2016 at 02:24:39PM -0700, Bryce Harrington wrote: > > > On Thu, Jul 07, 2016 at 02:08:28PM -0700, Bryce Harrington wrote: > > > > Check errno, which is set of over/underflow, out of range, etc. Also > > > > check for empty strings (the usages covered in this patch already also > > > > cover the case where there are non-digits present). Set errno to 0 > > > > before making the strto*l call in case of pre-existing errors > > > > (i.e. ENOTTY when running under the testsuite). > > > > > > > > This follows the error checking style used in Wayland > > > > (c.f. wayland-client.c and scanner.c). > > > > > > > > In tests, also check errno, and add testcases for parsing '0'. > > > > > > > > Signed-off-by: Bryce Harrington > > > > > > If this approach looks ok to folks, there are other irregular uses of > > > strto*l I'll clean up as well subsequently. > > > > it'd be best to have a safe_strtol helper function in a weston-util.h > > header and just enforce use of that. because most of the time all we care > > about is "has it parsed and what's the result", you can get that with a > > bool safe_strtol(const char *string, long *result); > > > > or safe_atoi, or whatever you want to call it. > > > > The problem is that it is a pain to have to pass a reference to the output > variable, which discourages use and why everybody keeps calling strtol. A > function that returns a value is much more usable and easy to read in the > source code. I'm really struggling to take you seriously here. For comparison: putting seatbelts on is a pain but the only time when it matters is when things go wrong and, oh boy, will you be glad when your feet don't ask your head to remove the windscreen so they can explore the space beyond. I also find it rather entertaining how you believe to speak for everybody. have you thought about how maybe "everybody" uses strtol because it's already implemented and the only one reliably available? have you given thought to all the projects that wrap it because strtol it's a terrible API? > To report the error setting errno will work. Programs can check this after > return. Returning a bool, or passing a pointer to a bool, really does not > help, the program can ignore that just as easily as ignoring errno, and you > have made it harder to call the function and thus discouraged it's use. > Have the function log errors to stderr if you want to make it "impossible" > to ignore errors. let me be clear about "the program can ignore $USEFUL_THING" because arguments similar to this have come from you in the past: This is a free software project with several gatekeepers with motivations that go beyond the mere monetary interests to make it work no matter what. On average, we not only care about the code right now but also whether this will make our lives easier, better or at least less embarrassing in the future. Yes, "a program can ignore $USEFUL_THING" is true but we are free to ignore to patches that don't do the right thing just because the author is lazy and, even better, we are free to ignore authors of patches that repeatedly show they value their own lazyness above everyone else's time. coincidentally, this thread started exactly *because* of such a discussion to improve things for the future even at the cost of minimally more effort right now. anyway, I really thought long about whether it's worth replying but in the end I decided I wanted the above paragraph in a readily accessible link. Don't expect me to continue this conversation otherwise. Cheers, Peter ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston v2 8/8] terminal: Silence maybe-uninitialized warning
On 10 July 2016 at 10:00, Quentin Glidic wrote: > From: Quentin Glidic > > clients/terminal.c: In function 'redraw_handler': > clients/terminal.c:213:28: warning: 'machine.unicode' may be used > uninitialized in this function [-Wmaybe-uninitialized] > struct utf8_state_machine machine; > ^~~ > > clients/terminal.c: In function 'handle_char': > clients/terminal.c:213:28: warning: 'machine.unicode' may be used > uninitialized in this function [-Wmaybe-uninitialized] > > Warning produced by GCC 5.3 and 6.1, with -O3. > > Signed-off-by: Quentin Glidic > --- > > You have to look at the file directly to completely understand this one. > Not sure that is the value to return in this case, but at least it > silences the warning. > ::unicode will never be read (get_unicode) when it's invalid, since ::state will equal to utf8state_reject (as set in utf8_next_char). Seems like the compiler/optimiser cannot see that far, thus throws a warning message. With anything vaguely like the above in the commit message, the patch is Reviewed-by: Emil Velikov -Emil ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston] gl-renderer: Silence silly warning
On 4 July 2016 at 15:00, Quentin Glidic wrote:
> From: Quentin Glidic
>
> Signed-off-by: Quentin Glidic
> ---
> libweston/gl-renderer.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libweston/gl-renderer.c b/libweston/gl-renderer.c
> index 28c0b50..73b6ccc 100644
> --- a/libweston/gl-renderer.c
> +++ b/libweston/gl-renderer.c
> @@ -455,7 +455,7 @@ static int
> compress_bands(pixman_box32_t *inrects, int nrects,
>pixman_box32_t **outrects)
> {
> - bool merged;
> + bool merged = false;
What is the "silly warning" here ? Please mention it just like you did
for the others.
If we get a "Wmaybe-uninitialized" then it's a compiler deficiency,
but it doesn't harm silencing it with this patch.
-Emil
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: Weston versioning (Re: [PATCH weston 6/6] libweston: do not use weston version in libweston.pc)
On 11 July 2016 at 17:58, Jan Engelhardt wrote: > > On Monday 2016-07-11 16:44, Emil Velikov wrote: http://ometer.com/parallel.html. I would strongly recommend giving it a look. >>> >>> I read it now, and I do not buy it - at least not for 2016 standards. >>> According to the page, it was written in 2002, and I can confirm that >>> the situation was much worse then it is now. I can practically refute >>> all his points from the "Some more issues:" section, but for brevity, >>> I spare you the details for now. >> >>Trying to get some understanding about your experience in the area - >>which distribution(s) do you work with, how many packages do you >>maintain ? > > openSUSE. Enlisted for 441, but my id appears in 2056 changelogs by now and I > bear the role of global co-reviewer, i.e. some 9100 packages. > > And in the paid part of life: SLE, RH6,7, Ubu14/16, Univention, Collax (things > you never heard of), Windows(*). So I am also aware about _their_ > skeletons-in-the-closet. > Damn, that's an impressive way of making me eat my own words ;-) Strange part is that neither of Fedora, Debian, Gentoo + Arch plus the Openhub $distro packages shows any results :-\ Guess I failed "at googling" That said, considering the discussion/confusion with others I'd suspect you've been at our level way back. Thus some of our assumptions/knowledge might be quite noobish. If you can share/recommend some reading material that covers your earlier/current concerns that'll be amazing. Thanks Emil [1] https://admin.fedoraproject.org/pkgdb/packagers/ [2] https://nm.debian.org/public/people [3] https://www.gentoo.org/inside-gentoo/developers/ [4] https://www.archlinux.org/people/developers/ > > (*) Not a distro, but an anarchy, which is also interesting to distribute for. Windows distribution is interesting in it's own crazy way :-P MSI, NSIS, InstallShield, downloading MS C/.NET runtimes... heh memories. ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH] scanner: Improve documentation for strtouint()
On Fri, Jul 08, 2016 at 07:38:13PM -0700, Yong Bakos wrote:
> On Jul 8, 2016, at 4:51 PM, Bryce Harrington wrote:
> >
> > From: Bryce Harrington
> >
> > Signed-off-by: Bryce Harrington
> > Signed-off-by: Bryce Harrington
>
> A definite improvement. However, a couple questions inline below.
>
>
> > ---
> > src/scanner.c | 13 +++--
> > 1 file changed, 11 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/scanner.c b/src/scanner.c
> > index 4708cae..a266da1 100644
> > --- a/src/scanner.c
> > +++ b/src/scanner.c
> > @@ -576,8 +576,17 @@ free_interface(struct interface *interface)
> > free(interface);
> > }
> >
> > -/* convert string to unsigned integer,
> > - * in the case of error, return -1 */
> > +/** Convert string to unsigned integer
>
> Should this be a standard comment block instead of a doc-comment?
> Since doxygen will not generate public documentation for scanner.c,
> and since this is an internal function, maybe this should just be a
> regular comment block.
Alright.
> > + *
> > + * Parses a non-negative base-10 number from the given string. If the
> > + * specified string is blank, contains non-numerical characters, is out
> > + * of range, or results in a negative number, -1 is returned to indicate
> > + * an error..
>
> Nit: single period here.
>
>
> > + *
> > + * This routine does not modify errno, nor sets errno on error.
> > + *
>
> This feels a little misleading as written, since it does potentially
> modify errno temporarily as an implementation detail. Perhaps "Upon error,
> this routine doe not modify or set errno" would be a little more clear.
Hmm, you're right it's a bit awkward. I'll use your text for now, we'll
get another go at the phrasing if/when the routine is moved to being a
shared utility routine.
> Other than that, this is
>
> Reviewed-by: Yong Bakos
Thanks, pushed with your suggested edits.
1cda73f..c88ec7e master -> master
> Regards,
> yong
>
>
> > + * \returns -1 on error, or a non-negative integer on success
> > + */
> > static int
> > strtouint(const char *str)
> > {
> > --
> > 1.9.1
> >
> > ___
> > 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] tests: Require base 10 for the string specifying the number of open fd's
On Fri, Jul 08, 2016 at 07:39:33PM -0700, Yong Bakos wrote: > On Jul 8, 2016, at 7:00 PM, Bryce Harrington wrote: > > > > The third arg to strtol() specifies the base to assume for the number. > > When 0 is passed, as is currently done in wayland-client.c, hexadecimal > > and octal numbers are permitted and automatically detected and > > converted. > > > > exec-fd-leak-checker's single argument is the count of file descriptors > > it should expect to be open. We should expect this to be specified only > > as a decimal number, there's no reason why one would want to use octal > > or hexadecimal for that. > > > > Suggested by Yong Bakos. > > > > Signed-off-by: Bryce Harrington > > Reviewed-by: Yong Bakos > > yong Thanks, pushed. Bryce > > > --- > > tests/exec-fd-leak-checker.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/tests/exec-fd-leak-checker.c b/tests/exec-fd-leak-checker.c > > index 0c69da3..5f3b395 100644 > > --- a/tests/exec-fd-leak-checker.c > > +++ b/tests/exec-fd-leak-checker.c > > @@ -37,7 +37,7 @@ parse_count(const char *str, int *value) > > long v; > > > > errno = 0; > > - v = strtol(str, &end, 0); > > + v = strtol(str, &end, 10); > > if ((errno == ERANGE && (v == LONG_MAX || v == LONG_MIN)) || > > (errno != 0 && v == 0) || > > (end == str) || > > -- > > 1.9.1 > > > > ___ > > 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] rdp: Check for non-numeric value in RDP_FD env var
On Fri, Jul 08, 2016 at 08:11:26PM -0700, Yong Bakos wrote:
> On Jul 8, 2016, at 6:54 PM, Bryce Harrington wrote:
> >
> > strtoul(nptr, endptr, ...) will set *endptr to nptr in the case of where
> > no digits were read from the string, and return 0. Running with
> > RDP_FD=foo would thus result in fd=0 being specified to
> > freerdp_peer_new(), which is unlikely to be the user's intent.
> >
> > Signed-off-by: Bryce Harrington
> > ---
> > libweston/compositor-rdp.c | 5 +++--
> > 1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/libweston/compositor-rdp.c b/libweston/compositor-rdp.c
> > index d74dd5e..79f0687 100644
> > --- a/libweston/compositor-rdp.c
> > +++ b/libweston/compositor-rdp.c
> > @@ -1209,6 +1209,7 @@ rdp_backend_create(struct weston_compositor
> > *compositor,
> > {
> > struct rdp_backend *b;
> > char *fd_str;
> > + char *fd_tail;
> > int fd;
> >
> > b = zalloc(sizeof *b);
> > @@ -1261,8 +1262,8 @@ rdp_backend_create(struct weston_compositor
> > *compositor,
> > goto err_output;
> > }
> >
> > - fd = strtoul(fd_str, NULL, 10);
> > - if (rdp_peer_init(freerdp_peer_new(fd), b))
> > + fd = strtoul(fd_str, &fd_tail, 10);
> > + if (fd_tail == fd_str || rdp_peer_init(freerdp_peer_new(fd), b))
>
> While you're at it, maybe this deserves to be a little more robust. When
> RDP_FD=42foo, fd_tail will != fd_str and fd will be 42. Something akin to
> the idiom:
>
> if (fd_tail == fd_str || *fd_tail != '\0' || errno == ERANGE || rdp_peer...
Yeah, good points. I'll catch this in followup, I think this specific
fix is good on its own, but agree there's more cleanup needed in this
call. For now I'll push as is.
> Regards,
> yong
Bryce
P.S. Apologies... When I pushed the patches I thought I'd applied the
patches with your R-b from patchwork but evidently they were my local
copies, so missed your R-b's. :-/
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston] config-parser: Improve error checks for strtol/strtoul calls
On Mon, Jul 11, 2016 at 09:28:16AM +1000, Peter Hutterer wrote:
> On Thu, Jul 07, 2016 at 02:24:39PM -0700, Bryce Harrington wrote:
> > On Thu, Jul 07, 2016 at 02:08:28PM -0700, Bryce Harrington wrote:
> > > Check errno, which is set of over/underflow, out of range, etc. Also
> > > check for empty strings (the usages covered in this patch already also
> > > cover the case where there are non-digits present). Set errno to 0
> > > before making the strto*l call in case of pre-existing errors
> > > (i.e. ENOTTY when running under the testsuite).
> > >
> > > This follows the error checking style used in Wayland
> > > (c.f. wayland-client.c and scanner.c).
> > >
> > > In tests, also check errno, and add testcases for parsing '0'.
> > >
> > > Signed-off-by: Bryce Harrington
> >
> > If this approach looks ok to folks, there are other irregular uses of
> > strto*l I'll clean up as well subsequently.
>
> it'd be best to have a safe_strtol helper function in a weston-util.h
> header and just enforce use of that. because most of the time all we care
> about is "has it parsed and what's the result", you can get that with a
> bool safe_strtol(const char *string, long *result);
>
> or safe_atoi, or whatever you want to call it.
I've pushed this specific cleanup with Eric's R-b but will follow up
with a proposal for this helper function later. I think there may be a
few more cleanups to do first.
Bryce
> Cheers,
>Peter
>
>
>
> >
> > > ---
> > > shared/config-parser.c | 6 --
> > > tests/config-parser-test.c | 34 ++
> > > 2 files changed, 38 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/shared/config-parser.c b/shared/config-parser.c
> > > index 2256469..151ae9c 100644
> > > --- a/shared/config-parser.c
> > > +++ b/shared/config-parser.c
> > > @@ -169,8 +169,9 @@ weston_config_section_get_int(struct
> > > weston_config_section *section,
> > > return -1;
> > > }
> > >
> > > + errno = 0;
> > > *value = strtol(entry->value, &end, 0);
> > > - if (*end != '\0') {
> > > + if (errno != 0 || end == entry->value || *end != '\0') {
> > > *value = default_value;
> > > errno = EINVAL;
> > > return -1;
> > > @@ -195,8 +196,9 @@ weston_config_section_get_uint(struct
> > > weston_config_section *section,
> > > return -1;
> > > }
> > >
> > > + errno = 0;
> > > *value = strtoul(entry->value, &end, 0);
> > > - if (*end != '\0') {
> > > + if (errno != 0 || end == entry->value || *end != '\0') {
> > > *value = default_value;
> > > errno = EINVAL;
> > > return -1;
> > > diff --git a/tests/config-parser-test.c b/tests/config-parser-test.c
> > > index 5dcafc4..735da4e 100644
> > > --- a/tests/config-parser-test.c
> > > +++ b/tests/config-parser-test.c
> > > @@ -116,6 +116,7 @@ static struct zuc_fixture config_test_t1 = {
> > > "[bar]\n"
> > > "# more comments\n"
> > > "number=5252\n"
> > > + "zero=0\n"
> > > "flag=false\n"
> > > "\n"
> > > "[stuff]\n"
> > > @@ -263,6 +264,7 @@ ZUC_TEST_F(config_test_t1, test006, data)
> > >
> > > ZUC_ASSERT_EQ(0, r);
> > > ZUC_ASSERT_EQ(5252, n);
> > > + ZUC_ASSERT_EQ(0, errno);
> > > }
> > >
> > > ZUC_TEST_F(config_test_t1, test007, data)
> > > @@ -289,8 +291,10 @@ ZUC_TEST_F(config_test_t1, test008, data)
> > >
> > > section = weston_config_get_section(config, "bar", NULL, NULL);
> > > r = weston_config_section_get_uint(section, "number", &u, 600);
> > > +
> > > ZUC_ASSERT_EQ(0, r);
> > > ZUC_ASSERT_EQ(5252, u);
> > > + ZUC_ASSERT_EQ(0, errno);
> > > }
> > >
> > > ZUC_TEST_F(config_test_t1, test009, data)
> > > @@ -427,6 +431,36 @@ ZUC_TEST_F(config_test_t1, test017, data)
> > > ZUC_ASSERT_EQ(5, i);
> > > }
> > >
> > > +ZUC_TEST_F(config_test_t1, test018, data)
> > > +{
> > > + int r;
> > > + int32_t n;
> > > + struct weston_config_section *section;
> > > + struct weston_config *config = data;
> > > +
> > > + section = weston_config_get_section(config, "bar", NULL, NULL);
> > > + r = weston_config_section_get_int(section, "zero", &n, 600);
> > > +
> > > + ZUC_ASSERT_EQ(0, r);
> > > + ZUC_ASSERT_EQ(0, n);
> > > + ZUC_ASSERT_EQ(0, errno);
> > > +}
> > > +
> > > +ZUC_TEST_F(config_test_t1, test019, data)
> > > +{
> > > + int r;
> > > + uint32_t n;
> > > + struct weston_config_section *section;
> > > + struct weston_config *config = data;
> > > +
> > > + section = weston_config_get_section(config, "bar", NULL, NULL);
> > > + r = weston_config_section_get_uint(section, "zero", &n, 600);
> > > +
> > > + ZUC_ASSERT_EQ(0, r);
> > > + ZUC_ASSERT_EQ(0, n);
> > > + ZUC_ASSERT_EQ(0, errno);
> > > +}
> > > +
> > > ZUC_TEST_F(config_test_t2, doesnt_parse, data)
> > > {
> > > struct weston_config *config = data;
> > > --
> > > 1.9.1
> > ___
> > wayland-devel mailing list
> > wayland-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/wayla
Re: [PATCH weston] Remove a wrong closing “extern "C"” in shared/xalloc.c
On Mon, Jul 11, 2016 at 05:31:59PM +0100, Emmanuel Gil Peyrot wrote: > Signed-off-by: Emmanuel Gil Peyrot Thanks, pushed: cbc0537..7fc000c master -> master > --- > shared/xalloc.c | 5 - > 1 file changed, 5 deletions(-) > > diff --git a/shared/xalloc.c b/shared/xalloc.c > index 4bf8a3e..9bf5245 100644 > --- a/shared/xalloc.c > +++ b/shared/xalloc.c > @@ -47,8 +47,3 @@ fail_on_null(void *p, size_t size, char *file, int32_t line) > > return p; > } > - > - > -#ifdef __cplusplus > -} > -#endif > -- > 2.9.0 > > ___ > 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] Remove a wrong closing “extern "C"” in shared/xalloc.c
On Mon, Jul 11, 2016 at 05:31:59PM +0100, Emmanuel Gil Peyrot wrote: > Signed-off-by: Emmanuel Gil Peyrot Reviewed-by: Bryce Harrington > --- > shared/xalloc.c | 5 - > 1 file changed, 5 deletions(-) > > diff --git a/shared/xalloc.c b/shared/xalloc.c > index 4bf8a3e..9bf5245 100644 > --- a/shared/xalloc.c > +++ b/shared/xalloc.c > @@ -47,8 +47,3 @@ fail_on_null(void *p, size_t size, char *file, int32_t line) > > return p; > } > - > - > -#ifdef __cplusplus > -} > -#endif > -- > 2.9.0 > > ___ > 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] config-parser: Improve error checks for strtol/strtoul calls
On Mon, Jul 11, 2016 at 09:39:00AM -0700, Bill Spitzak wrote: > On Sun, Jul 10, 2016 at 4:28 PM, Peter Hutterer > wrote: > > > On Thu, Jul 07, 2016 at 02:24:39PM -0700, Bryce Harrington wrote: > > > On Thu, Jul 07, 2016 at 02:08:28PM -0700, Bryce Harrington wrote: > > > > Check errno, which is set of over/underflow, out of range, etc. Also > > > > check for empty strings (the usages covered in this patch already also > > > > cover the case where there are non-digits present). Set errno to 0 > > > > before making the strto*l call in case of pre-existing errors > > > > (i.e. ENOTTY when running under the testsuite). > > > > > > > > This follows the error checking style used in Wayland > > > > (c.f. wayland-client.c and scanner.c). > > > > > > > > In tests, also check errno, and add testcases for parsing '0'. > > > > > > > > Signed-off-by: Bryce Harrington > > > > > > If this approach looks ok to folks, there are other irregular uses of > > > strto*l I'll clean up as well subsequently. > > > > it'd be best to have a safe_strtol helper function in a weston-util.h > > header and just enforce use of that. because most of the time all we care > > about is "has it parsed and what's the result", you can get that with a > > bool safe_strtol(const char *string, long *result); > > > > or safe_atoi, or whatever you want to call it. > > > > The problem is that it is a pain to have to pass a reference to the output > variable, which discourages use and why everybody keeps calling strtol. A > function that returns a value is much more usable and easy to read in the > source code. > > To report the error setting errno will work. Programs can check this after > return. Returning a bool, or passing a pointer to a bool, really does not > help, the program can ignore that just as easily as ignoring errno, and you > have made it harder to call the function and thus discouraged it's use. Hmm, I get your point, but checking bool error code returns is way more common than checking for errno, and I would think less likely to be ignored. As far as convenience goes... well error checking is annoying in general, so it's a bit of a wash, but checking errno with a value return is going to require slightly more typing than checking return codes with a value passed by reference. Using errno also places a requirement for including errno.h on the caller. But to me, those points are both really minor. The real benefits to relying on errno here would be: 1) the errors are more expressive (e.g. ERANGE or EINVAL rather than just a simple true/false), and 2) errno can be checked after making a sequence of conversion calls to check if any of them failed, rather than checking the return code on each individually. On the first point, most strtol/strtoul calls in Weston don't seem to care much why it failed. For the second point, there is indeed a sequence of calls in the config parser, where checking errno just once after all calls were made might make for more concise code, but if there *is* an error, then for debugging purposes I'm guessing folks would want to know what line in the config file errored, in which case we'd be back to needing to check errors after each conversion. So, considering all these points it seems like the trade-off favors something more like what Peter suggests - pass the value by reference and indicate success or failure with a bool return value. We could also set errno in addition, in case the caller might want to know more details about why the failure occurred, but since none of the callers seem to care to that level of detail maybe it'd be better to just leave errno unchanged, as Wayland's strtouint() does; a caller that *did* care about the exact errno can always just do their own direct strtol call. > Have the function log errors to stderr if you want to make it "impossible" > to ignore errors. Hmm, not sure on this. From the software's point of view, I would argue that logging messages to stderr and continuing on with processing is indeed ignoring the errors. Also, for general purpose utility functions like this, it seems better to leave decisions about error logging to the caller, because it'll have more context over what the parsing is trying to do. Some things might be best sent to weston_log() rather than stderr for example. Bryce ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH wayland-protocols v5 4/4] tablet: Add pad support to the tablet protocol
On 07/11/2016 08:13 AM, Carlos Garnacho wrote: > The pad's interface is similar to the tool interface, a client is notified of > the pad after the tablet_added event. > > The pad has three functionalities: buttons, rings and strips. > Buttons are fairly straightforward, rings and strips are separate interfaces > with a pointer-axis-like source/value/frame events. > The two interfaces are effectively identical but for the actual value they > send (degrees vs normalized position). > > Buttons are sequentially indexed starting with zero, unlike other protocols > where a linux/input.h-style semantic event code is used. Since we expect all > buttons to have client-specific functionality, an additional event tells the > client when a given button index is not available, usually because the > compositor assignes some function to it (e.g. mode switching, see below). > > Specific to the pad device is the set_feedback request which enables a client > to set a user-defined string to display for an OSD on the current mappings. > This request is available for buttons, rings and strips. > > Finally, the pad supports groups, effectively sets of button/ring/strip > configurations. Those groups may have multiple modes each, so that > users/clients may map several actions to a single element. > > Signed-off-by: Carlos Garnacho > Signed-off-by: Peter Hutterer > Reviewed-by: Jason Gerecke > --- > unstable/tablet/tablet-unstable-v2.xml | 540 > + > 1 file changed, 540 insertions(+) > > diff --git a/unstable/tablet/tablet-unstable-v2.xml > b/unstable/tablet/tablet-unstable-v2.xml > index 77b185c..2cc3a92 100644 > --- a/unstable/tablet/tablet-unstable-v2.xml > +++ b/unstable/tablet/tablet-unstable-v2.xml > @@ -172,6 +172,22 @@ > > summary="the newly added tablet tool"/> > > + > + > + > + This event is sent whenever a new pad is known to the system. Typically, > + pads are physically attached to tablets and a pad_added event is > + sent immediately after the wp_tablet_seat.tablet_added. > + However, some standalone pad devices logically attach to tablets at > + runtime, and the client must wait for wp_tablet_pad.enter to know > + the tablet a pad is attached to. > + > + This event only provides the object id of the pad. All further > + features (buttons, strips, rings) are sent through the wp_tablet_pad > + interface. > + > + summary="the newly added pad"/> > + > > > > @@ -636,4 +652,528 @@ > > > > + > + > + A circular interaction area, such as the touch ring on the Wacom Intuos > + Pro series tablets. > + > + Events on a ring are logically grouped by the wl_tablet_pad_ring.frame > + event. > + > + > + > + > + Request that the compositor use the provided feedback string > + associated with this ring. This request should be issued immediately > + after a wp_tablet_pad_group.mode_switch event from the corresponding > + group is received, or whenever the ring is mapped to a different > + action. See wp_tablet_pad_group.mode_switch for more details. > + > + Clients are encouraged to provide context-aware descriptions for > + the actions associated with the ring; compositors may use this > + information to offer visual feedback about the button layout > + (eg. on-screen displays). > + > + The provided string 'description' is a UTF-8 encoded string to be > + associated with this ring, and is considered user-visible; general > + internationalization rules apply. > + > + The serial argument will be that of the last > + wp_tablet_pad_group.mode_switch event received for the group of this > + ring. Requests providing other serials than the most recent one will be > + ignored. > + > + > + > + > + > + > + > + This destroys the client's resource for this ring object. > + > + > + > + > + > + Describes the source types for ring events. This indicates to the > + client how a ring event was physically generated; a client may > + adjust the user interface accordingly. For example, events > + from a "finger" source may trigger kinetic scrolling. > + > + > + > + > + > + > + Source information for ring events. > + > + This event does not occur on its own. It is sent before a > + wp_tablet_pad_ring.frame event and carries the source information > + for all events within that frame. > + > + The source specifies how this event was generated. If the source is > + wp_tablet_pad_ring.source.finger, a wp_tablet_pad_ring.stop event > + will be sent when the user lifts the finger off the device. > + > + This event is optional. If the source is unknown for an interaction, > + no event is sent. > + > + > + > + > + > + > + Sent whenever the angle on a
Re: Weston versioning (Re: [PATCH weston 6/6] libweston: do not use weston version in libweston.pc)
On Monday 2016-07-11 16:44, Emil Velikov wrote: >>>http://ometer.com/parallel.html. I would strongly recommend giving it >>>a look. >> >> I read it now, and I do not buy it - at least not for 2016 standards. >> According to the page, it was written in 2002, and I can confirm that >> the situation was much worse then it is now. I can practically refute >> all his points from the "Some more issues:" section, but for brevity, >> I spare you the details for now. > >Trying to get some understanding about your experience in the area - >which distribution(s) do you work with, how many packages do you >maintain ? openSUSE. Enlisted for 441, but my id appears in 2056 changelogs by now and I bear the role of global co-reviewer, i.e. some 9100 packages. And in the paid part of life: SLE, RH6,7, Ubu14/16, Univention, Collax (things you never heard of), Windows(*). So I am also aware about _their_ skeletons-in-the-closet. (*) Not a distro, but an anarchy, which is also interesting to distribute for. ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston] Remove a wrong closing “extern "C"” in shared/xalloc.c
Signed-off-by: Emmanuel Gil Peyrot --- shared/xalloc.c | 5 - 1 file changed, 5 deletions(-) diff --git a/shared/xalloc.c b/shared/xalloc.c index 4bf8a3e..9bf5245 100644 --- a/shared/xalloc.c +++ b/shared/xalloc.c @@ -47,8 +47,3 @@ fail_on_null(void *p, size_t size, char *file, int32_t line) return p; } - - -#ifdef __cplusplus -} -#endif -- 2.9.0 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH wayland-protocols v5 4/4] tablet: Add pad support to the tablet protocol
The pad's interface is similar to the tool interface, a client is notified of the pad after the tablet_added event. The pad has three functionalities: buttons, rings and strips. Buttons are fairly straightforward, rings and strips are separate interfaces with a pointer-axis-like source/value/frame events. The two interfaces are effectively identical but for the actual value they send (degrees vs normalized position). Buttons are sequentially indexed starting with zero, unlike other protocols where a linux/input.h-style semantic event code is used. Since we expect all buttons to have client-specific functionality, an additional event tells the client when a given button index is not available, usually because the compositor assignes some function to it (e.g. mode switching, see below). Specific to the pad device is the set_feedback request which enables a client to set a user-defined string to display for an OSD on the current mappings. This request is available for buttons, rings and strips. Finally, the pad supports groups, effectively sets of button/ring/strip configurations. Those groups may have multiple modes each, so that users/clients may map several actions to a single element. Signed-off-by: Carlos Garnacho Signed-off-by: Peter Hutterer Reviewed-by: Jason Gerecke --- unstable/tablet/tablet-unstable-v2.xml | 540 + 1 file changed, 540 insertions(+) diff --git a/unstable/tablet/tablet-unstable-v2.xml b/unstable/tablet/tablet-unstable-v2.xml index 77b185c..2cc3a92 100644 --- a/unstable/tablet/tablet-unstable-v2.xml +++ b/unstable/tablet/tablet-unstable-v2.xml @@ -172,6 +172,22 @@ + + + + This event is sent whenever a new pad is known to the system. Typically, + pads are physically attached to tablets and a pad_added event is + sent immediately after the wp_tablet_seat.tablet_added. + However, some standalone pad devices logically attach to tablets at + runtime, and the client must wait for wp_tablet_pad.enter to know + the tablet a pad is attached to. + + This event only provides the object id of the pad. All further + features (buttons, strips, rings) are sent through the wp_tablet_pad + interface. + + + @@ -636,4 +652,528 @@ + + + A circular interaction area, such as the touch ring on the Wacom Intuos + Pro series tablets. + + Events on a ring are logically grouped by the wl_tablet_pad_ring.frame + event. + + + + + Request that the compositor use the provided feedback string + associated with this ring. This request should be issued immediately + after a wp_tablet_pad_group.mode_switch event from the corresponding + group is received, or whenever the ring is mapped to a different + action. See wp_tablet_pad_group.mode_switch for more details. + + Clients are encouraged to provide context-aware descriptions for + the actions associated with the ring; compositors may use this + information to offer visual feedback about the button layout + (eg. on-screen displays). + + The provided string 'description' is a UTF-8 encoded string to be + associated with this ring, and is considered user-visible; general + internationalization rules apply. + + The serial argument will be that of the last + wp_tablet_pad_group.mode_switch event received for the group of this + ring. Requests providing other serials than the most recent one will be + ignored. + + + + + + + + This destroys the client's resource for this ring object. + + + + + + Describes the source types for ring events. This indicates to the + client how a ring event was physically generated; a client may + adjust the user interface accordingly. For example, events + from a "finger" source may trigger kinetic scrolling. + + + + + + + Source information for ring events. + + This event does not occur on its own. It is sent before a + wp_tablet_pad_ring.frame event and carries the source information + for all events within that frame. + + The source specifies how this event was generated. If the source is + wp_tablet_pad_ring.source.finger, a wp_tablet_pad_ring.stop event + will be sent when the user lifts the finger off the device. + + This event is optional. If the source is unknown for an interaction, + no event is sent. + + + + + + + Sent whenever the angle on a ring changes. + + The angle is provided in degrees clockwise from the logical + north of the ring in the pad's current rotation. + + + + + + + Stop notification for ring events. + + For some wp_tablet_pad_ring.source types, a wp_tablet_pad_ring.stop + ev
Re: Weston versioning (Re: [PATCH weston 6/6] libweston: do not use weston version in libweston.pc)
On 9 July 2016 at 17:26, Pekka Paalanen wrote: > On Thu, 7 Jul 2016 17:45:24 +0100 > Emil Velikov wrote: > >> On 7 July 2016 at 10:46, Pekka Paalanen wrote: >> > On Mon, 4 Jul 2016 16:25:54 +0100 >> > Emil Velikov wrote: >> > >> >> On 4 July 2016 at 15:35, Quentin Glidic >> >> wrote: >> >> > On 04/07/2016 16:23, Emil Velikov wrote: >> >> >> >> >> >> From: Emil Velikov >> >> >> >> >> >> Signed-off-by: Emil Velikov >> >> >> --- >> >> >> configure.ac | 1 + >> >> >> libweston/libweston.pc.in | 2 +- >> >> >> 2 files changed, 2 insertions(+), 1 deletion(-) >> >> >> >> >> >> diff --git a/configure.ac b/configure.ac >> >> >> index be40f10..46b61ae 100644 >> >> >> --- a/configure.ac >> >> >> +++ b/configure.ac >> >> >> @@ -21,6 +21,7 @@ AC_SUBST([WESTON_VERSION_MINOR], >> >> >> [weston_minor_version]) >> >> >> AC_SUBST([WESTON_VERSION_MICRO], [weston_micro_version]) >> >> >> AC_SUBST([WESTON_VERSION], [weston_version]) >> >> >> AC_SUBST([LIBWESTON_MAJOR], [libweston_major_version]) >> >> >> +AC_SUBST([LIBWESTON_VERSION], >> >> >> [libweston_major_version.libweston_minor_version.libweston_patch_version]) >> >> > >> >> > >> >> > That makes packaging a pain. Although the whole libweston (supposedly >> >> > parallel-installable) is already a pain. >> >> > >> >> > When you have a project with a dep on libweston, you’ll have to dig the >> >> > weston version because the tarball is versioned as weston. >> >> > >> >> > I would only do that once (and if) we split libweston and weston to >> >> > different repositories. >> >> > >> >> Yes splitting libweston into separate repo makes sense. Yet I failed >> >> to see where the actual pain is - there is a minor annoyance, and devs >> >> and/or distro maintainers have learned to deal with a lot nastier >> >> things through the years. >> >> >> >> If anything, having libweston-2 provided by (a future) >> >> libweston-1.12.0 tarball/package would make things even more >> >> confusing/annoying. That is unless one is planning to say "f**k it, >> >> let's decrease the version" upon the split. With the later upsetting a >> >> lot of people. >> > >> > Hi guys, >> > >> > my 2c again: >> > >> > I don't think splitting weston and libweston to separate >> > repositories can happen any time soon, because of the test suite >> > that is specific to weston. I do not want to duplicate the test >> > suite, and I do not want 'make check' in libweston to be useless, >> > so we need to keep them together for now. >> > >> If there were more developers, one could also move the tests into a >> separate repo. Although that (plus libweston) would require extensive >> audit of the private and public headers since atm, things are quite >> fragile (one might even call them busted). > > More developers or not, I am not so optimistic that it would work. > > The tests have code intimately tied to internal APIs of both weston > and libweston. That alone is reason enough. It may not even be > possible to get rid of that, realistically. > > To be able to run libweston, we'd essentially need a copy of weston > anyway, for initializing it. > Had no idea about those. I'd imagine that one could handle those with git submodules, although that might be a bit fragile and it'll require a ton more development effort that there is atm. Just food for though, than a serious suggestion. >> > I also think that we don't need separate versioning for weston and >> > libweston. Let's just use the same for both, now that the plans are >> > clarifying. >> > >> > Yes, it does mean the following: >> > >> > - weston version number will start to deviate from wayland, even >> > when both are still released at the same time >> > >> Don't think this is a serious problem, I've even recall people being >> confused why they are still in sync. > > Heh, cool. > >> > - weston version number will be the same as libweston it uses >> > >> > - MAJOR will start running like hell, we'll probably get to weston >> > 27.0.0 before it slows down, or whatever >> > >> > But, that's all fine with me, considering the confusion any other >> > scheme would raise. >> > >> > I suppose (lib)weston release 1.12.0 would be last MAJOR=1 release >> > of weston, installing libweston-1.so, and then we'd just bump to >> > weston 2.0.0 on the next final release. >> > >> > How's that? >> > >> This sounds like a good plan to me, fwiw. > > Alright. > >> > Hmm... but again, if we use MAJOR like that, then during the >> > development of 2.0.0 we would be still installing libweston-1.so >> > because our version in git master is 1.12.90 until the first >> > pre-release (alpha) of 1.12.91, and so on. That doesn't seem good. >> > How to solve that? Or is it not a problem? >> > >> > The thing is, libweston 1.12.90 will be completely incompatible >> > with libweston 1.12.0. >> > >> > Should .9x be a special case somehow, installed as >> > libweston-1-90.so or such? >> > >> As mentioned elsewhere - one should not rely on development version of >> proje
[PATCH wayland-protocols v5 1/4] tablet: add v2 of the tablet protocol
From: Peter Hutterer This is a straightforward copy/paste with a _v1 -> _v2 rename. No functional changes otherwise. Signed-off-by: Peter Hutterer Reviewed-by: Jason Gerecke Reviewed-by: Carlos Garnacho --- Makefile.am| 1 + unstable/tablet/tablet-unstable-v2.xml | 641 + 2 files changed, 642 insertions(+) create mode 100644 unstable/tablet/tablet-unstable-v2.xml diff --git a/Makefile.am b/Makefile.am index 71d2632..9e2a029 100644 --- a/Makefile.am +++ b/Makefile.am @@ -8,6 +8,7 @@ unstable_protocols = \ unstable/relative-pointer/relative-pointer-unstable-v1.xml \ unstable/pointer-constraints/pointer-constraints-unstable-v1.xml \ unstable/tablet/tablet-unstable-v1.xml \ + unstable/tablet/tablet-unstable-v2.xml \ $(NULL) stable_protocols = \ diff --git a/unstable/tablet/tablet-unstable-v2.xml b/unstable/tablet/tablet-unstable-v2.xml new file mode 100644 index 000..81e9835 --- /dev/null +++ b/unstable/tablet/tablet-unstable-v2.xml @@ -0,0 +1,641 @@ + + + + +Copyright 2014 © Stephen "Lyude" Chandler Paul +Copyright 2015-2016 © Red Hat, Inc. + +Permission is hereby granted, free of charge, to any person +obtaining a copy of this software and associated documentation files +(the "Software"), to deal in the Software without restriction, +including without limitation the rights to use, copy, modify, merge, +publish, distribute, sublicense, and/or sell copies of the Software, +and to permit persons to whom the Software is furnished to do so, +subject to the following conditions: + +The above copyright notice and this permission notice (including the +next paragraph) shall be included in all copies or substantial +portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, +EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF +MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND +NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS +BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN +ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN +CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +SOFTWARE. + + + +This description provides a high-level overview of the interplay between +the interfaces defined this protocol. For details, see the protocol +specification. + +More than one tablet may exist, and device-specifics matter. Tablets are +not represented by a single virtual device like wl_pointer. A client +binds to the tablet manager object which is just a proxy object. From +that, the client requests wp_tablet_manager.get_tablet_seat(wl_seat) +and that returns the actual interface that has all the tablets. With +this indirection, we can avoid merging wp_tablet into the actual Wayland +protocol, a long-term benefit. + +The wp_tablet_seat sends a "tablet added" event for each tablet +connected. That event is followed by descriptive events about the +hardware; currently that includes events for name, vid/pid and +a wp_tablet.path event that describes a local path. This path can be +used to uniquely identify a tablet or get more information through +libwacom. Emulated or nested tablets can skip any of those, e.g. a +virtual tablet may not have a vid/pid. The sequence of descriptive +events is terminated by a wp_tablet.done event to signal that a client +may now finalize any initialization for that tablet. + +Events from tablets require a tool in proximity. Tools are also managed +by the tablet seat; a "tool added" event is sent whenever a tool is new +to the compositor. That event is followed by a number of descriptive +events about the hardware; currently that includes capabilities, +hardware id and serial number, and tool type. Similar to the tablet +interface, a wp_tablet_tool.done event is sent to terminate that initial +sequence. + +Any event from a tool happens on the wp_tablet_tool interface. When the +tool gets into proximity of the tablet, a proximity_in event is sent on +the wp_tablet_tool interface, listing the tablet and the surface. That +event is followed by a motion event with the coordinates. After that, +it's the usual motion, axis, button, etc. events. The protocol's +serialisation means events are grouped by wp_tablet_tool.frame events. + +Two special events (that don't exist in X) are down and up. They signal +"tip touching the surface". For tablets without real proximity +detection, the sequence is: proximity_in, motion, down, frame. + +When the to
[PATCH wayland-protocols v5 3/4] tablet: restrict the cursor surface to one per tool
From: Peter Hutterer The initial approach was to allow one surface to be re-used between tools, seats and even used together as wl_pointer cursor surface. This has a few drawbacks, most of which are related to managing the surface correctly in the compositor. For example, the same cursor surface could have two different hotspots. Animated cursors should animate independently rather than update at the same time. Furthermore: a client cannot know when a surface will cease being used as a cursor surface. The basic assumption of "after focus out" is an implementation detail in the compositor and unless the client unsets the cursor it is not guaranteed that the surface is released. This again makes sharing a surface less obvious - you cannot know if the wl_pointer surface is still in use when you set it for a new wp_tablet_tool. Avoid these headaches (and push some of them to the client) by simply restricting a wl_surface to be assigned to a single tool. For the 99% use case where we have one tablet with two tools (pen + eraser) this means we merely get two extra surfaces, and the two don't usually share the same cursor shape anyway. If sharing is absolutely necessary, a client may still opt to share the underlying wl_buffer. Signed-off-by: Peter Hutterer Reviewed-by: Jason Gerecke Reviewed-by: Carlos Garnacho --- unstable/tablet/tablet-unstable-v2.xml | 12 +--- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/unstable/tablet/tablet-unstable-v2.xml b/unstable/tablet/tablet-unstable-v2.xml index de9217b..77b185c 100644 --- a/unstable/tablet/tablet-unstable-v2.xml +++ b/unstable/tablet/tablet-unstable-v2.xml @@ -225,13 +225,11 @@ and pending input regions become undefined, and the wl_surface is unmapped. - This request gives the surface the role of a cursor. The role - assigned by this request is the same as assigned by - wl_pointer.set_cursor meaning the same surface can be - used both as a wl_pointer cursor and a wp_tablet cursor. If the - surface already has another role, it raises a protocol error. - The surface may be used on multiple tablets and across multiple - seats. + This request gives the surface the role of a wp_tablet_tool cursor. A + surface may only ever be used as the cursor surface for one + wp_tablet_tool. If the surface already has another role or has + previously been used as cursor surface for a different tool, a + protocol error is raised. -- 2.7.4 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH wayland-protocols v5 2/4] tablet: change all degree values from int to wl_fixed
From: Peter Hutterer Signed-off-by: Peter Hutterer Reviewed-by: Jason Gerecke Reviewed-by: Carlos Garnacho --- unstable/tablet/tablet-unstable-v2.xml | 20 ++-- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/unstable/tablet/tablet-unstable-v2.xml b/unstable/tablet/tablet-unstable-v2.xml index 81e9835..de9217b 100644 --- a/unstable/tablet/tablet-unstable-v2.xml +++ b/unstable/tablet/tablet-unstable-v2.xml @@ -478,21 +478,21 @@ Sent whenever one or both of the tilt axes on a tool change. Each tilt - value is in 0.01 of a degree, relative to the z-axis of the tablet. + value is in degrees, relative to the z-axis of the tablet. The angle is positive when the top of a tool tilts along the positive x or y axis. - - + + Sent whenever the z-rotation axis on the tool changes. The - rotation value is in 0.01 of a degree clockwise from the tool's + rotation value is in degrees clockwise from the tool's logical neutral position. - + @@ -510,10 +510,10 @@ Sent whenever the wheel on the tool emits an event. This event contains two values for the same axis change. The degrees value is - in 0.01 of a degree in the same orientation as the - wl_pointer.vertical_scroll axis. The clicks value is in discrete - logical clicks of the mouse wheel. This value may be zero if the - movement of the wheel was less than one logical click. + in the same orientation as the wl_pointer.vertical_scroll axis. The + clicks value is in discrete logical clicks of the mouse wheel. This + value may be zero if the movement of the wheel was less + than one logical click. Clients should choose either value and avoid mixing degrees and clicks. The compositor may accumulate values smaller than a logical @@ -521,7 +521,7 @@ Thus, wl_tablet_tool.wheel events with non-zero clicks values may have different degrees values. - + -- 2.7.4 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH v4 wayland-protocols 4/4] tablet: Add pad support to the tablet protocol
Hi!, On Thu, Jul 7, 2016 at 11:30 PM, Jason Gerecke wrote: > On 06/30/2016 05:16 AM, Jonas Ådahl wrote: >> On Thu, Jun 30, 2016 at 01:10:33PM +0200, Carlos Garnacho wrote: >>> Hi!, >>> >>> On Thu, Jun 30, 2016 at 6:27 AM, Jonas Ådahl wrote: On Tue, Jun 28, 2016 at 11:21:50PM +0200, Carlos Garnacho wrote: > The pad's interface is similar to the tool interface, a client is > notified of > the pad after the tablet_added event. > > The pad has three functionalities: buttons, rings and strips. > Buttons are fairly straightforward, rings and strips are separate > interfaces > with a pointer-axis-like source/value/frame events. > The two interfaces are effectively identical but for the actual value they > send (degrees vs normalized position). > > Buttons are sequentially indexed starting with zero, unlike other > protocols > where a linux/input.h-style semantic event code is used. Since we expect > all > buttons to have client-specific functionality, an additional event tells > the > client when a given button index is not available, usually because the > compositor assignes some function to it (e.g. mode switching, see below). > > Specific to the pad device is the set_feedback request which enables a > client > to set a user-defined string to display for an OSD on the current > mappings. > This request is available for buttons, rings and strips. > > Finally, the pad supports groups, effectively sets of button/ring/strip > configurations. Those groups may have multiple modes each, so that > users/clients may map several actions to a single element. > > Signed-off-by: Carlos Garnacho > Signed-off-by: Peter Hutterer > Reviewed-by: Jason Gerecke > --- > >> >> ... snip ... >> > + > + > + > + Sent on wp_tablet_pad_group initialization to announce available > rings. > + One event is sent for each ring available on this pad group. > + > + This event is sent in the initial burst of events before the > + wp_tablet_pad_group.done event. This event is only sent when at > least > + one ring is available. The last sentence is redundant. It already says that there'll be one event per ring, thus no rings, no events. It also makes it slightly unclear (by the description alone) whethere multiple rings will result in multiple events. >>> >>> Agreed, removed. As for the last part... I guess the "One event is >>> sent for each ring available..." bit makes it clear enough already? >> >> Yes, I think that part makes it clear. It was only the part you now >> removed that made it possible to doubt the first part for a second. >> >>> >> >> ... snip ... >> > + > + > + > + Notification that the mode was switched. > + > + A mode applies to all buttons, rings and strips in a group > + simultaneously, but a client is not required to assign different > actions > + for each mode. For example, a client may have mode-specific button > + mappings but map the ring to vertical scrolling in all modes. Mode > + indices start at 0. > + > + Switching modes is compositor-dependent. The compositor may provide > + visual cues to the client about the mode, e.g. by toggling LEDs on > + the tablet device. Mode-switching may be software-controlled or > + controlled by one or more physical buttons. For example, on a Wacom > + Intuos Pro, the button inside the ring may be assigned to switch > + between modes. > + > + The current mode is compositor-global, the compositor will also send Isn't it just seat global? And why would this event be on the pad group and not on the seat wide interface? >>> >>> perhaps I should expand that first phrase :). >>> >>> The extent of the current mode is pretty much group local, it only >>> affects the buttons/rings/strips that have been announced through this >>> group. >>> >>> However, the current mode of every given group is global to all >>> clients. If you have a client focused with a group in mode=1, focus >>> another client, switch that group to mode=3 and go back to the first >>> client, you'll receive a .mode_switch with mode=3 after >>> wp_tablet_pad.enter. >>> >>> This could be considered similar to keyboard modifiers, you don't >>> reset those (eg. caps lock) when focusing another client, the current >>> state is rather propagated to the newly focused client. >>> >>> I'm replacing the beginning of that paragraph with this: >>> "Compositors will globally track the current mode that every >>> wp_tablet_pad_group has. This event will also be sent after >>> wp_tablet_pad.enter for all groups in the pad in order to..." >> >> I see. Any reason on why it needs to be compositor global? Couldn't that >> be a decision l
Re: [PATCH weston 3/6] README: Document versioning scheme, forward compatibility
On 9 July 2016 at 16:32, Pekka Paalanen wrote:
> On Thu, 7 Jul 2016 20:08:40 +0200
> Quentin Glidic wrote:
>
>> On 07/07/2016 18:11, Emil Velikov wrote:
>> > On 7 July 2016 at 10:05, Pekka Paalanen wrote:
>> >>
>> >> [snip]
>> >>
>> >> Now that you mentioned the semantics could be of upper or lower
>> >> limit, the name should imply the meaning. I only thought of using
>> >> it as both lower limit (as in pkg-config check) *and* upper limit
>> >> (do not declare API that was added after the given version or
>> >> remove API that was deprecated after the given version).
>> >>
>> > Ack. Considering I already know how it would/should be used, I'm a bit
>> > short on unbiased name. Your earlier suggestion sounds great imho, but
>> > if you have something else in mind please share it :-)
>> >
>> >> However, I also realized something more.
>> >>
>> >> If you do like I suggested for configure.ac, you would have trouble
>> >> using pre-releases of libweston.
>> >>
>> >> Pre-releases of, say 1.13.0, will be released as 1.12.9x. That
>> >> would be MINOR=12, so if you use the same numbers for both
>> >> pkg-config check and for the CPP macro for API, you cannot test the
>> >> 13 API until it has been finally released.
>> >>
>> >> So maybe those two need to be separate anyway?
>> >>
>> > The example will cover this confusion, but the gist is that:
>> > - No released version of software X should rely on development
>> > version of libweston/libfoo.
>
> Hi Emil,
>
> an unreleased project must be able to depend on unreleased
> libweston.
>
> That's how the project communicates to its testers and developers
> explicitly in configure.ac that it needs an unreleased version of
> libweston. And we need projects to be able to do that, so that we
> can have people test unreleased APIs of libweston.
>
> This is what we currently do with 1.y.90 version. We never release
> a 1.y.90, but projects can still check for and require it. It's
> been working fine between wayland and weston so far, AFAIK.
>
> Version 1.y.90 is not a proper version, it does not tell you which
> revision of the code is the right one, but it does say "WIP" which
> IMO is an important message.
>
> If the user project then gets released with a dependency to a
> library version 1.y.90, that's their fault. Hopefully they kept up
> to date, so that release 1.y.91 satisfies the dependency.
>
Fully agree.
Speaking of which... I don't think we got the wayland patch for mesa
in. /me double-checks.
>> > - If there's a serious need to void the above, one should set the
>> > REQUIRED_API to the number it would be in an official/final version.
>
> Indeed.
>
> This is why my desire to derive both pkg-config version check and
> the REQUIRED_API define from the single same source variable does
> not work after all.
>
> So, I think you're right, and we agree.
>
>> >
>> > Example:
>> > libweston (devel. process):
>> > - git tag 1.12.90
>> > - Add API X, guarded behind version 0x010d version check (one could
>> > keep the numbers decimal and/or other)
>> > - other...
>> >
>> > User:
>> > - Bump required version to 1.12.90
>> > - Use "-DREQUIRED_API_VERSION=0x010d"
>> > Personally I'm leaning towards adding the define only in the
>> > respective places that need the newer API, but having it in configure
>> > (global scale) would be nice as well.
>>
>> Regarding that, I’d go the GLib/GTK+ way completely.
>> That means each function has a
>> WESTON_AVAILABLE_IN(1, 13)
>>
>> and *if* a project set
>> WESTON_VERSION_{MIN,MAX}_ALLOWED, that will make these functions
>> deprecated (not sure for the ones above the MAX).
>>
>> The maint point is that I would rather have the new functions without
>> needing to update the define. Or most projects will use 0x as the
>> value. :-)
>
> Hi Quentin,
>
> I would not have new functions exposed without updating the define
> value in user projects. Why? It gives the libweston users *both*
> options.
>
> I don't consider using 0x as a hack. I see it as an opt-in to
> the scheme you are proposing, to get new functions exposed
> automatically, which also puts you at risk of accidentally
> depending on a newer version of libweston than before.
>
> If we expose new functions regardless the define, then no-one can
> choose to be safe from that accident, even if they wanted to.
>
> Oh, reading your later replies, it seems you are only arguing over
> which policy should be the default. I'd say neither. Make the build
> just error out if the user defines neither way. The definition will
> be just one line in the user project's config.h anyway.
>
>
You're right - calling 0x names isn't the most mature thing, sorry
about that.
And yes, the goal here is/was to error out by default in order to
avoid accidental damage. Anyone not interested in this safety switch
can add one line to override it.
Thanks for guidance in all this.
Emil
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.or
Re: Weston versioning (Re: [PATCH weston 6/6] libweston: do not use weston version in libweston.pc)
On 10 July 2016 at 13:23, Jan Engelhardt wrote: > > On Sunday 2016-07-10 12:46, Emil Velikov wrote: >>> PKG_CHECK_EXISTS([gtk-3.0], [PKG_CHECK_MODULES([gtk], [gtk-3.0])], [ >>> ...repeat the fun... >>> ])] >>> >>Yes, it's one line of fun for each version that you want to be >>compatible with. It's not ideal, but it's a price to pay, for keeping >>things compatible/sane. > > Without pkgconfig supporting some new alias tag (hint, hint) to cover > such a case, No idea what such a "alias tag" is supposed to do/look like. Do you have examples ? > I predict that whoever uses libfoo-LotsANumbersHere > limits themselves to 3 or maybe 4 PKG_CHECK_MODULES calls, because it > is looking real silly. > If (and that's a _huge_ if) they support more than 3-4 versions of libfoo, then the configure.ac will be the least ugly thing. >>> The .so symlink is really only for /usr/bin/ld. It does not >>> play a role for ld.so (ld-linux.so.*) and therefore won't >>> interfere with plugins living in versioned directories. >>> (Cf. libmirage from cdemu) >>> >>The (main?) problem [...] is that we [...] rely on libfoo.so being >>correct. >>I get the feeling that you have not read the following >>http://ometer.com/parallel.html. I would strongly recommend giving it >>a look. > > I read it now, and I do not buy it - at least not for 2016 standards. > According to the page, it was written in 2002, and I can confirm that > the situation was much worse then it is now. I can practically refute > all his points from the "Some more issues:" section, but for brevity, > I spare you the details for now. > It's fine to disagree. From what I've seen working with package maintainers and other my own experience, most people (still) share the sentiments mentioned in the page. Trying to get some understanding about your experience in the area - which distribution(s) do you work with, how many packages do you maintain ? > >>> Or you can just have two lines in configure.ac next to each other >>> which will scream at you "update me in lockstep" everytime you look >>> at them because you modify one of them. >>> >>> AC_INIT([libweston], [1.12.0]) >>> libweston_SONUM=3 >>> -> >>> AC_INIT([libweston], [1.12.90]) >>> libweston_SONUM=27 >>> >>This could also work, but Pekka's idea sounds more robust and simpler >>in the long run. Add the logic once and don't bother checking/keeping >>things in sync ;-) > > Feels wrong to pollute configure.ac with code that tries to be > "smart" but is foremost disproportional in size to the problem. One could handle this in multiple ways - in configure, separate script and/or hooks. > (And > you wonder why people hate $build_system_du_jour.) Would a commit > hook not be better to run a check that SONUM is in good shape w.r.t. > VERSION? > As _everything_ in life, people hate X because they don't understand it. Once your project gets complex enough and your build system is comprehensive enough to handle it things always looks scary/nasty. But all of that is an orthogonal, imho. -Emil ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston v3] libweston: Position layers in an absolute way
From: Quentin Glidic
Currently, layers’ order depends on the module loading order and it does
not survive runtime modifications (like shell locking/unlocking).
With this patch, modules can safely add their own layer at the expected
position in the stack, with runtime persistence.
Signed-off-by: Quentin Glidic
Acked-by: Pekka Paalanen
---
v3:
- Added weston_layer_unset_position
- HIDDEN is now still rendered
(Feature needed by Giulio Camuffo)
- weston_layer now stores the weston_compositor pointer to avoid its
need in set_position
- Reordered weston_layer members (we break the ABI anyway)
- weston_layer_init now only initialize the struct, you must
call set_position to add it to the layer list
- BACKGROUND is now 2 instead of 1, as these values are mainly meant
for libweston modules. The compositor should simply use
(BACKGROUND - 1) if it wants to support such modules with
a fallback surface
(Suggestion by Bill Spitzak)
v2:
- Tiny commit message addition: added runtime behaviour comment.
- Reworked (a lot) the enum comment to explain further the position
values and their actual expected scope. I hope their are clear enough.
Here are the biggest changes:
- Added a BOTTOM_UI value for widgets and conky-like applications.
- Changed BACKGROUND to be 1, as nothing should be under it
- Added a comment about mandatory background
desktop-shell/input-panel.c | 6 ++--
desktop-shell/shell.c | 64 +
fullscreen-shell/fullscreen-shell.c | 4 ++-
ivi-shell/input-panel-ivi.c | 6 ++--
ivi-shell/ivi-layout.c | 4 ++-
ivi-shell/ivi-shell.c | 2 +-
libweston/compositor.c | 52 ---
libweston/compositor.h | 70 +++--
tests/weston-test.c | 3 +-
9 files changed, 163 insertions(+), 48 deletions(-)
diff --git a/desktop-shell/input-panel.c b/desktop-shell/input-panel.c
index 0d003b1..23e22b1 100644
--- a/desktop-shell/input-panel.c
+++ b/desktop-shell/input-panel.c
@@ -114,8 +114,8 @@ show_input_panels(struct wl_listener *listener, void *data)
shell->showing_input_panels = true;
if (!shell->locked)
- wl_list_insert(&shell->compositor->cursor_layer.link,
- &shell->input_panel_layer.link);
+ weston_layer_set_position(&shell->input_panel_layer,
+ WESTON_LAYER_POSITION_TOP_UI);
wl_list_for_each_safe(ipsurf, next,
&shell->input_panel.surfaces, link) {
@@ -140,7 +140,7 @@ hide_input_panels(struct wl_listener *listener, void *data)
shell->showing_input_panels = false;
if (!shell->locked)
- wl_list_remove(&shell->input_panel_layer.link);
+ weston_layer_unset_position(&shell->input_panel_layer);
wl_list_for_each_safe(view, next,
&shell->input_panel_layer.view_list.link,
diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
index c72f801..bdaad87 100644
--- a/desktop-shell/shell.c
+++ b/desktop-shell/shell.c
@@ -1079,13 +1079,13 @@ seat_destroyed(struct wl_listener *listener, void *data)
}
static struct workspace *
-workspace_create(void)
+workspace_create(struct desktop_shell *shell)
{
struct workspace *ws = malloc(sizeof *ws);
if (ws == NULL)
return NULL;
- weston_layer_init(&ws->layer, NULL);
+ weston_layer_init(shell->compositor, &ws->layer);
wl_list_init(&ws->focus_list);
wl_list_init(&ws->seat_destroyed_listener.link);
@@ -1124,7 +1124,7 @@ activate_workspace(struct desktop_shell *shell, unsigned
int index)
struct workspace *ws;
ws = get_workspace(shell, index);
- wl_list_insert(&shell->panel_layer.link, &ws->layer.link);
+ weston_layer_set_position(&ws->layer, WESTON_LAYER_POSITION_NORMAL);
shell->workspaces.current = index;
}
@@ -4596,20 +4596,16 @@ resume_desktop(struct desktop_shell *shell)
{
struct workspace *ws = get_current_workspace(shell);
- wl_list_remove(&shell->lock_layer.link);
- if (shell->showing_input_panels) {
- wl_list_insert(&shell->compositor->cursor_layer.link,
- &shell->input_panel_layer.link);
- wl_list_insert(&shell->input_panel_layer.link,
- &shell->fullscreen_layer.link);
- } else {
- wl_list_insert(&shell->compositor->cursor_layer.link,
- &shell->fullscreen_layer.link);
- }
- wl_list_insert(&shell->fullscreen_layer.link,
- &shell->panel_layer.link);
- wl_list_insert(&shell->panel_layer.link,
- &ws->layer.link),
+ weston_layer_unset_position(&shell->lock_layer);
+
+ if (shell->showing_
Re: [PATCH weston v2] libweston: Position layers in an absolute way
On 11/07/2016 10:23, Giulio Camuffo wrote:
2016-07-09 20:45 GMT+02:00 Quentin Glidic :
From: Quentin Glidic
Currently, layers’ order depends on the module loading order and it does
not survive runtime modifications (like shell locking/unlocking).
With this patch, modules can safely add their own layer at the expected
position in the stack, with runtime persistence.
Signed-off-by: Quentin Glidic
---
v2:
- Tiny commit message addition: added runtime behaviour comment.
- Reworked (a lot) the enum comment to explain further the position
values and their actual expected scope. I hope their are clear enough.
Here are the biggest changes:
- Added a BOTTOM_UI value for widgets and conky-like applications.
- Changed BACKGROUND to be 1, as nothing should be under it
- Added a comment about mandatory background
As a side question: if this semantic regarding background surface is
accepted, would that be relevant for libweston to provide a “real” fallback
black surface if no BACKGROUND layer is found?
desktop-shell/input-panel.c | 9 +++--
desktop-shell/shell.c | 80 +++--
fullscreen-shell/fullscreen-shell.c | 3 +-
ivi-shell/input-panel-ivi.c | 9 +++--
ivi-shell/ivi-layout.c | 3 +-
ivi-shell/ivi-shell.c | 3 +-
libweston/compositor.c | 34 +---
libweston/compositor.h | 59 ++-
tests/weston-test.c | 2 +-
9 files changed, 156 insertions(+), 46 deletions(-)
diff --git a/desktop-shell/input-panel.c b/desktop-shell/input-panel.c
index 0d003b1..f712289 100644
--- a/desktop-shell/input-panel.c
+++ b/desktop-shell/input-panel.c
@@ -114,8 +114,9 @@ show_input_panels(struct wl_listener *listener, void *data)
shell->showing_input_panels = true;
if (!shell->locked)
- wl_list_insert(&shell->compositor->cursor_layer.link,
- &shell->input_panel_layer.link);
+ weston_layer_set_position(shell->compositor,
+ &shell->input_panel_layer,
+ WESTON_LAYER_POSITION_TOP_UI);
wl_list_for_each_safe(ipsurf, next,
&shell->input_panel.surfaces, link) {
@@ -140,7 +141,9 @@ hide_input_panels(struct wl_listener *listener, void *data)
shell->showing_input_panels = false;
if (!shell->locked)
- wl_list_remove(&shell->input_panel_layer.link);
+ weston_layer_set_position(shell->compositor,
+ &shell->input_panel_layer,
+ WESTON_LAYER_POSITION_HIDDEN);
wl_list_for_each_safe(view, next,
&shell->input_panel_layer.view_list.link,
diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
index c72f801..803ff33 100644
--- a/desktop-shell/shell.c
+++ b/desktop-shell/shell.c
@@ -1079,13 +1079,14 @@ seat_destroyed(struct wl_listener *listener, void *data)
}
static struct workspace *
-workspace_create(void)
+workspace_create(struct desktop_shell *shell)
{
struct workspace *ws = malloc(sizeof *ws);
if (ws == NULL)
return NULL;
- weston_layer_init(&ws->layer, NULL);
+ weston_layer_init(shell->compositor, &ws->layer,
+ WESTON_LAYER_POSITION_HIDDEN);
wl_list_init(&ws->focus_list);
wl_list_init(&ws->seat_destroyed_listener.link);
@@ -1124,7 +1125,9 @@ activate_workspace(struct desktop_shell *shell, unsigned
int index)
struct workspace *ws;
ws = get_workspace(shell, index);
- wl_list_insert(&shell->panel_layer.link, &ws->layer.link);
+ weston_layer_set_position(shell->compositor,
+ &ws->layer,
+ WESTON_LAYER_POSITION_NORMAL);
shell->workspaces.current = index;
}
@@ -4596,20 +4599,22 @@ resume_desktop(struct desktop_shell *shell)
{
struct workspace *ws = get_current_workspace(shell);
- wl_list_remove(&shell->lock_layer.link);
- if (shell->showing_input_panels) {
- wl_list_insert(&shell->compositor->cursor_layer.link,
- &shell->input_panel_layer.link);
- wl_list_insert(&shell->input_panel_layer.link,
- &shell->fullscreen_layer.link);
- } else {
- wl_list_insert(&shell->compositor->cursor_layer.link,
- &shell->fullscreen_layer.link);
- }
- wl_list_insert(&shell->fullscreen_layer.link,
- &shell->panel_layer.link);
- wl_list_insert(&shell->panel_layer.link,
- &ws->layer.link),
+ weston_layer_set_position(shell->compositor,
+ &shell->lock_layer,
+
Re: [PATCH weston 2/3] modules: Drop module_init as a shared init function
2016-07-04 15:58 GMT+02:00 Quentin Glidic :
> From: Quentin Glidic
>
> Use different functions so we cannot load a libweston module in weston
> or the other way around.
>
> Also properly namespace backend_init and use a different name for weston
> shells.
>
> Signed-off-by: Quentin Glidic
> ---
> compositor/cms-colord.c | 5 +++--
> compositor/cms-static.c | 4 ++--
> compositor/main.c | 45
> ++---
> compositor/screen-share.c | 4 ++--
> compositor/weston.h | 13 ++-
> desktop-shell/shell.c | 4 ++--
> fullscreen-shell/fullscreen-shell.c | 5 +++--
> ivi-shell/ivi-layout.c | 4 +++-
> ivi-shell/ivi-shell.c | 4 ++--
> libweston/compositor-drm.c | 4 ++--
> libweston/compositor-fbdev.c| 4 ++--
> libweston/compositor-headless.c | 4 ++--
> libweston/compositor-rdp.c | 4 ++--
> libweston/compositor-wayland.c | 4 ++--
> libweston/compositor-x11.c | 4 ++--
> libweston/compositor.c | 23 ---
> libweston/compositor.h | 7 +++---
> tests/plugin-registry-test.c| 4 +++-
> tests/surface-global-test.c | 4 +++-
> tests/surface-screenshot.c | 5 +++--
> tests/surface-test.c| 4 +++-
> tests/weston-test.c | 4 ++--
> xwayland/launcher.c | 3 +--
> 23 files changed, 111 insertions(+), 55 deletions(-)
>
> diff --git a/compositor/cms-colord.c b/compositor/cms-colord.c
> index b9bc9e3..b9938b9 100644
> --- a/compositor/cms-colord.c
> +++ b/compositor/cms-colord.c
> @@ -33,6 +33,7 @@
> #include
>
> #include "compositor.h"
> +#include "weston.h"
> #include "cms-helper.h"
> #include "shared/helpers.h"
>
> @@ -494,8 +495,8 @@ colord_cms_output_destroy(gpointer data)
> }
>
> WL_EXPORT int
> -module_init(struct weston_compositor *ec,
> - int *argc, char *argv[])
> +wet_module_init(struct weston_compositor *ec,
> + int *argc, char *argv[])
> {
> gboolean ret;
> GError *error = NULL;
> diff --git a/compositor/cms-static.c b/compositor/cms-static.c
> index a6bbfd4..e24501b 100644
> --- a/compositor/cms-static.c
> +++ b/compositor/cms-static.c
> @@ -91,8 +91,8 @@ cms_notifier_destroy(struct wl_listener *listener, void
> *data)
>
>
> WL_EXPORT int
> -module_init(struct weston_compositor *ec,
> - int *argc, char *argv[])
> +wet_module_init(struct weston_compositor *ec,
> + int *argc, char *argv[])
> {
> struct cms_static *cms;
> struct weston_output *output;
> diff --git a/compositor/main.c b/compositor/main.c
> index 4e6b7ae..88f7911 100644
> --- a/compositor/main.c
> +++ b/compositor/main.c
> @@ -704,7 +704,7 @@ weston_create_listening_socket(struct wl_display
> *display, const char *socket_na
> }
>
> WL_EXPORT void *
> -wet_load_module(const char *name, const char *entrypoint)
> +wet_load_module_entrypoint(const char *name, const char *entrypoint)
> {
> const char *builddir = getenv("WESTON_BUILD_DIR");
> char path[PATH_MAX];
> @@ -746,14 +746,46 @@ wet_load_module(const char *name, const char
> *entrypoint)
> return init;
> }
>
> +static int
> +wet_load_shell(struct weston_compositor *compositor,
> + const char *name, int *argc, char *argv[])
> +{
> + int (*shell_init)(struct weston_compositor *ec,
> + int *argc, char *argv[]);
> +
> + shell_init = wet_load_module_entrypoint(name, "wet_shell_init");
> + if (!shell_init)
> + shell_init = wet_load_module_entrypoint(name, "module_init");
Why do you keep the fallback for "module_init" here? Modules developed
against an older weston won't likely work anyway due to other changes,
so i think we can just jump to the new entrypoint name.
> + if (!shell_init)
> + return -1;
> + if (shell_init(compositor, argc, argv) < 0)
> + return -1;
> + return 0;
> +}
> +
> +WL_EXPORT int
> +wet_load_module(struct weston_compositor *compositor,
> + const char *name, int *argc, char *argv[])
> +{
> + int (*module_init)(struct weston_compositor *ec,
> + int *argc, char *argv[]);
> +
> + module_init = wet_load_module_entrypoint(name, "wet_module_init");
> + if (!module_init)
> + module_init = wet_load_module_entrypoint(name, "module_init");
> + if (!module_init)
> + return -1;
> + if (module_init(compositor, argc, argv) < 0)
> + return -1;
> + return 0;
> +}
> +
> static int
> load_modules(struct weston_compositor *ec, const char *modules,
> int *argc, char *argv[])
> {
> const char *p, *end;
> char buffer[256];
> - int (*module_init)(struct weston_compositor *ec,
> -
Re: [PATCH weston 1/3] weston: Add a specific option to load XWayland
Hi,
This looks good to me but i wonder, since it's a behavior change, it
should be mentioned in the changelog. I think we should start using a
[Changelog] tag or something in the commit message to make fishing
them out of the git history easier.
With that, Reviewed-by: Giulio Camuffo
2016-07-04 16:04 GMT+02:00 Quentin Glidic :
> On 04/07/2016 16:02, Michael Schellenberger Costa wrote:
>>
>> Hi Quentin,
>>
>> Am 04.07.2016 um 15:58 schrieb Quentin Glidic:
>>>
>>> From: Quentin Glidic
>>>
>>> Signed-off-by: Quentin Glidic
>>> ---
>>> compositor/main.c | 25 +++--
>>> man/weston.ini.man | 7 +--
>>> man/weston.man | 7 +--
>>> tests/weston-tests-env | 7 ---
>>> weston.ini.in | 3 ++-
>>> 5 files changed, 31 insertions(+), 18 deletions(-)
>>>
>>> diff --git a/compositor/main.c b/compositor/main.c
>>> index 6cf9194..4e6b7ae 100644
>>> --- a/compositor/main.c
>>> +++ b/compositor/main.c
>>> [snip]
>>> + if (!xwayland)
>>> + weston_config_section_get_bool(section, "xwayland",
>>> &xwayland,
>>> + false);
>>> + if (xwayland) {
>>
>> Can you make that the else clause?
>
>
> No, this pattern is there so we get the config file value as fallback.
>
>
>> --Michael
>>
>>> [snip]
>
>
>
> --
>
> Quentin “Sardem FF7” Glidic
>
> ___
> 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 v2] libweston: Position layers in an absolute way
2016-07-09 20:45 GMT+02:00 Quentin Glidic :
> From: Quentin Glidic
>
> Currently, layers’ order depends on the module loading order and it does
> not survive runtime modifications (like shell locking/unlocking).
> With this patch, modules can safely add their own layer at the expected
> position in the stack, with runtime persistence.
>
> Signed-off-by: Quentin Glidic
> ---
>
>
> v2:
> - Tiny commit message addition: added runtime behaviour comment.
> - Reworked (a lot) the enum comment to explain further the position
>values and their actual expected scope. I hope their are clear enough.
>Here are the biggest changes:
> - Added a BOTTOM_UI value for widgets and conky-like applications.
> - Changed BACKGROUND to be 1, as nothing should be under it
> - Added a comment about mandatory background
>
> As a side question: if this semantic regarding background surface is
> accepted, would that be relevant for libweston to provide a “real” fallback
> black surface if no BACKGROUND layer is found?
>
> desktop-shell/input-panel.c | 9 +++--
> desktop-shell/shell.c | 80
> +++--
> fullscreen-shell/fullscreen-shell.c | 3 +-
> ivi-shell/input-panel-ivi.c | 9 +++--
> ivi-shell/ivi-layout.c | 3 +-
> ivi-shell/ivi-shell.c | 3 +-
> libweston/compositor.c | 34 +---
> libweston/compositor.h | 59 ++-
> tests/weston-test.c | 2 +-
> 9 files changed, 156 insertions(+), 46 deletions(-)
>
> diff --git a/desktop-shell/input-panel.c b/desktop-shell/input-panel.c
> index 0d003b1..f712289 100644
> --- a/desktop-shell/input-panel.c
> +++ b/desktop-shell/input-panel.c
> @@ -114,8 +114,9 @@ show_input_panels(struct wl_listener *listener, void
> *data)
> shell->showing_input_panels = true;
>
> if (!shell->locked)
> - wl_list_insert(&shell->compositor->cursor_layer.link,
> - &shell->input_panel_layer.link);
> + weston_layer_set_position(shell->compositor,
> + &shell->input_panel_layer,
> + WESTON_LAYER_POSITION_TOP_UI);
>
> wl_list_for_each_safe(ipsurf, next,
> &shell->input_panel.surfaces, link) {
> @@ -140,7 +141,9 @@ hide_input_panels(struct wl_listener *listener, void
> *data)
> shell->showing_input_panels = false;
>
> if (!shell->locked)
> - wl_list_remove(&shell->input_panel_layer.link);
> + weston_layer_set_position(shell->compositor,
> + &shell->input_panel_layer,
> + WESTON_LAYER_POSITION_HIDDEN);
>
> wl_list_for_each_safe(view, next,
> &shell->input_panel_layer.view_list.link,
> diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
> index c72f801..803ff33 100644
> --- a/desktop-shell/shell.c
> +++ b/desktop-shell/shell.c
> @@ -1079,13 +1079,14 @@ seat_destroyed(struct wl_listener *listener, void
> *data)
> }
>
> static struct workspace *
> -workspace_create(void)
> +workspace_create(struct desktop_shell *shell)
> {
> struct workspace *ws = malloc(sizeof *ws);
> if (ws == NULL)
> return NULL;
>
> - weston_layer_init(&ws->layer, NULL);
> + weston_layer_init(shell->compositor, &ws->layer,
> + WESTON_LAYER_POSITION_HIDDEN);
>
> wl_list_init(&ws->focus_list);
> wl_list_init(&ws->seat_destroyed_listener.link);
> @@ -1124,7 +1125,9 @@ activate_workspace(struct desktop_shell *shell,
> unsigned int index)
> struct workspace *ws;
>
> ws = get_workspace(shell, index);
> - wl_list_insert(&shell->panel_layer.link, &ws->layer.link);
> + weston_layer_set_position(shell->compositor,
> + &ws->layer,
> + WESTON_LAYER_POSITION_NORMAL);
>
> shell->workspaces.current = index;
> }
> @@ -4596,20 +4599,22 @@ resume_desktop(struct desktop_shell *shell)
> {
> struct workspace *ws = get_current_workspace(shell);
>
> - wl_list_remove(&shell->lock_layer.link);
> - if (shell->showing_input_panels) {
> - wl_list_insert(&shell->compositor->cursor_layer.link,
> - &shell->input_panel_layer.link);
> - wl_list_insert(&shell->input_panel_layer.link,
> - &shell->fullscreen_layer.link);
> - } else {
> - wl_list_insert(&shell->compositor->cursor_layer.link,
> - &shell->fullscreen_layer.link);
> - }
> - wl_list_insert(&shell->fullscreen_layer.link,
> - &shell->panel_layer.link);
> - wl_list_insert(&shell->panel_layer.link,
> -
