Modify sample buffer size for larger displays.

Modify sample buffer size for larger displays.

Increases the intermediate buffer size for sample pixel indexes,
used in the sample proc function calls. If the operation is bigger
than the buffer it's split into multiple calls, creating overhead.
This would especially impact the performance of SIMD optimizations.
Also, aligns the start address of the buffer to 16 bytes, to enable
more efficient SIMD optimizations.

Author: henrik.smiding@intel.com

Signed-off-by: Henrik Smiding <henrik.smiding@intel.com>;

Committed: http://code.google.com/p/skia/source/detail?r=14825

Committed: http://code.google.com/p/skia/source/detail?r=14872

[tomhudson, 2014-04-17 13:42:50]

We at least need Derek, and maybe somebody deeper in Android, to verify that
ANDROID_LARGE_MEMORY_DEVICE is expected to be true for large screen.

Instead of making that kind of assumption, why did you not set BUF_MAX to 1024?
It's stack space, not a heap allocation, so it ought not be too expensive?

What clients do we have that could make use of this optimization? Chrome on
Android uses tiled rendering, and desktop Chrome is either there or shifting
there as far as I know, so I wouldn't expect to see large spans. (Although,
ironically, a 256px tile width means we need 129 words in the buffer, and we
only have 128, don't we? So doing something along this line sounds really good.)

[henrik.smiding, 2014-04-18 17:33:49]

On 2014/04/17 13:42:50, tomhudson wrote:
> We at least need Derek, and maybe somebody deeper in Android, to verify that
> ANDROID_LARGE_MEMORY_DEVICE is expected to be true for large screen.
> 
> Instead of making that kind of assumption, why did you not set BUF_MAX to
1024?
> It's stack space, not a heap allocation, so it ought not be too expensive?
> 
> What clients do we have that could make use of this optimization? Chrome on
> Android uses tiled rendering, and desktop Chrome is either there or shifting
> there as far as I know, so I wouldn't expect to see large spans. (Although,
> ironically, a 256px tile width means we need 129 words in the buffer, and we
> only have 128, don't we? So doing something along this line sounds really
good.)

The only reason I used the ANDROID_LARGE_MEMORY_DEVICE flag was to keep memory
footprint in check and reduce the impact on other platforms than Android.
I would gladly remove the flag check and set the buffer size to something
higher, like 541 or 961 (1920 width), if the memory is not that big of an issue.

It's true that Chrome might not benefit a lot from a large buffer anymore, but
there are still legacy applications/benchmarks in Android that should benefit
from it. Also, applications using the 'DXDY' functions will put even greater
demand on the buffer size, compared to the 'DX-only' functions that I used in my
example.

Are there any additional reviewers that you could recommend for this patch?

[tomhudson, 2014-04-22 09:49:29]

Derek or Leon (+scroggo@) can sign off on Android repercussions or will know if
we should ask somebody else from core Android to review.

[scroggo, 2014-04-22 15:18:40]

On 2014/04/17 13:42:50, tomhudson wrote:
> We at least need Derek, and maybe somebody deeper in Android, to verify that
> ANDROID_LARGE_MEMORY_DEVICE is expected to be true for large screen.

We define ANDROID_LARGE_MEMORY_DEVICE for all arm devices with a VFP, and all
non-arm devices (which I think may be all current Android devices).

https://codereview.chromium.org/240433002/diff/1/src/core/SkBitmapProcShader.cpp
File src/core/SkBitmapProcShader.cpp (right):

https://codereview.chromium.org/240433002/diff/1/src/core/SkBitmapProcShader....
src/core/SkBitmapProcShader.cpp:239: __declspec(align(16)) uint32_t
buffer[BUF_MAX + TEST_BUFFER_EXTRA];
Is there a way to turn this into a macro? Or does the different order for
align(ed) and uint32_t prevent that?

[henrik.smiding, 2014-04-23 13:34:46]

https://codereview.chromium.org/240433002/diff/1/src/core/SkBitmapProcShader.cpp
File src/core/SkBitmapProcShader.cpp (right):

https://codereview.chromium.org/240433002/diff/1/src/core/SkBitmapProcShader....
src/core/SkBitmapProcShader.cpp:239: __declspec(align(16)) uint32_t
buffer[BUF_MAX + TEST_BUFFER_EXTRA];
On 2014/04/22 15:18:40, scroggo wrote:
> Is there a way to turn this into a macro? Or does the different order for
> align(ed) and uint32_t prevent that?

Done.

[reed1, 2014-04-23 14:38:25]

I'm ok boosting the 128 to something larger.

1. Why not just always make it larger (no need for the #if test)?
2. What is the measured speed gain w/ and w/o this patch
3. Do we see a win on ARM and INTEL?

[djsollen, 2014-04-23 14:40:53]

https://codereview.chromium.org/240433002/diff/20001/src/core/SkBitmapProcSha...
File src/core/SkBitmapProcShader.cpp (right):

https://codereview.chromium.org/240433002/diff/20001/src/core/SkBitmapProcSha...
src/core/SkBitmapProcShader.cpp:214: #ifdef ANDROID_LARGE_MEMORY_DEVICE
my goal is to remove ANDROID_LARGE_MEMORY_DEVICE so I'm in agreement with Mike
that we should just go with the larger number.

[henrik.smiding, 2014-04-23 15:20:21]

https://codereview.chromium.org/240433002/diff/20001/src/core/SkBitmapProcSha...
File src/core/SkBitmapProcShader.cpp (right):

https://codereview.chromium.org/240433002/diff/20001/src/core/SkBitmapProcSha...
src/core/SkBitmapProcShader.cpp:214: #ifdef ANDROID_LARGE_MEMORY_DEVICE
On 2014/04/23 14:40:53, djsollen wrote:
> my goal is to remove ANDROID_LARGE_MEMORY_DEVICE so I'm in agreement with Mike
> that we should just go with the larger number.

Done.

[henrik.smiding, 2014-04-25 16:35:37]

On 2014/04/23 14:38:25, reed1 wrote:
> I'm ok boosting the 128 to something larger.
> 
> 1. Why not just always make it larger (no need for the #if test)?
> 2. What is the measured speed gain w/ and w/o this patch
> 3. Do we see a win on ARM and INTEL?

I modified the Bitmap benchmark to render using larger source bitmaps. With an
average width of ~500 pixels and the buffer size set to 1081, tests like
bitmap_8888_update_scale_rotate_bilerp,
bitmap_8888_update_volatile_scale_rotate_bilerp,
bitmap_8888_scale_rotate_bilerp, and others, showed a 4% performance increase on
ARGB8888, and 2.2% on RGB565 (which is not SIMD optimized).
This was using a 'affine/perspective filter' proc which ran with an even 8
pixels per iteration (64 pixels, using org. buffer size of 128). A
'scale/translate filter' proc should show even higher performance increase,
since it would only run 127 pixels per iteration using the org. buffer size.
This means that the SIMD opt. would probably have to fall back to doing only one
pixel at a time at the end.
Since ARM has more SIMD optimizations using this buffer, the effect should be
even more noticeable.

[tomhudson, 2014-04-29 11:39:24]

Firing off some test trybots, and if those come back then performance trybots;
if those are good, any reason not to land?

[djsollen, 2014-04-29 12:09:48]

if those bots come back green I'm fine landing the change.

[tomhudson, 2014-04-29 12:13:03]

Henrik: there are some mismatches on Linux that we'd have to work on, but
Windows compile is failing completely.

[04:40:47.595000]
c:\0\test-win7-shuttlea-hd2000-x86-debug-trybot\build\skia\src\core\skbitmapprocshader.cpp(237)
: error C3861: 'aligned': identifier not found

The "#if defined(WIN32)" doesn't seem to be working.

Try using SK_BUILD_FOR_WIN, which is set up at the top of SkPostConfig.h?

[henrik.smiding, 2014-04-30 08:45:00]

On 2014/04/29 12:13:03, tomhudson wrote:
> Henrik: there are some mismatches on Linux that we'd have to work on, but
> Windows compile is failing completely.
> 
> [04:40:47.595000]
>
c:\0\test-win7-shuttlea-hd2000-x86-debug-trybot\build\skia\src\core\skbitmapprocshader.cpp(237)
> : error C3861: 'aligned': identifier not found
> 
> The "#if defined(WIN32)" doesn't seem to be working.
> 
> Try using SK_BUILD_FOR_WIN, which is set up at the top of SkPostConfig.h?

I have pushed a patch that looks at the compiler version, to determine if it's
MS VS or gcc/clang.
It's used for similar things at other locations in SkPostConfig.

I have been unable to trigger the trybots, though. I've tried both submit_try
script and via the UI. I'm guessing it's a problem with access right.

[henrik.smiding, 2014-05-06 13:17:34]

On 2014/04/30 08:45:00, henrik.smiding wrote:
> On 2014/04/29 12:13:03, tomhudson wrote:
> > Henrik: there are some mismatches on Linux that we'd have to work on, but
> > Windows compile is failing completely.
> > 
> > [04:40:47.595000]
> >
>
c:\0\test-win7-shuttlea-hd2000-x86-debug-trybot\build\skia\src\core\skbitmapprocshader.cpp(237)
> > : error C3861: 'aligned': identifier not found
> > 
> > The "#if defined(WIN32)" doesn't seem to be working.
> > 
> > Try using SK_BUILD_FOR_WIN, which is set up at the top of SkPostConfig.h?
> 
> I have pushed a patch that looks at the compiler version, to determine if it's
> MS VS or gcc/clang.
> It's used for similar things at other locations in SkPostConfig.
> 
> I have been unable to trigger the trybots, though. I've tried both submit_try
> script and via the UI. I'm guessing it's a problem with access right.

It looks like the build for Win7/VS2010 passed.
What's the next step? The mismatches in the gm tests?

[tomhudson, 2014-05-07 10:19:44]

On 2014/05/06 13:17:34, henrik.smiding wrote:
> On 2014/04/30 08:45:00, henrik.smiding wrote:
> > I have pushed a patch that looks at the compiler version, to determine if
it's
> > MS VS or gcc/clang.
> > It's used for similar things at other locations in SkPostConfig.

I dont' know that it's future-proof, but since it's used throughout
SkPostConfig, you at least aren't introducing any new problems. :)

> It looks like the build for Win7/VS2010 passed.
> What's the next step? The mismatches in the gm tests?

Yep. Somebody from core Skia will have to chime in there for the current best
practice; the infrastructure has hugely improved since I was last in that
office. Leon? Derek? Mikes?

[henrik.smiding, 2014-05-12 11:53:41]

Ping, anyone?
Could the gm base references be automatically regenerated?

[epoger, 2014-05-14 15:08:32]

On 2014/05/12 11:53:41, henrik.smiding wrote:
> Ping, anyone?
> Could the gm base references be automatically regenerated?

Looking at
http://108.170.220.120:10117/builders/Test-Mac10.7-MacMini4.1-GeForce320M-x86...
(one of the trybot results from patchset 5), it looks like the change broke a
subset of the GMs, not most of them:

[*] 56 ExpectationsMismatch: drawbitmapmatrix_565.png
downsamplebitmap_image_none_mandrill_512.png_8888.png  ...

That being the case, your best bet is to follow the directions in
https://github.com/google/skia/blob/master/expectations/gm/ignored-tests.txt .

Which Googler is willing to put his name on the lines that Henrik will add to
the ignored-tests.txt file, to follow up on rebaselines once the CL is
committed?

[scroggo, 2014-05-14 15:26:56]

On 2014/05/14 15:08:32, epoger wrote:
> On 2014/05/12 11:53:41, henrik.smiding wrote:
> > Ping, anyone?
> > Could the gm base references be automatically regenerated?
> 
> Looking at
>
http://108.170.220.120:10117/builders/Test-Mac10.7-MacMini4.1-GeForce320M-x86...
> (one of the trybot results from patchset 5), it looks like the change broke a
> subset of the GMs, not most of them:
> 
> [*] 56 ExpectationsMismatch: drawbitmapmatrix_565.png
> downsamplebitmap_image_none_mandrill_512.png_8888.png  ...
> 
> That being the case, your best bet is to follow the directions in
> https://github.com/google/skia/blob/master/expectations/gm/ignored-tests.txt .
> 
> Which Googler is willing to put his name on the lines that Henrik will add to
> the ignored-tests.txt file, to follow up on rebaselines once the CL is
> committed?

I'll do it. Henrik, please add a line to ignored-tests.txt to skip the failing
test(s),
and include my name (scroggo).

[henrik.smiding, 2014-05-15 13:47:44]

On 2014/05/14 15:26:56, scroggo wrote:
> On 2014/05/14 15:08:32, epoger wrote:
> > On 2014/05/12 11:53:41, henrik.smiding wrote:
> > > Ping, anyone?
> > > Could the gm base references be automatically regenerated?
> > 
> > Looking at
> >
>
http://108.170.220.120:10117/builders/Test-Mac10.7-MacMini4.1-GeForce320M-x86...
> > (one of the trybot results from patchset 5), it looks like the change broke
a
> > subset of the GMs, not most of them:
> > 
> > [*] 56 ExpectationsMismatch: drawbitmapmatrix_565.png
> > downsamplebitmap_image_none_mandrill_512.png_8888.png  ...
> > 
> > That being the case, your best bet is to follow the directions in
> > https://github.com/google/skia/blob/master/expectations/gm/ignored-tests.txt
.
> > 
> > Which Googler is willing to put his name on the lines that Henrik will add
to
> > the ignored-tests.txt file, to follow up on rebaselines once the CL is
> > committed?
> 
> I'll do it. Henrik, please add a line to ignored-tests.txt to skip the failing
> test(s),
> and include my name (scroggo).

Done.

[scroggo, 2014-05-15 14:04:43]

lgtm

[commit-bot: I haz the power, 2014-05-15 14:04:58]

CQ is trying da patch. Follow status at

https://skia-tree-status.appspot.com/cq/henrik.smiding@intel.com/240433002/14...

[commit-bot: I haz the power, 2014-05-15 14:05:02]

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

[commit-bot: I haz the power, 2014-05-15 14:05:02]

Presubmit check for 240433002-140001 failed and returned exit status 1.

Running presubmit commit checks ...

** Presubmit ERRORS **
Since the CL is editing public API, you must have an LGTM from one of:
('reed@chromium.org', 'reed@google.com', 'bsalomon@chromium.org',
'bsalomon@google.com')

[henrik.smiding, 2014-05-20 16:31:59]

On 2014/05/15 14:05:02, I haz the power (commit-bot) wrote:
> Presubmit check for 240433002-140001 failed and returned exit status 1.
> 
> Running presubmit commit checks ...
> 
> ** Presubmit ERRORS **
> Since the CL is editing public API, you must have an LGTM from one of:
> (mailto:, mailto:, mailto:,
> mailto:)

reed? bsalomon?

[bsalomon, 2014-05-20 17:25:53]

public header lgtm

[henrik.smiding, 2014-05-21 08:27:06]

The CQ bit was checked by henrik.smiding@intel.com

[commit-bot: I haz the power, 2014-05-21 08:27:26]

CQ is trying da patch. Follow status at

https://skia-tree-status.appspot.com/cq/henrik.smiding@intel.com/240433002/14...

[commit-bot: I haz the power, 2014-05-21 08:27:28]

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

[commit-bot: I haz the power, 2014-05-21 08:27:29]

Failed to apply patch for expectations/gm/ignored-tests.txt:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file expectations/gm/ignored-tests.txt
  Hunk #1 FAILED at 41.
  1 out of 1 hunk FAILED -- saving rejects to file
expectations/gm/ignored-tests.txt.rej

Patch:       expectations/gm/ignored-tests.txt
Index: expectations/gm/ignored-tests.txt
diff --git a/expectations/gm/ignored-tests.txt
b/expectations/gm/ignored-tests.txt
index
9afd2ef44e1e41b69a0fd54f0bd9bc9bba61a6e7..89790e1ee93bcd1e03eac0551829c96290a4a7b3
100644
--- a/expectations/gm/ignored-tests.txt
+++ b/expectations/gm/ignored-tests.txt
@@ -41,3 +41,62 @@ canvas-layer-state
 # bsalomon: https://codereview.chromium.org/264303008/
 # bsalomon@ will rebaseline this test
 ninepatch-stretch
+
+# https://codereview.chromium.org/240433002/
+# scroggo will rebaseline this test
+shadertext3_8888
+shadertext3_565
+pictureshader_8888
+pictureshader_565
+giantbitmap_mirror_bilerp_rotate_8888
+giantbitmap_mirror_bilerp_rotate_565
+giantbitmap_repeat_bilerp_rotate_8888
+giantbitmap_repeat_bilerp_rotate_565
+filterbitmap_image_mandrill_512.png_8888
+filterbitmap_image_mandrill_512.png_565
+filterbitmap_image_mandrill_256.png_8888
+filterbitmap_image_mandrill_256.png_565
+filterbitmap_image_mandrill_128.png_8888
+filterbitmap_image_mandrill_128.png_565
+filterbitmap_image_mandrill_64.png_8888
+filterbitmap_image_mandrill_64.png_565
+filterbitmap_image_mandrill_32.png_8888
+filterbitmap_image_mandrill_32.png_565
+filterbitmap_image_mandrill_16.png_8888
+filterbitmap_image_mandrill_16.png_565
+filterbitmap_checkerboard_192_192_8888
+filterbitmap_checkerboard_192_192_565
+filterbitmap_checkerboard_32_2_8888
+filterbitmap_checkerboard_32_2_565
+filterbitmap_checkerboard_32_8_8888
+filterbitmap_checkerboard_32_8_565
+filterbitmap_checkerboard_32_32_8888
+filterbitmap_checkerboard_32_32_565
+filterbitmap_checkerboard_4_4_8888
+filterbitmap_checkerboard_4_4_565
+filterbitmap_text_10.00pt_8888
+filterbitmap_text_10.00pt_565
+filterbitmap_text_7.00pt_8888
+filterbitmap_text_7.00pt_565
+filterbitmap_text_3.00pt_8888
+filterbitmap_text_3.00pt_565
+downsamplebitmap_image_none_mandrill_512.png_8888
+downsamplebitmap_image_none_mandrill_512.png_565
+downsamplebitmap_checkerboard_none_512_256_8888
+downsamplebitmap_checkerboard_none_512_256_565
+downsamplebitmap_text_none_72.00pt_8888
+downsamplebitmap_text_none_72.00pt_565
+downsamplebitmap_image_low_mandrill_512.png_8888
+downsamplebitmap_image_low_mandrill_512.png_565
+downsamplebitmap_checkerboard_low_512_256_8888
+downsamplebitmap_checkerboard_low_512_256_565
+downsamplebitmap_text_low_72.00pt_8888
+downsamplebitmap_text_low_72.00pt_565
+downsamplebitmap_image_medium_mandrill_512.png_8888
+downsamplebitmap_image_medium_mandrill_512.png_565
+downsamplebitmap_checkerboard_medium_512_256_8888
+downsamplebitmap_checkerboard_medium_512_256_565
+downsamplebitmap_text_medium_72.00pt_8888
+downsamplebitmap_text_medium_72.00pt_565
+drawbitmapmatrix_8888
+drawbitmapmatrix_565

[henrik.smiding, 2014-05-21 12:40:05]

On 2014/05/21 08:27:29, I haz the power (commit-bot) wrote:
> Failed to apply patch for expectations/gm/ignored-tests.txt:
> While running patch -p1 --forward --force --no-backup-if-mismatch;
>   patching file expectations/gm/ignored-tests.txt
>   Hunk #1 FAILED at 41.
>   1 out of 1 hunk FAILED -- saving rejects to file
> expectations/gm/ignored-tests.txt.rej
> 
> Patch:       expectations/gm/ignored-tests.txt
> Index: expectations/gm/ignored-tests.txt
> diff --git a/expectations/gm/ignored-tests.txt
> b/expectations/gm/ignored-tests.txt
> index
>
9afd2ef44e1e41b69a0fd54f0bd9bc9bba61a6e7..89790e1ee93bcd1e03eac0551829c96290a4a7b3
> 100644
> --- a/expectations/gm/ignored-tests.txt
> +++ b/expectations/gm/ignored-tests.txt
> @@ -41,3 +41,62 @@ canvas-layer-state
>  # bsalomon: https://codereview.chromium.org/264303008/
>  # bsalomon@ will rebaseline this test
>  ninepatch-stretch
> +
> +# https://codereview.chromium.org/240433002/
> +# scroggo will rebaseline this test
> +shadertext3_8888
> +shadertext3_565
> +pictureshader_8888
> +pictureshader_565
> +giantbitmap_mirror_bilerp_rotate_8888
> +giantbitmap_mirror_bilerp_rotate_565
> +giantbitmap_repeat_bilerp_rotate_8888
> +giantbitmap_repeat_bilerp_rotate_565
> +filterbitmap_image_mandrill_512.png_8888
> +filterbitmap_image_mandrill_512.png_565
> +filterbitmap_image_mandrill_256.png_8888
> +filterbitmap_image_mandrill_256.png_565
> +filterbitmap_image_mandrill_128.png_8888
> +filterbitmap_image_mandrill_128.png_565
> +filterbitmap_image_mandrill_64.png_8888
> +filterbitmap_image_mandrill_64.png_565
> +filterbitmap_image_mandrill_32.png_8888
> +filterbitmap_image_mandrill_32.png_565
> +filterbitmap_image_mandrill_16.png_8888
> +filterbitmap_image_mandrill_16.png_565
> +filterbitmap_checkerboard_192_192_8888
> +filterbitmap_checkerboard_192_192_565
> +filterbitmap_checkerboard_32_2_8888
> +filterbitmap_checkerboard_32_2_565
> +filterbitmap_checkerboard_32_8_8888
> +filterbitmap_checkerboard_32_8_565
> +filterbitmap_checkerboard_32_32_8888
> +filterbitmap_checkerboard_32_32_565
> +filterbitmap_checkerboard_4_4_8888
> +filterbitmap_checkerboard_4_4_565
> +filterbitmap_text_10.00pt_8888
> +filterbitmap_text_10.00pt_565
> +filterbitmap_text_7.00pt_8888
> +filterbitmap_text_7.00pt_565
> +filterbitmap_text_3.00pt_8888
> +filterbitmap_text_3.00pt_565
> +downsamplebitmap_image_none_mandrill_512.png_8888
> +downsamplebitmap_image_none_mandrill_512.png_565
> +downsamplebitmap_checkerboard_none_512_256_8888
> +downsamplebitmap_checkerboard_none_512_256_565
> +downsamplebitmap_text_none_72.00pt_8888
> +downsamplebitmap_text_none_72.00pt_565
> +downsamplebitmap_image_low_mandrill_512.png_8888
> +downsamplebitmap_image_low_mandrill_512.png_565
> +downsamplebitmap_checkerboard_low_512_256_8888
> +downsamplebitmap_checkerboard_low_512_256_565
> +downsamplebitmap_text_low_72.00pt_8888
> +downsamplebitmap_text_low_72.00pt_565
> +downsamplebitmap_image_medium_mandrill_512.png_8888
> +downsamplebitmap_image_medium_mandrill_512.png_565
> +downsamplebitmap_checkerboard_medium_512_256_8888
> +downsamplebitmap_checkerboard_medium_512_256_565
> +downsamplebitmap_text_medium_72.00pt_8888
> +downsamplebitmap_text_medium_72.00pt_565
> +drawbitmapmatrix_8888
> +drawbitmapmatrix_565

Rebased ignored-tests.txt...

[scroggo, 2014-05-21 13:39:49]

On 2014/05/21 12:40:05, henrik.smiding wrote:
> On 2014/05/21 08:27:29, I haz the power (commit-bot) wrote:
> > Failed to apply patch for expectations/gm/ignored-tests.txt:
> > While running patch -p1 --forward --force --no-backup-if-mismatch;
> >   patching file expectations/gm/ignored-tests.txt
> >   Hunk #1 FAILED at 41.
> >   1 out of 1 hunk FAILED -- saving rejects to file
> > expectations/gm/ignored-tests.txt.rej
> > 
> > Patch:       expectations/gm/ignored-tests.txt
> > Index: expectations/gm/ignored-tests.txt
> > diff --git a/expectations/gm/ignored-tests.txt
> > b/expectations/gm/ignored-tests.txt
> > index
> >
>
9afd2ef44e1e41b69a0fd54f0bd9bc9bba61a6e7..89790e1ee93bcd1e03eac0551829c96290a4a7b3
> > 100644
> > --- a/expectations/gm/ignored-tests.txt
> > +++ b/expectations/gm/ignored-tests.txt
> > @@ -41,3 +41,62 @@ canvas-layer-state
> >  # bsalomon: https://codereview.chromium.org/264303008/
> >  # bsalomon@ will rebaseline this test
> >  ninepatch-stretch
> > +
> > +# https://codereview.chromium.org/240433002/
> > +# scroggo will rebaseline this test
> > +shadertext3_8888
> > +shadertext3_565
> > +pictureshader_8888
> > +pictureshader_565
> > +giantbitmap_mirror_bilerp_rotate_8888
> > +giantbitmap_mirror_bilerp_rotate_565
> > +giantbitmap_repeat_bilerp_rotate_8888
> > +giantbitmap_repeat_bilerp_rotate_565
> > +filterbitmap_image_mandrill_512.png_8888
> > +filterbitmap_image_mandrill_512.png_565
> > +filterbitmap_image_mandrill_256.png_8888
> > +filterbitmap_image_mandrill_256.png_565
> > +filterbitmap_image_mandrill_128.png_8888
> > +filterbitmap_image_mandrill_128.png_565
> > +filterbitmap_image_mandrill_64.png_8888
> > +filterbitmap_image_mandrill_64.png_565
> > +filterbitmap_image_mandrill_32.png_8888
> > +filterbitmap_image_mandrill_32.png_565
> > +filterbitmap_image_mandrill_16.png_8888
> > +filterbitmap_image_mandrill_16.png_565
> > +filterbitmap_checkerboard_192_192_8888
> > +filterbitmap_checkerboard_192_192_565
> > +filterbitmap_checkerboard_32_2_8888
> > +filterbitmap_checkerboard_32_2_565
> > +filterbitmap_checkerboard_32_8_8888
> > +filterbitmap_checkerboard_32_8_565
> > +filterbitmap_checkerboard_32_32_8888
> > +filterbitmap_checkerboard_32_32_565
> > +filterbitmap_checkerboard_4_4_8888
> > +filterbitmap_checkerboard_4_4_565
> > +filterbitmap_text_10.00pt_8888
> > +filterbitmap_text_10.00pt_565
> > +filterbitmap_text_7.00pt_8888
> > +filterbitmap_text_7.00pt_565
> > +filterbitmap_text_3.00pt_8888
> > +filterbitmap_text_3.00pt_565
> > +downsamplebitmap_image_none_mandrill_512.png_8888
> > +downsamplebitmap_image_none_mandrill_512.png_565
> > +downsamplebitmap_checkerboard_none_512_256_8888
> > +downsamplebitmap_checkerboard_none_512_256_565
> > +downsamplebitmap_text_none_72.00pt_8888
> > +downsamplebitmap_text_none_72.00pt_565
> > +downsamplebitmap_image_low_mandrill_512.png_8888
> > +downsamplebitmap_image_low_mandrill_512.png_565
> > +downsamplebitmap_checkerboard_low_512_256_8888
> > +downsamplebitmap_checkerboard_low_512_256_565
> > +downsamplebitmap_text_low_72.00pt_8888
> > +downsamplebitmap_text_low_72.00pt_565
> > +downsamplebitmap_image_medium_mandrill_512.png_8888
> > +downsamplebitmap_image_medium_mandrill_512.png_565
> > +downsamplebitmap_checkerboard_medium_512_256_8888
> > +downsamplebitmap_checkerboard_medium_512_256_565
> > +downsamplebitmap_text_medium_72.00pt_8888
> > +downsamplebitmap_text_medium_72.00pt_565
> > +drawbitmapmatrix_8888
> > +drawbitmapmatrix_565
> 
> Rebased ignored-tests.txt...

FYI - 'lgtm' is 'sticky', i.e. uploading a new patch does not remove it, so you
can check 'Commit' to re-attempt to submit. It may be worth waiting to submit
until the tree is a little more green though (I think once the current build
finishes everywhere it will be green though).

[henrik.smiding, 2014-05-21 15:02:11]

On 2014/05/21 13:39:49, scroggo wrote:
> On 2014/05/21 12:40:05, henrik.smiding wrote:
> > On 2014/05/21 08:27:29, I haz the power (commit-bot) wrote:
> > > Failed to apply patch for expectations/gm/ignored-tests.txt:
> > > While running patch -p1 --forward --force --no-backup-if-mismatch;
> > >   patching file expectations/gm/ignored-tests.txt
> > >   Hunk #1 FAILED at 41.
> > >   1 out of 1 hunk FAILED -- saving rejects to file
> > > expectations/gm/ignored-tests.txt.rej
> > > 
> > > Patch:       expectations/gm/ignored-tests.txt
> > > Index: expectations/gm/ignored-tests.txt
> > > diff --git a/expectations/gm/ignored-tests.txt
> > > b/expectations/gm/ignored-tests.txt
> > > index
> > >
> >
>
9afd2ef44e1e41b69a0fd54f0bd9bc9bba61a6e7..89790e1ee93bcd1e03eac0551829c96290a4a7b3
> > > 100644
> > > --- a/expectations/gm/ignored-tests.txt
> > > +++ b/expectations/gm/ignored-tests.txt
> > > @@ -41,3 +41,62 @@ canvas-layer-state
> > >  # bsalomon: https://codereview.chromium.org/264303008/
> > >  # bsalomon@ will rebaseline this test
> > >  ninepatch-stretch
> > > +
> > > +# https://codereview.chromium.org/240433002/
> > > +# scroggo will rebaseline this test
> > > +shadertext3_8888
> > > +shadertext3_565
> > > +pictureshader_8888
> > > +pictureshader_565
> > > +giantbitmap_mirror_bilerp_rotate_8888
> > > +giantbitmap_mirror_bilerp_rotate_565
> > > +giantbitmap_repeat_bilerp_rotate_8888
> > > +giantbitmap_repeat_bilerp_rotate_565
> > > +filterbitmap_image_mandrill_512.png_8888
> > > +filterbitmap_image_mandrill_512.png_565
> > > +filterbitmap_image_mandrill_256.png_8888
> > > +filterbitmap_image_mandrill_256.png_565
> > > +filterbitmap_image_mandrill_128.png_8888
> > > +filterbitmap_image_mandrill_128.png_565
> > > +filterbitmap_image_mandrill_64.png_8888
> > > +filterbitmap_image_mandrill_64.png_565
> > > +filterbitmap_image_mandrill_32.png_8888
> > > +filterbitmap_image_mandrill_32.png_565
> > > +filterbitmap_image_mandrill_16.png_8888
> > > +filterbitmap_image_mandrill_16.png_565
> > > +filterbitmap_checkerboard_192_192_8888
> > > +filterbitmap_checkerboard_192_192_565
> > > +filterbitmap_checkerboard_32_2_8888
> > > +filterbitmap_checkerboard_32_2_565
> > > +filterbitmap_checkerboard_32_8_8888
> > > +filterbitmap_checkerboard_32_8_565
> > > +filterbitmap_checkerboard_32_32_8888
> > > +filterbitmap_checkerboard_32_32_565
> > > +filterbitmap_checkerboard_4_4_8888
> > > +filterbitmap_checkerboard_4_4_565
> > > +filterbitmap_text_10.00pt_8888
> > > +filterbitmap_text_10.00pt_565
> > > +filterbitmap_text_7.00pt_8888
> > > +filterbitmap_text_7.00pt_565
> > > +filterbitmap_text_3.00pt_8888
> > > +filterbitmap_text_3.00pt_565
> > > +downsamplebitmap_image_none_mandrill_512.png_8888
> > > +downsamplebitmap_image_none_mandrill_512.png_565
> > > +downsamplebitmap_checkerboard_none_512_256_8888
> > > +downsamplebitmap_checkerboard_none_512_256_565
> > > +downsamplebitmap_text_none_72.00pt_8888
> > > +downsamplebitmap_text_none_72.00pt_565
> > > +downsamplebitmap_image_low_mandrill_512.png_8888
> > > +downsamplebitmap_image_low_mandrill_512.png_565
> > > +downsamplebitmap_checkerboard_low_512_256_8888
> > > +downsamplebitmap_checkerboard_low_512_256_565
> > > +downsamplebitmap_text_low_72.00pt_8888
> > > +downsamplebitmap_text_low_72.00pt_565
> > > +downsamplebitmap_image_medium_mandrill_512.png_8888
> > > +downsamplebitmap_image_medium_mandrill_512.png_565
> > > +downsamplebitmap_checkerboard_medium_512_256_8888
> > > +downsamplebitmap_checkerboard_medium_512_256_565
> > > +downsamplebitmap_text_medium_72.00pt_8888
> > > +downsamplebitmap_text_medium_72.00pt_565
> > > +drawbitmapmatrix_8888
> > > +drawbitmapmatrix_565
> > 
> > Rebased ignored-tests.txt...
> 
> FYI - 'lgtm' is 'sticky', i.e. uploading a new patch does not remove it, so
you
> can check 'Commit' to re-attempt to submit. It may be worth waiting to submit
> until the tree is a little more green though (I think once the current build
> finishes everywhere it will be green though).

Thanks, didn't know that.
Who do I check the green-ness of the tree? Are you talking about the 'Skia
buildbot' page?

[henrik.smiding, 2014-05-21 15:10:01]

The CQ bit was checked by henrik.smiding@intel.com

[commit-bot: I haz the power, 2014-05-21 15:10:53]

CQ is trying da patch. Follow status at

https://skia-tree-status.appspot.com/cq/henrik.smiding@intel.com/240433002/16...

[scroggo, 2014-05-21 15:15:20]

On 2014/05/21 15:02:11, henrik.smiding wrote:

> Thanks, didn't know that.
> Who do I check the green-ness of the tree? Are you talking about the 'Skia
> buildbot' page?

Yes: http://skia-tree-status.appspot.com/buildbots/console

(sorry for being less than clear!)

[commit-bot: I haz the power, 2014-05-21 15:16:17]

Change committed as 14825

[scroggo, 2014-05-21 16:24:38]

A revert of this CL has been created in
https://codereview.chromium.org/297803005/ by scroggo@google.com.

The reason for reverting is: Sorry, Henrik, but after all the churn, we're
reverting :(

We are having unrelated DEPS roll issues, and since this may affect layout tests
(which I suspect it will because it affects our GMs), we do not want to do
anything to further prevent the DEPS roll.

The ignored-tests changes are also not correct, leading to redness in our tree.
Will you please add the changes from https://codereview.chromium.org/296923002/,
and then we'll resubmit once the DEPS roll goes through?.

[henrik.smiding, 2014-05-22 14:28:03]

On 2014/05/21 16:24:38, scroggo wrote:
> A revert of this CL has been created in
> https://codereview.chromium.org/297803005/ by mailto:scroggo@google.com.
> 
> The reason for reverting is: Sorry, Henrik, but after all the churn, we're
> reverting :(
> 
> We are having unrelated DEPS roll issues, and since this may affect layout
tests
> (which I suspect it will because it affects our GMs), we do not want to do
> anything to further prevent the DEPS roll.
> 
> The ignored-tests changes are also not correct, leading to redness in our
tree.
> Will you please add the changes from
https://codereview.chromium.org/296923002/,
> and then we'll resubmit once the DEPS roll goes through?.

Fixed.
Commit when you feel ready.

[scroggo, 2014-05-23 13:32:17]

The CQ bit was checked by scroggo@google.com

[commit-bot: I haz the power, 2014-05-23 13:32:27]

CQ is trying da patch. Follow status at

https://skia-tree-status.appspot.com/cq/henrik.smiding@intel.com/240433002/18...

[commit-bot: I haz the power, 2014-05-23 13:32:31]

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

[commit-bot: I haz the power, 2014-05-23 13:32:32]

Failed to apply patch for expectations/gm/ignored-tests.txt:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file expectations/gm/ignored-tests.txt
  Hunk #1 FAILED at 51.
  1 out of 1 hunk FAILED -- saving rejects to file
expectations/gm/ignored-tests.txt.rej

Patch:       expectations/gm/ignored-tests.txt
Index: expectations/gm/ignored-tests.txt
diff --git a/expectations/gm/ignored-tests.txt
b/expectations/gm/ignored-tests.txt
index
c1343824206baf7c4ff98a20dd666ea859d53afb..4936ae6adf44fbf0df2d1488ffa0584d71809407
100644
--- a/expectations/gm/ignored-tests.txt
+++ b/expectations/gm/ignored-tests.txt
@@ -51,4 +51,35 @@ simpleblurroundrect
 downsamplebitmap_checkerboard_high_512_256
 downsamplebitmap_image_high_mandrill_512.png
 filterbitmap_checkerboard_192_192
-downsamplebitmap_text_high_72.00pt
\ No newline at end of file
+downsamplebitmap_text_high_72.00pt
+
+# https://codereview.chromium.org/240433002/
+# scroggo will rebaseline this test
+shadertext3
+pictureshader
+giantbitmap_mirror_bilerp_rotate
+giantbitmap_repeat_bilerp_rotate
+filterbitmap_image_mandrill_512.png
+filterbitmap_image_mandrill_256.png
+filterbitmap_image_mandrill_128.png
+filterbitmap_image_mandrill_64.png
+filterbitmap_image_mandrill_32.png
+filterbitmap_image_mandrill_16.png
+filterbitmap_checkerboard_192_192
+filterbitmap_checkerboard_32_2
+filterbitmap_checkerboard_32_8
+filterbitmap_checkerboard_32_32
+filterbitmap_checkerboard_4_4
+filterbitmap_text_10.00pt
+filterbitmap_text_7.00pt
+filterbitmap_text_3.00pt
+downsamplebitmap_image_none_mandrill_512.png
+downsamplebitmap_checkerboard_none_512_256
+downsamplebitmap_text_none_72.00pt
+downsamplebitmap_image_low_mandrill_512.png
+downsamplebitmap_checkerboard_low_512_256
+downsamplebitmap_text_low_72.00pt
+downsamplebitmap_image_medium_mandrill_512.png
+downsamplebitmap_checkerboard_medium_512_256
+downsamplebitmap_text_medium_72.00pt
+drawbitmapmatrix

[henrik.smiding, 2014-05-23 16:00:10]

The CQ bit was checked by henrik.smiding@intel.com

[commit-bot: I haz the power, 2014-05-23 16:00:30]

CQ is trying da patch. Follow status at

https://skia-tree-status.appspot.com/cq/henrik.smiding@intel.com/240433002/20...

[commit-bot: I haz the power, 2014-05-23 16:05:47]

Change committed as 14872

[scroggo, 2014-05-23 19:11:18]

A revert of this CL has been created in
https://codereview.chromium.org/294023016/ by scroggo@google.com.

The reason for reverting is: This also changes verylargebitmap, and the
difference appears to be meaningful. Henrik, I have emailed you the images that
differ..