[Pixman] [PATCH] Don't discriminate PAD and REFLECT repeat in standard fast paths
sandmann at daimi.au.dk
Tue Sep 28 20:01:31 PDT 2010
Siarhei Siamashka <siarhei.siamashka at gmail.com> writes:
> On Tuesday 28 September 2010 17:24:14 Siarhei Siamashka wrote:
> > The flags in pixman are implemented in such a way that they all have some
> > kind of "positive" meaning, indicating that the operation is going to be
> > more simple than the general case in one way or another. Having more flags
> > set on the initial analysis stage is always good. Forgetting to set some
> > of the flags is safe in the sense that it will not make pixman misbehave,
> > pixman will just run slower because of not taking some fast paths.
To rephrase: the flags should be though of as properties that the
If the FAST_PATH_BLAH flag is set, then the image has property
If a fast path includes BLAH in its mask, then it is saying "I can
only run if the image has property BLAH".
For fast paths, that's really all there is to it, bugs in the
PIXMAN_STD_FAST_PATH macro notwithstanding.
So the logic we rely on is:
BLAH is set => image has property BLAH
But there is no need for the arrow to go the other way because if an
image has property BLAH and the flag isn't set, then we just won't run
that particular fast path.
Ie., the flags have a positive meaning, as you say.
> Tried to actually verify this claim by artificially clearing flags in
> 'compute_image_info' and running pixman tests. Appears that not everything
> is so simple.
> Some flags are a bit special:
> - NARROW_FORMAT flag needs to be propagated to 'general_composite_rect'
> function because it is used there
> - calculation of NEEDS_WORKAROUND flag also can't be skipped
That is a good point.
Yes, in the cases you point out (NARROW_FORMAT and NEEDS_WORKAROUND),
we do make use of the negative meaning of the flag. If NARROW_FORMAT
is *not* set, then wide fetchers are used. So, we rely on this:
BLAH is not set => image does not have property BLAH
which by contraposition gives
image has property BLAH => BLAH is set
So those two flags really are special in that they *must* be set if
the image actually has the property.
This is actually kind of bad. I don't like special cases, and I hadn't
realized we had one here. But maybe we can get rid of NEEDS_WORKAROUND
in 0.22, and if we add a floating point implementation as a fallback
for the general one, that would allow the general one to just put
NARROW_FORMAT flags on its "fast path", using the flag in a more
> - flags ID_TRANSFORM, NO_ALPHA_MAP, NO_CONVOLUTION_FILTER, NO_PAD_REPEAT
> and NO_REFLECT_REPEAT are needed to select untransformed fetchers
> bit_image_fetch_untransformed_32/bit_image_fetch_untransformed_64 instead of
> bit_image_fetch_general/_pixman_image_get_scanline_generic_64, and apparently
> these behave differently
This is in *principle* the same positive use of the flags. It should
in *principle* be possible to ignore all these fetchers and just use
the bits_image_fetch_general() function. However, there are
differences because when you have a transformation, the 64 bit
pipeline is actually using 32 bit pixels expanded to 64 bits.
Which is broken, of course. So hopefully, this is another case where a
floating point implementation can help. However, for that to work, we
will need a couple of things from the floating point implemenetation:
- The floating point implementation must do transformations and
gradients (unlike the existing 64 bit pipe).
- We need the ability to plug in implementation specific
fetchers. Otherwise we can't do proper fallback from the general to
the floating point implementation.
So if Dmitri and Jonathan are still reading, I think the above points
are worth keeping in mind.
Here are some notes on how CPU specific fetchers could be done:
More information about the Pixman