[PATCH weston 07/10] tests: Add screenshot recording capability to weston-test

[Bryce Harrington]

On Sat, May 09, 2015 at 02:56:54PM +0100, Daniel Stone wrote:
> Hi,
> 
> On 7 May 2015 at 01:44, Bryce Harrington <bryce at osg.samsung.com> wrote:
> > +enum weston_test_screenshot_outcome {
> > +       WESTON_TEST_SCREENSHOT_SUCCESS,
> > +       WESTON_TEST_SCREENSHOT_NO_MEMORY,
> > +       WESTON_TEST_SCREENSHOT_BAD_BUFFER
> > +       };
> 
> This should be declared as an enum in the protocol definition.
> 
> > +static void
> > +copy_bgra_yflip(uint8_t *dst, uint8_t *src, int height, int stride)
> > +
> > +static void
> > +copy_bgra(uint8_t *dst, uint8_t *src, int height, int stride)
> > +
> > +static void
> > +copy_row_swap_RB(void *vdst, void *vsrc, int bytes)
> > +
> > +static void
> > +copy_rgba_yflip(uint8_t *dst, uint8_t *src, int height, int stride)
> > +
> > +static void
> > +copy_rgba(uint8_t *dst, uint8_t *src, int height, int stride)
> 
> Can we please use Pixman for these instead of open-coding them?

I don't understand, explain?  These routines already existing in the
screenshooter client, I just moved them to shared so I could reuse them.
 
> > +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
> 
> C++ comment. :(
> 
> > +/**
> > + * Grabs a snapshot of the screen.
> > + */
> > +static void
> > +capture_screenshot(struct wl_client *client,
> > +                  struct wl_resource *resource,
> > +                  struct wl_resource *output_resource,
> > +                  struct wl_resource *buffer_resource)
> > +{
> > +//     struct weston_test *test =
> > +//             wl_resource_get_user_data(resource);
> 
> Unused declaration.
> 
> > @@ -316,7 +559,6 @@ idle_launch_client(void *data)
> >                 sigfillset(&allsigs);
> >                 sigprocmask(SIG_UNBLOCK, &allsigs, NULL);
> >                 execl(path, path, NULL);
> > -               weston_log("compositor: executing '%s' failed: %m\n", path);
> >                 exit(EXIT_FAILURE);
> >         }
> 
> Unrelated change?
> 
> Cheers,
> Daniel