Re: [AD] blender operations SRC and DST

[Jon Rafkind <workmin@xxxxxxxxxx>]

On 06/08/2011 07:26 PM, Peter Wang wrote:
> On 2011-06-08, Jon Rafkind <workmin@xxxxxxxxxx> wrote:
>> On 06/08/2011 09:15 AM, Jon Rafkind wrote:
>>> On 06/08/2011 10:14 AM, Peter Wang wrote:
>>>> On 2011-06-08, Elias Pschernig <elias.pschernig@xxxxxxxxxx> wrote:
>>>>> On Wed, Jun 8, 2011 at 5:29 PM, Peter Wang <novalazy@xxxxxxxxxx> wrote:
>>>>>> It's much worse on my machine, 10-12%.  I'd want to see what we can do
>>>>>> to avoid that kind of regression.
>>>>>>
>>>>>> Peter
>>>>>>
>>>>>> before patch:
>>>>>> Plain blit: 659.283 FPS
>>>>>> Scaled blit: 663.27 FPS
>>>>>> Rotated blit: 678.819 FPS
>>>>>>
>>>>>> after patch:
>>>>>> Plain blit: 576.974 FPS
>>>>>> Scaled blit: 592.111 FPS
>>>>>> Rotated blit: 610.166 FPS
>>>>>>
>>>>> Looking at _al_blend_inline, I guess we can shuffle the code around so
>>>>> only the two new blend modes do anything differently but the existing
>>>>> ones remain as is.
>>>> Yes.  In addition (or afterwards) we might want to generate specialised
>>>> copies of _al_blend_inline for common operations, rather than relying on
>>>> the compiler to prune the dead branches in each inlined copy.  Manually
>>>> commenting out some of the (untaken) switch cases affects the benchmark
>>>> results so perhaps the compiler isn't doing that as well as I hoped.
>>>>
>>> Since the slowdowns come from scanline_drawers we could make it use a
>>> more specific al_blend_alpha_inline when it knows the arguments only
>>> deal with alpha (or ONE / ZERO). So like
>>>
>>> if (OP == ALLEGRO_ADD && src_mode == ALLEGRO_ALPHA ...){
>>>   ...
>>>   _al_blend_inline_alpha(...);
>>> } else {
>>>    /* do general blending */
>>>    _al_blend_inline(...);
>>> }
>>>
>>> I haven't tested this yet because it requires some non-trivial changes
>>> to make_scanline_drawers.py. Hm but probably I could just make local
>>> changes to scanline_drawers.c and see how it goes..
>> Making scanline_drawers.c call a the alpha blender if its only using
>> alpha channel factors seems to work. This patch adds
>> _al_blend_alpha_inline which is what the old _al_blend_inline was. Now
>> _al_blend_inline deals with ALLEGRO_COLOR in its full generality. Also I
>> changed make_scanline_drawers.py call _al_blend_alpha_inline when
>> appropriate.
>>
>> You'll have to apply this patch as well as the original patch that added
>> ALLEGRO_DST_COLOR to get things to compile. Theres some cruft leftover I
>> will eventually clean up if this is all ok.
>>
>> Before patch:
>>  Plain blit: 213.454 FPS
>>  Scaled blit: 213.883 FPS
>>  Rotated blit: 223.992 FPS
>>
>> After:
>>  Plain blit: 207.739 FPS
>>  Scaled blit: 212.275 FPS
>>  Rotated blit: 220.875 FPS
> The change looks better, but...
>
> I wrote make_scanline_drawers.py when ADD/ALPHA/INVERSE_ALPHA was the
> default blending mode.  The current default of ADD/ONE/INVERSE_ALPHA
> (which is what ex_blend_bench tests by default) is *not* actually
> specialised!  I don't know how this new patch manages to get a minor
> speedup compared to your previous patch, but it does.
>
> Obviously we should be specialising the current default blender mode
> as well.  Can you make that as a separate change first?  Then your
> patch should be okay (with style/docs fixups).
>
Ok sure I can make _al_blend_add_alpha_inversealpha_inline or something
but that would be a new blender. That is I don't think there is a
blender optimized for ADD/ALPHA/INVERSE, all the calls from
scanline_drawers.c were just calling al_blend_inline.

It looks like scanline_drawers.c only calls either ADD/ALPHA/INVERSE and
ADD/ONE/ONE, so maybe I should make both specialized blenders?

(BTW, when I ran make_scanline_drawers.py the indentation got all messed
up, at least the generated file didn't look like src/scanline_drawers.c.
Was scanline_drawers.c manually fixed up after generation? In my own
python-generating-c++ things I tend to have an indent() function that
takes care of this stuff, I can try to add it to
make_scanline_drawers.py if you like..)