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