tests: make screenshooting return a buffer
authorPekka Paalanen <pekka.paalanen@collabora.co.uk>
Fri, 27 May 2016 11:11:01 +0000 (14:11 +0300)
committerPekka Paalanen <pekka.paalanen@collabora.co.uk>
Wed, 29 Jun 2016 09:41:31 +0000 (12:41 +0300)
Screenshooting does not involve creating a wl_surface, so using struct
surface is superfluous.

Return a struct buffer instead. It could have been just a
pixman_image_t, but setting up proper destruction would be a bit more
work. Should not hurt to keep the wl_buffer around until the user is
ready to free the image.

Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Daniel Stone <daniels@collabora.com>
tests/internal-screenshot-test.c
tests/weston-test-client-helper.c
tests/weston-test-client-helper.h

index dfa7ac1..81ff013 100644 (file)
@@ -65,7 +65,7 @@ TEST(internal_screenshot)
        struct buffer *buf;
        struct client *client;
        struct wl_surface *surface;
-       struct surface *screenshot = NULL;
+       struct buffer *screenshot = NULL;
        pixman_image_t *reference_good = NULL;
        pixman_image_t *reference_bad = NULL;
        struct rectangle clip;
@@ -123,8 +123,7 @@ TEST(internal_screenshot)
        /* Test check_images_match() without a clip.
         * We expect this to fail since we use a bad reference image
         */
-       match = check_images_match(screenshot->buffer->image,
-                                  reference_bad, NULL);
+       match = check_images_match(screenshot->image, reference_bad, NULL);
        printf("Screenshot %s reference image\n", match? "equal to" : "different from");
        assert(!match);
        pixman_image_unref(reference_bad);
@@ -138,18 +137,17 @@ TEST(internal_screenshot)
        clip.width = 100;
        clip.height = 100;
        printf("Clip: %d,%d %d x %d\n", clip.x, clip.y, clip.width, clip.height);
-       match = check_images_match(screenshot->buffer->image,
-                                  reference_good, &clip);
+       match = check_images_match(screenshot->image, reference_good, &clip);
        printf("Screenshot %s reference image in clipped area\n", match? "matches" : "doesn't match");
        pixman_image_unref(reference_good);
 
        /* Test dumping of non-matching images */
        if (!match || dump_all_images) {
                fname = screenshot_output_filename("internal-screenshot", 0);
-               write_image_as_png(screenshot->buffer->image, fname);
+               write_image_as_png(screenshot->image, fname);
        }
 
-       free(screenshot);
+       buffer_destroy(screenshot);
 
        printf("Test complete\n");
        assert(match);
index 377508f..421edb9 100644 (file)
@@ -1255,54 +1255,29 @@ load_image_from_png(const char *fname)
        return converted;
 }
 
-/** 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.
- */
-struct surface *
-create_screenshot_surface(struct client *client)
-{
-       struct surface *screenshot;
-       screenshot = zalloc(sizeof *screenshot);
-       if (screenshot == NULL)
-               return NULL;
-
-       screenshot->buffer = create_shm_buffer_a8r8g8b8(client,
-                                                       client->output->width,
-                                                       client->output->height);
-       screenshot->height = client->output->height;
-       screenshot->width = client->output->width;
-
-       return screenshot;
-}
-
-/** capture_screenshot_of_output()
+/**
+ * Take screenshot of a single 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.
+ * client appears on. This implies that the compositor goes through an output
+ * repaint to provide the screenshot before this function returns. This
+ * function is therefore both a server roundtrip and a wait for a repaint.
  *
- * @returns a new surface object, which should be free'd when no
- * longer needed.
+ * @returns A new buffer object, that should be freed with buffer_destroy().
  */
-struct surface *
+struct buffer *
 capture_screenshot_of_output(struct client *client)
 {
-       struct surface *screenshot;
+       struct buffer *buffer;
 
-       /* Create a surface to hold the screenshot */
-       screenshot = create_screenshot_surface(client);
+       buffer = create_shm_buffer_a8r8g8b8(client,
+                                           client->output->width,
+                                           client->output->height);
 
        client->test->buffer_copy_done = 0;
        weston_test_capture_screenshot(client->test->weston_test,
                                       client->output->wl_output,
-                                      screenshot->buffer->proxy);
+                                      buffer->proxy);
        while (client->test->buffer_copy_done == 0)
                if (wl_display_dispatch(client->wl_display) < 0)
                        break;
@@ -1313,5 +1288,5 @@ capture_screenshot_of_output(struct client *client)
         * Protocol docs in the XML, comparison function docs in Doxygen style.
         */
 
-       return screenshot;
+       return buffer;
 }
index adeeee3..794abb3 100644 (file)
@@ -207,10 +207,7 @@ write_image_as_png(pixman_image_t *image, const char *fname);
 pixman_image_t *
 load_image_from_png(const char *fname);
 
-struct surface *
-create_screenshot_surface(struct client *client);
-
-struct surface *
+struct buffer *
 capture_screenshot_of_output(struct client *client);
 
 #endif