Two new benches for ETC1 bitmaps.

Two new benches for ETC1 bitmaps.

First bench: Simply render from a compressed ETC1 bitmap. This is roughly
equal between compressed and uncompressed data.

Second bench: Notify the bitmap that the pixels have changed (that's right,
like a liar) to force a re-upload to the GPU. On this bench, decompressed
textures seem to inexplicably do an order of magnitude better than
compressed textures when it should be the other way around, but that
investigation is reserved for a future CL.

Committed: https://skia.googlesource.com/skia/+/9a3cdbb3d0957ad3f04d02277a4ce839752f4efd

[krajcevski, 2014-06-04 18:21:31]

I updated the comments.

[robertphillips, 2014-06-04 18:53:36]

https://codereview.chromium.org/317023002/diff/20001/bench/ETCBitmapBench.cpp
File bench/ETCBitmapBench.cpp (right):

https://codereview.chromium.org/317023002/diff/20001/bench/ETCBitmapBench.cpp...
bench/ETCBitmapBench.cpp:1: /*
2014 ?

https://codereview.chromium.org/317023002/diff/20001/bench/ETCBitmapBench.cpp...
bench/ETCBitmapBench.cpp:8: #include "SkBenchmark.h"
Do we need SkBitmapDevice.h ?

https://codereview.chromium.org/317023002/diff/20001/bench/ETCBitmapBench.cpp...
bench/ETCBitmapBench.cpp:39: // The picture must be at least as large as a
block.
Is the block width & height already #defined somewhere ?

https://codereview.chromium.org/317023002/diff/20001/bench/ETCBitmapBench.cpp...
bench/ETCBitmapBench.cpp:64: for(etc1_uint32 i = 0; i < newWidth; i +=
origWidth) {
rename rowStart to be srcRow and move it out of the inner loop?

https://codereview.chromium.org/317023002/diff/20001/bench/ETCBitmapBench.cpp...
bench/ETCBitmapBench.cpp:78: public:
Don't think you need explicit here.
this->loadPKM() ?

https://codereview.chromium.org/317023002/diff/20001/bench/ETCBitmapBench.cpp...
bench/ETCBitmapBench.cpp:99: 
static const etc1_uint32 kExpansionFactor = 8; ?

https://codereview.chromium.org/317023002/diff/20001/bench/ETCBitmapBench.cpp...
bench/ETCBitmapBench.cpp:131: 
I think we should actually parameterize the bench based on whether it should be
gpu or raster only.

https://codereview.chromium.org/317023002/diff/20001/src/gpu/SkGr.cpp
File src/gpu/SkGr.cpp (right):

https://codereview.chromium.org/317023002/diff/20001/src/gpu/SkGr.cpp#newcode248
src/gpu/SkGr.cpp:248: &&
ctx->getGpu()->caps()->isConfigTexturable(kETC1_GrPixelConfig)
What is this fixing?

[krajcevski, 2014-06-04 19:26:52]

https://codereview.chromium.org/317023002/diff/20001/bench/ETCBitmapBench.cpp
File bench/ETCBitmapBench.cpp (right):

https://codereview.chromium.org/317023002/diff/20001/bench/ETCBitmapBench.cpp...
bench/ETCBitmapBench.cpp:1: /*
On 2014/06/04 18:53:36, robertphillips wrote:
> 2014 ?

Done.

https://codereview.chromium.org/317023002/diff/20001/bench/ETCBitmapBench.cpp...
bench/ETCBitmapBench.cpp:8: #include "SkBenchmark.h"
On 2014/06/04 18:53:36, robertphillips wrote:
> Do we need SkBitmapDevice.h ?

Nope. Removed.

https://codereview.chromium.org/317023002/diff/20001/bench/ETCBitmapBench.cpp...
bench/ETCBitmapBench.cpp:39: // The picture must be at least as large as a
block.
On 2014/06/04 18:53:36, robertphillips wrote:
> Is the block width & height already #defined somewhere ?

No, but "everyone knows" etc1 data is 4x4 pixel blocks. Should I put a #define
in etc.h?

https://codereview.chromium.org/317023002/diff/20001/bench/ETCBitmapBench.cpp...
bench/ETCBitmapBench.cpp:64: for(etc1_uint32 i = 0; i < newWidth; i +=
origWidth) {
On 2014/06/04 18:53:36, robertphillips wrote:
> rename rowStart to be srcRow and move it out of the inner loop?

Done.

https://codereview.chromium.org/317023002/diff/20001/bench/ETCBitmapBench.cpp...
bench/ETCBitmapBench.cpp:78: public:
On 2014/06/04 18:53:36, robertphillips wrote:
> Don't think you need explicit here.
> this->loadPKM() ?

Done.

https://codereview.chromium.org/317023002/diff/20001/bench/ETCBitmapBench.cpp...
bench/ETCBitmapBench.cpp:99: 
On 2014/06/04 18:53:36, robertphillips wrote:
> static const etc1_uint32 kExpansionFactor = 8; ?

Done.

https://codereview.chromium.org/317023002/diff/20001/bench/ETCBitmapBench.cpp...
bench/ETCBitmapBench.cpp:131: 
On 2014/06/04 18:53:36, robertphillips wrote:
> I think we should actually parameterize the bench based on whether it should
be
> gpu or raster only.

Done.

https://codereview.chromium.org/317023002/diff/20001/src/gpu/SkGr.cpp
File src/gpu/SkGr.cpp (right):

https://codereview.chromium.org/317023002/diff/20001/src/gpu/SkGr.cpp#newcode248
src/gpu/SkGr.cpp:248: &&
ctx->getGpu()->caps()->isConfigTexturable(kETC1_GrPixelConfig)
On 2014/06/04 18:53:36, robertphillips wrote:
> What is this fixing?

If the bitmap had compressed data and was then uncompressed, it'll still return
compressed data on 'refEncodedData' and upload it. Probably not good, since if
the bitmap has available pixels, then they might not be what the decompressed
data is.

[robertphillips, 2014-06-04 19:46:37]

https://codereview.chromium.org/317023002/diff/20001/bench/ETCBitmapBench.cpp
File bench/ETCBitmapBench.cpp (right):

https://codereview.chromium.org/317023002/diff/20001/bench/ETCBitmapBench.cpp...
bench/ETCBitmapBench.cpp:39: // The picture must be at least as large as a
block.
Probably just put a "static const int kETC1BlockSize = 4;" here somewhere.
Maybe:
kETC1BlockWidthInPixels ? kETC1BlockHeightInPixels ?

https://codereview.chromium.org/317023002/diff/20001/src/gpu/SkGr.cpp
File src/gpu/SkGr.cpp (right):

https://codereview.chromium.org/317023002/diff/20001/src/gpu/SkGr.cpp#newcode248
src/gpu/SkGr.cpp:248: &&
ctx->getGpu()->caps()->isConfigTexturable(kETC1_GrPixelConfig)
That needs a comment!

https://codereview.chromium.org/317023002/diff/40001/bench/ETCBitmapBench.cpp
File bench/ETCBitmapBench.cpp (right):

https://codereview.chromium.org/317023002/diff/40001/bench/ETCBitmapBench.cpp...
bench/ETCBitmapBench.cpp:164: virtual void onDraw(const int loops, SkCanvas*
canvas) SK_OVERRIDE {
++i ?

https://codereview.chromium.org/317023002/diff/40001/bench/ETCBitmapBench.cpp...
bench/ETCBitmapBench.cpp:206: virtual void onDraw(const int loops, SkCanvas*
canvas) SK_OVERRIDE {
++i ?

[robertphillips, 2014-06-04 19:46:48]

lgtm + nits

[krajcevski, 2014-06-04 19:57:16]

https://codereview.chromium.org/317023002/diff/20001/bench/ETCBitmapBench.cpp
File bench/ETCBitmapBench.cpp (right):

https://codereview.chromium.org/317023002/diff/20001/bench/ETCBitmapBench.cpp...
bench/ETCBitmapBench.cpp:39: // The picture must be at least as large as a
block.
On 2014/06/04 19:46:37, robertphillips wrote:
> Probably just put a "static const int kETC1BlockSize = 4;" here somewhere.
> Maybe:
> kETC1BlockWidthInPixels ? kETC1BlockHeightInPixels ?

Done.

https://codereview.chromium.org/317023002/diff/20001/src/gpu/SkGr.cpp
File src/gpu/SkGr.cpp (right):

https://codereview.chromium.org/317023002/diff/20001/src/gpu/SkGr.cpp#newcode248
src/gpu/SkGr.cpp:248: &&
ctx->getGpu()->caps()->isConfigTexturable(kETC1_GrPixelConfig)
On 2014/06/04 19:46:37, robertphillips wrote:
> That needs a comment!

Done.

https://codereview.chromium.org/317023002/diff/40001/bench/ETCBitmapBench.cpp
File bench/ETCBitmapBench.cpp (right):

https://codereview.chromium.org/317023002/diff/40001/bench/ETCBitmapBench.cpp...
bench/ETCBitmapBench.cpp:164: virtual void onDraw(const int loops, SkCanvas*
canvas) SK_OVERRIDE {
On 2014/06/04 19:46:37, robertphillips wrote:
> ++i ?

Done.

https://codereview.chromium.org/317023002/diff/40001/bench/ETCBitmapBench.cpp...
bench/ETCBitmapBench.cpp:206: virtual void onDraw(const int loops, SkCanvas*
canvas) SK_OVERRIDE {
On 2014/06/04 19:46:37, robertphillips wrote:
> ++i ?

Done.

[krajcevski, 2014-06-04 20:27:50]

The CQ bit was checked by krajcevski@google.com

[commit-bot: I haz the power, 2014-06-04 20:28:41]

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

[commit-bot: I haz the power, 2014-06-04 20:49:26]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  Test-Ubuntu13.10-ShuttleA-NoGPU-x86_64-Debug-Trybot on tryserver.chromium
(http://108.170.220.120:10117/builders/Test-Ubuntu13.10-ShuttleA-NoGPU-x86_64-...)

[commit-bot: I haz the power, 2014-06-04 21:00:26]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-06-04 21:00:27]

Try jobs failed on following builders:
  Test-Ubuntu13.10-ShuttleA-NoGPU-x86_64-Debug-Trybot on tryserver.chromium
(http://108.170.220.120:10117/builders/Test-Ubuntu13.10-ShuttleA-NoGPU-x86_64-...)

[krajcevski, 2014-06-05 13:52:06]

The CQ bit was checked by krajcevski@google.com

[commit-bot: I haz the power, 2014-06-05 13:53:55]

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

[commit-bot: I haz the power, 2014-06-05 14:03:44]

Change committed as 9a3cdbb3d0957ad3f04d02277a4ce839752f4efd