[Pixman] [PATCH 1/2] allow runtime switching of pixman_implementation_t's for testing.
sandmann at daimi.au.dk
Fri May 21 13:42:14 PDT 2010
> define a new public entry point _pixman_debug_set_implementation.
> _pixman_choose_implementation now takes a "char* imp_name" parameter,
> with NULL being equivalent to "general". unrecognized names also result
> in the general implementation being chosen.
> the thread local cache now includes a pixman_implementation_t* which is
> used to clear the fastpath cache when a change in pixman_implementation_t
> is noticed.
Testing implmentations against each other is a very good idea. I think
these patches are a big improvement over what we have now, so apart
from the comments below, it looks fine to go in.
- Would it be worthwhile using an environment variable along with some
API entrypoint to make pixman recompute what implementation it is
using? The advantage of that is that it would allow us to isolate
bugs more quickly by asking users to reproduce it with something
I'm not sure of the answer here; there may be drawbacks to the
environment variable that I'm missing.
- I think whatever entrypoint we end up adding should be protected
- Maybe call the fast path implementation "fast-path" just for
consistency with the _create_fast_path constructor (though I know I
said otherwise on IRC).
- Style comments:
- braces go on their own line
- multiline if/while/for bodies should have braces around them even
if they are just one C statement.
More information about the Pixman