Thanks. We should probably have a discussion at some point as to the best guidelines on how to split the code up for unit testing purposes. For now I'll move ahead with getting the vertex clipping code to support N vertices while still passing these tests. I probably won't be able to start that for a few weeks though. Sam. <div class="gmail_quote">On 17/09/2013 12:41 PM, "Kristian Høgsberg" <<a href="mailto:hoegsberg@gmail.com">hoegsberg@gmail.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> On Fri, Sep 13, 2013 at 10:01:22AM +0800, Sam Spilsbury wrote: > This tests (via the table-driven testing method) that the correct > number of vertices and also the correct vertices themselves > are generated for an clip box and polygon of up to eight vertices. > > Also add a <a href="http://libshared-test.la" target="_blank">libshared-test.la</a> so that we don't have to build weston-test-runner > all the time Hi Sam, I've committed these two, let's not get stuck on how to split it. Kristian > --- > tests/Makefile.am | 37 +++++--- > tests/vertex-clip-test.c | 219 +++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 245 insertions(+), 11 deletions(-) > create mode 100644 tests/vertex-clip-test.c > > diff --git a/tests/Makefile.am b/tests/Makefile.am > index 3a36619..5be52c6 100644 > --- a/tests/Makefile.am > +++ b/tests/Makefile.am > @@ -1,7 +1,8 @@ > TESTS = $(shared_tests) $(module_tests) $(weston_tests) > > shared_tests = \ > - config-parser.test > + config-parser.test \ > + vertex-clip.test > > module_tests = \ > <a href="http://surface-test.la" target="_blank">surface-test.la</a> \ > @@ -48,13 +49,6 @@ AM_CPPFLAGS = \ > -DUNIT_TEST \ > $(COMPOSITOR_CFLAGS) > > - > -config_parser_test_LDADD = \ > - ../shared/<a href="http://libshared.la" target="_blank">libshared.la</a> \ > - $(COMPOSITOR_LIBS) > -config_parser_test_SOURCES = \ > - config-parser-test.c > - > surface_global_test_la_SOURCES = surface-global-test.c > surface_global_test_la_LDFLAGS = -module -avoid-version -rpath $(libdir) > surface_test_la_SOURCES = surface-test.c > @@ -72,17 +66,38 @@ weston_test_la_SOURCES = \ > weston_test_runner_src = \ > weston-test-runner.c \ > weston-test-runner.h > + > +check_LTLIBRARIES = <a href="http://libshared-test.la" target="_blank">libshared-test.la</a> > + > +libshared_test_la_SOURCES = \ > + $(weston_test_runner_src) > +libshared_test_la_CFLAGS = $(GCC_CFLAGS) $(COMPOSITOR_CFLAGS) > + > +config_parser_test_LDADD = \ > + ../shared/<a href="http://libshared.la" target="_blank">libshared.la</a> \ > + <a href="http://libshared-test.la" target="_blank">libshared-test.la</a> \ > + $(COMPOSITOR_LIBS) > +config_parser_test_SOURCES = \ > + config-parser-test.c > +vertex_clip_test_SOURCES = \ > + vertex-clip-test.c \ > + ../src/vertex-clipping.c \ > + ../src/vertex-clipping.h > +vertex_clip_test_LDADD = \ > + <a href="http://libshared-test.la" target="_blank">libshared-test.la</a> \ > + -lm -lrt > + > weston_test_client_src = \ > weston-test-client-helper.c \ > weston-test-client-helper.h \ > wayland-test-protocol.c \ > wayland-test-client-protocol.h \ > subsurface-protocol.c \ > - subsurface-client-protocol.h \ > - $(weston_test_runner_src) > + subsurface-client-protocol.h > weston_test_client_libs = \ > $(SIMPLE_CLIENT_LIBS) \ > - ../shared/<a href="http://libshared.la" target="_blank">libshared.la</a> > + ../shared/<a href="http://libshared.la" target="_blank">libshared.la</a> \ > + <a href="http://libshared-test.la" target="_blank">libshared-test.la</a> > > keyboard_weston_SOURCES = keyboard-test.c $(weston_test_client_src) > keyboard_weston_LDADD = $(weston_test_client_libs) > diff --git a/tests/vertex-clip-test.c b/tests/vertex-clip-test.c > new file mode 100644 > index 0000000..5b2e08c > --- /dev/null > +++ b/tests/vertex-clip-test.c > @@ -0,0 +1,219 @@ > +/* > + * Copyright © 2013 Sam Spilsbury <<a href="mailto:smspillaz@gmail.com">smspillaz@gmail.com</a>> > + * > + * 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 <assert.h> > +#include <stdio.h> > +#include <stdlib.h> > +#include <string.h> > +#include <math.h> > + > +#include "weston-test-runner.h" > + > +#include "../src/vertex-clipping.h" > + > +#define BOUNDING_BOX_TOP_Y 100.0f > +#define BOUNDING_BOX_LEFT_X 50.0f > +#define BOUNDING_BOX_RIGHT_X 100.0f > +#define BOUNDING_BOX_BOTTOM_Y 50.0f > + > +#define INSIDE_X1 (BOUNDING_BOX_LEFT_X + 1.0f) > +#define INSIDE_X2 (BOUNDING_BOX_RIGHT_X - 1.0f) > +#define INSIDE_Y1 (BOUNDING_BOX_BOTTOM_Y + 1.0f) > +#define INSIDE_Y2 (BOUNDING_BOX_TOP_Y - 1.0f) > + > +#define OUTSIDE_X1 (BOUNDING_BOX_LEFT_X - 1.0f) > +#define OUTSIDE_X2 (BOUNDING_BOX_RIGHT_X + 1.0f) > +#define OUTSIDE_Y1 (BOUNDING_BOX_BOTTOM_Y - 1.0f) > +#define OUTSIDE_Y2 (BOUNDING_BOX_TOP_Y + 1.0f) > + > +static void > +populate_clip_context (struct clip_context *ctx) > +{ > + ctx->clip.x1 = BOUNDING_BOX_LEFT_X; > + ctx->clip.y1 = BOUNDING_BOX_BOTTOM_Y; > + ctx->clip.x2 = BOUNDING_BOX_RIGHT_X; > + ctx->clip.y2 = BOUNDING_BOX_TOP_Y; > +} > + > +static int > +clip_polygon (struct clip_context *ctx, > + struct polygon8 *polygon, > + GLfloat *vertices_x, > + GLfloat *vertices_y) > +{ > + populate_clip_context(ctx); > + return clip_transformed(ctx, polygon, vertices_x, vertices_y); > +} > + > +struct vertex_clip_test_data > +{ > + struct polygon8 surface; > + struct polygon8 expected; > +}; > + > +const struct vertex_clip_test_data test_data[] = > +{ > + /* All inside */ > + { > + { > + { INSIDE_X1, INSIDE_X2, INSIDE_X2, INSIDE_X1 }, > + { INSIDE_Y1, INSIDE_Y1, INSIDE_Y2, INSIDE_Y2 }, > + 4 > + }, > + { > + { INSIDE_X1, INSIDE_X2, INSIDE_X2, INSIDE_X1 }, > + { INSIDE_Y1, INSIDE_Y1, INSIDE_Y2, INSIDE_Y2 }, > + 4 > + } > + }, > + /* Top outside */ > + { > + { > + { INSIDE_X1, INSIDE_X2, INSIDE_X2, INSIDE_X1 }, > + { INSIDE_Y1, INSIDE_Y1, OUTSIDE_Y2, OUTSIDE_Y2 }, > + 4 > + }, > + { > + { INSIDE_X1, INSIDE_X1, INSIDE_X2, INSIDE_X2 }, > + { BOUNDING_BOX_TOP_Y, INSIDE_Y1, INSIDE_Y1, BOUNDING_BOX_TOP_Y }, > + 4 > + } > + }, > + /* Bottom outside */ > + { > + { > + { INSIDE_X1, INSIDE_X2, INSIDE_X2, INSIDE_X1 }, > + { OUTSIDE_Y1, OUTSIDE_Y1, INSIDE_Y2, INSIDE_Y2 }, > + 4 > + }, > + { > + { INSIDE_X1, INSIDE_X2, INSIDE_X2, INSIDE_X1 }, > + { BOUNDING_BOX_BOTTOM_Y, BOUNDING_BOX_BOTTOM_Y, INSIDE_Y2, INSIDE_Y2 }, > + 4 > + } > + }, > + /* Left outside */ > + { > + { > + { OUTSIDE_X1, INSIDE_X2, INSIDE_X2, OUTSIDE_X1 }, > + { INSIDE_Y1, INSIDE_Y1, INSIDE_Y2, INSIDE_Y2 }, > + 4 > + }, > + { > + { BOUNDING_BOX_LEFT_X, INSIDE_X2, INSIDE_X2, BOUNDING_BOX_LEFT_X }, > + { INSIDE_Y1, INSIDE_Y1, INSIDE_Y2, INSIDE_Y2 }, > + 4 > + } > + }, > + /* Right outside */ > + { > + { > + { INSIDE_X1, OUTSIDE_X2, OUTSIDE_X2, INSIDE_X1 }, > + { INSIDE_Y1, INSIDE_Y1, INSIDE_Y2, INSIDE_Y2 }, > + 4 > + }, > + { > + { INSIDE_X1, BOUNDING_BOX_RIGHT_X, BOUNDING_BOX_RIGHT_X, INSIDE_X1 }, > + { INSIDE_Y1, INSIDE_Y1, INSIDE_Y2, INSIDE_Y2 }, > + 4 > + } > + }, > + /* Diamond extending from bounding box edges, clip to bounding box */ > + { > + { > + { BOUNDING_BOX_LEFT_X - 25, BOUNDING_BOX_LEFT_X + 25, BOUNDING_BOX_RIGHT_X + 25, BOUNDING_BOX_RIGHT_X - 25 }, > + { BOUNDING_BOX_BOTTOM_Y + 25, BOUNDING_BOX_TOP_Y + 25, BOUNDING_BOX_TOP_Y - 25, BOUNDING_BOX_BOTTOM_Y - 25 }, > + 4 > + }, > + { > + { BOUNDING_BOX_LEFT_X, BOUNDING_BOX_LEFT_X, BOUNDING_BOX_RIGHT_X, BOUNDING_BOX_RIGHT_X }, > + { BOUNDING_BOX_BOTTOM_Y, BOUNDING_BOX_TOP_Y, BOUNDING_BOX_TOP_Y, BOUNDING_BOX_BOTTOM_Y }, > + 4 > + } > + }, > + /* Diamond inside of bounding box edges, clip t bounding box, 8 resulting vertices */ > + { > + { > + { BOUNDING_BOX_LEFT_X - 12.5, BOUNDING_BOX_LEFT_X + 25, BOUNDING_BOX_RIGHT_X + 12.5, BOUNDING_BOX_RIGHT_X - 25 }, > + { BOUNDING_BOX_BOTTOM_Y + 25, BOUNDING_BOX_TOP_Y + 12.5, BOUNDING_BOX_TOP_Y - 25, BOUNDING_BOX_BOTTOM_Y - 12.5 }, > + 4 > + }, > + { > + { BOUNDING_BOX_LEFT_X + 12.5, BOUNDING_BOX_LEFT_X, BOUNDING_BOX_LEFT_X, BOUNDING_BOX_LEFT_X + 12.5, > + BOUNDING_BOX_RIGHT_X - 12.5, BOUNDING_BOX_RIGHT_X, BOUNDING_BOX_RIGHT_X, BOUNDING_BOX_RIGHT_X - 12.5 }, > + { BOUNDING_BOX_BOTTOM_Y, BOUNDING_BOX_BOTTOM_Y + 12.5, BOUNDING_BOX_TOP_Y - 12.5, BOUNDING_BOX_TOP_Y, > + BOUNDING_BOX_TOP_Y, BOUNDING_BOX_TOP_Y - 12.5, BOUNDING_BOX_BOTTOM_Y + 12.5, BOUNDING_BOX_BOTTOM_Y }, > + 8 > + } > + } > +}; > + > +/* clip_polygon modifies the source operand and the test data must > + * be const, so we need to deep copy it */ > +static void > +deep_copy_polygon8(const struct polygon8 *src, struct polygon8 *dst) > +{ > + dst->n = src->n; > + memcpy((void *) dst->x, src->x, sizeof (src->x)); > + memcpy((void *) dst->y, src->y, sizeof (src->y)); > +} > + > +TEST_P(clip_polygon_n_vertices_emitted, test_data) > +{ > + struct vertex_clip_test_data *tdata = data; > + struct clip_context ctx; > + struct polygon8 polygon; > + GLfloat vertices_x[8]; > + GLfloat vertices_y[8]; > + deep_copy_polygon8(&tdata->surface, &polygon); > + int emitted = clip_polygon(&ctx, &polygon, vertices_x, vertices_y); > + > + assert(emitted == tdata->expected.n); > +} > + > +TEST_P(clip_polygon_expected_vertices, test_data) > +{ > + struct vertex_clip_test_data *tdata = data; > + struct clip_context ctx; > + struct polygon8 polygon; > + GLfloat vertices_x[8]; > + GLfloat vertices_y[8]; > + deep_copy_polygon8(&tdata->surface, &polygon); > + int emitted = clip_polygon(&ctx, &polygon, vertices_x, vertices_y); > + int i = 0; > + > + for (; i < emitted; ++i) > + { > + assert(vertices_x[i] == tdata->expected.x[i]); > + assert(vertices_y[i] == tdata->expected.y[i]); > + } > +} > + > +TEST(float_difference_different) > +{ > + assert(float_difference(1.0f, 0.0f) == 1.0f); > +} > + > +TEST(float_difference_same) > +{ > + assert(float_difference(1.0f, 1.0f) == 0.0f); > +} > + > -- > 1.8.3.2 > > _______________________________________________ > wayland-devel mailing list > <a href="mailto:wayland-devel@lists.freedesktop.org">wayland-devel@lists.freedesktop.org</a> > <a href="http://lists.freedesktop.org/mailman/listinfo/wayland-devel" target="_blank">http://lists.freedesktop.org/mailman/listinfo/wayland-devel</a> </blockquote></div>