[Pixman] [PATCH 0/7] lowlevel-blt-bench test pattern parser

Pekka Paalanen ppaalanen at gmail.com
Mon Apr 13 04:31:35 PDT 2015


From: Pekka Paalanen <pekka.paalanen at collabora.co.uk>

Hi,

this is a retake of Ben Avison's series starting with
http://lists.freedesktop.org/archives/pixman/2015-March/003506.html

Of that series, the first patch has landed. Here I am replacing patches 3
and 4. Patch 2 is included in my series as is, with my R-b added.  I am leaving
patch 5 of that series for a bit later, including some other clean-ups to
lowlevel-blt-bench.

The point of my series is to try and take into accout Søren's review comments
from
http://lists.freedesktop.org/archives/pixman/2014-October/003457.html
particularly:

> ==================================================================
> 0002:	lowlevel parse				Needs work 
> 0003:	affine benchmarker			Needs work 
> ==================================================================
> 
> Both of these could benefit from taking the support for
> format/operator parsing and printing in check-format.c and moving it
> to utils.[ch], perhaps extended to deal with strings like 'none' etc.
> 
> Also some coding style issues. (Use a typedef for structs, don't
> define a struct on a single line, braces around statements that span
> multiple lines -- see CODING_STYLE for more information.

This series tries to do just that: move the parsing/printing code to utils and
extend it to be usable from lowlevel-blt-bench.

Apart from restructuring, there is one significant(?) difference to Ben's
patches: for a solid mask, Ben used a8r8g8b8, but my code uses a8. I hope this
is fine in general, but should we also add a condition, that if a test has CA
flag and a solid mask, then that mask needs to be a8r8g8b8?

None of the existing tests has a solid mask for CA tests. I'm not sure how much
it makes sense.

I have tested this series with the usual 'make check' after each commit, plus
verified that the output of check-formats (the lists of operators and formats)
is the same before and after.

This series allows to attempt even impossible tests like src_8888_n. A solid
destination is obviously nonsense, and triggers a segfault:
  pixman_image_composite32
    general_composite_rect
      dest_write_back_narrow
        NULL function pointer
To me it seems that Pixman does not validate its arguments well enough, but
that's off-topic here. I don't think this kind of validation would belong in
lowlevel-blt-bench but libpixman itself.

Ben, when we eventually get to the actual optimization patches, I expect I will
reproduce the benchmarks myself while reviewing. Is that ok?
What do you think of this series?


Thanks,
pq


Ben Avison (1):
  test: Move format and operator string functions to utils.[ch]

Pekka Paalanen (6):
  test/utils: support operator name aliases
  test/utils: support format name aliases
  test/utils: add operator aliases for lowlevel-blt-bench
  test/utils: add format aliases used by lowlevel-blt-bench
  lowlevel-blt-bench: add test name parser and self-test
  lowlevel-blt-bench: use the test pattern parser

 test/check-formats.c      | 192 -------------------
 test/lowlevel-blt-bench.c | 295 ++++++++++++++++++++++++++---
 test/utils.c              | 463 ++++++++++++++++++++++++++++++++--------------
 test/utils.h              |  12 ++
 4 files changed, 608 insertions(+), 354 deletions(-)

-- 
2.0.5



More information about the Pixman mailing list