SSE2 optimization for blitmask

this improves the skia performance on SSE2-supporting platform. (please refer to below issues)

Issue 2765043: SSE2 optimizations for 32bit Color operation
Issue 171055: More SSE2ification
Issue 157141: More SSE2ification
Issue 150060: minor tweaks to SSE2 code for -fPIC
Issue 144072: SSE2 optimizations for 32bit blending blitters


This CL implements SSE2 optimizations for the ARGB32 blitmask. Like above issues, it uses CPUID to detect for SSE2 and changes the platform procs at
runtime as well. 

blitmask is very common operations for blend source image to a destination background. With the workload http://www.feedtank.com/labs/html_canvas/, SkARGB32_Blitter::blitMask() occupies (10-12)% cpu cycles of chrome (the performance data is from oprofile in Meego).

this patch just focus on SkARGB32_Blitter, but it is extendable to other Blitter.

Additional, this CL has passed the skia bench & tests validation, the result is pretty good. We also apply this CL to the latest chromium, and re-run http://www.feedtank.com/labs/html_canvas/, the performance is improved by almost 6~8%.

[reed1, 2011-01-17 16:13:39]

trying out the patch...

[reed1, 2011-01-17 18:29:28]

A good start, but a few issues.

1) need a more complete framework, to handle
- black and opaque fast cases
- 565 device config
- shader input (instead of just a color)

2) at the moment, the code is slightly slower for black text than the portable
code for body-size text (e.g. 9pt). Possibly easy to fix with special cases for
opaque and black input color.

Lets keep the CL in the wings. I think having an opts extension for mask-blits
is a good idea, just need a little more infrastructure (see #1) in place before
we land this (and we need to extend the bench-tool to explicitly exercise the
aformentioned cases [black, opaque, color, shader].

[yaojie.yan, 2011-01-19 03:44:36]

thanks for your good comments, and I am not familiar with Skia design mechanism,
so please forgive me to ask some more trivial questions.

1. to impelement a complete opt framework to handle [black, opaque, color,
shader] blitmask, Is it ok to define Factory32() or Factory() to return the
corresponding blit functions.

class SkBlitMask {
public:
    enum kBlitMask_Ops {
        kBlack_Flag 	= 0x00, //black
        kOpaque_Flag   	= 0x01, // opaque
        kARGB32_Flag      = 0x02, // color
        KShader_Flag	= 0x03  // shader
    };
    typedef void (*Proc)(SkPMColor* dst, 
			          const SkPMColor src,
			          const uint8_t* alpha,
			          int width, int height, 
			          int maskRB, int devRB);
    // to get different mask operators
    static Proc Factory(unsigned flags, SkBitmap::Config);
    static Proc Factory32(unsigned flags, SkBitmap::Config config =
SkBitmap::kARGB_8888_Config);
    // for different color space
    static Proc PlatformProcs32(unsigned flags);
    static Proc PlatformProcs565(unsigned flags);
    static Proc PlatformProcs4444(unsigned flags);
private:
    enum {
        kFlags16_Mask = 7,
        kFlags32_Mask = 3
    };
};

in SkBlitRow_D32.cpp

// add the portable blit functions.
static const SkBlitMask::Proc SkBlitMask::gDefault_Procs32[] = {
    NULL, // TODO: for Black_BlitMask_portable
    NULL, // TODO: for Opaque_BlitMask_portable
    SkBlitMask::SkARGB32_BlitMask_portable,
    NULL //TODO: for Shader_BlitMask_portable
};

and in SkBlitRow_opts_XXX.cpp
// add the opt blit functions
static const SkBlitMask::Proc platform_32_procs[] = {
    NULL,   // S32_Black,
    NULL,   // S32_Opaque,
    NULL,   // S32A_Color,
    NULL    // S32A_Shader
}

2. to handle 565 device config. Above changes will be duplicated to
SkBlitRow_D16.cpp.

3. regarding to slightly slower performance for black text, do you mean to
bypass the blitmask opt for such special case? 
4. and the bench-tool, I will follow up after completing the framework.

[yaojie.yan, 2011-01-27 07:38:05]

updated version to implement a completed framework for blitmask

[yaojie.yan, 2011-01-27 07:40:43]

Hi Reed,
Thanks a lot for your comments! Could you please review this updated patch,
which implements a completed framework for all kinds of blitmask[opaque, black,
color and shader] in both 32 bits color and 16 bits color. 

For your comments [2], there seems no clear clue that the performance would
degrade for opaque and black input color, because flow exactly follows the old
logic before my optimization. From my investigation, it will follow a shortcut
by calling SkARGB32_BlitBW() to avoid the framework overhead if the format is
kBW_Format, as illustrated by the snippet below. Would you please share the
special test cases exposing the issue you pointed out so that I can look into
that? 

void SkARGB32_Blitter::blitMask(const SkMask& mask, 
                                const SkIRect& clip) 
{ ...
    if (mask.fFormat == SkMask::kBW_Format) 
    {
        SkARGB32_BlendBW(fDevice, mask, clip, fPMColor, 
                         SkAlpha255To256(255 - fSrcA));
        return;
    }
...
}
  
This patch only provides the framework and the SSE2 optimizing for ARGB32
blitmask. Other blitmask types are currently leaving as blank entries and may be
filled up by later on patches, if you showed OK with this framework.
 
The patch also extends the bench tool to cover blitmask. 
Look forward to your further comments!

Thanks
Yaojie

[reed1, 2011-02-24 16:09:47]

I think we don't need/want to follow BlitRow's pattern of Proc and Proc32. That
is just legacy, as initially I only thought about a 16bit destination.

I propose the Proc definition to look more like this:


/**
 *  Function pointer that blits the mask into a device (dst) colorized
 *  by color. The number of pixels to blit is specified by width and height,
 *  but each scanline is offset by dstRB (rowbytes) and srcRB respectively.
 */
typedef void (*Proc)(void* dst, size_t dstRB, SkBitmap::Config dstConfig,
                     const uint8_t* mask, size_t maskRB, SkColor color,
                     int width, int height);

For simplicity and generality, I propose we just give the color itself to the
Factory, eliminating the need for flags.

static Proc Factory(SkBitmap::Config dstConfig, SkColor color);

I would have expected the PlatfromProc function to take the same parameters as
Factory. Is there a reason for the difference?

To better match the call pattern with BlitRow, can we supply default
implementations for the Factory, rather than returning NULL?

[reed1, 2011-02-24 16:11:08]

ah, I see that you have TODOs for the other default implementations.

[yaojie.yan, 2011-03-02 09:13:09]

Hi Reed,
According to your comments, some changes are made in this version. It has been
tested and verified by skia benchtool and workload
http://www.feedtank.com/labs/html_canvas/, the performance gain is almost 8%
when mask size is 15*15.

Thanks
Yaojie

[yaojie.yan, 2011-03-02 09:16:05]

new patch for 2nd review

[yaojie.yan, 2011-03-07 09:47:30]

upload a new patch diff from the recent code base, thanks

[yaojie.yan, 2011-03-09 09:18:45]

Minor changes to pass "full" rowBytes to the proc. Please provide your comments,
thanks Reed

[reed1, 2011-03-18 21:04:34]

This has been committed.

http://codereview.appspot.com/3980041/diff/12001/include/core/SkBlitRow.h
File include/core/SkBlitRow.h (right):

http://codereview.appspot.com/3980041/diff/12001/include/core/SkBlitRow.h#new...
include/core/SkBlitRow.h:91: *  but each scanline is offset by dstRB (rowbytes)
and srcRB respectively.
The dstRB and maskRB should both be complete values, meaning they are the true
distance between rows. It looks like from SkBlitter_ARGB32.cpp, that you are
passing in the "leftover" amounts after taking into account the width. I know
that is what my original code did, but I think this new interface should not
specify that. The implementation can always perform that substract before its
loop.

Lets document this here, and update the call site and impls. (should be trivial)