Buffer four scanlines prior to blitting. We will eventually use this

Buffer four scanlines prior to blitting. We will eventually use this
to blit four scanlines in a row in order to aggressively compress alpha
masks as the information comes in. This CL is only to introduce the API
and to make sure that nothing breaks in the general case.

Committed: https://skia.googlesource.com/skia/+/2ec93fc1d3206db4dcf74ccfc1c995badbc135e9

[robertphillips, 2014-07-14 15:06:29]

I defer to Mike on this one.

[reed1, 2014-07-14 21:31:46]

Hmm, that seems like a fair amount of complexity to place on the core
scan-converter.

I had imagined spreading out the complexity, so that we pay for most of it only
in the 1 case where we want it. Something like:

core-code: allocate 4x buffer, and round-robin through the buffer pointers on
each scanline.

special-blitter: record the ptrs that it receives and only process 4 of them
every 4th call (or on the final flush).

Do you think the above could be done?

[krajcevski, 2014-07-14 21:40:16]

On 2014/07/14 21:31:46, reed1 wrote:
> Hmm, that seems like a fair amount of complexity to place on the core
> scan-converter.
> 
> I had imagined spreading out the complexity, so that we pay for most of it
only
> in the 1 case where we want it. Something like:
> 
> core-code: allocate 4x buffer, and round-robin through the buffer pointers on
> each scanline.
> 
> special-blitter: record the ptrs that it receives and only process 4 of them
> every 4th call (or on the final flush).
> 
> Do you think the above could be done?

Yea, that's totally doable.

The "classical" compression formats operate on 4x4 blocks, but some newer ones
can go as high as 12x12. Do you want to hardcode the '4' in 4x buffer
allocation? It might be good to have a function of the form:

SkBlitter::getPreferredNumRows()

This will return 1 in the general case, but for special texture compression
blitters it can return the number of rows it can handle per scanline?

[reed1, 2014-07-15 13:38:12]

On 2014/07/14 21:40:16, krajcevski wrote:
> On 2014/07/14 21:31:46, reed1 wrote:
> > Hmm, that seems like a fair amount of complexity to place on the core
> > scan-converter.
> > 
> > I had imagined spreading out the complexity, so that we pay for most of it
> only
> > in the 1 case where we want it. Something like:
> > 
> > core-code: allocate 4x buffer, and round-robin through the buffer pointers
on
> > each scanline.
> > 
> > special-blitter: record the ptrs that it receives and only process 4 of them
> > every 4th call (or on the final flush).
> > 
> > Do you think the above could be done?
> 
> Yea, that's totally doable.
> 
> The "classical" compression formats operate on 4x4 blocks, but some newer ones
> can go as high as 12x12. Do you want to hardcode the '4' in 4x buffer
> allocation? It might be good to have a function of the form:
> 
> SkBlitter::getPreferredNumRows()
> 
> This will return 1 in the general case, but for special texture compression
> blitters it can return the number of rows it can handle per scanline?

Sure, add a virtual to return the multiplier for #rows

[krajcevski, 2014-07-15 20:20:21]

Should be updated with the changes you suggested.

[reed1, 2014-07-15 21:02:58]

That looks very manageable. lgtm

[robertphillips, 2014-07-16 12:33:11]

lgtm + comment suggestions

https://codereview.chromium.org/387953005/diff/80001/src/core/SkScan_AntiPath...
File src/core/SkScan_AntiPath.cpp (right):

https://codereview.chromium.org/387953005/diff/80001/src/core/SkScan_AntiPath...
src/core/SkScan_AntiPath.cpp:125: private:
Move this comment down into getRunsSz ?

https://codereview.chromium.org/387953005/diff/80001/src/core/SkScan_AntiPath...
src/core/SkScan_AntiPath.cpp:126: // extra one to store the zero at the end
// The next 3 variables implement a circular buffer of the last 'n' runs. It is
used by the compression code to buffer up a block of runs to compress together ?

https://codereview.chromium.org/387953005/diff/80001/src/core/SkScan_AntiPath...
src/core/SkScan_AntiPath.cpp:133: 
// This updates 'fRuns' to point to the current buffered run in 'fRunsBuffer' ?

https://codereview.chromium.org/387953005/diff/80001/src/core/SkScan_AntiPath...
src/core/SkScan_AntiPath.cpp:162: if (!fRuns.empty()) {
Usually we put the ';' inside the ')' of the SkDEBUGCODE

[robertphillips, 2014-07-16 12:50:33]

https://codereview.chromium.org/387953005/diff/80001/src/core/SkScan_AntiPath...
File src/core/SkScan_AntiPath.cpp (right):

https://codereview.chromium.org/387953005/diff/80001/src/core/SkScan_AntiPath...
src/core/SkScan_AntiPath.cpp:127: int         fRunsToBuffer;
void* ?

[krajcevski, 2014-07-16 16:36:03]

https://codereview.chromium.org/387953005/diff/80001/src/core/SkScan_AntiPath...
File src/core/SkScan_AntiPath.cpp (right):

https://codereview.chromium.org/387953005/diff/80001/src/core/SkScan_AntiPath...
src/core/SkScan_AntiPath.cpp:125: private:
On 2014/07/16 12:33:11, robertphillips wrote:
> Move this comment down into getRunsSz ?

Done.

https://codereview.chromium.org/387953005/diff/80001/src/core/SkScan_AntiPath...
src/core/SkScan_AntiPath.cpp:126: // extra one to store the zero at the end
On 2014/07/16 12:33:11, robertphillips wrote:
> // The next 3 variables implement a circular buffer of the last 'n' runs. It
is
> used by the compression code to buffer up a block of runs to compress together
?

Done.

https://codereview.chromium.org/387953005/diff/80001/src/core/SkScan_AntiPath...
src/core/SkScan_AntiPath.cpp:127: int         fRunsToBuffer;
On 2014/07/16 12:50:33, robertphillips wrote:
> void* ?

Done.

https://codereview.chromium.org/387953005/diff/80001/src/core/SkScan_AntiPath...
src/core/SkScan_AntiPath.cpp:133: 
On 2014/07/16 12:33:11, robertphillips wrote:
> // This updates 'fRuns' to point to the current buffered run in 'fRunsBuffer'
?

Done.

https://codereview.chromium.org/387953005/diff/80001/src/core/SkScan_AntiPath...
src/core/SkScan_AntiPath.cpp:162: if (!fRuns.empty()) {
On 2014/07/16 12:33:11, robertphillips wrote:
> Usually we put the ';' inside the ')' of the SkDEBUGCODE

Done.

[krajcevski, 2014-07-16 16:36:06]

The CQ bit was checked by krajcevski@google.com

[commit-bot: I haz the power, 2014-07-16 16:37:11]

CQ is trying da patch. Follow status at
 https://skia-tree-status.appspot.com/cq/krajcevski@google.com/387953005/100001

[commit-bot: I haz the power, 2014-07-16 20:31:44]

Change committed as 2ec93fc1d3206db4dcf74ccfc1c995badbc135e9