| Message ID | 20230327133848.5250-2-arthurgrillo@riseup.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Create tests for the drm_rect functions | expand |
Hi Arthur, On 3/27/23 10:38, Arthur Grillo wrote: > Insert test for the drm_rect_intersect() function, it also create a > helper for comparing drm_rects more easily. > > Signed-off-by: Arthur Grillo <arthurgrillo@riseup.net> > --- > drivers/gpu/drm/tests/drm_rect_test.c | 139 ++++++++++++++++++++++++++ > 1 file changed, 139 insertions(+) > > diff --git a/drivers/gpu/drm/tests/drm_rect_test.c b/drivers/gpu/drm/tests/drm_rect_test.c > index e9809ea32696..3654c0be3d6b 100644 > --- a/drivers/gpu/drm/tests/drm_rect_test.c > +++ b/drivers/gpu/drm/tests/drm_rect_test.c > @@ -9,6 +9,17 @@ > > #include <drm/drm_rect.h> > > +#include <linux/string_helpers.h> Is this include really needed? I was able to compile without it. > + > +static void drm_rect_compare(struct kunit *test, const struct drm_rect *r, > + const struct drm_rect *expected) > +{ > + KUNIT_EXPECT_EQ(test, r->x1, expected->x1); Maybe it would be nice to have a message here that shows the current x1 and the expected x1. Same for the other dimensions. > + KUNIT_EXPECT_EQ(test, r->y1, expected->y1); > + KUNIT_EXPECT_EQ(test, drm_rect_width(r), drm_rect_width(expected)); > + KUNIT_EXPECT_EQ(test, drm_rect_height(r), drm_rect_height(expected)); > +} > + > static void drm_test_rect_clip_scaled_div_by_zero(struct kunit *test) > { > struct drm_rect src, dst, clip; > @@ -196,11 +207,139 @@ static void drm_test_rect_clip_scaled_signed_vs_unsigned(struct kunit *test) > KUNIT_EXPECT_FALSE_MSG(test, drm_rect_visible(&src), "Source should not be visible\n"); > } > > +struct drm_rect_intersect_case { > + const char *description; > + struct drm_rect r1, r2; > + bool should_be_visible; > + struct drm_rect expected_intersection; > +}; > + > +static const struct drm_rect_intersect_case drm_rect_intersect_cases[] = { > + { > + .description = "top-left X bottom-right", > + .r1 = DRM_RECT_INIT(1, 1, 2, 2), > + .r2 = DRM_RECT_INIT(0, 0, 2, 2), > + .should_be_visible = true, > + .expected_intersection = DRM_RECT_INIT(1, 1, 1, 1), > + }, > + { > + .description = "top-right X bottom-left", > + .r1 = DRM_RECT_INIT(0, 0, 2, 2), > + .r2 = DRM_RECT_INIT(1, -1, 2, 2), > + .should_be_visible = true, > + .expected_intersection = DRM_RECT_INIT(1, 0, 1, 1), > + }, > + { > + .description = "bottom-left X top-right", > + .r1 = DRM_RECT_INIT(1, -1, 2, 2), > + .r2 = DRM_RECT_INIT(0, 0, 2, 2), > + .should_be_visible = true, > + .expected_intersection = DRM_RECT_INIT(1, 0, 1, 1), > + }, > + { > + .description = "bottom-right X top-left", > + .r1 = DRM_RECT_INIT(0, 0, 2, 2), > + .r2 = DRM_RECT_INIT(1, 1, 2, 2), > + .should_be_visible = true, > + .expected_intersection = DRM_RECT_INIT(1, 1, 1, 1), > + }, > + { > + .description = "right X left", > + .r1 = DRM_RECT_INIT(0, 0, 2, 1), > + .r2 = DRM_RECT_INIT(1, 0, 3, 1), > + .should_be_visible = true, > + .expected_intersection = DRM_RECT_INIT(1, 0, 1, 1), > + }, > + { > + .description = "left X right", > + .r1 = DRM_RECT_INIT(1, 0, 3, 1), > + .r2 = DRM_RECT_INIT(0, 0, 2, 1), > + .should_be_visible = true, > + .expected_intersection = DRM_RECT_INIT(1, 0, 1, 1), > + }, > + { > + .description = "up X bottom", > + .r1 = DRM_RECT_INIT(0, 0, 1, 2), > + .r2 = DRM_RECT_INIT(0, -1, 1, 3), > + .should_be_visible = true, > + .expected_intersection = DRM_RECT_INIT(0, 0, 1, 2), > + }, > + { > + .description = "bottom X up", > + .r1 = DRM_RECT_INIT(0, -1, 1, 3), > + .r2 = DRM_RECT_INIT(0, 0, 1, 2), > + .should_be_visible = true, > + .expected_intersection = DRM_RECT_INIT(0, 0, 1, 2), > + }, > + { > + .description = "touching corner", > + .r1 = DRM_RECT_INIT(0, 0, 1, 1), > + .r2 = DRM_RECT_INIT(1, 1, 2, 2), > + .should_be_visible = false, > + .expected_intersection = DRM_RECT_INIT(1, 1, 0, 0), > + }, > + { > + .description = "touching side", > + .r1 = DRM_RECT_INIT(0, 0, 1, 1), > + .r2 = DRM_RECT_INIT(1, 0, 1, 1), > + .should_be_visible = false, > + .expected_intersection = DRM_RECT_INIT(1, 0, 0, 1), > + }, > + { > + .description = "equal rects", > + .r1 = DRM_RECT_INIT(0, 0, 2, 2), > + .r2 = DRM_RECT_INIT(0, 0, 2, 2), > + .should_be_visible = true, > + .expected_intersection = DRM_RECT_INIT(0, 0, 2, 2), > + }, > + { > + .description = "inside another", > + .r1 = DRM_RECT_INIT(0, 0, 2, 2), > + .r2 = DRM_RECT_INIT(1, 1, 1, 1), > + .should_be_visible = true, > + .expected_intersection = DRM_RECT_INIT(1, 1, 1, 1), > + }, > + { > + .description = "far away", > + .r1 = DRM_RECT_INIT(0, 0, 1, 1), > + .r2 = DRM_RECT_INIT(3, 6, 1, 1), > + .should_be_visible = false, > + .expected_intersection = DRM_RECT_INIT(3, 6, -2, -5), > + }, > +}; What happens if width = height = 0? It would be nice to have a test case for this scenario. > + > +static void drm_rect_intersect_case_desc(const struct drm_rect_intersect_case *t, char *desc) > +{ > + if (!t->description) > + snprintf(desc, KUNIT_PARAM_DESC_SIZE, > + DRM_RECT_FMT " X " DRM_RECT_FMT, > + DRM_RECT_ARG(&t->r1), DRM_RECT_ARG(&t->r2)); Is this conditional clause really needed? All parameters have description. > + else > + snprintf(desc, KUNIT_PARAM_DESC_SIZE, > + "%s: " DRM_RECT_FMT " X " DRM_RECT_FMT, > + t->description, DRM_RECT_ARG(&t->r1), DRM_RECT_ARG(&t->r2)); > +} > + > +KUNIT_ARRAY_PARAM(drm_rect_intersect, drm_rect_intersect_cases, drm_rect_intersect_case_desc); > + > +static void drm_test_rect_intersect(struct kunit *test) > +{ > + const struct drm_rect_intersect_case *params = test->param_value; > + struct drm_rect r1_aux = params->r1; Does this variable needs to exist? I guess you could just use params->r1. Best Regards, - Maíra Canal > + bool visible; > + > + visible = drm_rect_intersect(&r1_aux, ¶ms->r2); > + > + KUNIT_EXPECT_EQ(test, visible, params->should_be_visible); > + drm_rect_compare(test, &r1_aux, ¶ms->expected_intersection); > +} > + > static struct kunit_case drm_rect_tests[] = { > KUNIT_CASE(drm_test_rect_clip_scaled_div_by_zero), > KUNIT_CASE(drm_test_rect_clip_scaled_not_clipped), > KUNIT_CASE(drm_test_rect_clip_scaled_clipped), > KUNIT_CASE(drm_test_rect_clip_scaled_signed_vs_unsigned), > + KUNIT_CASE_PARAM(drm_test_rect_intersect, drm_rect_intersect_gen_params), > { } > }; >
On 03/04/23 12:33, Maíra Canal wrote: > Hi Arthur, > > On 3/27/23 10:38, Arthur Grillo wrote: >> Insert test for the drm_rect_intersect() function, it also create a >> helper for comparing drm_rects more easily. >> >> Signed-off-by: Arthur Grillo <arthurgrillo@riseup.net> >> --- >> drivers/gpu/drm/tests/drm_rect_test.c | 139 ++++++++++++++++++++++++++ >> 1 file changed, 139 insertions(+) >> >> diff --git a/drivers/gpu/drm/tests/drm_rect_test.c b/drivers/gpu/drm/tests/drm_rect_test.c >> index e9809ea32696..3654c0be3d6b 100644 >> --- a/drivers/gpu/drm/tests/drm_rect_test.c >> +++ b/drivers/gpu/drm/tests/drm_rect_test.c >> @@ -9,6 +9,17 @@ >> #include <drm/drm_rect.h> >> +#include <linux/string_helpers.h> > > Is this include really needed? I was able to compile without it. > >> + >> +static void drm_rect_compare(struct kunit *test, const struct drm_rect *r, >> + const struct drm_rect *expected) >> +{ >> + KUNIT_EXPECT_EQ(test, r->x1, expected->x1); > > Maybe it would be nice to have a message here that shows the current x1 > and the expected x1. Same for the other dimensions. > Doesn't KUnit already output this information when the values don't match? >> + KUNIT_EXPECT_EQ(test, r->y1, expected->y1); >> + KUNIT_EXPECT_EQ(test, drm_rect_width(r), drm_rect_width(expected)); >> + KUNIT_EXPECT_EQ(test, drm_rect_height(r), drm_rect_height(expected)); >> +} >> + >> static void drm_test_rect_clip_scaled_div_by_zero(struct kunit *test) >> { >> struct drm_rect src, dst, clip; >> @@ -196,11 +207,139 @@ static void drm_test_rect_clip_scaled_signed_vs_unsigned(struct kunit *test) >> KUNIT_EXPECT_FALSE_MSG(test, drm_rect_visible(&src), "Source should not be visible\n"); >> } >> +struct drm_rect_intersect_case { >> + const char *description; >> + struct drm_rect r1, r2; >> + bool should_be_visible; >> + struct drm_rect expected_intersection; >> +}; >> + >> +static const struct drm_rect_intersect_case drm_rect_intersect_cases[] = { >> + { >> + .description = "top-left X bottom-right", >> + .r1 = DRM_RECT_INIT(1, 1, 2, 2), >> + .r2 = DRM_RECT_INIT(0, 0, 2, 2), >> + .should_be_visible = true, >> + .expected_intersection = DRM_RECT_INIT(1, 1, 1, 1), >> + }, >> + { >> + .description = "top-right X bottom-left", >> + .r1 = DRM_RECT_INIT(0, 0, 2, 2), >> + .r2 = DRM_RECT_INIT(1, -1, 2, 2), >> + .should_be_visible = true, >> + .expected_intersection = DRM_RECT_INIT(1, 0, 1, 1), >> + }, >> + { >> + .description = "bottom-left X top-right", >> + .r1 = DRM_RECT_INIT(1, -1, 2, 2), >> + .r2 = DRM_RECT_INIT(0, 0, 2, 2), >> + .should_be_visible = true, >> + .expected_intersection = DRM_RECT_INIT(1, 0, 1, 1), >> + }, >> + { >> + .description = "bottom-right X top-left", >> + .r1 = DRM_RECT_INIT(0, 0, 2, 2), >> + .r2 = DRM_RECT_INIT(1, 1, 2, 2), >> + .should_be_visible = true, >> + .expected_intersection = DRM_RECT_INIT(1, 1, 1, 1), >> + }, >> + { >> + .description = "right X left", >> + .r1 = DRM_RECT_INIT(0, 0, 2, 1), >> + .r2 = DRM_RECT_INIT(1, 0, 3, 1), >> + .should_be_visible = true, >> + .expected_intersection = DRM_RECT_INIT(1, 0, 1, 1), >> + }, >> + { >> + .description = "left X right", >> + .r1 = DRM_RECT_INIT(1, 0, 3, 1), >> + .r2 = DRM_RECT_INIT(0, 0, 2, 1), >> + .should_be_visible = true, >> + .expected_intersection = DRM_RECT_INIT(1, 0, 1, 1), >> + }, >> + { >> + .description = "up X bottom", >> + .r1 = DRM_RECT_INIT(0, 0, 1, 2), >> + .r2 = DRM_RECT_INIT(0, -1, 1, 3), >> + .should_be_visible = true, >> + .expected_intersection = DRM_RECT_INIT(0, 0, 1, 2), >> + }, >> + { >> + .description = "bottom X up", >> + .r1 = DRM_RECT_INIT(0, -1, 1, 3), >> + .r2 = DRM_RECT_INIT(0, 0, 1, 2), >> + .should_be_visible = true, >> + .expected_intersection = DRM_RECT_INIT(0, 0, 1, 2), >> + }, >> + { >> + .description = "touching corner", >> + .r1 = DRM_RECT_INIT(0, 0, 1, 1), >> + .r2 = DRM_RECT_INIT(1, 1, 2, 2), >> + .should_be_visible = false, >> + .expected_intersection = DRM_RECT_INIT(1, 1, 0, 0), >> + }, >> + { >> + .description = "touching side", >> + .r1 = DRM_RECT_INIT(0, 0, 1, 1), >> + .r2 = DRM_RECT_INIT(1, 0, 1, 1), >> + .should_be_visible = false, >> + .expected_intersection = DRM_RECT_INIT(1, 0, 0, 1), >> + }, >> + { >> + .description = "equal rects", >> + .r1 = DRM_RECT_INIT(0, 0, 2, 2), >> + .r2 = DRM_RECT_INIT(0, 0, 2, 2), >> + .should_be_visible = true, >> + .expected_intersection = DRM_RECT_INIT(0, 0, 2, 2), >> + }, >> + { >> + .description = "inside another", >> + .r1 = DRM_RECT_INIT(0, 0, 2, 2), >> + .r2 = DRM_RECT_INIT(1, 1, 1, 1), >> + .should_be_visible = true, >> + .expected_intersection = DRM_RECT_INIT(1, 1, 1, 1), >> + }, >> + { >> + .description = "far away", >> + .r1 = DRM_RECT_INIT(0, 0, 1, 1), >> + .r2 = DRM_RECT_INIT(3, 6, 1, 1), >> + .should_be_visible = false, >> + .expected_intersection = DRM_RECT_INIT(3, 6, -2, -5), >> + }, >> +}; > > What happens if width = height = 0? It would be nice to have a test > case for this scenario. > >> + >> +static void drm_rect_intersect_case_desc(const struct drm_rect_intersect_case *t, char *desc) >> +{ >> + if (!t->description) >> + snprintf(desc, KUNIT_PARAM_DESC_SIZE, >> + DRM_RECT_FMT " X " DRM_RECT_FMT, >> + DRM_RECT_ARG(&t->r1), DRM_RECT_ARG(&t->r2)); > > Is this conditional clause really needed? All parameters have > description. > >> + else >> + snprintf(desc, KUNIT_PARAM_DESC_SIZE, >> + "%s: " DRM_RECT_FMT " X " DRM_RECT_FMT, >> + t->description, DRM_RECT_ARG(&t->r1), DRM_RECT_ARG(&t->r2)); >> +} >> + >> +KUNIT_ARRAY_PARAM(drm_rect_intersect, drm_rect_intersect_cases, drm_rect_intersect_case_desc); >> + >> +static void drm_test_rect_intersect(struct kunit *test) >> +{ >> + const struct drm_rect_intersect_case *params = test->param_value; >> + struct drm_rect r1_aux = params->r1; > > Does this variable needs to exist? I guess you could just use params->r1. > I think it does because the params variable is const. Best Regards, ~Arthur Grillo > Best Regards, > - Maíra Canal > >> + bool visible; >> + >> + visible = drm_rect_intersect(&r1_aux, ¶ms->r2); >> + >> + KUNIT_EXPECT_EQ(test, visible, params->should_be_visible); >> + drm_rect_compare(test, &r1_aux, ¶ms->expected_intersection); >> +} >> + >> static struct kunit_case drm_rect_tests[] = { >> KUNIT_CASE(drm_test_rect_clip_scaled_div_by_zero), >> KUNIT_CASE(drm_test_rect_clip_scaled_not_clipped), >> KUNIT_CASE(drm_test_rect_clip_scaled_clipped), >> KUNIT_CASE(drm_test_rect_clip_scaled_signed_vs_unsigned), >> + KUNIT_CASE_PARAM(drm_test_rect_intersect, drm_rect_intersect_gen_params), >> { } >> }; >>
diff --git a/drivers/gpu/drm/tests/drm_rect_test.c b/drivers/gpu/drm/tests/drm_rect_test.c index e9809ea32696..3654c0be3d6b 100644 --- a/drivers/gpu/drm/tests/drm_rect_test.c +++ b/drivers/gpu/drm/tests/drm_rect_test.c @@ -9,6 +9,17 @@ #include <drm/drm_rect.h> +#include <linux/string_helpers.h> + +static void drm_rect_compare(struct kunit *test, const struct drm_rect *r, + const struct drm_rect *expected) +{ + KUNIT_EXPECT_EQ(test, r->x1, expected->x1); + KUNIT_EXPECT_EQ(test, r->y1, expected->y1); + KUNIT_EXPECT_EQ(test, drm_rect_width(r), drm_rect_width(expected)); + KUNIT_EXPECT_EQ(test, drm_rect_height(r), drm_rect_height(expected)); +} + static void drm_test_rect_clip_scaled_div_by_zero(struct kunit *test) { struct drm_rect src, dst, clip; @@ -196,11 +207,139 @@ static void drm_test_rect_clip_scaled_signed_vs_unsigned(struct kunit *test) KUNIT_EXPECT_FALSE_MSG(test, drm_rect_visible(&src), "Source should not be visible\n"); } +struct drm_rect_intersect_case { + const char *description; + struct drm_rect r1, r2; + bool should_be_visible; + struct drm_rect expected_intersection; +}; + +static const struct drm_rect_intersect_case drm_rect_intersect_cases[] = { + { + .description = "top-left X bottom-right", + .r1 = DRM_RECT_INIT(1, 1, 2, 2), + .r2 = DRM_RECT_INIT(0, 0, 2, 2), + .should_be_visible = true, + .expected_intersection = DRM_RECT_INIT(1, 1, 1, 1), + }, + { + .description = "top-right X bottom-left", + .r1 = DRM_RECT_INIT(0, 0, 2, 2), + .r2 = DRM_RECT_INIT(1, -1, 2, 2), + .should_be_visible = true, + .expected_intersection = DRM_RECT_INIT(1, 0, 1, 1), + }, + { + .description = "bottom-left X top-right", + .r1 = DRM_RECT_INIT(1, -1, 2, 2), + .r2 = DRM_RECT_INIT(0, 0, 2, 2), + .should_be_visible = true, + .expected_intersection = DRM_RECT_INIT(1, 0, 1, 1), + }, + { + .description = "bottom-right X top-left", + .r1 = DRM_RECT_INIT(0, 0, 2, 2), + .r2 = DRM_RECT_INIT(1, 1, 2, 2), + .should_be_visible = true, + .expected_intersection = DRM_RECT_INIT(1, 1, 1, 1), + }, + { + .description = "right X left", + .r1 = DRM_RECT_INIT(0, 0, 2, 1), + .r2 = DRM_RECT_INIT(1, 0, 3, 1), + .should_be_visible = true, + .expected_intersection = DRM_RECT_INIT(1, 0, 1, 1), + }, + { + .description = "left X right", + .r1 = DRM_RECT_INIT(1, 0, 3, 1), + .r2 = DRM_RECT_INIT(0, 0, 2, 1), + .should_be_visible = true, + .expected_intersection = DRM_RECT_INIT(1, 0, 1, 1), + }, + { + .description = "up X bottom", + .r1 = DRM_RECT_INIT(0, 0, 1, 2), + .r2 = DRM_RECT_INIT(0, -1, 1, 3), + .should_be_visible = true, + .expected_intersection = DRM_RECT_INIT(0, 0, 1, 2), + }, + { + .description = "bottom X up", + .r1 = DRM_RECT_INIT(0, -1, 1, 3), + .r2 = DRM_RECT_INIT(0, 0, 1, 2), + .should_be_visible = true, + .expected_intersection = DRM_RECT_INIT(0, 0, 1, 2), + }, + { + .description = "touching corner", + .r1 = DRM_RECT_INIT(0, 0, 1, 1), + .r2 = DRM_RECT_INIT(1, 1, 2, 2), + .should_be_visible = false, + .expected_intersection = DRM_RECT_INIT(1, 1, 0, 0), + }, + { + .description = "touching side", + .r1 = DRM_RECT_INIT(0, 0, 1, 1), + .r2 = DRM_RECT_INIT(1, 0, 1, 1), + .should_be_visible = false, + .expected_intersection = DRM_RECT_INIT(1, 0, 0, 1), + }, + { + .description = "equal rects", + .r1 = DRM_RECT_INIT(0, 0, 2, 2), + .r2 = DRM_RECT_INIT(0, 0, 2, 2), + .should_be_visible = true, + .expected_intersection = DRM_RECT_INIT(0, 0, 2, 2), + }, + { + .description = "inside another", + .r1 = DRM_RECT_INIT(0, 0, 2, 2), + .r2 = DRM_RECT_INIT(1, 1, 1, 1), + .should_be_visible = true, + .expected_intersection = DRM_RECT_INIT(1, 1, 1, 1), + }, + { + .description = "far away", + .r1 = DRM_RECT_INIT(0, 0, 1, 1), + .r2 = DRM_RECT_INIT(3, 6, 1, 1), + .should_be_visible = false, + .expected_intersection = DRM_RECT_INIT(3, 6, -2, -5), + }, +}; + +static void drm_rect_intersect_case_desc(const struct drm_rect_intersect_case *t, char *desc) +{ + if (!t->description) + snprintf(desc, KUNIT_PARAM_DESC_SIZE, + DRM_RECT_FMT " X " DRM_RECT_FMT, + DRM_RECT_ARG(&t->r1), DRM_RECT_ARG(&t->r2)); + else + snprintf(desc, KUNIT_PARAM_DESC_SIZE, + "%s: " DRM_RECT_FMT " X " DRM_RECT_FMT, + t->description, DRM_RECT_ARG(&t->r1), DRM_RECT_ARG(&t->r2)); +} + +KUNIT_ARRAY_PARAM(drm_rect_intersect, drm_rect_intersect_cases, drm_rect_intersect_case_desc); + +static void drm_test_rect_intersect(struct kunit *test) +{ + const struct drm_rect_intersect_case *params = test->param_value; + struct drm_rect r1_aux = params->r1; + bool visible; + + visible = drm_rect_intersect(&r1_aux, ¶ms->r2); + + KUNIT_EXPECT_EQ(test, visible, params->should_be_visible); + drm_rect_compare(test, &r1_aux, ¶ms->expected_intersection); +} + static struct kunit_case drm_rect_tests[] = { KUNIT_CASE(drm_test_rect_clip_scaled_div_by_zero), KUNIT_CASE(drm_test_rect_clip_scaled_not_clipped), KUNIT_CASE(drm_test_rect_clip_scaled_clipped), KUNIT_CASE(drm_test_rect_clip_scaled_signed_vs_unsigned), + KUNIT_CASE_PARAM(drm_test_rect_intersect, drm_rect_intersect_gen_params), { } };
Insert test for the drm_rect_intersect() function, it also create a helper for comparing drm_rects more easily. Signed-off-by: Arthur Grillo <arthurgrillo@riseup.net> --- drivers/gpu/drm/tests/drm_rect_test.c | 139 ++++++++++++++++++++++++++ 1 file changed, 139 insertions(+)