Re: [weston, v2, 17/17] configure: add an option to allow building only the libraries
2015-05-16 7:43 GMT+03:00 Bryce Harrington : > On Mon, May 11, 2015 at 09:27:38AM +0300, Pekka Paalanen wrote: >> On Sat, 9 May 2015 10:09:21 +0300 >> Giulio Camuffo wrote: >> >> > 2015-05-09 3:15 GMT+03:00 Bryce Harrington : >> > > On Thu, Dec 04, 2014 at 11:01:23PM +0200, Giulio Camuffo wrote: >> > >> the --enable/disable-weston-binaries emable or disable the creation >> > >> of the 'weston', 'weston-launch' and all the binaries that are >> > >> installed in $prefix/lib/libexec. This allows, together with >> > >> --enable-clients, to only build the libraries, making possible to >> > >> build and install different libweston versions at the same time. >> > > >> > > Prior to 1.7 there was uncertainty whether to have a libweston. Since >> > > then, the uncertainty has gone away and it sounds like we're now all >> > > more or less in consensus favoring this change? >> > > >> > > However, this patchset hasn't yet landed, and there's still some >> > > questions about specifics. >> > > >> > > Should we press ahead with an eye towards getting this landed for 1.8? >> > > Or should we postpone and target landing it as soon as the 1.9 tree >> > > opens? >> > >> > I don't think it'd make sense to push it into 1.8 now, it is a rather >> > big change and may introduce regressions. 1.9 seems a better idea >> > imho. >> >> Yeah, it is far far too late for 1.8. >> >> This is such a big change, that it would be better to land it early in >> a development cycle, like before the mid-point to the next alpha. That >> would give us more experience on how it works, and if there are >> problems, we should have time to fix them properly. > > Okay, in this case Guilio, in the next couple weeks can you rebase this > to trunk (hopefully there's not a huge amount changed) and re-propose it > on the mailing list? Hopefully as things are stabilized for the release > we shouldn't see much changing in the repo. Sure. Actually my branch on github is already on top of master, or almost. -- Giulio > > Then, once 1.9 is open we can proceed with getting it landed first thing. > > We will also require tests and API documentation. > > Bryce ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH xwayland] xwayland: do not add output into output_list multiple times
I just made that change in my set of patches, and I think it fixes the
problem.
On Thu, May 14, 2015 at 9:43 AM, Dima Ryazanov wrote:
> Actually, why not just move "xorg_list_append(&xwl_output->link,
> &xwl_screen->output_list);" to xwl_output_create?
>
> I can't tell if there's a reason it's in xwl_output_done, or if it's just
> an oversight.
>
> On Thu, May 14, 2015 at 9:37 AM, Dima Ryazanov wrote:
>
>> Oh wow, I was playing around with outputs, and never realized
>> output_handle_done
>> was being called after any geometry change, not just after the output was
>> created.
>>
>> On Thu, May 14, 2015 at 2:58 AM, Marek Chalupa
>> wrote:
>>
>>> output.done event can be sent even on some property change, not only
>>> when announcing the output. Therefore we must check if we already have it
>>> otherwise we may corrupt the list by adding it multiple times.
>>>
>>> This fixes bug when xwayland looped indefinitely in output.done handler
>>> and that can be reproduced following these steps (under X without
>>> multi-monitor setup):
>>> 1) run weston --output-count=2
>>> 2) run xterm, move it so that half is on one output
>>> and half on the other
>>> 3) close second output, try run weston-terminal
>>>
>>
>> (I can only repro it after closing the first output, not the second one;
>> is that what you meant?)
>>
>>
>>> weston sends updated outputs which trigger this bug.
>>>
>>> Signed-off-by: Marek Chalupa
>>> ---
>>> hw/xwayland/xwayland-output.c | 25 +++--
>>> 1 file changed, 19 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/hw/xwayland/xwayland-output.c
>>> b/hw/xwayland/xwayland-output.c
>>> index 155cbc1..0c96e87 100644
>>> --- a/hw/xwayland/xwayland-output.c
>>> +++ b/hw/xwayland/xwayland-output.c
>>> @@ -116,15 +116,28 @@ output_handle_mode(void *data, struct wl_output
>>> *wl_output, uint32_t flags,
>>> static void
>>> output_handle_done(void *data, struct wl_output *wl_output)
>>> {
>>> -struct xwl_output *xwl_output = data;
>>> +struct xwl_output *it, *xwl_output = data;
>>> struct xwl_screen *xwl_screen = xwl_output->xwl_screen;
>>> -int width, height;
>>> +int width = 0, height = 0, has_this_output = 0;
>>> +
>>> +xorg_list_for_each_entry(it, &xwl_screen->output_list, link) {
>>> +/* output done event is sent even when some property
>>> + * of output is changed. That means that we may already
>>> + * have this output. If it is true, we must not add it
>>> + * into the output_list otherwise we'll corrupt it */
>>> +if (it == xwl_output)
>>> +has_this_output = 1;
>>> +
>>> +if (width < it->x + it->width)
>>> +width = it->x + it->width;
>>> +if (height < it->y + it->height)
>>> +height = it->y + it->height;
>>> +}
>>>
>>> -xorg_list_append(&xwl_output->link, &xwl_screen->output_list);
>>> +if (!has_this_output) {
>>> +xorg_list_append(&xwl_output->link, &xwl_screen->output_list);
>>>
>>
>> I think this line should also be moved here:
>> xwl_screen->expecting_event--;
>>
>> (It might not matter since it's only used by xwl_screen_init - but the
>> code seems to assume it would only be decremented once after the output is
>> created.)
>>
>> -width = 0;
>>> -height = 0;
>>> -xorg_list_for_each_entry(xwl_output, &xwl_screen->output_list,
>>> link) {
>>> +/* we did not check this output for new screen size, do it now
>>> */
>>> if (width < xwl_output->x + xwl_output->width)
>>> width = xwl_output->x + xwl_output->width;
>>> if (height < xwl_output->y + xwl_output->height)
>>> --
>>> 2.1.0
>>>
>>> ___
>>> wayland-devel mailing list
>>> wayland-devel@lists.freedesktop.org
>>> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
>>>
>>
>> Just a thought... You could instead:
>> - check if the output already exists
>> - add it if necessary
>> - update the width and height
>>
>> That way, the width/height calculation code won't be duplicated. (Though
>> it will require iterating over output_list twice.) Anyways, it's up to you.
>>
>>
>
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH v2 3/3] xwayland: Destroy xwl_output when wl_output gets removed
This makes Xwayland correctly handle a monitor getting unplugged.
Signed-off-by: Dima Ryazanov
---
hw/xwayland/xwayland-output.c | 1 +
hw/xwayland/xwayland.c| 10 +-
hw/xwayland/xwayland.h| 1 +
3 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/hw/xwayland/xwayland-output.c b/hw/xwayland/xwayland-output.c
index 41937b8..9ef8a48 100644
--- a/hw/xwayland/xwayland-output.c
+++ b/hw/xwayland/xwayland-output.c
@@ -163,6 +163,7 @@ xwl_output_create(struct xwl_screen *xwl_screen, uint32_t
id)
xwl_output->output = wl_registry_bind(xwl_screen->registry, id,
&wl_output_interface, 2);
+xwl_output->server_output_id = id;
wl_output_add_listener(xwl_output->output, &output_listener, xwl_output);
snprintf(name, sizeof name, "XWAYLAND%d", serial++);
diff --git a/hw/xwayland/xwayland.c b/hw/xwayland/xwayland.c
index 7e8d667..7c2eaed 100644
--- a/hw/xwayland/xwayland.c
+++ b/hw/xwayland/xwayland.c
@@ -410,7 +410,15 @@ registry_global(void *data, struct wl_registry *registry,
uint32_t id,
static void
global_remove(void *data, struct wl_registry *registry, uint32_t name)
{
-/* Nothing to do here, wl_compositor and wl_shm should not be removed */
+struct xwl_screen *xwl_screen = data;
+struct xwl_output *xwl_output;
+
+xorg_list_for_each_entry(xwl_output, &xwl_screen->output_list, link) {
+if (xwl_output->server_output_id == name) {
+xwl_output_destroy(xwl_output);
+break;
+}
+}
}
static const struct wl_registry_listener registry_listener = {
diff --git a/hw/xwayland/xwayland.h b/hw/xwayland/xwayland.h
index cfb343d..70875e7 100644
--- a/hw/xwayland/xwayland.h
+++ b/hw/xwayland/xwayland.h
@@ -130,6 +130,7 @@ struct xwl_seat {
struct xwl_output {
struct xorg_list link;
struct wl_output *output;
+uint32_t server_output_id;
struct xwl_screen *xwl_screen;
RROutputPtr randr_output;
RRCrtcPtr randr_crtc;
--
2.4.0
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH v2 2/3] xwayland: Remove a useless out-of-memory check
snprintf does not allocate memory, so we can never get an out-of-memory error.
(Also, the error handler would free xwl_output after it was already registered
as an event listener.)
Signed-off-by: Dima Ryazanov
---
hw/xwayland/xwayland-output.c | 6 +-
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/hw/xwayland/xwayland-output.c b/hw/xwayland/xwayland-output.c
index 9baf4eb..41937b8 100644
--- a/hw/xwayland/xwayland-output.c
+++ b/hw/xwayland/xwayland-output.c
@@ -165,11 +165,7 @@ xwl_output_create(struct xwl_screen *xwl_screen, uint32_t
id)
&wl_output_interface, 2);
wl_output_add_listener(xwl_output->output, &output_listener, xwl_output);
-if (snprintf(name, sizeof name, "XWAYLAND%d", serial++) < 0) {
-ErrorF("create_output ENOMEM\n");
-free(xwl_output);
-return NULL;
-}
+snprintf(name, sizeof name, "XWAYLAND%d", serial++);
xwl_output->xwl_screen = xwl_screen;
xwl_output->randr_crtc = RRCrtcCreate(xwl_screen->screen, xwl_output);
--
2.4.0
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH v2 1/3] xwayland: Fix the addition and removal of outputs
Add the output to the list when it's created rather than when its properties
change (as pointed out by Marek Chalupa).
Remove the output from the list when it's destroyed.
Signed-off-by: Dima Ryazanov
---
hw/xwayland/xwayland-output.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/hw/xwayland/xwayland-output.c b/hw/xwayland/xwayland-output.c
index 155cbc1..9baf4eb 100644
--- a/hw/xwayland/xwayland-output.c
+++ b/hw/xwayland/xwayland-output.c
@@ -120,8 +120,6 @@ output_handle_done(void *data, struct wl_output *wl_output)
struct xwl_screen *xwl_screen = xwl_output->xwl_screen;
int width, height;
-xorg_list_append(&xwl_output->link, &xwl_screen->output_list);
-
width = 0;
height = 0;
xorg_list_for_each_entry(xwl_output, &xwl_screen->output_list, link) {
@@ -177,6 +175,8 @@ xwl_output_create(struct xwl_screen *xwl_screen, uint32_t
id)
xwl_output->randr_crtc = RRCrtcCreate(xwl_screen->screen, xwl_output);
xwl_output->randr_output = RROutputCreate(xwl_screen->screen, name,
strlen(name), xwl_output);
+xorg_list_append(&xwl_output->link, &xwl_screen->output_list);
+
RRCrtcGammaSetSize(xwl_output->randr_crtc, 256);
RROutputSetCrtcs(xwl_output->randr_output, &xwl_output->randr_crtc, 1);
RROutputSetConnection(xwl_output->randr_output, RR_Connected);
@@ -190,6 +190,7 @@ xwl_output_destroy(struct xwl_output *xwl_output)
wl_output_destroy(xwl_output->output);
RRCrtcDestroy(xwl_output->randr_crtc);
RROutputDestroy(xwl_output->randr_output);
+xorg_list_del(&xwl_output->link);
free(xwl_output);
}
--
2.4.0
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [weston, v2, 17/17] configure: add an option to allow building only the libraries
On Mon, May 11, 2015 at 09:27:38AM +0300, Pekka Paalanen wrote: > On Sat, 9 May 2015 10:09:21 +0300 > Giulio Camuffo wrote: > > > 2015-05-09 3:15 GMT+03:00 Bryce Harrington : > > > On Thu, Dec 04, 2014 at 11:01:23PM +0200, Giulio Camuffo wrote: > > >> the --enable/disable-weston-binaries emable or disable the creation > > >> of the 'weston', 'weston-launch' and all the binaries that are > > >> installed in $prefix/lib/libexec. This allows, together with > > >> --enable-clients, to only build the libraries, making possible to > > >> build and install different libweston versions at the same time. > > > > > > Prior to 1.7 there was uncertainty whether to have a libweston. Since > > > then, the uncertainty has gone away and it sounds like we're now all > > > more or less in consensus favoring this change? > > > > > > However, this patchset hasn't yet landed, and there's still some > > > questions about specifics. > > > > > > Should we press ahead with an eye towards getting this landed for 1.8? > > > Or should we postpone and target landing it as soon as the 1.9 tree > > > opens? > > > > I don't think it'd make sense to push it into 1.8 now, it is a rather > > big change and may introduce regressions. 1.9 seems a better idea > > imho. > > Yeah, it is far far too late for 1.8. > > This is such a big change, that it would be better to land it early in > a development cycle, like before the mid-point to the next alpha. That > would give us more experience on how it works, and if there are > problems, we should have time to fix them properly. Okay, in this case Guilio, in the next couple weeks can you rebase this to trunk (hopefully there's not a huge amount changed) and re-propose it on the mailing list? Hopefully as things are stabilized for the release we shouldn't see much changing in the repo. Then, once 1.9 is open we can proceed with getting it landed first thing. We will also require tests and API documentation. Bryce ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[ANNOUNCE] weston 1.7.92
Here is the Weston's RC1 release for the 1.8.0 cycle. This requires Wayland 1.7.92 (just for sake of keeping things in sync). Changes this release since the alpha include adding Cut/Copy/Paste support to the editor, and a handful of bugfixes to xwm, input, and compositors. Prior development this cycle focused around the IVI shell and keeping up with the EGL modernization. Feature changes include: repaint scheduling, named outputs, output transformations, EGL improvements to gl-renderer, surface-shooting API, source clipping and view scissor in the pixman renderer, ivi shell testing, and ivi layout. Remaining changes are a random assortment of cleanups, fixes, documentation, and upstreaming pieces of Collabora's work this past year. Bryce Harrington (1): configure.ac: bump to version 1.7.92 for the RC1 release Derek Foreman (4): text: Fix text-input for multi-seat text-input: Replace model with input logind: actually close fd in weston_launcher_close() launcher-util: Force all weston_launcher_open()s to use O_CLOEXEC Dima Ryazanov (1): xwm: Fix the window decoration hints. Giulio Camuffo (3): compositor: send the output_created signal after inserting it in the list xwm: make X windows of type 'utility' inactive man: use Xwayland instead of Xorg Hardening (1): RDP compositor: fixes for multiple connections, mstsc and FreeRDP master compilation Jonas Ådahl (1): protocol: Improve formatting of input method and text protocols Manuel Bachmann (1): editor: implement Cut,Copy,Paste Michael Vetter (1): remove trailing whitespaces Ryo Munakata (1): desktop-shell: set the current size in the first 'resizing' configure event git tag: 1.7.92 http://wayland.freedesktop.org/releases/weston-1.7.92.tar.xz MD5: 979ab73d54fc3097eac941a5729cb1b6 weston-1.7.92.tar.xz SHA1: 829dfeb795e7eaa67c61ff658e372cdc149faa79 weston-1.7.92.tar.xz SHA256: 628d9d18d0e72806216623ecd7d8884f83b23bf72796a6e61ad8f9cb0ec8bcd9 weston-1.7.92.tar.xz PGP: http://wayland.freedesktop.org/releases/weston-1.7.92.tar.xz.sig ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[ANNOUNCE] wayland 1.7.92
Here's RC1 for Wayland as we head to 1.8. Changes this time are pretty modest, just some light code cleanup. The main change in 1.8 is the addition of a new scanner option --include-core-only, and adds new headers wayland-client-core.h and wayland-server-core.h, to avoid dependency on the generated protocol headers. This makes life a bit easier for language bindings and other libwayland users that generate their own protocol code from newer wayland.xml files. Our schedule: √ Alpha (1.7.91) √ RC1 (1.7.92) on Friday, May 15th - RC2 (1.7.93) on Friday, May 22nd - 1.8.0 on Friday, May 29th Bryce Harrington (1): configure.ac: bump to version 1.7.92 for the RC1 release Michael Vetter (1): remove trailing whitespaces Pekka Paalanen (1): scanner: simplify the getopt logic git tag: 1.7.92 http://wayland.freedesktop.org/releases/wayland-1.7.92.tar.xz MD5: 0d4fcfbb13101f694bf9bf256b595c54 wayland-1.7.92.tar.xz SHA1: b8c497a1abb37bf1dfdbe44361371fb7735c6183 wayland-1.7.92.tar.xz SHA256: c057f546030ecc277f4c4c2726a725f9496b42d41563b293a222e48e77c221f8 wayland-1.7.92.tar.xz PGP: http://wayland.freedesktop.org/releases/wayland-1.7.92.tar.xz.sig ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston v2 04/14] tests: Add surface checks
On Fri, May 15, 2015 at 11:12:45AM +0100, Daniel Stone wrote:
> Hi,
>
> On 15 May 2015 at 09:21, Bryce Harrington wrote:
> > +/**
> > + * check_surfaces_equal() - tests if two surfaces are pixel-identical
> > + *
> > + * Returns true if surface buffers have all the same byte values,
> > + * false if the surfaces don't match or can't be compared due to
> > + * different dimensions.
> > + */
> > +bool
> > +check_surfaces_equal(const struct surface *a, const struct surface *b)
> > +{
> > + int y;
> > + void *p, *q;
> > +
> > + if (a == NULL || b == NULL)
> > + return false;
> > + if (a->width != b->width || a->height != a->height)
> > + return false;
>
> I realise you're going to hate me here, but I have led you astray a bit ...
>
> > + if (a->stride == b->stride) {
>
> /* Second comparison needed to avoid checking potentially garbage
> values in the padding. */
> if (a->stride == b->stride && a->stride == (a->width * a->bpp))
There is actually no bpp member to the surface structure. Adding that
(and putting in the relevant numbers) is going to be a fair bit of
plumbing. I actually wondered about that, but throughout the existing
code I see assumptions that hardcode it to just '4' anyway, so it didn't
seem worth worrying about. But I guess it's possible it'll be different
some day.
> > + printf("Checking data for equivalent strides\n");
> > + return (memcmp(a->data, b->data, a->stride * a->height) ==
> > 0);
> > + } else {
> > + printf("Manually comparing due to differing strides\n");
> > + for (y = 0; y < a->height; y++) {
> > + p = a->data + (y * a->stride);
> > + q = b->data + (y * b->stride);
> > + if (memcmp(p, q, a->stride) != 0)
>
> if (memcmp(p, q, a->width * a->bpp) != 0) /* don't check the garbage
> values in the stride */
>
> So sorry.
>
> If you just fix these up (and a->bpp is actually a real thing which is
> bytes per pixel), then I'm happy for it to get merged without a third
> pass through the list.
>
> > + printf("Bytewise comparison inside clip\n");
> > + a_bpp = a->stride / a->width;
> > + b_bpp = b->stride / b->width;
>
> This isn't necessarily true, e.g. if you have a 1366x768 buffer padded
> out to 1344px to meet a 32px alignment requirement, at 32bpp, then
> deriving bpp from stride/width would give you a bpp of 3.9355.
> Probably enough to get away with, but really bpp needs to come from
> the format.
>
> > + for (i=y0; i > + p = a->data + i * a->stride + x0 * a_bpp;
> > + q = b->data + i * b->stride + x0 * b_bpp;
> > + if (a->stride == b->stride) {
> > + if (memcmp(p, q, (x1-x0)*a_bpp) != 0) {
> > + // Dump the bad row
>
> Stray C++ comment.
>
> > + printf("Mismatched image on row %d\n", i);
> > + for (j=0; j<(x1-x0)*a_bpp; j++) {
> > + a_char = *((char*)(p+j*a_bpp));
> > + b_char = *((char*)(q+j*b_bpp));
> > + printf("%d,%d: %8x %8x %s\n", i, j,
> > a_char, b_char,
> > + (a_char != b_char)? " <---":
> > "");
> > + }
> > + return false;
> > + }
> > + } else {
> > + /* account for bpp differences */
> > + for (j=0; j > + a_char = *((char*)(p+j*a_bpp));
> > + b_char = *((char*)(q+j*b_bpp));
> > + if (a_char != b_char) {
> > + printf("%d,%d: %8x %8x %s\n", i, j,
> > a_char, b_char,
> > + (a_char != b_char)? " <---":
> > "");
> > + return false;
> > + }
> > + }
> > + }
>
> Hmm, maybe a stupid question, but don't these branches fundamentally
> do the same thing ... ? Except that in the 'account for bpp
> differences' branch, it only iterates for as many bytes as there are
> pixels to compare, which probably isn't what you want. I think the
> memcmp should be perfectly sufficient, and you don't need to test the
> stride at all.
Well, if the bpp for p and q differ, then comparing an N pixel row from
each is not going to result in a correct match, right?
The latter branch is more generalized and would work correctly for both,
but lacks the optimizations that the memset function has and so would
likely be slower in execution. Maybe that matters less in test code,
especially if we continue not writing test cases. ;-)
Bryce
_
Re: [PATCH] compositor-fbdev: Wait and retry before failing on reconnect to the framebuffer
On Friday, May 15, 2015 12:30:30 PM David Herrmann wrote:
> Hi
>
> On Fri, May 15, 2015 at 8:39 AM, Pekka Paalanen wrote:
> >
> > On Wed, 13 May 2015 21:43:39 -0400
> > nerdopolis wrote:
> >
> > > Resolving https://bugs.freedesktop.org/show_bug.cgi?id=73782
> > > udev might be configured to set the permissions on framebuffer devices
> > > with the UACCESS attribute.
> > > Weston currently attempts to reconnect to the framebuffer device before
> > > udev can set the permissions back.
> > >
> > > It waits 3 times in case if the system is heavily paging, or slowed down,
> > > and prevents udev from working.
> > > In my testing the delay is long enough where it works on the first try
> > > ---
> > > src/compositor-fbdev.c | 12 +++-
> > > 1 file changed, 11 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/src/compositor-fbdev.c b/src/compositor-fbdev.c
> > > index 7c505ce..42d2881 100644
> > > --- a/src/compositor-fbdev.c
> > > +++ b/src/compositor-fbdev.c
> > > @@ -663,13 +663,23 @@ fbdev_output_reenable(struct fbdev_compositor
> > > *compositor,
> > > struct fbdev_output *output = to_fbdev_output(base);
> > > struct fbdev_screeninfo new_screen_info;
> > > int fb_fd;
> > > + int retries;
> > > const char *device;
> > >
> > > weston_log("Re-enabling fbdev output.\n");
> > >
> > > /* Create the frame buffer. */
> > > - fb_fd = fbdev_frame_buffer_open(output, output->device,
> > > + fb_fd = -1;
> > > + retries = 0;
> > > + while (fb_fd < 0 && retries < 3) {
> > > + usleep(1);
> > > + fb_fd = fbdev_frame_buffer_open(output, output->device,
> > > &new_screen_info);
> > > + if (fb_fd < 0) {
> > > + weston_log("Creating frame buffer failed. Retrying...\n");
> > > + }
> > > + retries++;
> > > + }
> > > if (fb_fd < 0) {
> > > weston_log("Creating frame buffer failed.\n");
> > > goto err;
> >
> > Hi,
> >
> > I hate sleeps. I'd really want an explanation in the commit message on
> > why this cannot be fixed properly at all, and you need a delay to have
> > it work.
> >
> > It is possible there is no better way, but I would like to understand
> > why first.
> >
> > For instance, should reenable() be waiting for some sort of udev
> > advertisement of the fb device? Is there any event from udev to signal
> > this?
> >
> > Do you have systemd involved in your use case? Could this be solved with
> > systemd or logind? Or should we let weston-launch open the fb device
> > instead?
> >
> > I'm CC'ing David, maybe he might have an idea?
>
> systemd-logind listens to VT events via poll(2) on
> /sys/class/tty/tty0/active. On VT switches it changes ACL permissions
> on dev-nodes underneath /dev, if the 'uaccess' tag is set on a device.
> But it cannot delay a VT-switch, it just reacts to it.
>
> It is inherent to this approach, that the permissions are set _after_
> the VT switch. Therefore, there's a race and I guess it's what is hit
> here. However, systemd-logind sends out dbus signals after it fully
> changed the permissions. Hence, if you rely on the 'uaccess'
> functionality by systemd, then you better also use the systemd APIs
> (either sd_login_monitor_new() or DBus). If you don't want to use the
> systemd APIs, then you cannot rely on 'uaccess' (i.e., you have to set
> static group/user permissions on your device nodes; it will not
> interfere with 'uaccess', logind allows both in parallel).
>
> Long story short: Please run weston directly (_without_ weston-launch,
> instead directly invoking weston from within a logged-in VT), with
> systemd support compiled it. It should work fine with 'uaccess'. If
> you don't have systemd support compiled in, please use static access
> permissions.
>
> Thanks
> David
> ___
> wayland-devel mailing list
> wayland-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Hi.
It seems that I am a bit mistaken. My scripts used to call Weston using
weston-launch, and then I configured fbdev for uaccess permissions, and stopped
setting static permissions.
I created the workaround locally... Then later on I updated my login script
manager to not use weston-launch anymore, and call systemd enabled login
sessions on TTYs with runuser...
I didn't bring it upstream until now, because I wasn't sure if it was actually
a good way to fix it...
I actually haven't tested it with _vanilla_ weston until now. Removing my patch
and tty switching with /dev/fb0 set with UACCESS, with --backend=fbdev-backend
_without_ weston-launch does *not* cause a crash... I called weston directly,
and tty switched fine on fbdev.
However, It still fails under weston-launch even when compiling against
systemd. TTY switching results in the race condition, and Weston crashes. But
now most systems, should
Re: [PATCH weston v2 14/21] Introduce pointer locking and confinement protocol
On 05/14/2015 07:53 PM, Jonas Ådahl wrote: but your notes were very misleading. What notes? The statement "Serials are dropped" that was in your header email is the one that really threw me. The primary problem I am having is all your description seems to be for setting up something like an X11 pointer grab that is permanent until the client cancels it. However both your examples, and virtually every single use of pointer lock I can think of other than point & shoot games, involve grabbing the pointer on mouse press and releasing it on mouse release. Ideas like the "slow scrollbar" or attaching the pointer to the surface of a 3D sphere, or a drawing program where a stroke off the edge of the window scrolls the canvas so the drawing remains visible. I believe your api, with minor changes, can support both of these styles of interaction. In particular there needs to be a type of pointer lock that is cancelled when all the mouse buttons are released (this would be identical to current grab behavior except the mouse cursor stops moving). I am assuming you get this type of lock if the triggering event is a mouse-down. Keystrokes, mouse-up, and mouse enter events get you the long-term type of lock you are talking about. The primary problem is that you removed the serial of the triggering event, which is VITAL for this to work. You have to put that back. The compositor has to know what event caused the lock! No I didn't. They have never been there. Then it is broken. This needs to be fixed so the compositor can know which event triggered the lock, since it makes a huge difference in whether to grant the lock or not. It also fixes race conditions. I removed other serials that was only used with set_cursor_position_hint, but since the lock/confine objects are one-shot objects they were useless. I *think* that makes sense. The only sync issue is that set_cursor_position requests may be sent after the compositor cancels the lock but before the client sees this. I think correct behavior is that the compositor should obey these, though it is probably not important if these are dropped. I am still fully mystified as to the purpose of the "region" in the request. I think the purpose of this could be handled by the client sending the request only when the mouse enters the region. However it is possible you intend to use the "region" as part of the compositor interaction, maybe it wants to highlight it, or place a dialog box centered in it. A pointer entering any region is should not be enough to trigger a lock. Can you please explain WHAT IS THE PURPOSE OF THE REGION??? I am taking wild guesses here because I just don't see it. How about a specific question: - The client gets a mouse press at x,y - Version 1: the client sends lock_pointer with the region set to x,y and a width and height of zero - Version 2: the client sends lock_pointer with the region set to the input area of the clicked surface. Question: what happens differently in these two cases??? I really am guessing wildly here that the "region" is supposed to be used by the compositor to control what events cause the lock to be triggered. But because you limit it to the input region of the surface and thus only to events the client would get anyway, I am unable to come up with any idea that could not be done by the client. For instance if a click inside the region is to trigger the lock, it is trivial for a client to instead wait for a mouse press, check if it is in the region, and then do the lock_pointer request only if it is inside. Can you please describe an example of user interaction you expect to trigger one of your deferred locks? And how the region is used in this interaction? A lock happens asynchronously. In order for the client to know when the lock is activated, an event is needed. Sometimes it will be immediate, sometimes not. If a client receives pointer events but no "locked" event, then it can deal with those events knowing the pointer is not locked. No, the client has to assume and treat any pointer motion events it gets as part of the lock. They may have been sent before the compositor received the lock_pointer request. It also seems to me that if the compositor pops up some kind of user interface to allow the user to choose whether the lock should happen, it will be eating all the pointer motion events anyway. Anyway, while we probably need to specify what happens if a surface's input region changes size. Right now a lock will be kept, no matter how the region changes, but that won't make sense for confine regions. What is the use case for resizing and changing the lock/confine region you are talking about? Because user interaction with the game changes the window size. If you need the cursor to be locked precisely to the graphic it is drawing the only possible way to achieve this is by drawing that cursor client side. As I tried to tell yo
Re: [PATCH] remove trailing whitespaces
On Fri, May 15, 2015 at 05:17:47PM +0200, Michael Vetter wrote:
> Remove trailing whitespaces because they are not needed and jumping to
> the end of al ine should do just that and not jump to the whitespace.
> ---
> clients/desktop-shell.c| 10 -
> clients/dnd.c | 12 +-
> clients/editor.c | 2 +-
> clients/flower.c | 6 ++---
> clients/fullscreen.c | 4 ++--
> clients/gears.c| 4 ++--
> clients/glmatrix.c | 12 +-
> clients/keyboard.c | 14 ++--
> clients/nested.c | 2 +-
> clients/terminal.c | 56
> +++---
> clients/transformed.c | 4 ++--
> clients/window.c | 2 +-
> desktop-shell/shell.c | 2 +-
> ivi-shell/hmi-controller.c | 2 +-
> 14 files changed, 66 insertions(+), 66 deletions(-)
pushed:
2663c68..2a18a52 master -> master
> diff --git a/clients/desktop-shell.c b/clients/desktop-shell.c
> index e2f9f80..777a50a 100644
> --- a/clients/desktop-shell.c
> +++ b/clients/desktop-shell.c
> @@ -34,7 +34,7 @@
> #include
> #include
> #include
> -#include
> +#include
> #include
> #include
> #include
> @@ -241,7 +241,7 @@ panel_launcher_motion_handler(struct widget *widget,
> struct input *input,
> static void
> set_hex_color(cairo_t *cr, uint32_t color)
> {
> - cairo_set_source_rgba(cr,
> + cairo_set_source_rgba(cr,
> ((color >> 16) & 0xff) / 255.0,
> ((color >> 8) & 0xff) / 255.0,
> ((color >> 0) & 0xff) / 255.0,
> @@ -319,7 +319,7 @@ panel_launcher_touch_down_handler(struct widget *widget,
> struct input *input,
>
> static void
> panel_launcher_touch_up_handler(struct widget *widget, struct input *input,
> - uint32_t serial, uint32_t time, int32_t id,
> + uint32_t serial, uint32_t time, int32_t id,
> void *data)
> {
> struct panel_launcher *launcher;
> @@ -440,7 +440,7 @@ panel_resize_handler(struct widget *widget,
> struct panel_launcher *launcher;
> struct panel *panel = data;
> int x, y, w, h;
> -
> +
> x = 10;
> y = 16;
> wl_list_for_each(launcher, &panel->launcher_list, link) {
> @@ -521,7 +521,7 @@ panel_create(struct desktop *desktop)
>
> widget_set_redraw_handler(panel->widget, panel_redraw_handler);
> widget_set_resize_handler(panel->widget, panel_resize_handler);
> -
> +
> panel_add_clock(panel);
>
> s = weston_config_get_section(desktop->config, "shell", NULL, NULL);
> diff --git a/clients/dnd.c b/clients/dnd.c
> index e893d36..b5a7d41 100644
> --- a/clients/dnd.c
> +++ b/clients/dnd.c
> @@ -103,12 +103,12 @@ item_create(struct display *display, int x, int y, int
> seed)
> item = malloc(sizeof *item);
> if (item == NULL)
> return NULL;
> -
> -
> +
> +
> gettimeofday(&tv, NULL);
> item->seed = seed ? seed : tv.tv_usec;
> srandom(item->seed);
> -
> +
> const int petal_count = 3 + random() % 5;
> const double r1 = 20 + random() % 10;
> const double r2 = 5 + random() % 12;
> @@ -289,7 +289,7 @@ static void
> data_source_send(void *data, struct wl_data_source *source,
>const char *mime_type, int32_t fd)
> {
> - struct dnd_flower_message dnd_flower_message;
> + struct dnd_flower_message dnd_flower_message;
> struct dnd_drag *dnd_drag = data;
> char buffer[128];
> int n;
> @@ -325,7 +325,7 @@ data_source_cancelled(void *data, struct wl_data_source
> *source)
>* up the drag object created and the local state. */
>
> wl_data_source_destroy(dnd_drag->data_source);
> -
> +
> /* Destroy the item that has been dragged out */
> cairo_surface_destroy(dnd_drag->item->surface);
> free(dnd_drag->item);
> @@ -619,7 +619,7 @@ dnd_receive_func(void *data, size_t len, int32_t x,
> int32_t y, void *user_data)
> len, sizeof *message);
> return;
> }
> -
> +
> widget_get_allocation(dnd->widget, &allocation);
> item = item_create(dnd->display,
> x - message->x_offset - allocation.x,
> diff --git a/clients/editor.c b/clients/editor.c
> index 29cab34..f77a48d 100644
> --- a/clients/editor.c
> +++ b/clients/editor.c
> @@ -612,7 +612,7 @@ editor_copy_cut(struct editor *editor, struct input
> *input, bool cut)
>
> if (!entry)
> return;
> -
> +
> if (entry->cursor != entry->anchor) {
> int start_index = MIN(entry->cursor, entry->anchor);
> int end_index = MAX(entry->cursor, entry->anchor);
> diff --git a/clients/flower.c b/clients/flower.c
> index 624525c..15315b2 100644
> --- a/clients/flower.c
> +++ b/clients/flower.c
> @@ -86,7 +86,7 @
Re: [PATCH] remove trailing whitespaces
On Fri, May 15, 2015 at 05:12:41PM +0200, Michael Vetter wrote:
> Remove trailing whitespaces because they are not needed and jumping to
> the end of al ine should do just that and not jump to the whitespace.
> ---
> cursor/cursor-data.h | 1004
> +++---
> src/scanner.c|2 +-
> tests/connection-test.c |8 +-
> tests/list-test.c|2 +-
> tests/os-wrappers-test.c |2 +-
> 5 files changed, 509 insertions(+), 509 deletions(-)
pushed:
d08c079..b409c91 master -> master
> diff --git a/cursor/cursor-data.h b/cursor/cursor-data.h
> index 4c5e672..b1026d7 100644
> --- a/cursor/cursor-data.h
> +++ b/cursor/cursor-data.h
> @@ -22,508 +22,508 @@
> */
>
> static uint32_t cursor_data[] = {
> - 0x, 0x, 0x, 0x, 0x, 0x,
> - 0x, 0x, 0x, 0x, 0x, 0x,
> - 0x, 0x, 0x, 0x, 0x, 0xff00,
> - 0xff00, 0x, 0x, 0x, 0x, 0x,
> - 0x, 0x, 0x, 0x, 0x, 0x,
> - 0x, 0x, 0x, 0xff00, 0xff00, 0x,
> - 0x, 0x, 0x, 0x, 0x, 0x,
> - 0x, 0x, 0x, 0x, 0x, 0x,
> - 0x, 0xff00, 0xff00, 0x, 0x, 0x,
> - 0xff00, 0x, 0x, 0x, 0x, 0x,
> - 0xff00, 0x, 0x, 0x, 0x, 0xff00,
> - 0xff00, 0x, 0x, 0x, 0xff00, 0x,
> - 0x, 0x, 0x, 0xff00, 0x, 0x,
> - 0x, 0x, 0x, 0xff00, 0xff00, 0x,
> - 0x, 0x, 0xff00, 0x, 0x, 0x,
> - 0xff00, 0x, 0x, 0x, 0x, 0x,
> - 0x, 0xff00, 0xff00, 0x, 0x, 0x,
> - 0xff00, 0x, 0x, 0xff00, 0x, 0x,
> - 0x, 0x, 0x, 0x, 0x, 0xff00,
> - 0xff00, 0x, 0x, 0x, 0xff00, 0x,
> - 0xff00, 0x, 0x, 0x, 0x, 0x,
> - 0x, 0x, 0x, 0xff00, 0xff00, 0x,
> - 0x, 0x, 0xff00, 0xff00, 0x, 0x,
> - 0x, 0x, 0x, 0x, 0x, 0x,
> - 0x, 0xff00, 0xff00, 0x, 0x, 0x,
> - 0xff00, 0xff00, 0xff00, 0xff00, 0xff00, 0xff00,
> - 0xff00, 0x, 0x, 0x, 0x, 0xff00,
> - 0xff00, 0x, 0x, 0x, 0x, 0x,
> - 0x, 0x, 0x, 0x, 0x, 0x,
> - 0x, 0x, 0x, 0xff00, 0xff00, 0x,
> - 0x, 0x, 0x, 0x, 0x, 0x,
> - 0x, 0x, 0x, 0x, 0x, 0x,
> - 0x, 0xff00, 0xff00, 0x, 0x, 0x,
> - 0x, 0x, 0x, 0x, 0x, 0x,
> - 0x, 0x, 0x, 0x, 0x, 0xff00,
> - 0xff00, 0xff00, 0xff00, 0xff00, 0xff00, 0xff00,
> - 0xff00, 0xff00, 0xff00, 0xff00, 0xff00, 0xff00,
> - 0xff00, 0x, 0x, 0xff00, 0xff00, 0xff00,
> - 0xff00, 0xff00, 0xff00, 0xff00, 0xff00, 0xff00,
> - 0xff00, 0xff00, 0xff00, 0xff00, 0xff00, 0x,
> - 0x, 0x, 0x, 0x, 0x, 0x,
> - 0x, 0x, 0x, 0x, 0x, 0x,
> - 0x, 0x, 0x, 0x, 0x, 0x,
> - 0x, 0x, 0x, 0x, 0x, 0x,
> - 0x, 0x, 0x, 0x, 0x, 0x,
> - 0x, 0x, 0x, 0x, 0x, 0x,
> - 0x, 0x, 0x, 0x, 0x, 0x,
> - 0x, 0x, 0x, 0xff00, 0xff00, 0x,
> - 0x, 0x, 0x, 0x, 0x, 0x,
> - 0x, 0x, 0x, 0x, 0x, 0x,
> - 0x, 0xff00, 0xff00, 0x, 0x, 0x,
> - 0x, 0xff00, 0x, 0x, 0x, 0x,
> - 0
Re: [PATCH weston v2 04/14] tests: Add surface checks
On Fri, May 15, 2015 at 04:55:48PM +0300, Pekka Paalanen wrote:
> On Fri, 15 May 2015 01:21:49 -0700
> Bryce Harrington wrote:
>
> > Introduce helper routines for testing surfaces against specific
> > conditions. These allow tests to validate screen captures as displaying
> > the correct rendering results.
> >
> > Signed-off-by: Bryce Harrington
> > ---
> > tests/weston-test-client-helper.c | 116
> > ++
> > tests/weston-test-client-helper.h | 15 +
> > 2 files changed, 131 insertions(+)
> >
> > diff --git a/tests/weston-test-client-helper.c
> > b/tests/weston-test-client-helper.c
> > index 080bb62..04b77d5 100644
> > --- a/tests/weston-test-client-helper.c
> > +++ b/tests/weston-test-client-helper.c
>
> > +bool
> > +check_surfaces_equal(const struct surface *a, const struct surface *b)
> > +{
> > + int y;
> > + void *p, *q;
> > +
> > + if (a == NULL || b == NULL)
> > + return false;
> > + if (a->width != b->width || a->height != a->height)
>
> The latter compares a to a, should be a to b.
Good catch, I actually fix this up in a latter patch.
I was just really tired of squashing patches at this point. ;-)
Bryce
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston v2 2/2] compositor-drm: pass ARGB fallback to gl create functions for XRGB formats
If the GL implementation doesn't provide an XRGB visual we may still be
able to proceed with an ARGB one. Since we're not changing the scanout
buffer format, and our current rendering loop always results in saturated
alpha in the frame buffer, it should be Just Fine(tm) - and probably better
than just exiting.
This is a workaround for https://bugs.freedesktop.org/show_bug.cgi?id=89689
Reviewed-By: Bryce Harrington
Tested-By: Pekka Paalanen
Reviewed-By: Pekka Paalanen
Signed-off-by: Derek Foreman
---
src/compositor-drm.c | 57 +++-
src/gl-renderer.c| 8
2 files changed, 55 insertions(+), 10 deletions(-)
diff --git a/src/compositor-drm.c b/src/compositor-drm.c
index 69bdcfd..313860b 100644
--- a/src/compositor-drm.c
+++ b/src/compositor-drm.c
@@ -1391,14 +1391,46 @@ create_gbm_device(int fd)
return gbm;
}
+/* When initializing EGL, if the preferred buffer format isn't availble
+ * we may be able to susbstitute an ARGB format for an XRGB one.
+ *
+ * This returns 0 if substitution isn't possible, but 0 might be a
+ * legitimate format for other EGL platforms, so the caller is
+ * responsible for checking for 0 before calling gl_renderer->create().
+ *
+ * This works around https://bugs.freedesktop.org/show_bug.cgi?id=89689
+ * but it's entirely possible we'll see this again on other implementations.
+ */
static int
-drm_compositor_create_gl_renderer(struct drm_compositor *ec)
+fallback_format_for(uint32_t format)
{
- EGLint format;
+ switch (format) {
+ case GBM_FORMAT_XRGB:
+ return GBM_FORMAT_ARGB;
+ case GBM_FORMAT_XRGB2101010:
+ return GBM_FORMAT_ARGB2101010;
+ default:
+ return 0;
+ }
+}
- format = ec->format;
- if (gl_renderer->create(&ec->base, EGL_PLATFORM_GBM_KHR, (void *)
ec->gbm,
- gl_renderer->opaque_attribs, &format, 1) < 0) {
+static int
+drm_compositor_create_gl_renderer(struct drm_compositor *ec)
+{
+ EGLint format[2] = {
+ ec->format,
+ fallback_format_for(ec->format),
+ };
+ int n_formats = 1;
+
+ if (format[1])
+ n_formats = 2;
+ if (gl_renderer->create(&ec->base,
+ EGL_PLATFORM_GBM_KHR,
+ (void *)ec->gbm,
+ gl_renderer->opaque_attribs,
+ format,
+ n_formats) < 0) {
return -1;
}
@@ -1602,13 +1634,16 @@ find_crtc_for_connector(struct drm_compositor *ec,
static int
drm_output_init_egl(struct drm_output *output, struct drm_compositor *ec)
{
- EGLint format = output->format;
- int i, flags;
+ EGLint format[2] = {
+ output->format,
+ fallback_format_for(output->format),
+ };
+ int i, flags, n_formats = 1;
output->surface = gbm_surface_create(ec->gbm,
output->base.current_mode->width,
output->base.current_mode->height,
-format,
+format[0],
GBM_BO_USE_SCANOUT |
GBM_BO_USE_RENDERING);
if (!output->surface) {
@@ -1616,12 +1651,14 @@ drm_output_init_egl(struct drm_output *output, struct
drm_compositor *ec)
return -1;
}
+ if (format[1])
+ n_formats = 2;
if (gl_renderer->output_create(&output->base,
(EGLNativeDisplayType)output->surface,
output->surface,
gl_renderer->opaque_attribs,
- &format,
- 1) < 0) {
+ format,
+ n_formats) < 0) {
weston_log("failed to create gl renderer output state\n");
gbm_surface_destroy(output->surface);
return -1;
diff --git a/src/gl-renderer.c b/src/gl-renderer.c
index 248c8aa..772c6a8 100644
--- a/src/gl-renderer.c
+++ b/src/gl-renderer.c
@@ -934,6 +934,14 @@ output_rotate_damage(struct weston_output *output,
go->border_damage[go->buffer_damage_index] = border_status;
}
+/* NOTE: We now allow falling back to ARGB gl visuals when XRGB is
+ * unavailable, so we're assuming the background has no transparency
+ * and that everything with a blend, like drop shadows, will have something
+ * opaque (like the background) drawn underneath it.
+ *
+ * Depending on the underlying hardware, violating that assumption could
+ * result in seeing through to another display plane.
+ */
static void
gl_renderer_repaint_output(
[PATCH weston v2 1/2] gl-renderer: Take a list of acceptable formats in create functions
Currently we pass either a single format or no formats to the gl renderer
create and output_create functions. We extend this to any number of
formats so we can allow fallback formats if we don't get our first pick.
Reviewed-By: Bryce Harrington
Reviewed-By: Pekka Paalanen
Signed-off-by: Derek Foreman
---
src/compositor-drm.c | 5 ++--
src/compositor-fbdev.c | 4 +--
src/compositor-wayland.c | 12
src/compositor-x11.c | 5 ++--
src/gl-renderer.c| 71
src/gl-renderer.h| 6 ++--
6 files changed, 66 insertions(+), 37 deletions(-)
diff --git a/src/compositor-drm.c b/src/compositor-drm.c
index 0cdb8f4..69bdcfd 100644
--- a/src/compositor-drm.c
+++ b/src/compositor-drm.c
@@ -1398,7 +1398,7 @@ drm_compositor_create_gl_renderer(struct drm_compositor
*ec)
format = ec->format;
if (gl_renderer->create(&ec->base, EGL_PLATFORM_GBM_KHR, (void *)
ec->gbm,
- gl_renderer->opaque_attribs, &format) < 0) {
+ gl_renderer->opaque_attribs, &format, 1) < 0) {
return -1;
}
@@ -1620,7 +1620,8 @@ drm_output_init_egl(struct drm_output *output, struct
drm_compositor *ec)
(EGLNativeDisplayType)output->surface,
output->surface,
gl_renderer->opaque_attribs,
- &format) < 0) {
+ &format,
+ 1) < 0) {
weston_log("failed to create gl renderer output state\n");
gbm_surface_destroy(output->surface);
return -1;
diff --git a/src/compositor-fbdev.c b/src/compositor-fbdev.c
index 7c505ce..3f3394f 100644
--- a/src/compositor-fbdev.c
+++ b/src/compositor-fbdev.c
@@ -570,7 +570,7 @@ fbdev_output_create(struct fbdev_compositor *compositor,
if (gl_renderer->output_create(&output->base,
(EGLNativeWindowType)NULL, NULL,
gl_renderer->opaque_attribs,
- NULL) < 0) {
+ NULL, 0) < 0) {
weston_log("gl_renderer_output_create failed.\n");
goto out_shadow_surface;
}
@@ -871,7 +871,7 @@ fbdev_compositor_create(struct wl_display *display, int
*argc, char *argv[],
if (gl_renderer->create(&compositor->base, NO_EGL_PLATFORM,
EGL_DEFAULT_DISPLAY,
gl_renderer->opaque_attribs,
- NULL) < 0) {
+ NULL, 0) < 0) {
weston_log("gl_renderer_create failed.\n");
goto out_launcher;
}
diff --git a/src/compositor-wayland.c b/src/compositor-wayland.c
index 303151c..c9983e0 100644
--- a/src/compositor-wayland.c
+++ b/src/compositor-wayland.c
@@ -648,7 +648,8 @@ wayland_output_init_gl_renderer(struct wayland_output
*output)
output->gl.egl_window,
output->gl.egl_window,
gl_renderer->alpha_attribs,
- NULL) < 0)
+ NULL,
+ 0) < 0)
goto cleanup_window;
return 0;
@@ -1970,10 +1971,11 @@ wayland_compositor_create(struct wl_display *display,
int use_pixman,
if (!c->use_pixman) {
if (gl_renderer->create(&c->base,
- EGL_PLATFORM_WAYLAND_KHR,
- c->parent.wl_display,
- gl_renderer->alpha_attribs,
- NULL) < 0) {
+ EGL_PLATFORM_WAYLAND_KHR,
+ c->parent.wl_display,
+ gl_renderer->alpha_attribs,
+ NULL,
+ 0) < 0) {
weston_log("Failed to initialize the GL renderer; "
"falling back to pixman.\n");
c->use_pixman = 1;
diff --git a/src/compositor-x11.c b/src/compositor-x11.c
index 5129e85..3565677 100644
--- a/src/compositor-x11.c
+++ b/src/compositor-x11.c
@@ -906,7 +906,8 @@ x11_compositor_create_output(struct x11_compositor *c, int
x, int y,
(EGLNativeWindowType)
output->window,
&xid,
Re: [PATCH weston 1/2] gl-renderer: Take a list of acceptable formats in create functions
On 15/05/15 02:18 AM, Pekka Paalanen wrote:
> On Thu, 14 May 2015 20:14:53 -0700
> Bryce Harrington wrote:
>
>> On Mon, May 11, 2015 at 02:19:02PM -0500, Derek Foreman wrote:
>>> Currently we pass either a single format or no formats to the gl renderer
>>> create and output_create functions. We extend this to any number of
>>> formats so we can allow fallback formats if we don't get our first pick.
>>>
>>> Signed-off-by: Derek Foreman
>>
>> This all looks fine, but I have some bikesheddy comments. I'll give a
>> R-B and you can decide whether or not my suggestions have any utility.
>>
>> Reviewed-by: Bryce Harrington
>
> Hi,
>
> I agree with Bryce's comments below, and also agree that this patch is
> also landable as is.
>
> Reviewed-by: Pekka Paalanen
>
> I have made three further notes below.
>
>>> ---
>>> src/compositor-drm.c | 5 ++--
>>> src/compositor-fbdev.c | 4 +--
>>> src/compositor-wayland.c | 12 +
>>> src/compositor-x11.c | 5 ++--
>>> src/gl-renderer.c| 68
>>> +++-
>>> src/gl-renderer.h| 6 +++--
>>> 6 files changed, 63 insertions(+), 37 deletions(-)
>>>
>>> diff --git a/src/compositor-drm.c b/src/compositor-drm.c
>>> index 0cdb8f4..69bdcfd 100644
>>> --- a/src/compositor-drm.c
>>> +++ b/src/compositor-drm.c
>>> @@ -1398,7 +1398,7 @@ drm_compositor_create_gl_renderer(struct
>>> drm_compositor *ec)
>>>
>>> format = ec->format;
>>> if (gl_renderer->create(&ec->base, EGL_PLATFORM_GBM_KHR, (void *)
>>> ec->gbm,
>>> - gl_renderer->opaque_attribs, &format) < 0) {
>>> + gl_renderer->opaque_attribs, &format, 1) < 0) {
>>> return -1;
>>> }
>>>
>>> @@ -1620,7 +1620,8 @@ drm_output_init_egl(struct drm_output *output, struct
>>> drm_compositor *ec)
>>>(EGLNativeDisplayType)output->surface,
>>>output->surface,
>>>gl_renderer->opaque_attribs,
>>> - &format) < 0) {
>>> + &format,
>>> + 1) < 0) {
>>> weston_log("failed to create gl renderer output state\n");
>>> gbm_surface_destroy(output->surface);
>>> return -1;
>>> diff --git a/src/compositor-fbdev.c b/src/compositor-fbdev.c
>>> index 7c505ce..3f3394f 100644
>>> --- a/src/compositor-fbdev.c
>>> +++ b/src/compositor-fbdev.c
>>> @@ -570,7 +570,7 @@ fbdev_output_create(struct fbdev_compositor *compositor,
>>> if (gl_renderer->output_create(&output->base,
>>>(EGLNativeWindowType)NULL, NULL,
>>>gl_renderer->opaque_attribs,
>>> - NULL) < 0) {
>>> + NULL, 0) < 0) {
>>> weston_log("gl_renderer_output_create failed.\n");
>>> goto out_shadow_surface;
>>> }
>>> @@ -871,7 +871,7 @@ fbdev_compositor_create(struct wl_display *display, int
>>> *argc, char *argv[],
>>> if (gl_renderer->create(&compositor->base, NO_EGL_PLATFORM,
>>> EGL_DEFAULT_DISPLAY,
>>> gl_renderer->opaque_attribs,
>>> - NULL) < 0) {
>>> + NULL, 0) < 0) {
>>> weston_log("gl_renderer_create failed.\n");
>>> goto out_launcher;
>>> }
>>> diff --git a/src/compositor-wayland.c b/src/compositor-wayland.c
>>> index 303151c..c9983e0 100644
>>> --- a/src/compositor-wayland.c
>>> +++ b/src/compositor-wayland.c
>>> @@ -648,7 +648,8 @@ wayland_output_init_gl_renderer(struct wayland_output
>>> *output)
>>>output->gl.egl_window,
>>>output->gl.egl_window,
>>>gl_renderer->alpha_attribs,
>>> - NULL) < 0)
>>> + NULL,
>>> + 0) < 0)
>>> goto cleanup_window;
>>>
>>> return 0;
>>> @@ -1970,10 +1971,11 @@ wayland_compositor_create(struct wl_display
>>> *display, int use_pixman,
>>>
>>> if (!c->use_pixman) {
>>> if (gl_renderer->create(&c->base,
>>> - EGL_PLATFORM_WAYLAND_KHR,
>>> - c->parent.wl_display,
>>> - gl_renderer->alpha_attribs,
>>> - NULL) < 0) {
>>> + EGL_PLATFORM_WAYLAND_KHR,
>>> + c->parent.wl_display,
>>> + gl_renderer->alpha_attribs,
>>> +
Re: [PATCH libinput] filter: add Simon's copyright
Hi Peter, that's fine, thanks! Sorry for being so formal, I was a bit upset and should have waited a little longer before hitting send. BTW it's great that you finally came to do that user study and all that jazz. If you like, just keep me in the loop for feedback. Cheers, Simon On 05/06/2015 05:20 AM, Peter Hutterer wrote: This code was largely lifted from the X server in bb25b2ad297891430606c367bfabc but didn't take the copyright messages that applied to that code. Signed-off-by: Peter Hutterer --- Simon: I think that should cover it. Sorry about that, it was certainly not intentional. Let me know if that covers it or if you think there's something else missing. COPYING | 1 + src/filter.c | 1 + 2 files changed, 2 insertions(+) diff --git a/COPYING b/COPYING index 8bbb3c3..efc1a94 100644 --- a/COPYING +++ b/COPYING @@ -1,3 +1,4 @@ +Copyright © 2006-2009 Simon Thum Copyright © 2008-2012 Kristian Høgsberg Copyright © 2010-2012 Intel Corporation Copyright © 2010-2011 Benjamin Franzke diff --git a/src/filter.c b/src/filter.c index b953bee..626cb0a 100644 --- a/src/filter.c +++ b/src/filter.c @@ -1,4 +1,5 @@ /* + * Copyright © 2006-2009 Simon Thum * Copyright © 2012 Jonas Ådahl * * Permission to use, copy, modify, distribute, and sell this software and ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH] remove trailing whitespaces
Remove trailing whitespaces because they are not needed and jumping to
the end of al ine should do just that and not jump to the whitespace.
---
cursor/cursor-data.h | 1004 +++---
src/scanner.c|2 +-
tests/connection-test.c |8 +-
tests/list-test.c|2 +-
tests/os-wrappers-test.c |2 +-
5 files changed, 509 insertions(+), 509 deletions(-)
diff --git a/cursor/cursor-data.h b/cursor/cursor-data.h
index 4c5e672..b1026d7 100644
--- a/cursor/cursor-data.h
+++ b/cursor/cursor-data.h
@@ -22,508 +22,508 @@
*/
static uint32_t cursor_data[] = {
- 0x, 0x, 0x, 0x, 0x, 0x,
- 0x, 0x, 0x, 0x, 0x, 0x,
- 0x, 0x, 0x, 0x, 0x, 0xff00,
- 0xff00, 0x, 0x, 0x, 0x, 0x,
- 0x, 0x, 0x, 0x, 0x, 0x,
- 0x, 0x, 0x, 0xff00, 0xff00, 0x,
- 0x, 0x, 0x, 0x, 0x, 0x,
- 0x, 0x, 0x, 0x, 0x, 0x,
- 0x, 0xff00, 0xff00, 0x, 0x, 0x,
- 0xff00, 0x, 0x, 0x, 0x, 0x,
- 0xff00, 0x, 0x, 0x, 0x, 0xff00,
- 0xff00, 0x, 0x, 0x, 0xff00, 0x,
- 0x, 0x, 0x, 0xff00, 0x, 0x,
- 0x, 0x, 0x, 0xff00, 0xff00, 0x,
- 0x, 0x, 0xff00, 0x, 0x, 0x,
- 0xff00, 0x, 0x, 0x, 0x, 0x,
- 0x, 0xff00, 0xff00, 0x, 0x, 0x,
- 0xff00, 0x, 0x, 0xff00, 0x, 0x,
- 0x, 0x, 0x, 0x, 0x, 0xff00,
- 0xff00, 0x, 0x, 0x, 0xff00, 0x,
- 0xff00, 0x, 0x, 0x, 0x, 0x,
- 0x, 0x, 0x, 0xff00, 0xff00, 0x,
- 0x, 0x, 0xff00, 0xff00, 0x, 0x,
- 0x, 0x, 0x, 0x, 0x, 0x,
- 0x, 0xff00, 0xff00, 0x, 0x, 0x,
- 0xff00, 0xff00, 0xff00, 0xff00, 0xff00, 0xff00,
- 0xff00, 0x, 0x, 0x, 0x, 0xff00,
- 0xff00, 0x, 0x, 0x, 0x, 0x,
- 0x, 0x, 0x, 0x, 0x, 0x,
- 0x, 0x, 0x, 0xff00, 0xff00, 0x,
- 0x, 0x, 0x, 0x, 0x, 0x,
- 0x, 0x, 0x, 0x, 0x, 0x,
- 0x, 0xff00, 0xff00, 0x, 0x, 0x,
- 0x, 0x, 0x, 0x, 0x, 0x,
- 0x, 0x, 0x, 0x, 0x, 0xff00,
- 0xff00, 0xff00, 0xff00, 0xff00, 0xff00, 0xff00,
- 0xff00, 0xff00, 0xff00, 0xff00, 0xff00, 0xff00,
- 0xff00, 0x, 0x, 0xff00, 0xff00, 0xff00,
- 0xff00, 0xff00, 0xff00, 0xff00, 0xff00, 0xff00,
- 0xff00, 0xff00, 0xff00, 0xff00, 0xff00, 0x,
- 0x, 0x, 0x, 0x, 0x, 0x,
- 0x, 0x, 0x, 0x, 0x, 0x,
- 0x, 0x, 0x, 0x, 0x, 0x,
- 0x, 0x, 0x, 0x, 0x, 0x,
- 0x, 0x, 0x, 0x, 0x, 0x,
- 0x, 0x, 0x, 0x, 0x, 0x,
- 0x, 0x, 0x, 0x, 0x, 0x,
- 0x, 0x, 0x, 0xff00, 0xff00, 0x,
- 0x, 0x, 0x, 0x, 0x, 0x,
- 0x, 0x, 0x, 0x, 0x, 0x,
- 0x, 0xff00, 0xff00, 0x, 0x, 0x,
- 0x, 0xff00, 0x, 0x, 0x, 0x,
- 0x, 0xff00, 0x, 0x, 0x, 0xff00,
- 0xff00, 0x, 0x, 0x, 0x, 0x
HiDPI problem with Gnome or Xwayland
I have a 4K display machine running Fedora 22. When I run a Wayland session it seems to me that applications which use X in some way get confused about the pixel size. The window manager (gnome-shell) will halve the size of the window whenever the window becomes active. Alt-tabbing can get pretty amusing. I'm not entirely sure whose software is doing it. If you're working on Wayland and read this, and have a 4K display maybe you'll try it out and see if it works for you. Gnome Terminal is one of the apps that does it. ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH] remove trailing whitespaces
Remove trailing whitespaces because they are not needed and jumping to
the end of al ine should do just that and not jump to the whitespace.
---
clients/desktop-shell.c| 10 -
clients/dnd.c | 12 +-
clients/editor.c | 2 +-
clients/flower.c | 6 ++---
clients/fullscreen.c | 4 ++--
clients/gears.c| 4 ++--
clients/glmatrix.c | 12 +-
clients/keyboard.c | 14 ++--
clients/nested.c | 2 +-
clients/terminal.c | 56 +++---
clients/transformed.c | 4 ++--
clients/window.c | 2 +-
desktop-shell/shell.c | 2 +-
ivi-shell/hmi-controller.c | 2 +-
14 files changed, 66 insertions(+), 66 deletions(-)
diff --git a/clients/desktop-shell.c b/clients/desktop-shell.c
index e2f9f80..777a50a 100644
--- a/clients/desktop-shell.c
+++ b/clients/desktop-shell.c
@@ -34,7 +34,7 @@
#include
#include
#include
-#include
+#include
#include
#include
#include
@@ -241,7 +241,7 @@ panel_launcher_motion_handler(struct widget *widget, struct
input *input,
static void
set_hex_color(cairo_t *cr, uint32_t color)
{
- cairo_set_source_rgba(cr,
+ cairo_set_source_rgba(cr,
((color >> 16) & 0xff) / 255.0,
((color >> 8) & 0xff) / 255.0,
((color >> 0) & 0xff) / 255.0,
@@ -319,7 +319,7 @@ panel_launcher_touch_down_handler(struct widget *widget,
struct input *input,
static void
panel_launcher_touch_up_handler(struct widget *widget, struct input *input,
- uint32_t serial, uint32_t time, int32_t id,
+ uint32_t serial, uint32_t time, int32_t id,
void *data)
{
struct panel_launcher *launcher;
@@ -440,7 +440,7 @@ panel_resize_handler(struct widget *widget,
struct panel_launcher *launcher;
struct panel *panel = data;
int x, y, w, h;
-
+
x = 10;
y = 16;
wl_list_for_each(launcher, &panel->launcher_list, link) {
@@ -521,7 +521,7 @@ panel_create(struct desktop *desktop)
widget_set_redraw_handler(panel->widget, panel_redraw_handler);
widget_set_resize_handler(panel->widget, panel_resize_handler);
-
+
panel_add_clock(panel);
s = weston_config_get_section(desktop->config, "shell", NULL, NULL);
diff --git a/clients/dnd.c b/clients/dnd.c
index e893d36..b5a7d41 100644
--- a/clients/dnd.c
+++ b/clients/dnd.c
@@ -103,12 +103,12 @@ item_create(struct display *display, int x, int y, int
seed)
item = malloc(sizeof *item);
if (item == NULL)
return NULL;
-
-
+
+
gettimeofday(&tv, NULL);
item->seed = seed ? seed : tv.tv_usec;
srandom(item->seed);
-
+
const int petal_count = 3 + random() % 5;
const double r1 = 20 + random() % 10;
const double r2 = 5 + random() % 12;
@@ -289,7 +289,7 @@ static void
data_source_send(void *data, struct wl_data_source *source,
const char *mime_type, int32_t fd)
{
- struct dnd_flower_message dnd_flower_message;
+ struct dnd_flower_message dnd_flower_message;
struct dnd_drag *dnd_drag = data;
char buffer[128];
int n;
@@ -325,7 +325,7 @@ data_source_cancelled(void *data, struct wl_data_source
*source)
* up the drag object created and the local state. */
wl_data_source_destroy(dnd_drag->data_source);
-
+
/* Destroy the item that has been dragged out */
cairo_surface_destroy(dnd_drag->item->surface);
free(dnd_drag->item);
@@ -619,7 +619,7 @@ dnd_receive_func(void *data, size_t len, int32_t x, int32_t
y, void *user_data)
len, sizeof *message);
return;
}
-
+
widget_get_allocation(dnd->widget, &allocation);
item = item_create(dnd->display,
x - message->x_offset - allocation.x,
diff --git a/clients/editor.c b/clients/editor.c
index 29cab34..f77a48d 100644
--- a/clients/editor.c
+++ b/clients/editor.c
@@ -612,7 +612,7 @@ editor_copy_cut(struct editor *editor, struct input *input,
bool cut)
if (!entry)
return;
-
+
if (entry->cursor != entry->anchor) {
int start_index = MIN(entry->cursor, entry->anchor);
int end_index = MAX(entry->cursor, entry->anchor);
diff --git a/clients/flower.c b/clients/flower.c
index 624525c..15315b2 100644
--- a/clients/flower.c
+++ b/clients/flower.c
@@ -86,7 +86,7 @@ draw_stuff(cairo_surface_t *surface, int width, int height)
cairo_curve_to(cr,
x1 - y1 * u, y1 + x1 * u,
x2 + y2 * v, y2 - x2 * v,
- x2, y2);
Re: [PATCH weston] launcher-util: Force all weston_launcher_open()s to use O_CLOEXEC
On Fri, 1 May 2015 11:46:36 -0500 Derek Foreman wrote: > Really, there's pretty much no time we'd ever want O_CLOEXEC unset, > as it will likely result in leaking fds to processes that aren't > interested in them or shouldn't have them. > > This also removes the (now unused) code from weston_logind_open() that > could drop O_CLOEXEC. > > Signed-off-by: Derek Foreman > --- > Daniel suggested I do something like this instead of fixing all the > callers as I did in my previous patches. > > I think this should supersede my previous two patches on the subject. > > It's still possible for someone to leak fds to child processes if they > really want to (by using fcntl to clear O_CLOEXEC). I really can't think > of a use case for that. By making it much harder to do on purpose it's > become almost impossible to do by accident... > > src/launcher-util.c | 8 +++- > src/logind-util.c | 20 ++-- > 2 files changed, 9 insertions(+), 19 deletions(-) Pushed. b0f5a25..2663c68 master -> master Thanks, pq ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston v2] logind: actually close fd in weston_logind_close()
On Thu, 30 Apr 2015 14:37:58 -0500
Derek Foreman wrote:
> Sigh.
>
> s/weston_logind_close/weston_launcher_close/
Fixed that when applying.
> On 30/04/15 02:37 PM, Derek Foreman wrote:
> > You had one job...
> >
> > Signed-off-by: Derek Foreman
> > ---
> > Recommended change by David Herrmann.
> >
> > src/launcher-util.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/src/launcher-util.c b/src/launcher-util.c
> > index e89710b..00cb935 100644
> > --- a/src/launcher-util.c
> > +++ b/src/launcher-util.c
> > @@ -190,7 +190,7 @@ void
> > weston_launcher_close(struct weston_launcher *launcher, int fd)
> > {
> > if (launcher->logind)
> > - return weston_logind_close(launcher->logind, fd);
> > + weston_logind_close(launcher->logind, fd);
> >
> > close(fd);
> > }
Pushed.
b0f5a25..2663c68 master -> master
Thanks,
pq
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH] compositor-fbdev: Wait and retry before failing on reconnect to the framebuffer
Hi On Fri, May 15, 2015 at 4:44 PM, Derek Foreman wrote: > On 15/05/15 05:30 AM, David Herrmann wrote: >> systemd-logind listens to VT events via poll(2) on >> /sys/class/tty/tty0/active. On VT switches it changes ACL permissions >> on dev-nodes underneath /dev, if the 'uaccess' tag is set on a device. >> But it cannot delay a VT-switch, it just reacts to it. >> >> It is inherent to this approach, that the permissions are set _after_ >> the VT switch. Therefore, there's a race and I guess it's what is hit >> here. However, systemd-logind sends out dbus signals after it fully >> changed the permissions. Hence, if you rely on the 'uaccess' >> functionality by systemd, then you better also use the systemd APIs >> (either sd_login_monitor_new() or DBus). If you don't want to use the >> systemd APIs, then you cannot rely on 'uaccess' (i.e., you have to set >> static group/user permissions on your device nodes; it will not >> interfere with 'uaccess', logind allows both in parallel). >> >> Long story short: Please run weston directly (_without_ weston-launch, >> instead directly invoking weston from within a logged-in VT), with >> systemd support compiled it. It should work fine with 'uaccess'. If >> you don't have systemd support compiled in, please use static access >> permissions. > > Does this mean weston-launch is always the wrong thing to do if systemd > support is compiled in? If systemd is running, weston-launch should probably execve() 'weston' directly without setting up WESTON_LAUNCHER_FD. > ie) should we refuse to build weston-launch if systemd support is enabled? This should be a run-time detection, not build-time, imho. Thanks David ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH] compositor-fbdev: Wait and retry before failing on reconnect to the framebuffer
On 15/05/15 05:30 AM, David Herrmann wrote:
> Hi
>
> On Fri, May 15, 2015 at 8:39 AM, Pekka Paalanen wrote:
>>
>> On Wed, 13 May 2015 21:43:39 -0400
>> nerdopolis wrote:
>>
>>> Resolving https://bugs.freedesktop.org/show_bug.cgi?id=73782
>>> udev might be configured to set the permissions on framebuffer devices with
>>> the UACCESS attribute.
>>> Weston currently attempts to reconnect to the framebuffer device before
>>> udev can set the permissions back.
>>>
>>> It waits 3 times in case if the system is heavily paging, or slowed down,
>>> and prevents udev from working.
>>> In my testing the delay is long enough where it works on the first try
>>> ---
>>> src/compositor-fbdev.c | 12 +++-
>>> 1 file changed, 11 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/src/compositor-fbdev.c b/src/compositor-fbdev.c
>>> index 7c505ce..42d2881 100644
>>> --- a/src/compositor-fbdev.c
>>> +++ b/src/compositor-fbdev.c
>>> @@ -663,13 +663,23 @@ fbdev_output_reenable(struct fbdev_compositor
>>> *compositor,
>>> struct fbdev_output *output = to_fbdev_output(base);
>>> struct fbdev_screeninfo new_screen_info;
>>> int fb_fd;
>>> + int retries;
>>> const char *device;
>>>
>>> weston_log("Re-enabling fbdev output.\n");
>>>
>>> /* Create the frame buffer. */
>>> - fb_fd = fbdev_frame_buffer_open(output, output->device,
>>> + fb_fd = -1;
>>> + retries = 0;
>>> + while (fb_fd < 0 && retries < 3) {
>>> + usleep(1);
>>> + fb_fd = fbdev_frame_buffer_open(output, output->device,
>>> &new_screen_info);
>>> + if (fb_fd < 0) {
>>> + weston_log("Creating frame buffer failed. Retrying...\n");
>>> + }
>>> + retries++;
>>> + }
>>> if (fb_fd < 0) {
>>> weston_log("Creating frame buffer failed.\n");
>>> goto err;
>>
>> Hi,
>>
>> I hate sleeps. I'd really want an explanation in the commit message on
>> why this cannot be fixed properly at all, and you need a delay to have
>> it work.
>>
>> It is possible there is no better way, but I would like to understand
>> why first.
>>
>> For instance, should reenable() be waiting for some sort of udev
>> advertisement of the fb device? Is there any event from udev to signal
>> this?
>>
>> Do you have systemd involved in your use case? Could this be solved with
>> systemd or logind? Or should we let weston-launch open the fb device
>> instead?
>>
>> I'm CC'ing David, maybe he might have an idea?
>
> systemd-logind listens to VT events via poll(2) on
> /sys/class/tty/tty0/active. On VT switches it changes ACL permissions
> on dev-nodes underneath /dev, if the 'uaccess' tag is set on a device.
> But it cannot delay a VT-switch, it just reacts to it.
>
> It is inherent to this approach, that the permissions are set _after_
> the VT switch. Therefore, there's a race and I guess it's what is hit
> here. However, systemd-logind sends out dbus signals after it fully
> changed the permissions. Hence, if you rely on the 'uaccess'
> functionality by systemd, then you better also use the systemd APIs
> (either sd_login_monitor_new() or DBus). If you don't want to use the
> systemd APIs, then you cannot rely on 'uaccess' (i.e., you have to set
> static group/user permissions on your device nodes; it will not
> interfere with 'uaccess', logind allows both in parallel).
>
> Long story short: Please run weston directly (_without_ weston-launch,
> instead directly invoking weston from within a logged-in VT), with
> systemd support compiled it. It should work fine with 'uaccess'. If
> you don't have systemd support compiled in, please use static access
> permissions.
Does this mean weston-launch is always the wrong thing to do if systemd
support is compiled in?
ie) should we refuse to build weston-launch if systemd support is enabled?
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston v2 04/14] tests: Add surface checks
On Fri, 15 May 2015 01:21:49 -0700
Bryce Harrington wrote:
> Introduce helper routines for testing surfaces against specific
> conditions. These allow tests to validate screen captures as displaying
> the correct rendering results.
>
> Signed-off-by: Bryce Harrington
> ---
> tests/weston-test-client-helper.c | 116
> ++
> tests/weston-test-client-helper.h | 15 +
> 2 files changed, 131 insertions(+)
>
> diff --git a/tests/weston-test-client-helper.c
> b/tests/weston-test-client-helper.c
> index 080bb62..04b77d5 100644
> --- a/tests/weston-test-client-helper.c
> +++ b/tests/weston-test-client-helper.c
> +bool
> +check_surfaces_equal(const struct surface *a, const struct surface *b)
> +{
> + int y;
> + void *p, *q;
> +
> + if (a == NULL || b == NULL)
> + return false;
> + if (a->width != b->width || a->height != a->height)
The latter compares a to a, should be a to b.
Thanks,
pq
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH libinput 0/3] touchpad: add disable-while-typing feature
Hi, On 07-05-15 08:53, Peter Hutterer wrote: This patchset adds the old feature "disable while typing" to libinput. We already have some palm detection but on some touchpads (or typing behaviours) it's not enough to analyse the edges alone. We have some plans to add general better detection of palms on touchpads that e.g. provide reliable finger width, but that simply won't work on all touchpads. Long term what's likely to happen is that depending on a touchpad's capability we'll enable various detection methods, with this one being the fallback when we don't trust the others to work. The entire set LGTM: Reviewed-by: Hans de Goede Regards, Hans ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH libinput] filter: add Simon's copyright
Hi, On 06-05-15 05:20, Peter Hutterer wrote: This code was largely lifted from the X server in bb25b2ad297891430606c367bfabc but didn't take the copyright messages that applied to that code. Signed-off-by: Peter Hutterer LGTM: Reviewed-by: Hans de Goede Regards, Hans --- Simon: I think that should cover it. Sorry about that, it was certainly not intentional. Let me know if that covers it or if you think there's something else missing. COPYING | 1 + src/filter.c | 1 + 2 files changed, 2 insertions(+) diff --git a/COPYING b/COPYING index 8bbb3c3..efc1a94 100644 --- a/COPYING +++ b/COPYING @@ -1,3 +1,4 @@ +Copyright © 2006-2009 Simon Thum Copyright © 2008-2012 Kristian Høgsberg Copyright © 2010-2012 Intel Corporation Copyright © 2010-2011 Benjamin Franzke diff --git a/src/filter.c b/src/filter.c index b953bee..626cb0a 100644 --- a/src/filter.c +++ b/src/filter.c @@ -1,4 +1,5 @@ /* + * Copyright © 2006-2009 Simon Thum * Copyright © 2012 Jonas Ådahl * * Permission to use, copy, modify, distribute, and sell this software and ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH libinput 1/3] test: add litest_add_ranged* functionality
Hi,
On 06-05-15 02:17, Peter Hutterer wrote:
litest_add_ranged* takes a range parameter that serves as the lower/upper
boundary for a loop. This enables tests to be run multiple times, avoiding the
timeouts we triggered by having the loops inside (e.g. see 2bf8d035c and
6dd02468).
This just wraps the underlying check framework, the ranged variable is
available as "_i" in the test.
Signed-off-by: Peter Hutterer
The entire set LGTM:
Reviewed-by: Hans de Goede
Regards,
Hans
---
test/litest.c | 64 +--
test/litest.h | 20 +++
2 files changed, 74 insertions(+), 10 deletions(-)
diff --git a/test/litest.c b/test/litest.c
index 91f2747..48d3ca4 100644
--- a/test/litest.c
+++ b/test/litest.c
@@ -184,7 +184,8 @@ litest_drop_udev_rules(void)
static void
litest_add_tcase_for_device(struct suite *suite,
void *func,
- const struct litest_test_device *dev)
+ const struct litest_test_device *dev,
+ const struct range *range)
{
struct test *t;
const char *test_name = dev->shortname;
@@ -193,7 +194,13 @@ litest_add_tcase_for_device(struct suite *suite,
if (strcmp(t->name, test_name) != 0)
continue;
- tcase_add_test(t->tc, func);
+ if (range)
+ tcase_add_loop_test(t->tc,
+ func,
+ range->lower,
+ range->upper);
+ else
+ tcase_add_test(t->tc, func);
return;
}
@@ -216,7 +223,9 @@ litest_add_tcase_for_device(struct suite *suite,
}
static void
-litest_add_tcase_no_device(struct suite *suite, void *func)
+litest_add_tcase_no_device(struct suite *suite,
+ void *func,
+ const struct range *range)
{
struct test *t;
const char *test_name = "no device";
@@ -225,7 +234,10 @@ litest_add_tcase_no_device(struct suite *suite, void *func)
if (strcmp(t->name, test_name) != 0)
continue;
- tcase_add_test(t->tc, func);
+ if (range)
+ tcase_add_loop_test(t->tc, func, range->lower,
range->upper);
+ else
+ tcase_add_test(t->tc, func);
return;
}
@@ -241,7 +253,8 @@ litest_add_tcase_no_device(struct suite *suite, void *func)
static void
litest_add_tcase(struct suite *suite, void *func,
enum litest_device_feature required,
-enum litest_device_feature excluded)
+enum litest_device_feature excluded,
+const struct range *range)
{
struct litest_test_device **dev = devices;
@@ -250,17 +263,17 @@ litest_add_tcase(struct suite *suite, void *func,
if (required == LITEST_DISABLE_DEVICE &&
excluded == LITEST_DISABLE_DEVICE) {
- litest_add_tcase_no_device(suite, func);
+ litest_add_tcase_no_device(suite, func, range);
} else if (required != LITEST_ANY || excluded != LITEST_ANY) {
while (*dev) {
if (((*dev)->features & required) == required &&
((*dev)->features & excluded) == 0)
- litest_add_tcase_for_device(suite, func, *dev);
+ litest_add_tcase_for_device(suite, func, *dev,
range);
dev++;
}
} else {
while (*dev) {
- litest_add_tcase_for_device(suite, func, *dev);
+ litest_add_tcase_for_device(suite, func, *dev, range);
dev++;
}
}
@@ -272,6 +285,18 @@ litest_add_no_device(const char *name, void *func)
litest_add(name, func, LITEST_DISABLE_DEVICE, LITEST_DISABLE_DEVICE);
}
+void
+litest_add_ranged_no_device(const char *name,
+ void *func,
+ const struct range *range)
+{
+ litest_add_ranged(name,
+ func,
+ LITEST_DISABLE_DEVICE,
+ LITEST_DISABLE_DEVICE,
+ range);
+}
+
static struct suite *
get_suite(const char *name)
{
@@ -302,7 +327,17 @@ litest_add(const char *name,
enum litest_device_feature required,
enum litest_device_feature excluded)
{
- litest_add_tcase(get_suite(name), func, required, excluded);
+ litest_add_ranged(name, func, required, excluded, NULL);
+}
+
+void
+litest_add_ranged(const char *name,
+ void *func,
+ enum litest_device_feature required,
+ enum litest_device_feature
Re: [PATCH libinput] timer: fix coverity warning about unused return value
Hi, On 06-05-15 02:01, Peter Hutterer wrote: "read(int, void *, size_t)" returns the number of bytes read, but it is ignored. We don't really care about the number of bytes, but let's complain if we get anything but EAGAIN. Signed-off-by: Peter Hutterer LGTM: Reviewed-by: Hans de Goede Regards, Hans --- src/timer.c | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/timer.c b/src/timer.c index 114a649..d1d3c10 100644 --- a/src/timer.c +++ b/src/timer.c @@ -102,8 +102,14 @@ libinput_timer_handler(void *data) struct libinput_timer *timer, *tmp; uint64_t now; uint64_t discard; + int r; - read(libinput->timer.fd, &discard, sizeof(discard)); + r = read(libinput->timer.fd, &discard, sizeof(discard)); + if (r == -1 && errno != EAGAIN) + log_bug_libinput(libinput, +"Error %d reading from timerfd (%s)", +errno, +strerror(errno)); now = libinput_now(libinput); if (now == 0) ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH] compositor-fbdev: Wait and retry before failing on reconnect to the framebuffer
Hi
On Fri, May 15, 2015 at 8:39 AM, Pekka Paalanen wrote:
>
> On Wed, 13 May 2015 21:43:39 -0400
> nerdopolis wrote:
>
> > Resolving https://bugs.freedesktop.org/show_bug.cgi?id=73782
> > udev might be configured to set the permissions on framebuffer devices with
> > the UACCESS attribute.
> > Weston currently attempts to reconnect to the framebuffer device before
> > udev can set the permissions back.
> >
> > It waits 3 times in case if the system is heavily paging, or slowed down,
> > and prevents udev from working.
> > In my testing the delay is long enough where it works on the first try
> > ---
> > src/compositor-fbdev.c | 12 +++-
> > 1 file changed, 11 insertions(+), 1 deletion(-)
> >
> > diff --git a/src/compositor-fbdev.c b/src/compositor-fbdev.c
> > index 7c505ce..42d2881 100644
> > --- a/src/compositor-fbdev.c
> > +++ b/src/compositor-fbdev.c
> > @@ -663,13 +663,23 @@ fbdev_output_reenable(struct fbdev_compositor
> > *compositor,
> > struct fbdev_output *output = to_fbdev_output(base);
> > struct fbdev_screeninfo new_screen_info;
> > int fb_fd;
> > + int retries;
> > const char *device;
> >
> > weston_log("Re-enabling fbdev output.\n");
> >
> > /* Create the frame buffer. */
> > - fb_fd = fbdev_frame_buffer_open(output, output->device,
> > + fb_fd = -1;
> > + retries = 0;
> > + while (fb_fd < 0 && retries < 3) {
> > + usleep(1);
> > + fb_fd = fbdev_frame_buffer_open(output, output->device,
> > &new_screen_info);
> > + if (fb_fd < 0) {
> > + weston_log("Creating frame buffer failed. Retrying...\n");
> > + }
> > + retries++;
> > + }
> > if (fb_fd < 0) {
> > weston_log("Creating frame buffer failed.\n");
> > goto err;
>
> Hi,
>
> I hate sleeps. I'd really want an explanation in the commit message on
> why this cannot be fixed properly at all, and you need a delay to have
> it work.
>
> It is possible there is no better way, but I would like to understand
> why first.
>
> For instance, should reenable() be waiting for some sort of udev
> advertisement of the fb device? Is there any event from udev to signal
> this?
>
> Do you have systemd involved in your use case? Could this be solved with
> systemd or logind? Or should we let weston-launch open the fb device
> instead?
>
> I'm CC'ing David, maybe he might have an idea?
systemd-logind listens to VT events via poll(2) on
/sys/class/tty/tty0/active. On VT switches it changes ACL permissions
on dev-nodes underneath /dev, if the 'uaccess' tag is set on a device.
But it cannot delay a VT-switch, it just reacts to it.
It is inherent to this approach, that the permissions are set _after_
the VT switch. Therefore, there's a race and I guess it's what is hit
here. However, systemd-logind sends out dbus signals after it fully
changed the permissions. Hence, if you rely on the 'uaccess'
functionality by systemd, then you better also use the systemd APIs
(either sd_login_monitor_new() or DBus). If you don't want to use the
systemd APIs, then you cannot rely on 'uaccess' (i.e., you have to set
static group/user permissions on your device nodes; it will not
interfere with 'uaccess', logind allows both in parallel).
Long story short: Please run weston directly (_without_ weston-launch,
instead directly invoking weston from within a logged-in VT), with
systemd support compiled it. It should work fine with 'uaccess'. If
you don't have systemd support compiled in, please use static access
permissions.
Thanks
David
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston v2 00/14] Implement screenshot-based testing with the headless renderer
Hi Bryce,
On 15 May 2015 at 09:21, Bryce Harrington wrote:
> This series adds support for implementing test cases that can check
> rendering output without needing connection to a physical output. It
> utilizes the pixman renderer in the headless backend to generate
> screenshots at the request of the test client.
>
> The test creates a shm buffer and passes it to the server via the
> Weston-Test protocol ('capture_screenshot'). The server then renders
> the display contents into the buffer and returns it as a response
> ('capture_screenshot_done').
>
> The test then loads a corresponding reference PNG from disk using Cairo,
> and then compares it with the captured screenshot. Note that the
> screenshot includes the current time in the desktop clock, which will of
> course be different in the captured screenshot from the reference image.
> So this checks a small clipped out section of each of the two images to
> verify congruence.
>
> By default, Wayland fades in the display and will show a patterned
> background. The former feature causes a black or nearly-black image to
> be captured (the darkness of which may vary from system to system), and
> the latter merely adds noise in our comparison, so both features are
> disabled via a test-specific configuration .ini file.
Thanks for these. There are still some stride issues on patch 4, but
for all the rest:
Reviewed-by: Daniel Stone
If you want a quicker pass over a revised 4, feel free to ping me
patches on IRC and I can give R-b there.
These look good to land in general; they're not perfect, but I'd much
rather have them in there and just fix them up later.
Cheers,
Daniel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston v2 04/14] tests: Add surface checks
Hi,
On 15 May 2015 at 09:21, Bryce Harrington wrote:
> +/**
> + * check_surfaces_equal() - tests if two surfaces are pixel-identical
> + *
> + * Returns true if surface buffers have all the same byte values,
> + * false if the surfaces don't match or can't be compared due to
> + * different dimensions.
> + */
> +bool
> +check_surfaces_equal(const struct surface *a, const struct surface *b)
> +{
> + int y;
> + void *p, *q;
> +
> + if (a == NULL || b == NULL)
> + return false;
> + if (a->width != b->width || a->height != a->height)
> + return false;
I realise you're going to hate me here, but I have led you astray a bit ...
> + if (a->stride == b->stride) {
/* Second comparison needed to avoid checking potentially garbage
values in the padding. */
if (a->stride == b->stride && a->stride == (a->width * a->bpp))
> + printf("Checking data for equivalent strides\n");
> + return (memcmp(a->data, b->data, a->stride * a->height) == 0);
> + } else {
> + printf("Manually comparing due to differing strides\n");
> + for (y = 0; y < a->height; y++) {
> + p = a->data + (y * a->stride);
> + q = b->data + (y * b->stride);
> + if (memcmp(p, q, a->stride) != 0)
if (memcmp(p, q, a->width * a->bpp) != 0) /* don't check the garbage
values in the stride */
So sorry.
If you just fix these up (and a->bpp is actually a real thing which is
bytes per pixel), then I'm happy for it to get merged without a third
pass through the list.
> + printf("Bytewise comparison inside clip\n");
> + a_bpp = a->stride / a->width;
> + b_bpp = b->stride / b->width;
This isn't necessarily true, e.g. if you have a 1366x768 buffer padded
out to 1344px to meet a 32px alignment requirement, at 32bpp, then
deriving bpp from stride/width would give you a bpp of 3.9355.
Probably enough to get away with, but really bpp needs to come from
the format.
> + for (i=y0; i + p = a->data + i * a->stride + x0 * a_bpp;
> + q = b->data + i * b->stride + x0 * b_bpp;
> + if (a->stride == b->stride) {
> + if (memcmp(p, q, (x1-x0)*a_bpp) != 0) {
> + // Dump the bad row
Stray C++ comment.
> + printf("Mismatched image on row %d\n", i);
> + for (j=0; j<(x1-x0)*a_bpp; j++) {
> + a_char = *((char*)(p+j*a_bpp));
> + b_char = *((char*)(q+j*b_bpp));
> + printf("%d,%d: %8x %8x %s\n", i, j,
> a_char, b_char,
> + (a_char != b_char)? " <---":
> "");
> + }
> + return false;
> + }
> + } else {
> + /* account for bpp differences */
> + for (j=0; j + a_char = *((char*)(p+j*a_bpp));
> + b_char = *((char*)(q+j*b_bpp));
> + if (a_char != b_char) {
> + printf("%d,%d: %8x %8x %s\n", i, j,
> a_char, b_char,
> + (a_char != b_char)? " <---":
> "");
> + return false;
> + }
> + }
> + }
Hmm, maybe a stupid question, but don't these branches fundamentally
do the same thing ... ? Except that in the 'account for bpp
differences' branch, it only iterates for as many bytes as there are
pixels to compare, which probably isn't what you want. I think the
memcmp should be perfectly sufficient, and you don't need to test the
stride at all.
Cheers,
Daniel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston v2 10/14] tests: Add write_surface_as_png() helper
And use the helper routine for generating the output filename.
Signed-off-by: Bryce Harrington
---
tests/internal-screenshot-test.c | 42
1 file changed, 30 insertions(+), 12 deletions(-)
diff --git a/tests/internal-screenshot-test.c b/tests/internal-screenshot-test.c
index 2d6f8e7..7f1a690 100644
--- a/tests/internal-screenshot-test.c
+++ b/tests/internal-screenshot-test.c
@@ -31,6 +31,34 @@
char *server_parameters="--use-pixman --width=320 --height=240";
+/** write_surface_as_png()
+ *
+ * Writes out a given weston test surface to disk as a PNG image
+ * using the provided filename (with path).
+ *
+ * @returns true if successfully saved file; false otherwise.
+ */
+static bool
+write_surface_as_png(const struct surface* weston_surface, const char *fname) {
+ cairo_surface_t *cairo_surface;
+ cairo_status_t status;
+
+ cairo_surface =
cairo_image_surface_create_for_data(weston_surface->data,
+ CAIRO_FORMAT_ARGB32,
+
weston_surface->width,
+
weston_surface->height,
+
weston_surface->stride);
+ printf("Writing PNG to disk\n");
+ status = cairo_surface_write_to_png(cairo_surface, fname);
+ if (status != CAIRO_STATUS_SUCCESS) {
+ printf("Failed to save screenshot: %s\n",
+ cairo_status_to_string(status));
+ return false;
+ }
+ cairo_surface_destroy(cairo_surface);
+ return true;
+}
+
TEST(internal_screenshot)
{
struct client *client;
@@ -123,18 +151,8 @@ TEST(internal_screenshot)
/* Test dumping of non-matching images */
if (!match || dump_all_images) {
- /* Write image to .png file */
- cairo_surface_t *surface;
-
- surface = cairo_image_surface_create_for_data(screenshot->data,
-
CAIRO_FORMAT_ARGB32,
- screenshot->width,
-
screenshot->height,
-
screenshot->stride);
-
- printf("Writing PNG to disk\n");
- cairo_surface_write_to_png(surface,
"clientside-screenshot.png");
- cairo_surface_destroy(surface);
+ fname = screenshot_output_filename("internal-screenshot", 0);
+ write_surface_as_png(screenshot, fname);
}
free(screenshot);
--
1.9.1
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston v2 05/14] tests: Support --config to enable tests to override config defaults
Implements a simple mechanism to allow tests to customize the
configuration. For a given -test.c just place a .ini file
at the same location as the test itself. Alternately, you can generate
a .ini in the same directory that the compiled test is placed
(i.e. the top builddir). If no configuration file is found, then no
configuration will be used (i.e. --no-config is specified.)
Signed-off-by: Bryce Harrington
---
Makefile.am| 3 ++-
tests/weston-tests-env | 14 --
2 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/Makefile.am b/Makefile.am
index 2f16fac..543c736 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -967,7 +967,8 @@ ivi_tests =
$(ivi_tests) : $(builddir)/tests/weston-ivi.ini
AM_TESTS_ENVIRONMENT = \
- abs_builddir='$(abs_builddir)'; export abs_builddir;
+ abs_builddir='$(abs_builddir)'; export abs_builddir; \
+ abs_top_srcdir='$(abs_top_srcdir)'; export abs_top_srcdir;
TEST_EXTENSIONS = .la .weston
LA_LOG_COMPILER = $(srcdir)/tests/weston-tests-env
diff --git a/tests/weston-tests-env b/tests/weston-tests-env
index f945ac6..4da908a 100755
--- a/tests/weston-tests-env
+++ b/tests/weston-tests-env
@@ -26,6 +26,16 @@ SHELL_PLUGIN=$MODDIR/desktop-shell.so
TEST_PLUGIN=$MODDIR/weston-test.so
XWAYLAND_PLUGIN=$MODDIR/xwayland.so
+CONFIG_FILE="${TEST_NAME}.ini"
+
+if [ -e "${abs_builddir}/${CONFIG_FILE}" ]; then
+ CONFIG="--config=${abs_builddir}/${CONFIG_FILE}"
+elif [ -e "${abs_top_srcdir}/tests/${CONFIG_FILE}" ]; then
+ CONFIG="--config=${abs_top_srcdir}/tests/${CONFIG_FILE}"
+else
+ CONFIG="--no-config"
+fi
+
case $TEST_FILE in
ivi-*.la|ivi-*.so)
SHELL_PLUGIN=$MODDIR/ivi-shell.so
@@ -43,11 +53,11 @@ case $TEST_FILE in
*.la|*.so)
WESTON_BUILD_DIR=$abs_builddir \
$WESTON --backend=$MODDIR/$BACKEND \
- --no-config \
--shell=$SHELL_PLUGIN \
--socket=test-${TEST_NAME} \
--modules=$MODDIR/${TEST_FILE/.la/.so},$XWAYLAND_PLUGIN
\
--log="$SERVERLOG" \
+ ${CONFIG} \
&> "$OUTLOG"
;;
ivi-*.weston)
@@ -69,10 +79,10 @@ case $TEST_FILE in
WESTON_TEST_CLIENT_PATH=$abs_builddir/$TEST_FILE $WESTON \
--socket=test-${TEST_NAME} \
--backend=$MODDIR/$BACKEND \
- --no-config \
--shell=$SHELL_PLUGIN \
--log="$SERVERLOG" \
--modules=$TEST_PLUGIN,$XWAYLAND_PLUGIN \
+ ${CONFIG} \
$($abs_builddir/$TEST_FILE --params) \
&> "$OUTLOG"
esac
--
1.9.1
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston v2 04/14] tests: Add surface checks
Introduce helper routines for testing surfaces against specific
conditions. These allow tests to validate screen captures as displaying
the correct rendering results.
Signed-off-by: Bryce Harrington
---
tests/weston-test-client-helper.c | 116 ++
tests/weston-test-client-helper.h | 15 +
2 files changed, 131 insertions(+)
diff --git a/tests/weston-test-client-helper.c
b/tests/weston-test-client-helper.c
index 080bb62..04b77d5 100644
--- a/tests/weston-test-client-helper.c
+++ b/tests/weston-test-client-helper.c
@@ -32,6 +32,10 @@
#include "../shared/os-compatibility.h"
#include "weston-test-client-helper.h"
+#define max(a, b) (((a) > (b)) ? (a) : (b))
+#define min(a, b) (((a) > (b)) ? (b) : (a))
+#define clip(x, a, b) min(max(x, a), b)
+
void *
fail_on_null(void *p)
{
@@ -861,3 +865,115 @@ screenshot_reference_filename(const char *basename,
uint32_t seq) {
return NULL;
return filename;
}
+
+/**
+ * check_surfaces_equal() - tests if two surfaces are pixel-identical
+ *
+ * Returns true if surface buffers have all the same byte values,
+ * false if the surfaces don't match or can't be compared due to
+ * different dimensions.
+ */
+bool
+check_surfaces_equal(const struct surface *a, const struct surface *b)
+{
+ int y;
+ void *p, *q;
+
+ if (a == NULL || b == NULL)
+ return false;
+ if (a->width != b->width || a->height != a->height)
+ return false;
+
+ if (a->stride == b->stride) {
+ printf("Checking data for equivalent strides\n");
+ return (memcmp(a->data, b->data, a->stride * a->height) == 0);
+ } else {
+ printf("Manually comparing due to differing strides\n");
+ for (y = 0; y < a->height; y++) {
+ p = a->data + (y * a->stride);
+ q = b->data + (y * b->stride);
+ if (memcmp(p, q, a->stride) != 0)
+ return false;
+ }
+ return true;
+ }
+}
+
+/**
+ * check_surfaces_match_in_clip() - tests if a given region within two
+ * surfaces are pixel-identical.
+ *
+ * Returns true if the two surfaces have the same byte values within the
+ * given clipping region, or false if they don't match or the surfaces
+ * can't be compared.
+ */
+bool
+check_surfaces_match_in_clip(const struct surface *a, const struct surface *b,
const struct rectangle *clip_rect)
+{
+ int i, j;
+ int a_bpp, b_bpp;
+ int x0, y0, x1, y1;
+ void *p, *q;
+ char a_char, b_char;
+
+ if (a == NULL || b == NULL || clip_rect == NULL)
+ return false;
+
+ if (a->data == NULL || b->data == NULL) {
+ printf("Undefined data\n");
+ return false;
+ }
+ if (a->width != b->width || a->height != b->height) {
+ printf("Mismatched dimensions: %d,%d != %d,%d\n",
+ a->width, a->height, b->width, b->height);
+ return false;
+ }
+ if (clip_rect->x > a->width || clip_rect->y > a->height) {
+ printf("Clip outside image boundaries\n");
+ return true;
+ }
+
+ x0 = max(0, clip_rect->x);
+ y0 = max(0, clip_rect->y);
+ x1 = min(a->width, clip_rect->x + clip_rect->width);
+ y1 = min(a->height, clip_rect->y + clip_rect->height);
+
+ if (x0 == x1 || y0 == y1) {
+ printf("Degenerate comparison\n");
+ return true;
+ }
+
+ printf("Bytewise comparison inside clip\n");
+ a_bpp = a->stride / a->width;
+ b_bpp = b->stride / b->width;
+ for (i=y0; idata + i * a->stride + x0 * a_bpp;
+ q = b->data + i * b->stride + x0 * b_bpp;
+ if (a->stride == b->stride) {
+ if (memcmp(p, q, (x1-x0)*a_bpp) != 0) {
+ // Dump the bad row
+ printf("Mismatched image on row %d\n", i);
+ for (j=0; j<(x1-x0)*a_bpp; j++) {
+ a_char = *((char*)(p+j*a_bpp));
+ b_char = *((char*)(q+j*b_bpp));
+ printf("%d,%d: %8x %8x %s\n", i, j,
a_char, b_char,
+ (a_char != b_char)? " <---": "");
+ }
+ return false;
+ }
+ } else {
+ /* account for bpp differences */
+ for (j=0; j
#include
+
#include "weston-test-runner.h"
#include "weston-test-client-protocol.h"
@@ -129,9 +130,17 @@ struct surface {
int y;
int width;
int height;
+ int stride;
void *data;
};
+struct rectangle {
+ int x;
+ int y;
+ int width;
+ int height;
+}
[PATCH weston v2 06/14] protocol: Add test screenshot capability
This adds a capture_screenshot request which returns an image of the screen in the client-supplied wl_buffer for the given display output. A 'done' event is used to indicate when screenshotting has finished and the wl_buffer is ready to be read. Signed-off-by: Bryce Harrington --- protocol/weston-test.xml | 16 1 file changed, 16 insertions(+) diff --git a/protocol/weston-test.xml b/protocol/weston-test.xml index 643a0c7..2fd6b7a 100644 --- a/protocol/weston-test.xml +++ b/protocol/weston-test.xml @@ -73,6 +73,22 @@ + + +Records an image of what is currently displayed on a given +display output, returning the image as an event. + + + + + + + The capture_screenshot_done signal is sent when a screenshot has been copied into the + provided buffer. + + -- 1.9.1 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston v2 09/14] tests: Add internal test for the weston test screenshot capability
This also serves as a proof of concept of the screen capture
functionality and as a demo for snapshot-based rendering verification.
Implements screenshot saving clientside in the test itself.
This also demonstrates use of test-specific configuration files, in this
case to disable fadein animations and background images.
Signed-off-by: Bryce Harrington
---
Makefile.am| 20 +++-
tests/internal-screenshot-test.c | 144 +
tests/internal-screenshot.ini | 3 +
tests/reference/internal-screenshot-00.png | Bin 0 -> 5149 bytes
4 files changed, 166 insertions(+), 1 deletion(-)
create mode 100644 tests/internal-screenshot-test.c
create mode 100644 tests/internal-screenshot.ini
create mode 100644 tests/reference/internal-screenshot-00.png
diff --git a/Makefile.am b/Makefile.am
index 543c736..e860e0e 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -941,7 +941,10 @@ libshared_cairo_la_SOURCES = \
# tests subdirectory
#
-TESTS = $(shared_tests) $(module_tests) $(weston_tests) $(ivi_tests)
+TESTS = $(internal_tests) $(shared_tests) $(module_tests) $(weston_tests)
$(ivi_tests)
+
+internal_tests = \
+ internal-screenshot.weston
shared_tests = \
config-parser.test \
@@ -988,6 +991,7 @@ noinst_LTLIBRARIES += \
noinst_PROGRAMS += \
$(setbacklight) \
+ $(internal_tests) \
$(shared_tests) \
$(weston_tests) \
$(ivi_tests)\
@@ -1040,6 +1044,20 @@ nodist_libtest_client_la_SOURCES = \
libtest_client_la_CFLAGS = $(AM_CFLAGS) $(TEST_CLIENT_CFLAGS)
libtest_client_la_LIBADD = $(TEST_CLIENT_LIBS) libshared.la libtest-runner.la
+
+#
+# Internal tests - tests functionality of the testsuite itself
+#
+
+internal_screenshot_weston_SOURCES = tests/internal-screenshot-test.c
+internal_screenshot_weston_CFLAGS = $(AM_CFLAGS) $(TEST_CLIENT_CFLAGS)
$(CAIRO_CFLAGS)
+internal_screenshot_weston_LDADD = libtest-client.la $(CAIRO_LIBS)
+
+
+#
+# Weston Tests
+#
+
bad_buffer_weston_SOURCES = tests/bad-buffer-test.c
bad_buffer_weston_CFLAGS = $(AM_CFLAGS) $(TEST_CLIENT_CFLAGS)
bad_buffer_weston_LDADD = libtest-client.la
diff --git a/tests/internal-screenshot-test.c b/tests/internal-screenshot-test.c
new file mode 100644
index 000..2d6f8e7
--- /dev/null
+++ b/tests/internal-screenshot-test.c
@@ -0,0 +1,144 @@
+/*
+ * Copyright © 2015 Samsung Electronics Co., Ltd
+ *
+ * Permission to use, copy, modify, distribute, and sell this software and
+ * its documentation for any purpose is hereby granted without fee, provided
+ * that the above copyright notice appear in all copies and that both that
+ * copyright notice and this permission notice appear in supporting
+ * documentation, and that the name of the copyright holders not be used in
+ * advertising or publicity pertaining to distribution of the software
+ * without specific, written prior permission. The copyright holders make
+ * no representations about the suitability of this software for any
+ * purpose. It is provided "as is" without express or implied warranty.
+ *
+ * THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS
+ * SOFTWARE, INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND
+ * FITNESS, IN NO EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY
+ * SPECIAL, INDIRECT OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER
+ * RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF
+ * CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN
+ * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+#include "config.h"
+
+#include
+#include
+#include /* memcpy */
+#include
+
+#include "weston-test-client-helper.h"
+
+char *server_parameters="--use-pixman --width=320 --height=240";
+
+TEST(internal_screenshot)
+{
+ struct client *client;
+ struct surface *screenshot = NULL;
+ struct surface *reference = NULL;
+ struct rectangle clip;
+ const char *fname;
+ cairo_surface_t *reference_cairo_surface;
+ cairo_status_t status;
+ bool match = false;
+ bool dump_all_images = true;
+
+ printf("Starting test\n");
+
+ /* Create the client */
+ client = create_client_and_test_surface(100, 100, 100, 100);
+ assert(client);
+ printf("Client created\n");
+
+ /* Create a surface to hold the screenshot */
+ screenshot = xzalloc(sizeof *screenshot);
+ assert(screenshot);
+
+ /* Create and attach buffer to our surface */
+ screenshot->wl_buffer = create_shm_buffer(client,
+ client->output->width,
+ client->output->height,
+
[PATCH weston v2 02/14] tests: Add client helper routines for output and reference filenames
Signed-off-by: Bryce Harrington
---
tests/weston-test-client-helper.c | 38 ++
tests/weston-test-client-helper.h | 6 ++
2 files changed, 44 insertions(+)
diff --git a/tests/weston-test-client-helper.c
b/tests/weston-test-client-helper.c
index 41ed208..00fdf8f 100644
--- a/tests/weston-test-client-helper.c
+++ b/tests/weston-test-client-helper.c
@@ -812,3 +812,41 @@ create_client_and_test_surface(int x, int y, int width,
int height)
return client;
}
+
+static const char*
+output_path(void) {
+ char *path = getenv("WESTON_TEST_OUTPUT_PATH");
+
+ if (!path)
+ return ".";
+ return path;
+ }
+
+char*
+screenshot_output_filename(const char *basename, uint32_t seq) {
+ char *filename;
+
+ if (asprintf(&filename, "%s/%s-%02d.png",
+output_path(), basename, seq) < 0)
+ return NULL;
+ return filename;
+}
+
+static const char*
+reference_path(void) {
+ char *path = getenv("WESTON_TEST_REFERENCE_PATH");
+
+ if (!path)
+ return "./tests/reference";
+ return path;
+}
+
+char*
+screenshot_reference_filename(const char *basename, uint32_t seq) {
+ char *filename;
+
+ if (asprintf(&filename, "%s/%s-%02d.png",
+reference_path(), basename, seq) < 0)
+ return NULL;
+ return filename;
+}
diff --git a/tests/weston-test-client-helper.h
b/tests/weston-test-client-helper.h
index 8635471..b46f158 100644
--- a/tests/weston-test-client-helper.h
+++ b/tests/weston-test-client-helper.h
@@ -180,4 +180,10 @@ void
expect_protocol_error(struct client *client,
const struct wl_interface *intf, uint32_t code);
+char*
+screenshot_output_filename(const char *basename, uint32_t seq);
+
+char*
+screenshot_reference_filename(const char *basename, uint32_t seq);
+
#endif
--
1.9.1
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston v2 08/14] tests: Handle screenshot done event in weston-test
Signed-off-by: Bryce Harrington
---
tests/weston-test-client-helper.c | 10 ++
tests/weston-test-client-helper.h | 1 +
2 files changed, 11 insertions(+)
diff --git a/tests/weston-test-client-helper.c
b/tests/weston-test-client-helper.c
index 04b77d5..f9de7c8 100644
--- a/tests/weston-test-client-helper.c
+++ b/tests/weston-test-client-helper.c
@@ -446,9 +446,19 @@ test_handle_n_egl_buffers(void *data, struct weston_test
*weston_test, uint32_t
test->n_egl_buffers = n;
}
+static void
+test_handle_capture_screenshot_done(void *data, struct weston_test
*weston_test)
+{
+ struct test *test = data;
+
+ printf("Screenshot has been captured\n");
+ test->buffer_copy_done = 1;
+}
+
static const struct weston_test_listener test_listener = {
test_handle_pointer_position,
test_handle_n_egl_buffers,
+ test_handle_capture_screenshot_done,
};
static void
diff --git a/tests/weston-test-client-helper.h
b/tests/weston-test-client-helper.h
index 8cbd4ba..1380bb6 100644
--- a/tests/weston-test-client-helper.h
+++ b/tests/weston-test-client-helper.h
@@ -64,6 +64,7 @@ struct test {
int pointer_x;
int pointer_y;
uint32_t n_egl_buffers;
+ int buffer_copy_done;
};
struct input {
--
1.9.1
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston v2 07/14] tests: Add screenshot recording capability to weston-test
Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=83981
Signed-off-by: Bryce Harrington
---
tests/weston-test.c | 243
1 file changed, 243 insertions(+)
diff --git a/tests/weston-test.c b/tests/weston-test.c
index 9f1f49b..8f8781f 100644
--- a/tests/weston-test.c
+++ b/tests/weston-test.c
@@ -269,6 +269,248 @@ get_n_buffers(struct wl_client *client, struct
wl_resource *resource)
weston_test_send_n_egl_buffers(resource, n_buffers);
}
+enum weston_test_screenshot_outcome {
+ WESTON_TEST_SCREENSHOT_SUCCESS,
+ WESTON_TEST_SCREENSHOT_NO_MEMORY,
+ WESTON_TEST_SCREENSHOT_BAD_BUFFER
+ };
+
+typedef void (*weston_test_screenshot_done_func_t)(void *data,
+ enum
weston_test_screenshot_outcome outcome);
+
+struct test_screenshot {
+ struct weston_compositor *compositor;
+ struct wl_global *global;
+ struct wl_client *client;
+ struct weston_process process;
+ struct wl_listener destroy_listener;
+};
+
+struct test_screenshot_frame_listener {
+ struct wl_listener listener;
+ struct weston_buffer *buffer;
+ weston_test_screenshot_done_func_t done;
+ void *data;
+};
+
+static void
+copy_bgra_yflip(uint8_t *dst, uint8_t *src, int height, int stride)
+{
+ uint8_t *end;
+
+ end = dst + height * stride;
+ while (dst < end) {
+ memcpy(dst, src, stride);
+ dst += stride;
+ src -= stride;
+ }
+}
+
+
+static void
+copy_bgra(uint8_t *dst, uint8_t *src, int height, int stride)
+{
+ /* TODO: optimize this out */
+ memcpy(dst, src, height * stride);
+}
+
+static void
+copy_row_swap_RB(void *vdst, void *vsrc, int bytes)
+{
+ uint32_t *dst = vdst;
+ uint32_t *src = vsrc;
+ uint32_t *end = dst + bytes / 4;
+
+ while (dst < end) {
+ uint32_t v = *src++;
+ /*A R G B */
+ uint32_t tmp = v & 0xff00ff00;
+ tmp |= (v >> 16) & 0x00ff;
+ tmp |= (v << 16) & 0x00ff;
+ *dst++ = tmp;
+ }
+}
+
+static void
+copy_rgba_yflip(uint8_t *dst, uint8_t *src, int height, int stride)
+{
+ uint8_t *end;
+
+ end = dst + height * stride;
+ while (dst < end) {
+ copy_row_swap_RB(dst, src, stride);
+ dst += stride;
+ src -= stride;
+ }
+}
+
+static void
+copy_rgba(uint8_t *dst, uint8_t *src, int height, int stride)
+{
+ uint8_t *end;
+
+ end = dst + height * stride;
+ while (dst < end) {
+ copy_row_swap_RB(dst, src, stride);
+ dst += stride;
+ src += stride;
+ }
+}
+
+static void
+test_screenshot_frame_notify(struct wl_listener *listener, void *data)
+{
+ struct test_screenshot_frame_listener *l =
+ container_of(listener,
+struct test_screenshot_frame_listener, listener);
+ struct weston_output *output = data;
+ struct weston_compositor *compositor = output->compositor;
+ int32_t stride;
+ uint8_t *pixels, *d, *s;
+
+ output->disable_planes--;
+ wl_list_remove(&listener->link);
+ stride = l->buffer->width * (PIXMAN_FORMAT_BPP(compositor->read_format)
/ 8);
+ pixels = malloc(stride * l->buffer->height);
+
+ if (pixels == NULL) {
+ l->done(l->data, WESTON_TEST_SCREENSHOT_NO_MEMORY);
+ free(l);
+ return;
+ }
+
+ // FIXME: Needs to handle output transformations
+
+ compositor->renderer->read_pixels(output,
+ compositor->read_format,
+ pixels,
+ 0, 0,
+ output->current_mode->width,
+ output->current_mode->height);
+
+ stride = wl_shm_buffer_get_stride(l->buffer->shm_buffer);
+
+ d = wl_shm_buffer_get_data(l->buffer->shm_buffer);
+ s = pixels + stride * (l->buffer->height - 1);
+
+ wl_shm_buffer_begin_access(l->buffer->shm_buffer);
+
+ /* XXX: It would be nice if we used Pixman to do all this rather
+* than our own implementation
+*/
+ switch (compositor->read_format) {
+ case PIXMAN_a8r8g8b8:
+ case PIXMAN_x8r8g8b8:
+ if (compositor->capabilities & WESTON_CAP_CAPTURE_YFLIP)
+ copy_bgra_yflip(d, s, output->current_mode->height,
stride);
+ else
+ copy_bgra(d, pixels, output->current_mode->height,
stride);
+ break;
+ case PIXMAN_x8b8g8r8:
+ case PIXMAN_a8b8g8r8:
+ if (compositor->capabilities & WESTON_CAP_CAPTURE_YFLIP)
+ copy_rgba_yflip(d, s, output->current_mode->height,
stri
[PATCH weston v2 12/14] tests: Add load_surface_from_png()
Loads an image from disk via cairo, and copies data into a weston test
surface for internal use.
Signed-off-by: Bryce Harrington
---
tests/internal-screenshot-test.c | 86 +---
1 file changed, 62 insertions(+), 24 deletions(-)
diff --git a/tests/internal-screenshot-test.c b/tests/internal-screenshot-test.c
index 3f8dcba..61eaab2 100644
--- a/tests/internal-screenshot-test.c
+++ b/tests/internal-screenshot-test.c
@@ -59,6 +59,59 @@ write_surface_as_png(const struct surface* weston_surface,
const char *fname) {
return true;
}
+/** load_surface_from_png()
+ *
+ * Reads a PNG image from disk using the given filename (and path)
+ * and returns as a freshly allocated weston test surface.
+ *
+ * @returns weston test surface with image, which should be free'd
+ * when no longer used; or, NULL in case of error.
+ */
+static struct surface*
+load_surface_from_png(const char *fname) {
+ struct surface *reference;
+ cairo_surface_t *reference_cairo_surface;
+ cairo_status_t status;
+ size_t source_data_size;
+
+ printf("Loading reference image %s\n", fname);
+ reference_cairo_surface = cairo_image_surface_create_from_png(fname);
+ status = cairo_surface_status(reference_cairo_surface);
+ if (status != CAIRO_STATUS_SUCCESS) {
+ printf("Could not open %s: %s\n", fname,
cairo_status_to_string(status));
+ cairo_surface_destroy(reference_cairo_surface);
+ return NULL;
+ }
+
+ /* Disguise the cairo surface in a weston test surface */
+ reference = xzalloc(sizeof *reference);
+ if (reference == NULL) {
+ perror("xzalloc reference");
+ cairo_surface_destroy(reference_cairo_surface);
+ return NULL;
+ }
+ reference->width =
cairo_image_surface_get_width(reference_cairo_surface);
+ reference->height =
cairo_image_surface_get_height(reference_cairo_surface);
+ reference->stride =
cairo_image_surface_get_stride(reference_cairo_surface);
+ source_data_size = reference->stride * reference->height;
+
+ /* Allocate new buffer for our weston reference, and copy the data from
+ the cairo surface so we can destroy it */
+ reference->data = xzalloc(source_data_size);
+ if (reference->data == NULL) {
+ perror("xzalloc reference data");
+ cairo_surface_destroy(reference_cairo_surface);
+ free(reference);
+ return NULL;
+ }
+ memcpy(reference->data,
+ cairo_image_surface_get_data(reference_cairo_surface),
+ source_data_size);
+
+ cairo_surface_destroy(reference_cairo_surface);
+ return reference;
+}
+
/** create_screenshot_surface()
*
* Allocates and initializes a weston test surface for use in
@@ -75,13 +128,13 @@ create_screenshot_surface(struct client *client) {
screenshot = xzalloc(sizeof *screenshot);
if (screenshot == NULL)
return NULL;
- screenshot->wl_buffer = create_shm_buffer(client,
- client->output->width,
- client->output->height,
- &screenshot->data);
- screenshot->height = client->output->height;
- screenshot->width = client->output->width;
- screenshot->stride = screenshot->width * 4; /* FIXME bpp? */
+ screenshot->wl_buffer = create_shm_buffer(client,
+ client->output->width,
+ client->output->height,
+ &screenshot->data);
+ screenshot->height = client->output->height;
+ screenshot->width = client->output->width;
+ screenshot->stride = screenshot->width * 4; /* FIXME bpp? */
return screenshot;
}
@@ -93,8 +146,6 @@ TEST(internal_screenshot)
struct surface *reference = NULL;
struct rectangle clip;
const char *fname;
- cairo_surface_t *reference_cairo_surface;
- cairo_status_t status;
bool match = false;
bool dump_all_images = true;
@@ -130,20 +181,8 @@ TEST(internal_screenshot)
/* Load reference image */
fname = screenshot_reference_filename("internal-screenshot", 0);
printf("Loading reference image %s\n", fname);
- reference_cairo_surface = cairo_image_surface_create_from_png(fname);
- status = cairo_sur
[PATCH weston v2 11/14] tests: Add create_screenshot_surface()
Refactor out the screenshot shm buffer creation code.
Signed-off-by: Bryce Harrington
---
tests/internal-screenshot-test.c | 39 ---
1 file changed, 28 insertions(+), 11 deletions(-)
diff --git a/tests/internal-screenshot-test.c b/tests/internal-screenshot-test.c
index 7f1a690..3f8dcba 100644
--- a/tests/internal-screenshot-test.c
+++ b/tests/internal-screenshot-test.c
@@ -59,6 +59,33 @@ write_surface_as_png(const struct surface* weston_surface,
const char *fname) {
return true;
}
+/** create_screenshot_surface()
+ *
+ * Allocates and initializes a weston test surface for use in
+ * storing a screenshot of the client's output. Establishes a
+ * shm backed wl_buffer for retrieving screenshot image data
+ * from the server, sized to match the client's output display.
+ *
+ * @returns stack allocated surface image, which should be
+ * free'd when done using it.
+ */
+static struct surface*
+create_screenshot_surface(struct client *client) {
+ struct surface* screenshot;
+ screenshot = xzalloc(sizeof *screenshot);
+ if (screenshot == NULL)
+ return NULL;
+ screenshot->wl_buffer = create_shm_buffer(client,
+ client->output->width,
+ client->output->height,
+ &screenshot->data);
+ screenshot->height = client->output->height;
+ screenshot->width = client->output->width;
+ screenshot->stride = screenshot->width * 4; /* FIXME bpp? */
+
+ return screenshot;
+}
+
TEST(internal_screenshot)
{
struct client *client;
@@ -79,18 +106,8 @@ TEST(internal_screenshot)
printf("Client created\n");
/* Create a surface to hold the screenshot */
- screenshot = xzalloc(sizeof *screenshot);
+ screenshot = create_screenshot_surface(client);
assert(screenshot);
-
- /* Create and attach buffer to our surface */
- screenshot->wl_buffer = create_shm_buffer(client,
- client->output->width,
- client->output->height,
- &screenshot->data);
- screenshot->height = client->output->height;
- screenshot->width = client->output->width;
- assert(screenshot->wl_buffer);
- screenshot->stride = screenshot->width * 4; /* meh */
printf("Screenshot buffer created and attached to surface\n");
/* Take a snapshot. Result will be in screenshot->wl_buffer. */
--
1.9.1
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston v2 13/14] tests: Add check_surfaces_geometry()
Minor refactoring to simplify initial sanity checks of surfaces.
Conceivably useful for other basic checking.
Signed-off-by: Bryce Harrington
---
tests/weston-test-client-helper.c | 41 ++-
tests/weston-test-client-helper.h | 3 +++
2 files changed, 30 insertions(+), 14 deletions(-)
diff --git a/tests/weston-test-client-helper.c
b/tests/weston-test-client-helper.c
index f9de7c8..b63634d 100644
--- a/tests/weston-test-client-helper.c
+++ b/tests/weston-test-client-helper.c
@@ -877,6 +877,31 @@ screenshot_reference_filename(const char *basename,
uint32_t seq) {
}
/**
+ * check_surfaces_geometry() - verifies two surfaces are same size
+ *
+ * @returns true if surfaces have the same width and height, or false
+ * if not, or if there is no actual data.
+ */
+bool
+check_surfaces_geometry(const struct surface *a, const struct surface *b)
+{
+ if (a == NULL || b == NULL) {
+ printf("Undefined surfaces\n");
+ return false;
+ }
+ else if (a->data == NULL || b->data == NULL) {
+ printf("Undefined data\n");
+ return false;
+ }
+ else if (a->width != b->width || a->height != b->height) {
+ printf("Mismatched dimensions: %d,%d != %d,%d\n",
+ a->width, a->height, b->width, b->height);
+ return false;
+ }
+ return true;
+}
+
+/**
* check_surfaces_equal() - tests if two surfaces are pixel-identical
*
* Returns true if surface buffers have all the same byte values,
@@ -889,11 +914,8 @@ check_surfaces_equal(const struct surface *a, const struct
surface *b)
int y;
void *p, *q;
- if (a == NULL || b == NULL)
+ if (!check_surfaces_geometry(a, b))
return false;
- if (a->width != b->width || a->height != a->height)
- return false;
-
if (a->stride == b->stride) {
printf("Checking data for equivalent strides\n");
return (memcmp(a->data, b->data, a->stride * a->height) == 0);
@@ -926,18 +948,9 @@ check_surfaces_match_in_clip(const struct surface *a,
const struct surface *b, c
void *p, *q;
char a_char, b_char;
- if (a == NULL || b == NULL || clip_rect == NULL)
+ if (!check_surfaces_geometry(a, b) || clip_rect == NULL)
return false;
- if (a->data == NULL || b->data == NULL) {
- printf("Undefined data\n");
- return false;
- }
- if (a->width != b->width || a->height != b->height) {
- printf("Mismatched dimensions: %d,%d != %d,%d\n",
- a->width, a->height, b->width, b->height);
- return false;
- }
if (clip_rect->x > a->width || clip_rect->y > a->height) {
printf("Clip outside image boundaries\n");
return true;
diff --git a/tests/weston-test-client-helper.h
b/tests/weston-test-client-helper.h
index 1380bb6..407a8ab 100644
--- a/tests/weston-test-client-helper.h
+++ b/tests/weston-test-client-helper.h
@@ -201,6 +201,9 @@ char*
screenshot_reference_filename(const char *basename, uint32_t seq);
bool
+check_surfaces_geometry(const struct surface *a, const struct surface *b);
+
+bool
check_surfaces_equal(const struct surface *a, const struct surface *b);
bool
--
1.9.1
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston v2 03/14] tests: Add an xmalloc helper function
Signed-off-by: Bryce Harrington
---
tests/weston-test-client-helper.c | 11 +++
tests/weston-test-client-helper.h | 16 ++--
2 files changed, 21 insertions(+), 6 deletions(-)
diff --git a/tests/weston-test-client-helper.c
b/tests/weston-test-client-helper.c
index 00fdf8f..080bb62 100644
--- a/tests/weston-test-client-helper.c
+++ b/tests/weston-test-client-helper.c
@@ -32,6 +32,17 @@
#include "../shared/os-compatibility.h"
#include "weston-test-client-helper.h"
+void *
+fail_on_null(void *p)
+{
+ if (p == NULL) {
+ fprintf(stderr, "out of memory\n");
+ exit(EXIT_FAILURE);
+ }
+ return p;
+}
+
+
int
surface_contains(struct surface *surface, int x, int y)
{
diff --git a/tests/weston-test-client-helper.h
b/tests/weston-test-client-helper.h
index b46f158..43a5aa7 100644
--- a/tests/weston-test-client-helper.h
+++ b/tests/weston-test-client-helper.h
@@ -132,15 +132,19 @@ struct surface {
void *data;
};
+void *
+fail_on_null(void *p);
+
static inline void *
-xzalloc(size_t size)
+xzalloc(size_t s)
{
-void *p;
-
-p = calloc(1, size);
-assert(p);
+ return fail_on_null(calloc(1, s));
+}
-return p;
+static inline void *
+xmalloc(size_t s)
+{
+ return fail_on_null(malloc(s));
}
struct client *
--
1.9.1
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston v2 01/14] tests: Add error handling for system calls
Signed-off-by: Bryce Harrington
---
tests/weston-tests-env | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tests/weston-tests-env b/tests/weston-tests-env
index ed8ff98..f945ac6 100755
--- a/tests/weston-tests-env
+++ b/tests/weston-tests-env
@@ -11,12 +11,12 @@ fi
WESTON=$abs_builddir/weston
LOGDIR=$abs_builddir/logs
-mkdir -p "$LOGDIR"
+mkdir -p "$LOGDIR" || exit
SERVERLOG="$LOGDIR/${TEST_NAME}-serverlog.txt"
OUTLOG="$LOGDIR/${TEST_NAME}-log.txt"
-rm -f "$SERVERLOG"
+rm -f "$SERVERLOG" || exit
BACKEND=${BACKEND:-headless-backend.so}
--
1.9.1
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston v2 14/14] tests: Add capture_screenshot_of_output()
Provides a convenience function for JFDI grabbing of a single
screenshot. Tests that are doing multiple screenshots or other
fanciness probably will bypass this routine and do things more manually,
but this'll provide a reference implementation. And hopefully there'll
be enough simple cases that this actually is useful.
Signed-off-by: Bryce Harrington
---
tests/internal-screenshot-test.c | 56 +---
1 file changed, 35 insertions(+), 21 deletions(-)
diff --git a/tests/internal-screenshot-test.c b/tests/internal-screenshot-test.c
index 61eaab2..761ad1e 100644
--- a/tests/internal-screenshot-test.c
+++ b/tests/internal-screenshot-test.c
@@ -139,38 +139,29 @@ create_screenshot_surface(struct client *client) {
return screenshot;
}
-TEST(internal_screenshot)
-{
- struct client *client;
- struct surface *screenshot = NULL;
- struct surface *reference = NULL;
- struct rectangle clip;
- const char *fname;
- bool match = false;
- bool dump_all_images = true;
-
- printf("Starting test\n");
-
- /* Create the client */
- client = create_client_and_test_surface(100, 100, 100, 100);
- assert(client);
- printf("Client created\n");
+/** capture_screenshot_of_output()
+ *
+ * Requests a screenshot from the server of the output that the
+ * client appears on. The image data returned from the server
+ * can be accessed from the screenshot surface's data member.
+ *
+ * @returns a new surface object, which should be free'd when no
+ * longer needed.
+ */
+static struct surface *
+capture_screenshot_of_output(struct client *client) {
+ struct surface *screenshot;
/* Create a surface to hold the screenshot */
screenshot = create_screenshot_surface(client);
- assert(screenshot);
- printf("Screenshot buffer created and attached to surface\n");
- /* Take a snapshot. Result will be in screenshot->wl_buffer. */
client->test->buffer_copy_done = 0;
weston_test_capture_screenshot(client->test->weston_test,
client->output->wl_output,
screenshot->wl_buffer);
- printf("Capture request sent\n");
while (client->test->buffer_copy_done == 0)
if (wl_display_dispatch(client->wl_display) < 0)
break;
- printf("Roundtrip done\n");
/* FIXME: Document somewhere the orientation the screenshot is taken
* and how the clip coords are interpreted, in case of
scaling/transform.
@@ -178,6 +169,29 @@ TEST(internal_screenshot)
* Protocol docs in the XML, comparison function docs in Doxygen style.
*/
+ return screenshot;
+}
+
+TEST(internal_screenshot)
+{
+ struct client *client;
+ struct surface *screenshot = NULL;
+ struct surface *reference = NULL;
+ struct rectangle clip;
+ const char *fname;
+ bool match = false;
+ bool dump_all_images = true;
+
+ /* Create the client */
+ printf("Creating client for test\n");
+ client = create_client_and_test_surface(100, 100, 100, 100);
+ assert(client);
+
+ /* Take a snapshot. Result will be in screenshot->wl_buffer. */
+ printf("Taking a screenshot\n");
+ screenshot = capture_screenshot_of_output(client);
+ assert(screenshot);
+
/* Load reference image */
fname = screenshot_reference_filename("internal-screenshot", 0);
printf("Loading reference image %s\n", fname);
--
1.9.1
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston v2 00/14] Implement screenshot-based testing with the headless renderer
This series adds support for implementing test cases that can check
rendering output without needing connection to a physical output. It
utilizes the pixman renderer in the headless backend to generate
screenshots at the request of the test client.
The test creates a shm buffer and passes it to the server via the
Weston-Test protocol ('capture_screenshot'). The server then renders
the display contents into the buffer and returns it as a response
('capture_screenshot_done').
The test then loads a corresponding reference PNG from disk using Cairo,
and then compares it with the captured screenshot. Note that the
screenshot includes the current time in the desktop clock, which will of
course be different in the captured screenshot from the reference image.
So this checks a small clipped out section of each of the two images to
verify congruence.
By default, Wayland fades in the display and will show a patterned
background. The former feature causes a black or nearly-black image to
be captured (the darkness of which may vary from system to system), and
the latter merely adds noise in our comparison, so both features are
disabled via a test-specific configuration .ini file.
[Update v2]
* Refactor cairo out of the test client backend code entirely
by utilizing the weston test surface structure to carry the
specific data that the check_ routines need.
* Drop refactoring redundant pixel copying code to shared
* Take stride and bpp into account in check functions
* In test case, set the bpp for the screenshot
* Fix screenshot protocol description
* Convert C++ style comments to C
* Don't leak screenshot object
* s/ec/compositor/
* Instead of spinning on client_roundtrip, check for client's wl_display
* Drop unneeded wl_surface in test case
* Restore missing weston_log in idle_launch_client()
* Drop unused test object from capture_screenshot
* check_ function variable cleanup
* Prefer tracking stride rather than bpp
* Cleanup character calculation in check_ routines
* Drop unnecessary data copying before write to disk
* Split up test case code into several functions
* Use screenshot_output_filename() so output files respect the
WESTON_TEST_OUTPUT_PATH env var
Bryce Harrington (14):
tests: Add error handling for system calls
tests: Add client helper routines for output and reference filenames
tests: Add an xmalloc helper function
tests: Add surface checks
tests: Support --config to enable tests to override config defaults
protocol: Add test screenshot capability
tests: Add screenshot recording capability to weston-test
tests: Handle screenshot done event in weston-test
tests: Add internal test for the weston test screenshot capability
tests: Add write_surface_as_png() helper
tests: Add create_screenshot_surface()
tests: Add load_surface_from_png()
tests: Add check_surfaces_geometry()
tests: Add capture_screenshot_of_output()
Makefile.am| 23 ++-
protocol/weston-test.xml | 16 ++
tests/internal-screenshot-test.c | 231 +++
tests/internal-screenshot.ini | 3 +
tests/reference/internal-screenshot-00.png | Bin 0 -> 5149 bytes
tests/weston-test-client-helper.c | 188 ++
tests/weston-test-client-helper.h | 41 -
tests/weston-test.c| 243 +
tests/weston-tests-env | 18 ++-
9 files changed, 751 insertions(+), 12 deletions(-)
create mode 100644 tests/internal-screenshot-test.c
create mode 100644 tests/internal-screenshot.ini
create mode 100644 tests/reference/internal-screenshot-00.png
--
1.9.1
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston 2/2] compositor-drm: pass ARGB fallback to gl create functions for XRGB formats
On Mon, 11 May 2015 14:19:03 -0500
Derek Foreman wrote:
> If the GL implementation doesn't provide an XRGB visual we may still be
> able to proceed with an ARGB one. Since we're not changing the scanout
> buffer format, and our current rendering loop always results in saturated
> alpha in the frame buffer, it should be Just Fine(tm) - and probably better
> than just exiting.
>
> Signed-off-by: Derek Foreman
Hi,
I have tested, that this patch does indeed work around
https://bugs.freedesktop.org/show_bug.cgi?id=89689
I think you should link to the bug report here. It provides the context
on when this workaround is needed. The link might even be in a comment
in the code.
> ---
> src/compositor-drm.c | 54
> ++--
> src/gl-renderer.c| 8
> 2 files changed, 52 insertions(+), 10 deletions(-)
>
> diff --git a/src/compositor-drm.c b/src/compositor-drm.c
> index 69bdcfd..48dfa29 100644
> --- a/src/compositor-drm.c
> +++ b/src/compositor-drm.c
> @@ -1391,14 +1391,43 @@ create_gbm_device(int fd)
> return gbm;
> }
>
> +/* When initializing EGL, if the preferred buffer format isn't availble
> + * we may be able to susbstitute an ARGB format for an XRGB one.
> + *
> + * This returns 0 if substitution isn't possible, but 0 might be a
> + * legitimate format for other EGL platforms, so the caller is
> + * responsible for checking for 0 before calling gl_renderer->create().
> + */
> static int
> -drm_compositor_create_gl_renderer(struct drm_compositor *ec)
> +fallback_format_for(uint32_t format)
> {
> - EGLint format;
> + switch (format) {
> + case GBM_FORMAT_XRGB:
> + return GBM_FORMAT_ARGB;
> + case GBM_FORMAT_XRGB2101010:
> + return GBM_FORMAT_ARGB2101010;
> + default:
> + return 0;
> + }
> +}
This is now very obvious on what it does. Great!
Tested-by: Pekka Paalanen
Reviewed-by: Pekka Paalanen
Thanks,
pq
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston 1/2] gl-renderer: Take a list of acceptable formats in create functions
On Thu, 14 May 2015 20:14:53 -0700
Bryce Harrington wrote:
> On Mon, May 11, 2015 at 02:19:02PM -0500, Derek Foreman wrote:
> > Currently we pass either a single format or no formats to the gl renderer
> > create and output_create functions. We extend this to any number of
> > formats so we can allow fallback formats if we don't get our first pick.
> >
> > Signed-off-by: Derek Foreman
>
> This all looks fine, but I have some bikesheddy comments. I'll give a
> R-B and you can decide whether or not my suggestions have any utility.
>
> Reviewed-by: Bryce Harrington
Hi,
I agree with Bryce's comments below, and also agree that this patch is
also landable as is.
Reviewed-by: Pekka Paalanen
I have made three further notes below.
> > ---
> > src/compositor-drm.c | 5 ++--
> > src/compositor-fbdev.c | 4 +--
> > src/compositor-wayland.c | 12 +
> > src/compositor-x11.c | 5 ++--
> > src/gl-renderer.c| 68
> > +++-
> > src/gl-renderer.h| 6 +++--
> > 6 files changed, 63 insertions(+), 37 deletions(-)
> >
> > diff --git a/src/compositor-drm.c b/src/compositor-drm.c
> > index 0cdb8f4..69bdcfd 100644
> > --- a/src/compositor-drm.c
> > +++ b/src/compositor-drm.c
> > @@ -1398,7 +1398,7 @@ drm_compositor_create_gl_renderer(struct
> > drm_compositor *ec)
> >
> > format = ec->format;
> > if (gl_renderer->create(&ec->base, EGL_PLATFORM_GBM_KHR, (void *)
> > ec->gbm,
> > - gl_renderer->opaque_attribs, &format) < 0) {
> > + gl_renderer->opaque_attribs, &format, 1) < 0) {
> > return -1;
> > }
> >
> > @@ -1620,7 +1620,8 @@ drm_output_init_egl(struct drm_output *output, struct
> > drm_compositor *ec)
> >(EGLNativeDisplayType)output->surface,
> >output->surface,
> >gl_renderer->opaque_attribs,
> > - &format) < 0) {
> > + &format,
> > + 1) < 0) {
> > weston_log("failed to create gl renderer output state\n");
> > gbm_surface_destroy(output->surface);
> > return -1;
> > diff --git a/src/compositor-fbdev.c b/src/compositor-fbdev.c
> > index 7c505ce..3f3394f 100644
> > --- a/src/compositor-fbdev.c
> > +++ b/src/compositor-fbdev.c
> > @@ -570,7 +570,7 @@ fbdev_output_create(struct fbdev_compositor *compositor,
> > if (gl_renderer->output_create(&output->base,
> >(EGLNativeWindowType)NULL, NULL,
> >gl_renderer->opaque_attribs,
> > - NULL) < 0) {
> > + NULL, 0) < 0) {
> > weston_log("gl_renderer_output_create failed.\n");
> > goto out_shadow_surface;
> > }
> > @@ -871,7 +871,7 @@ fbdev_compositor_create(struct wl_display *display, int
> > *argc, char *argv[],
> > if (gl_renderer->create(&compositor->base, NO_EGL_PLATFORM,
> > EGL_DEFAULT_DISPLAY,
> > gl_renderer->opaque_attribs,
> > - NULL) < 0) {
> > + NULL, 0) < 0) {
> > weston_log("gl_renderer_create failed.\n");
> > goto out_launcher;
> > }
> > diff --git a/src/compositor-wayland.c b/src/compositor-wayland.c
> > index 303151c..c9983e0 100644
> > --- a/src/compositor-wayland.c
> > +++ b/src/compositor-wayland.c
> > @@ -648,7 +648,8 @@ wayland_output_init_gl_renderer(struct wayland_output
> > *output)
> >output->gl.egl_window,
> >output->gl.egl_window,
> >gl_renderer->alpha_attribs,
> > - NULL) < 0)
> > + NULL,
> > + 0) < 0)
> > goto cleanup_window;
> >
> > return 0;
> > @@ -1970,10 +1971,11 @@ wayland_compositor_create(struct wl_display
> > *display, int use_pixman,
> >
> > if (!c->use_pixman) {
> > if (gl_renderer->create(&c->base,
> > - EGL_PLATFORM_WAYLAND_KHR,
> > - c->parent.wl_display,
> > - gl_renderer->alpha_attribs,
> > - NULL) < 0) {
> > + EGL_PLATFORM_WAYLAND_KHR,
> > + c->parent.wl_display,
> > + gl_renderer->alpha_attribs,
> > + NULL,
> > + 0) < 0) {
> >
