Siarhei Siamashka <siarhei.siamas...@gmail.com> writes: > On Wednesday 22 September 2010 17:56:20 Soeren Sandmann wrote: > > Siarhei Siamashka <siarhei.siamas...@gmail.com> writes: > > > But right now setting PAD repeat for everying will cause serious > > > performance problems for pixman, because it will stop using simple > > > non-transformed fast paths. > > > > > > This patch tries to address the problem. It passes current pixman tests > > > and I think that it is most likely fine. But I'm still not totally sure > > > if it is really safe in all possible cases. > > > > I can't think of a case where it would be broken, but I don't think > > it's really *right* either. The COVERS_CLIP flag is supposed to > > indicate that the image doesn't have any undefined parts, which is > > certainly the case when you have PAD or REFLECT set. > > I don't think the patch is more broken than the current code. Both NONE > and PAD repeats are very similar, as they both define the values of pixels > outside image boundaries according to some predefined rules. Both can't > be used with simple non-transformed fast paths if the operation involves > accessing any pixels outside image boundaries. So in my opinion treating > them equally in this part of code is the right thing.
You can make exactly the same argument about NORMAL: it defines values of pixels outside image boundaries according to some predefined rules. The reason you can't remove COVERS_CLIP in that case is that solid images created by NORMAL repeating a 1x1 image is still a common way to generate solid images. But PAD and REFLECT *could* be used that way too, even if in practice they aren't. I guess that's an example of something that is in principle more broken with the patch that without: with the patch you can't use PAD and REFLECT with a 1x1 image to make solid images. But my main objection is still that it uses the flags in a meaningless way that just happen to give a desired result. With the patch you can't assign any real meaning to the COVERS_CLIP flag. Ie., it's a hack to get around some admittedly not-too-well-though-out existing code, instead of fixing it. > Even though this treatment itself is not perfect and can be improved > as you explained below. > > Would it be a bad idea to apply this patch now, and then proceed with > a larger scale refactoring next? If I thought that a larger scale refactoring was likely to actually happen, then I'd be more willing to consider applying this patch. But once the immediate symptom is solved, people inevitably end up not fixing the real problem. Actually, I don't even think a large scale refactoring is necessary - most of the real fix is just deleting code. The main piece of new logic is the extending of PIXMAN_STD_FAST_PATH to generate different flags for solid and bits images. Soren _______________________________________________ Pixman mailing list Pixman@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pixman