Revision 1.6 of fbcompose.c added an optimization to fbCompositeRect where the src pixels would only be fetched once if this was possible (solid fills and the like). However this doesn't work for some operations which overwrite the src buffer, for example fbCombineAddC (via fbCombineMaskValueC). Such operations need to be checked for when deciding when the src fetch-once optimization can occur.
I'm pretty sure that all my patches have not landed in the cairo tree yet? If I remeber correctly the second iteration which contained mostly performance improvments also had some minor fixes that could correct this. Is there a test program that expose this? rendercheck doesn't complain about the same code in the server (that's code with all my patches applied though).
I don't have a test program - doing text with a clip path set on win32 triggered this composite operation. Which server are you talking about? xc head fbcompose.c doesn't have this optimization, and the xserver one looks to have the same problem at first glance.
Created attachment 4546 [details] testcase Here's a cairo test that shows the problem. You'll need to disable the mmx versions of the composite functions (hack fbHaveMMX), as the mmx implementation of CombineAddC doesn't have this problem.
Thanks for the test case, I'll look at this asap. Not being exposed when using the mmx code explaines why I didn't notice it.
Created attachment 4577 [details] [review] possible bandaid fix
David, have you managed to make any progress on a fix for this bug?
Created attachment 9469 [details] [review] Fix The previous fix is fine too but here's another one which I think is a bit more appropriate. It changes so fbCombineMaskC, fbCombineMaskValueC and fbCombineMaskAlphaC operate on pixels instead of scanlines and can be used without modifying src and mask scanlines.
What's the effect on performance of the patch? Function-call-per-pixel might be noticeable and I don't think gcc will inline at -O2 unless you add 'inline'
I don't know if the current patch got any effect on performance. We should add 'inline' to make sure gcc will inline those functions.
commit d3c7942fb271fe8d1df7ca3205b41601abdcb5c8 Author: Behdad Esfahbod <behdad@behdad.org> Date: Thu Apr 12 14:22:02 2007 -0400 [pixman] Mark some small functions that are called per-pixel inline These uses were introduced in the previous commit. commit e3b3d22999a130f7017e8e20a432a0d8a7f48f3b Author: David Reveman <davidr@novell.com> Date: Thu Apr 12 14:14:12 2007 -0400 [pixman] Fix fbcompose.c that was broken for some composite operations (#5777)
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.