winex11.drv: Implement FIXME in AlphaBlend

Peter Urbanec winehq.org at urbanec.net
Sat Nov 7 22:08:35 CST 2009


I've taken the following feedback on board and resubmitted the patch.


Andrew Eikum wrote:
> This is less important, but you're not following the code conventions 
> of the surrounding code.  The surrounding code does not have spaces 
> around function parameters, so yours shouldn't either.

The existing code uses a mixture of styles in the AlphaBlend function 
alone. My original patch did not have spaces around function parameters, 
however it did have spaces after C language keywords. The resubmitted 
patch follows the style of the code immediately before and after the 
patch, which does not have spaces after keywords.



Roderick Colenbrander wrote:
> I didn't see this patch before because I was away at that time. The
> SourceConstantAlpha part looks correct to me. It might make sense to
> calculate blendfn.SourceConstantAlpha / 255 before the loop, so that
> it isn't recalculated each time.

I considered this, but because the alpha is calculated using integer 
arithmetic on BYTE values, a pre-calculated blend factor would always 
degenerate to the value 0. The alternative is to use floating point 
arithmetic for the blend factor calculation, but that would most likely 
end up being a performance killer. Hopefully the compiler will do a 
reasonable optimising job if it is possible to improve the code.

I guess the code could be hand optimised to use of a vectorised 
instruction set where available. However, I think that at this point 
providing a concise and readable implementation is probably a more 
valuable contribution.


> The other half of your patch is a separate change and it would require
> a separate patch. The part looks suspicious though. I don't understand
> why you need special locking there.

Yes, the locking code is not related to the missing AlphaBlend 
functionality. It got rolled into the patch because I was fixing some 
other issues at the same time. I have a heavily multithreaded 
application and without that locking code I end up with areas of the 
screen that are not updated correctly. To be honest, I don't have a very 
clear picture of how the various locking mechanisms interact, so it is 
feasible that the locking code I added does not address the underlying 
problem, but only fixes the symptom as a side effect.

I'd be happy to learn more about the interaction between X11, OpenGL and 
wine synchronisation mechanisms - any pointers where to look for 
explanations?

Best regards,

    Peter Urbanec




More information about the wine-devel mailing list