[Pixman] [PATCH 4/5] lowlevel-blt-bench: Parse test name strings in general case

[Ben Avison]

Hi,

Sorry for the delay in following up...

On Mon, 16 Mar 2015 14:33:47 -0000, Pekka Paalanen <ppaalanen at gmail.com> wrote:
> I understood these arrays should have been replaced by
> format_from_string/operator_from_string functions from patch 2 and
> similar new functions doing lookups in the same arrays.
>
> Hmm, but I see there are several alternative spellings for operators,
> and the formats follow a whole new convention.

Some background to this might help. Pixman supports a lot of pixel
formats; of particular significance is the fact that in many cases you
can have the same bits-per-pixel, divided up into the same bitpattern of
colour components, but with the red and blue components exchanged.

Each of Pixman's operations act upon between 1 and 3 bitmaps, each of
which may have any supported pixel format. But in practice, any given
application tends to stick to the same colour component ordering for most
or all of its images, so the actual number of operations you're likely to
encounter in practice is 1/2 (for 2-image operations) or 1/4 (for 3-image
operations) of what you might otherwise expect. Furthermore,
mathematically, an operation on (for example) two a8r8g8b8 images is
identical to one on two a8b8g8r8 images, so typically a fast path written
for one will be listed in the fast path table under both image formats.
Because of this, a naming convention has evolved in the source code where
fast path names include the string 8888 as an indication that it can be
applied to either a8r8g8b8 or a8b8g8r8, with the implicit assumption that
the other image has the same colour component ordering.

lowlevel-blt-bench is most useful when you're testing the effectiveness
of a particular fast path, so it makes sense that its test pattern names
reflect the names of the fast path function that you're interested in.
However, there are a few tests, like "src_0888_8888_rev" or "rpixbuf"
where the limitations of this approach start to show. I suspected I would
get objections if I changed the command line of lowlevel-blt-bench, but
in introducing a new tool, affine-bench, I saw an opportunity to allow
the pixel formats to be specified more directly, and deliberately made
its syntax different.

It is a matter for debate as to whether the two tools should use the same
syntax or not, and if so, which one they should standardise on. I'd be a
bit uncomfortable with saying that "8888" is an alias for "a8r8g8b8" or
"0565" for "r5g6b5", because that's not true in both directions, as I
have described. I'd probably choose the more verbose form if the
consensus was that the two tools should match. Is there anyone who cares
either way, though?

> Personally I'm not really much of a fan of this much nesting,
> especially where you have a number of parsing steps and each step nests
> a bit more. I'd refactor this code into a function and use early (error)
> returns instead of nesting.

Quick-and-dirty code there - it was only a test harness after all. But
easily fixed.

> Looks like there is also no complaint, if the given string is not a
> valid test name.

It never did generate any sort of error if you specified a bad test
string - you just wouldn't have seen any results. Another easy fix -
updated patch following shortly.

Ben