revise row blits to keep intermediate precision so that color is preserved when blended against its…

SkBlendARGB32 and S32[A]_Blend_BlitRow32 are currently formulated as: SkAlphaMulQ(src, src_scale) + SkAlphaMulQ(dst, dst_scale), which boils down to ((src*src_scale)>>8) + ((dst*dst_scale)>>8). In particular, note that the intermediate precision is discarded before the two parts are added together, causing the final result to possibly inaccurate. 

In Firefox, we use SkCanvas::saveLayer in combination with a backdrop that initializes the layer to the background. When this is blended back onto background using transparency, where the source and destination pixel colors are the same, the resulting color after the blend is not preserved due to the lost precision mentioned above. In cases where this operation is repeatedly performed, this causes substantially noticeable differences in color as evidenced in this downstream Firefox bug report:  https://bugzilla.mozilla.org/show_bug.cgi?id=1200684

In the test-case in the downstream report, essentially it does blend(src=0xFF2E3338, dst=0xFF2E3338, scale=217), which gives the result 0xFF2E3237, while we would expect to get back 0xFF2E3338.

This problem goes away if the blend is instead reformulated to effectively do (src*src_scale + dst*dst_scale)>>8, which keeps the intermediate precision during the addition before shifting it off.

This modifies the blending operations thusly. The performance should remain mostly unchanged, or possibly improve slightly, so there should be no real downside to doing this, with the benefit of making the results more accurate. Without this, it is currently unsafe for Firefox to blend a layer back onto itself that was initialized with a copy of its background.

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2097883002
CQ_INCLUDE_TRYBOTS=master.client.skia:Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-SKNX_NO_SIMD-Trybot

[mtklein adds...]
No public API changes.
TBR=reed@google.com

Committed: https://skia.googlesource.com/skia/+/40254c2c2dc28a34f96294d5a1ad94a99b0be8a6

[lsalzman1, 2016-06-24 18:05:01]

Description was changed from

==========
revise row blits to keep intermediate precision so that color is preserved when
blended against itself

BUG=skia:
==========

to

==========
revise row blits to keep intermediate precision so that color is preserved when
blended against itself

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2097883002
CQ_EXTRA_TRYBOTS=client.skia:Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-SKNX_NO_SIMD-Trybot
==========

[lsalzman1, 2016-06-24 18:23:37]

Description was changed from

==========
revise row blits to keep intermediate precision so that color is preserved when
blended against itself

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2097883002
CQ_EXTRA_TRYBOTS=client.skia:Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-SKNX_NO_SIMD-Trybot
==========

to

==========
SkBlendARGB32 and S32[A]_Blend_BlitRow32 are currently formulated as:
SkAlphaMulQ(src, src_scale) + SkAlphaMulQ(dst, dst_scale), which boils down to
((src*src_scale)>>8) + ((dst*dst_scale)>>8). In particular, not that the
intermediate precision is discarded before the two parts are added together,
causing the final result to possibly inaccurate. 

In Firefox, we use SkCanvas::saveLayer in combination with a backdrop that
initializes the layer to the background. When this is blended back onto
background using transparency, where the source and destination pixel colors are
the same, the resulting color after the blend is not preserved due to the lost
precision mentioned above. In cases where this operation is repeatedly
performed, this causes substantially noticeable differences in color as
evidenced in this upstream Firefox bug report: 
https://bugzilla.mozilla.org/show_bug.cgi?id=1200684

In the test-case in the upstream report, essentially it does
blend(src=0xFF2E3338, dst=0xFF2E3338, scale=217), which gives the result
0xFF2E3237, while we would expect to get back 0xFF2E3338.

This problem goes away if the blend is instead reformulated to effectively do
(src*src_scale + dst*dst_scale)>>8, which keeps the intermediate precision
during the addition before shifting it off.

This modifies the blending operations thusly. The performance should remain
mostly unchanged, or possibly improve slightly, so there should be no real
downside to doing this, with the benefit of making the results more accurate.
Without this, it is currently unsafe for Firefox to blend a layer back onto
itself that was initialized with a copy of its background.

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2097883002
CQ_EXTRA_TRYBOTS=client.skia:Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-SKNX_NO_SIMD-Trybot
==========

[lsalzman1, 2016-06-24 18:23:37]

lsalzman@mozilla.com changed reviewers:
+ mtklein@google.com

[lsalzman1, 2016-06-24 18:29:12]

Description was changed from

==========
SkBlendARGB32 and S32[A]_Blend_BlitRow32 are currently formulated as:
SkAlphaMulQ(src, src_scale) + SkAlphaMulQ(dst, dst_scale), which boils down to
((src*src_scale)>>8) + ((dst*dst_scale)>>8). In particular, not that the
intermediate precision is discarded before the two parts are added together,
causing the final result to possibly inaccurate. 

In Firefox, we use SkCanvas::saveLayer in combination with a backdrop that
initializes the layer to the background. When this is blended back onto
background using transparency, where the source and destination pixel colors are
the same, the resulting color after the blend is not preserved due to the lost
precision mentioned above. In cases where this operation is repeatedly
performed, this causes substantially noticeable differences in color as
evidenced in this upstream Firefox bug report: 
https://bugzilla.mozilla.org/show_bug.cgi?id=1200684

In the test-case in the upstream report, essentially it does
blend(src=0xFF2E3338, dst=0xFF2E3338, scale=217), which gives the result
0xFF2E3237, while we would expect to get back 0xFF2E3338.

This problem goes away if the blend is instead reformulated to effectively do
(src*src_scale + dst*dst_scale)>>8, which keeps the intermediate precision
during the addition before shifting it off.

This modifies the blending operations thusly. The performance should remain
mostly unchanged, or possibly improve slightly, so there should be no real
downside to doing this, with the benefit of making the results more accurate.
Without this, it is currently unsafe for Firefox to blend a layer back onto
itself that was initialized with a copy of its background.

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2097883002
CQ_EXTRA_TRYBOTS=client.skia:Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-SKNX_NO_SIMD-Trybot
==========

to

==========
SkBlendARGB32 and S32[A]_Blend_BlitRow32 are currently formulated as:
SkAlphaMulQ(src, src_scale) + SkAlphaMulQ(dst, dst_scale), which boils down to
((src*src_scale)>>8) + ((dst*dst_scale)>>8). In particular, note that the
intermediate precision is discarded before the two parts are added together,
causing the final result to possibly inaccurate. 

In Firefox, we use SkCanvas::saveLayer in combination with a backdrop that
initializes the layer to the background. When this is blended back onto
background using transparency, where the source and destination pixel colors are
the same, the resulting color after the blend is not preserved due to the lost
precision mentioned above. In cases where this operation is repeatedly
performed, this causes substantially noticeable differences in color as
evidenced in this downstream Firefox bug report: 
https://bugzilla.mozilla.org/show_bug.cgi?id=1200684

In the test-case in the downstream report, essentially it does
blend(src=0xFF2E3338, dst=0xFF2E3338, scale=217), which gives the result
0xFF2E3237, while we would expect to get back 0xFF2E3338.

This problem goes away if the blend is instead reformulated to effectively do
(src*src_scale + dst*dst_scale)>>8, which keeps the intermediate precision
during the addition before shifting it off.

This modifies the blending operations thusly. The performance should remain
mostly unchanged, or possibly improve slightly, so there should be no real
downside to doing this, with the benefit of making the results more accurate.
Without this, it is currently unsafe for Firefox to blend a layer back onto
itself that was initialized with a copy of its background.

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2097883002
CQ_EXTRA_TRYBOTS=client.skia:Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-SKNX_NO_SIMD-Trybot
==========

[lsalzman1, 2016-06-29 22:15:38]

lsalzman@mozilla.com changed reviewers:
+ reed@google.com
- mtklein@google.com

[mtklein, 2016-06-29 22:37:52]

The CQ bit was checked by mtklein@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-06-29 22:37:58]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[mtklein, 2016-06-29 22:41:27]

The CQ bit was unchecked by mtklein@google.com

[mtklein, 2016-06-29 22:44:35]

Do you have someone (including yourself) fluent enough in MIPS assembly to vouch
for that part of the change?  We don't have any MIPS bots or anyone who
regularly reads and writes MIPS assembly, so that part will be a bit blind.

Other than that concern, the mechanics of this code LGTM.  This code is clearer,
more accurate, and I agree likely faster than the code it replaces.

We may need to stage landing this so it can roll into Chrome without upsetting
the Blink layout tests.  Generally we guard a code change like this with an
#ifndef SK_SUPPORT_LEGACY_SOMETHING_OR_OTHER flag, then ask for the legacy
behavior in Chrome by defining that flag, then landing the Skia change, and then
finally removing that flag in Chrome, rebaseing the Blink layout tests in the
same change.  The trybots I triggered should give us some indication of whether
that'll be necessary.

I wouldn't anticipate Mike will object to this, but we'd better let him chime in
if he'd like.

[lsalzman1, 2016-06-29 23:51:41]

On 2016/06/29 22:44:35, mtklein wrote:
> Do you have someone (including yourself) fluent enough in MIPS assembly to
vouch
> for that part of the change?  We don't have any MIPS bots or anyone who
> regularly reads and writes MIPS assembly, so that part will be a bit blind.
> 
> Other than that concern, the mechanics of this code LGTM.  This code is
clearer,
> more accurate, and I agree likely faster than the code it replaces.
> 
> We may need to stage landing this so it can roll into Chrome without upsetting
> the Blink layout tests.  Generally we guard a code change like this with an
> #ifndef SK_SUPPORT_LEGACY_SOMETHING_OR_OTHER flag, then ask for the legacy
> behavior in Chrome by defining that flag, then landing the Skia change, and
then
> finally removing that flag in Chrome, rebaseing the Blink layout tests in the
> same change.  The trybots I triggered should give us some indication of
whether
> that'll be necessary.
> 
> I wouldn't anticipate Mike will object to this, but we'd better let him chime
in
> if he'd like.

I don't know anyone offhand who knows MIPS assembly. I admit
I read the instruction set manual to verify if my changes were correct, while at
the
same time trying to minimize the changed instructions. The precrq instructions
are packing
the temps together while discarding the least significant byte, and the addu
does what you'd think.
https://imgtec.com/mips/architectures/dsp/

At least the C, SSE2, and NEON versions passed the Firefox unit tests since we
deploy on those architectures.

[mtklein, 2016-06-30 13:29:53]

On 2016/06/29 23:51:41, lsalzman1 wrote:
> On 2016/06/29 22:44:35, mtklein wrote:
> > Do you have someone (including yourself) fluent enough in MIPS assembly to
> vouch
> > for that part of the change?  We don't have any MIPS bots or anyone who
> > regularly reads and writes MIPS assembly, so that part will be a bit blind.
> > 
> > Other than that concern, the mechanics of this code LGTM.  This code is
> clearer,
> > more accurate, and I agree likely faster than the code it replaces.
> > 
> > We may need to stage landing this so it can roll into Chrome without
upsetting
> > the Blink layout tests.  Generally we guard a code change like this with an
> > #ifndef SK_SUPPORT_LEGACY_SOMETHING_OR_OTHER flag, then ask for the legacy
> > behavior in Chrome by defining that flag, then landing the Skia change, and
> then
> > finally removing that flag in Chrome, rebaseing the Blink layout tests in
the
> > same change.  The trybots I triggered should give us some indication of
> whether
> > that'll be necessary.
> > 
> > I wouldn't anticipate Mike will object to this, but we'd better let him
chime
> in
> > if he'd like.
> 
> I don't know anyone offhand who knows MIPS assembly. I admit
> I read the instruction set manual to verify if my changes were correct, while
at
> the
> same time trying to minimize the changed instructions. The precrq instructions
> are packing
> the temps together while discarding the least significant byte, and the addu
> does what you'd think.
> https://imgtec.com/mips/architectures/dsp/
> 
> At least the C, SSE2, and NEON versions passed the Firefox unit tests since we
> deploy on those architectures.

Hmm.  This looks wrong.

That red linux_blink_rel bot indicates layout tests are going to change.  That's
expected, but some of the changes look pretty bad.  Have a look:
https://storage.googleapis.com/chromium-layout-test-archives/linux_blink_rel/...

css3/blending/effect-background-blend-mode-stacking.html in particular appears
to have gone very wrong.

That GOLD_TRYBOT_URL link in the description also now shows a bunch of our Skia
tests that have changed, most subtly but several dramatically, e.g. gradtext.

[lsalzman1, 2016-06-30 14:53:53]

On 2016/06/30 13:29:53, mtklein wrote:
> Hmm.  This looks wrong.
> 
> That red linux_blink_rel bot indicates layout tests are going to change. 
That's
> expected, but some of the changes look pretty bad.  Have a look:
>
https://storage.googleapis.com/chromium-layout-test-archives/linux_blink_rel/...
> 
> css3/blending/effect-background-blend-mode-stacking.html in particular appears
> to have gone very wrong.
> 
> That GOLD_TRYBOT_URL link in the description also now shows a bunch of our
Skia
> tests that have changed, most subtly but several dramatically, e.g. gradtext.

I have an idea what might be going wrong. It could be it is not liking the 
dst + (src-dst)*src_scale formulation in the SSE2 version. Should be doable
to calculate the alternate way, although not quite as fast. I'll investigate
today and fix this.

Strange that this didn't trigger in any of the Firefox unit tests.

[mtklein, 2016-06-30 15:10:54]

On 2016/06/30 14:53:53, lsalzman1 wrote:
> On 2016/06/30 13:29:53, mtklein wrote:
> > Hmm.  This looks wrong.
> > 
> > That red linux_blink_rel bot indicates layout tests are going to change. 
> That's
> > expected, but some of the changes look pretty bad.  Have a look:
> >
>
https://storage.googleapis.com/chromium-layout-test-archives/linux_blink_rel/...
> > 
> > css3/blending/effect-background-blend-mode-stacking.html in particular
appears
> > to have gone very wrong.
> > 
> > That GOLD_TRYBOT_URL link in the description also now shows a bunch of our
> Skia
> > tests that have changed, most subtly but several dramatically, e.g.
gradtext.
> 
> I have an idea what might be going wrong. It could be it is not liking the 
> dst + (src-dst)*src_scale formulation in the SSE2 version. Should be doable
> to calculate the alternate way, although not quite as fast. I'll investigate
> today and fix this.
> 
> Strange that this didn't trigger in any of the Firefox unit tests.

Sounds good.

[lsalzman1, 2016-06-30 21:01:07]

The CQ bit was checked by lsalzman@mozilla.com to run a CQ dry run

[commit-bot: I haz the power, 2016-06-30 21:01:17]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[lsalzman1, 2016-06-30 21:08:47]

I managed to reproduce at least some of the problems in Firefox using some
adapted webkit tests. The problem turned out to not be what I thought it was,
though...

When calculating the destination scale with 256 - ((a*scale)>>8), a*scale is
truncated before the inversion. This can result in a set of scales that when
used can cause overflow with the enhanced precision introduced by this patch.

Incidentally, I found this same bug lurking in a few other places in Skia.

Should be fixed now.

[mtklein, 2016-06-30 21:12:07]

The CQ bit was unchecked by mtklein@google.com

[mtklein, 2016-06-30 21:12:56]

On 2016/06/30 21:08:47, lsalzman1 wrote:
> I managed to reproduce at least some of the problems in Firefox using some
> adapted webkit tests. The problem turned out to not be what I thought it was,
> though...
> 
> When calculating the destination scale with 256 - ((a*scale)>>8), a*scale is
> truncated before the inversion. This can result in a set of scales that when
> used can cause overflow with the enhanced precision introduced by this patch.
> 
> Incidentally, I found this same bug lurking in a few other places in Skia.
> 
> Should be fixed now.

Great!  I'm eager to look at this tomorrow.  I've kicked off another run of the
Blink layout tests too.

[lsalzman1, 2016-07-01 14:27:58]

Le sigh, the gm blend failures seem to be with regards to my second change and
some specific vias (like pic/2ndpic/serialize). They go through
SkPictureRecorder, and that does some optimizations of the op stream that
antagonizes this.

Before optimization:
canvas->saveLayerAlpha(0xFC);
paint.setColor(0xFF008000);
canvas->drawRect(paint);
canvas->restore();

After optimization:
paint.setColor(0xFC008000);
canvas->drawRect(paint);

The before version goes through S32A_Blend_BlitRow32, while the latter goes
through blit_row_color32. That these functions even agree at all in this
particular gm before is somewhat stunning, but that they disagree after my
changes is not surprising either.

So I could either modify the gm's color so it rounds similarly, or I could
modify the src scale further in the blend routines (which might have further
adverse affects on other tests). I am somewhat undecided on how best to fix this
one...

[lsalzman1, 2016-07-01 18:56:28]

On 2016/07/01 14:27:58, lsalzman1 wrote:
> Le sigh, the gm blend failures seem to be with regards to my second change and
> some specific vias (like pic/2ndpic/serialize). They go through
> SkPictureRecorder, and that does some optimizations of the op stream that
> antagonizes this.
> 
> Before optimization:
> canvas->saveLayerAlpha(0xFC);
> paint.setColor(0xFF008000);
> canvas->drawRect(paint);
> canvas->restore();
> 
> After optimization:
> paint.setColor(0xFC008000);
> canvas->drawRect(paint);
> 
> The before version goes through S32A_Blend_BlitRow32, while the latter goes
> through blit_row_color32. That these functions even agree at all in this
> particular gm before is somewhat stunning, but that they disagree after my
> changes is not surprising either.
> 
> So I could either modify the gm's color so it rounds similarly, or I could
> modify the src scale further in the blend routines (which might have further
> adverse affects on other tests). I am somewhat undecided on how best to fix
this
> one...

Looks like trying to modify src scale significantly causes adverse movement on
Firefox's tests. So just slightly modifyi

[lsalzman1, 2016-07-01 19:02:18]

On 2016/07/01 14:27:58, lsalzman1 wrote:
> Le sigh, the gm blend failures seem to be with regards to my second change and
> some specific vias (like pic/2ndpic/serialize). They go through
> SkPictureRecorder, and that does some optimizations of the op stream that
> antagonizes this.
> 
> Before optimization:
> canvas->saveLayerAlpha(0xFC);
> paint.setColor(0xFF008000);
> canvas->drawRect(paint);
> canvas->restore();
> 
> After optimization:
> paint.setColor(0xFC008000);
> canvas->drawRect(paint);
> 
> The before version goes through S32A_Blend_BlitRow32, while the latter goes
> through blit_row_color32. That these functions even agree at all in this
> particular gm before is somewhat stunning, but that they disagree after my
> changes is not surprising either.
> 
> So I could either modify the gm's color so it rounds similarly, or I could
> modify the src scale further in the blend routines (which might have further
> adverse affects on other tests). I am somewhat undecided on how best to fix
this
> one...

Looks like trying to modify src scale significantly causes adverse movement on
Firefox's tests and other general bad juju. The lesser evil seems to just be
to slightly modify the gm's color, something like 0xFC208000 seems to make it
pass just fine.

As for the webkit tests, clicking through them it looks like just off-by-1s due
to rounding differences. I am not sure how the references work for the webkit
tests? We'd normally just fuzzy compare away differences like that in Firefox,
but if exact image comparisons are used for references, I could see why they
might cause those to superficially fail.

I didn't see any other types of failures in the tests, though, from the sampling
I inspected. The real ones should have been addressed by the overflow fix.

[lsalzman1, 2016-07-18 17:53:45]

Any decisions?

[mtklein, 2016-07-18 18:21:07]

On 2016/07/18 17:53:45, lsalzman1 wrote:
> Any decisions?

Sorry, just haven't circled back yet.

[mtklein, 2016-07-26 19:19:51]

The CQ bit was checked by mtklein@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-07-26 19:20:05]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-26 19:20:08]

Your CL relies on deprecated CQ feature(s):
 * Specifying master names in CQ_INCLUDE_TRYBOTS part of description without
"master." prefix is deprecated:
     client.skia
   For more details, see http://crbug.com/617627.

[commit-bot: I haz the power, 2016-07-26 19:20:09]

There were warnings when CQ was processing your CL:
 * CQ_EXTRA_TRYBOTS flag is deprecated and will soon be removed. Use
CQ_INCLUDE_TRYBOTS instead.

[mtklein, 2016-07-26 19:29:33]

> Looks like trying to modify src scale significantly causes adverse movement on
> Firefox's tests and other general bad juju. The lesser evil seems to just be
> to slightly modify the gm's color, something like 0xFC208000 seems to make it
> pass just fine.

This sounds fine to me.  Why don't you send me a CL that we can land first, so
that
when we land this one we see no diff?

> As for the webkit tests, clicking through them it looks like just off-by-1s
due
> to rounding differences. I am not sure how the references work for the webkit
> tests? We'd normally just fuzzy compare away differences like that in Firefox,
> but if exact image comparisons are used for references, I could see why they
> might cause those to superficially fail.

There's no fuzziness for those tests.  Instead, we've got to guard this code,
let's say with #if defined(SK_SUPPORT_LEGACY_BROKEN_LERP), and keep using the
old broken path in the #else case.  Then we'll define
SK_SUPPORT_LEGACY_BROKEN_LERP
in Chrome, then land this, then undefine it while updating Blink's expected
images.

[mtklein, 2016-07-26 19:29:43]

https://codereview.chromium.org/2097883002/diff/20001/include/core/SkColorPriv.h
File include/core/SkColorPriv.h (right):

https://codereview.chromium.org/2097883002/diff/20001/include/core/SkColorPri...
include/core/SkColorPriv.h:206: #define SkAlphaMulInv256(value, alpha256)
(((256<<8) - (value) * (alpha256)) >> 8)
Let's make this a static function and maybe try to make the comment clearer?  It
wasn't immediately clear whether this was (1-aa)*x or 1 - (x*aa).

Maybe:
    // 1 - (x*aa), for [0,255] x and [0,256] aa.

[commit-bot: I haz the power, 2016-07-26 19:30:56]

There were warnings when CQ was processing your CL:
 * CQ_EXTRA_TRYBOTS flag is deprecated and will soon be removed. Use
CQ_INCLUDE_TRYBOTS instead.

[commit-bot: I haz the power, 2016-07-26 19:36:44]

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

[commit-bot: I haz the power, 2016-07-26 19:36:45]

Dry run: Try jobs failed on following builders:
  Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-SKNX_NO_SIMD-Trybot on client.skia
(JOB_FAILED,
http://build.chromium.org/p/client.skia/builders/Test-Ubuntu-GCC-GCE-CPU-AVX2...)

[lsalzman1, 2016-07-28 02:15:10]

Okay, I'll try to get to fixing those issues this week. We're in the final
stages
of trying to get Skia content working in Firefox, and we're turning up other
possible
cases of the legacy lerp biting us that I may want to bundle in with this patch,
or
save for a later follow-up patch. So I just want to see if I can enumerate those
remaining cases first.

[mtklein, 2016-07-28 13:31:53]

On 2016/07/28 02:15:10, lsalzman1 wrote:
> Okay, I'll try to get to fixing those issues this week. We're in the final
> stages
> of trying to get Skia content working in Firefox, and we're turning up other
> possible
> cases of the legacy lerp biting us that I may want to bundle in with this
patch,
> or
> save for a later follow-up patch. So I just want to see if I can enumerate
those
> remaining cases first.

sounds good.

sorry you're having to slog through this.  we know much of this code is broken,
and we've mostly decided to leave it that way.

[lsalzman1, 2016-08-03 22:09:15]

Description was changed from

==========
SkBlendARGB32 and S32[A]_Blend_BlitRow32 are currently formulated as:
SkAlphaMulQ(src, src_scale) + SkAlphaMulQ(dst, dst_scale), which boils down to
((src*src_scale)>>8) + ((dst*dst_scale)>>8). In particular, note that the
intermediate precision is discarded before the two parts are added together,
causing the final result to possibly inaccurate. 

In Firefox, we use SkCanvas::saveLayer in combination with a backdrop that
initializes the layer to the background. When this is blended back onto
background using transparency, where the source and destination pixel colors are
the same, the resulting color after the blend is not preserved due to the lost
precision mentioned above. In cases where this operation is repeatedly
performed, this causes substantially noticeable differences in color as
evidenced in this downstream Firefox bug report: 
https://bugzilla.mozilla.org/show_bug.cgi?id=1200684

In the test-case in the downstream report, essentially it does
blend(src=0xFF2E3338, dst=0xFF2E3338, scale=217), which gives the result
0xFF2E3237, while we would expect to get back 0xFF2E3338.

This problem goes away if the blend is instead reformulated to effectively do
(src*src_scale + dst*dst_scale)>>8, which keeps the intermediate precision
during the addition before shifting it off.

This modifies the blending operations thusly. The performance should remain
mostly unchanged, or possibly improve slightly, so there should be no real
downside to doing this, with the benefit of making the results more accurate.
Without this, it is currently unsafe for Firefox to blend a layer back onto
itself that was initialized with a copy of its background.

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2097883002
CQ_EXTRA_TRYBOTS=client.skia:Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-SKNX_NO_SIMD-Trybot
==========

to

==========
SkBlendARGB32 and S32[A]_Blend_BlitRow32 are currently formulated as:
SkAlphaMulQ(src, src_scale) + SkAlphaMulQ(dst, dst_scale), which boils down to
((src*src_scale)>>8) + ((dst*dst_scale)>>8). In particular, note that the
intermediate precision is discarded before the two parts are added together,
causing the final result to possibly inaccurate. 

In Firefox, we use SkCanvas::saveLayer in combination with a backdrop that
initializes the layer to the background. When this is blended back onto
background using transparency, where the source and destination pixel colors are
the same, the resulting color after the blend is not preserved due to the lost
precision mentioned above. In cases where this operation is repeatedly
performed, this causes substantially noticeable differences in color as
evidenced in this downstream Firefox bug report: 
https://bugzilla.mozilla.org/show_bug.cgi?id=1200684

In the test-case in the downstream report, essentially it does
blend(src=0xFF2E3338, dst=0xFF2E3338, scale=217), which gives the result
0xFF2E3237, while we would expect to get back 0xFF2E3338.

This problem goes away if the blend is instead reformulated to effectively do
(src*src_scale + dst*dst_scale)>>8, which keeps the intermediate precision
during the addition before shifting it off.

This modifies the blending operations thusly. The performance should remain
mostly unchanged, or possibly improve slightly, so there should be no real
downside to doing this, with the benefit of making the results more accurate.
Without this, it is currently unsafe for Firefox to blend a layer back onto
itself that was initialized with a copy of its background.

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2097883002
CQ_EXTRA_TRYBOTS=client.skia:Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-SKNX_NO_SIMD-Trybot
CQ_INCLUDE_TRYBOTS=master.client.skia:Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-SKNX_NO_SIMD-Trybot
==========

[lsalzman1, 2016-08-03 22:12:58]

I finally got around to tracking down some last issues in Firefox's unit tests.
I had to change the way the blend scale was calculated in SkBlendARGB32 to be
more accurate using the approximate divide-by-255 to fix some problems we were
having with opaque surfaces de-opaquing due to rounding error. On net, the code
should still be about as fast as the old code, but with fewer artifacts and
makes our unit tests happy now.

I also added in the SK_SUPPORT_LEGACY_BROKEN_LERP option as requested.

[mtklein_C, 2016-08-04 16:40:27]

The CQ bit was checked by mtklein@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-04 16:40:33]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-04 16:40:35]

Your CL relies on deprecated CQ feature(s):
 * Specifying master names in CQ_INCLUDE_TRYBOTS part of description without
"master." prefix is deprecated:
     client.skia
   For more details, see http://crbug.com/617627.

[commit-bot: I haz the power, 2016-08-04 16:40:38]

There were warnings when CQ was processing your CL:
 * CQ_EXTRA_TRYBOTS flag is deprecated and will soon be removed. Use
CQ_INCLUDE_TRYBOTS instead.

[mtklein_C, 2016-08-04 16:57:17]

Description was changed from

==========
SkBlendARGB32 and S32[A]_Blend_BlitRow32 are currently formulated as:
SkAlphaMulQ(src, src_scale) + SkAlphaMulQ(dst, dst_scale), which boils down to
((src*src_scale)>>8) + ((dst*dst_scale)>>8). In particular, note that the
intermediate precision is discarded before the two parts are added together,
causing the final result to possibly inaccurate. 

In Firefox, we use SkCanvas::saveLayer in combination with a backdrop that
initializes the layer to the background. When this is blended back onto
background using transparency, where the source and destination pixel colors are
the same, the resulting color after the blend is not preserved due to the lost
precision mentioned above. In cases where this operation is repeatedly
performed, this causes substantially noticeable differences in color as
evidenced in this downstream Firefox bug report: 
https://bugzilla.mozilla.org/show_bug.cgi?id=1200684

In the test-case in the downstream report, essentially it does
blend(src=0xFF2E3338, dst=0xFF2E3338, scale=217), which gives the result
0xFF2E3237, while we would expect to get back 0xFF2E3338.

This problem goes away if the blend is instead reformulated to effectively do
(src*src_scale + dst*dst_scale)>>8, which keeps the intermediate precision
during the addition before shifting it off.

This modifies the blending operations thusly. The performance should remain
mostly unchanged, or possibly improve slightly, so there should be no real
downside to doing this, with the benefit of making the results more accurate.
Without this, it is currently unsafe for Firefox to blend a layer back onto
itself that was initialized with a copy of its background.

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2097883002
CQ_EXTRA_TRYBOTS=client.skia:Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-SKNX_NO_SIMD-Trybot
CQ_INCLUDE_TRYBOTS=master.client.skia:Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-SKNX_NO_SIMD-Trybot
==========

to

==========
SkBlendARGB32 and S32[A]_Blend_BlitRow32 are currently formulated as:
SkAlphaMulQ(src, src_scale) + SkAlphaMulQ(dst, dst_scale), which boils down to
((src*src_scale)>>8) + ((dst*dst_scale)>>8). In particular, note that the
intermediate precision is discarded before the two parts are added together,
causing the final result to possibly inaccurate. 

In Firefox, we use SkCanvas::saveLayer in combination with a backdrop that
initializes the layer to the background. When this is blended back onto
background using transparency, where the source and destination pixel colors are
the same, the resulting color after the blend is not preserved due to the lost
precision mentioned above. In cases where this operation is repeatedly
performed, this causes substantially noticeable differences in color as
evidenced in this downstream Firefox bug report: 
https://bugzilla.mozilla.org/show_bug.cgi?id=1200684

In the test-case in the downstream report, essentially it does
blend(src=0xFF2E3338, dst=0xFF2E3338, scale=217), which gives the result
0xFF2E3237, while we would expect to get back 0xFF2E3338.

This problem goes away if the blend is instead reformulated to effectively do
(src*src_scale + dst*dst_scale)>>8, which keeps the intermediate precision
during the addition before shifting it off.

This modifies the blending operations thusly. The performance should remain
mostly unchanged, or possibly improve slightly, so there should be no real
downside to doing this, with the benefit of making the results more accurate.
Without this, it is currently unsafe for Firefox to blend a layer back onto
itself that was initialized with a copy of its background.

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2097883002
CQ_INCLUDE_TRYBOTS=master.client.skia:Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-SKNX_NO_SIMD-Trybot
==========

[mtklein_C, 2016-08-04 17:04:55]

On 2016/08/03 at 22:12:58, lsalzman wrote:
> I finally got around to tracking down some last issues in Firefox's unit
tests. I had to change the way the blend scale was calculated in SkBlendARGB32
to be more accurate using the approximate divide-by-255 to fix some problems we
were having with opaque surfaces de-opaquing due to rounding error. On net, the
code should still be about as fast as the old code, but with fewer artifacts and
makes our unit tests happy now.
> 
> I also added in the SK_SUPPORT_LEGACY_BROKEN_LERP option as requested.

This looks good to me.  All the diffs I see in Gold look pretty minor.  I'm
going to trigger the layout test bot to see what that'll look like, but it's not
going to gate us landing this CL.  Instead, we just wait for
https://codereview.chromium.org/2203213004 to land, then we land this, then I'll
follow up to rebaseline the layout tests after that all rolls into Chrome.

[mtklein_C, 2016-08-04 17:06:07]

The CQ bit was unchecked by mtklein@chromium.org

[lsalzman1, 2016-08-05 14:19:48]

Is this safe to commit now?

[mtklein_C, 2016-08-05 14:23:01]

On 2016/08/05 at 14:19:48, lsalzman wrote:
> Is this safe to commit now?

Yes, I think so.  The layout test diffs look numerous but minor.  Let's just
wait for this latest run of linux_precise_blink_rel I just kicked off to confirm
that we really did disable this correctly in Blink.  It should go green now.

[lsalzman1, 2016-08-05 14:40:24]

On 2016/08/05 14:23:01, mtklein_C wrote:
> On 2016/08/05 at 14:19:48, lsalzman wrote:
> > Is this safe to commit now?
> 
> Yes, I think so.  The layout test diffs look numerous but minor.  Let's just
> wait for this latest run of linux_precise_blink_rel I just kicked off to
confirm
> that we really did disable this correctly in Blink.  It should go green now.

Le sigh, fixed typo and should build now. Can we try the blink tests again? :)

[mtklein_C, 2016-08-05 15:36:37]

The CQ bit was checked by mtklein@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-05 15:36:40]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[mtklein_C, 2016-08-05 15:37:11]

On 2016/08/05 at 14:40:24, lsalzman wrote:
> On 2016/08/05 14:23:01, mtklein_C wrote:
> > On 2016/08/05 at 14:19:48, lsalzman wrote:
> > > Is this safe to commit now?
> > 
> > Yes, I think so.  The layout test diffs look numerous but minor.  Let's just
> > wait for this latest run of linux_precise_blink_rel I just kicked off to
confirm
> > that we really did disable this correctly in Blink.  It should go green now.
> 
> Le sigh, fixed typo and should build now. Can we try the blink tests again? :)

Of course!  Good thing we ran them again eh?

[mtklein_C, 2016-08-05 15:37:16]

The CQ bit was unchecked by mtklein@chromium.org

[lsalzman1, 2016-08-05 17:24:56]

On 2016/08/05 15:37:11, mtklein_C wrote:
> On 2016/08/05 at 14:40:24, lsalzman wrote:
> > On 2016/08/05 14:23:01, mtklein_C wrote:
> > > On 2016/08/05 at 14:19:48, lsalzman wrote:
> > > > Is this safe to commit now?
> > > 
> > > Yes, I think so.  The layout test diffs look numerous but minor.  Let's
just
> > > wait for this latest run of linux_precise_blink_rel I just kicked off to
> confirm
> > > that we really did disable this correctly in Blink.  It should go green
now.
> > 
> > Le sigh, fixed typo and should build now. Can we try the blink tests again?
:)
> 
> Of course!  Good thing we ran them again eh?

Okay, not my lucky day. There were still one or two more changes that needed to
be
guarded with SK_SUPPORT_LEGACY_BROKEN_LERP. But now every si

[lsalzman1, 2016-08-05 17:26:24]

On 2016/08/05 15:37:11, mtklein_C wrote:
> On 2016/08/05 at 14:40:24, lsalzman wrote:
> > On 2016/08/05 14:23:01, mtklein_C wrote:
> > > On 2016/08/05 at 14:19:48, lsalzman wrote:
> > > > Is this safe to commit now?
> > > 
> > > Yes, I think so.  The layout test diffs look numerous but minor.  Let's
just
> > > wait for this latest run of linux_precise_blink_rel I just kicked off to
> confirm
> > > that we really did disable this correctly in Blink.  It should go green
now.
> > 
> > Le sigh, fixed typo and should build now. Can we try the blink tests again?
:)
> 
> Of course!  Good thing we ran them again eh?

Okay, not my lucky day. There were still one or two more changes that needed to
be guarded with SK_SUPPORT_LEGACY_BROKEN_LERP. But now every single one should
be. One more try, with feeling?

[mtklein_C, 2016-08-05 17:48:55]

The CQ bit was checked by mtklein@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-05 17:49:02]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[mtklein_C, 2016-08-05 17:49:07]

On 2016/08/05 at 17:26:24, lsalzman wrote:
> On 2016/08/05 15:37:11, mtklein_C wrote:
> > On 2016/08/05 at 14:40:24, lsalzman wrote:
> > > On 2016/08/05 14:23:01, mtklein_C wrote:
> > > > On 2016/08/05 at 14:19:48, lsalzman wrote:
> > > > > Is this safe to commit now?
> > > > 
> > > > Yes, I think so.  The layout test diffs look numerous but minor.  Let's
just
> > > > wait for this latest run of linux_precise_blink_rel I just kicked off to
> > confirm
> > > > that we really did disable this correctly in Blink.  It should go green
now.
> > > 
> > > Le sigh, fixed typo and should build now. Can we try the blink tests
again? :)
> > 
> > Of course!  Good thing we ran them again eh?
> 
> Okay, not my lucky day. There were still one or two more changes that needed
to be guarded with SK_SUPPORT_LEGACY_BROKEN_LERP. But now every single one
should be. One more try, with feeling?

here goes..

[commit-bot: I haz the power, 2016-08-05 18:07:21]

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

[commit-bot: I haz the power, 2016-08-05 18:07:22]

Dry run: This issue passed the CQ dry run.

[lsalzman1, 2016-08-05 18:41:16]

On 2016/08/05 17:49:07, mtklein_C wrote:
> On 2016/08/05 at 17:26:24, lsalzman wrote:
> > On 2016/08/05 15:37:11, mtklein_C wrote:
> > > On 2016/08/05 at 14:40:24, lsalzman wrote:
> > > > On 2016/08/05 14:23:01, mtklein_C wrote:
> > > > > On 2016/08/05 at 14:19:48, lsalzman wrote:
> > > > > > Is this safe to commit now?
> > > > > 
> > > > > Yes, I think so.  The layout test diffs look numerous but minor. 
Let's
> just
> > > > > wait for this latest run of linux_precise_blink_rel I just kicked off
to
> > > confirm
> > > > > that we really did disable this correctly in Blink.  It should go
green
> now.
> > > > 
> > > > Le sigh, fixed typo and should build now. Can we try the blink tests
> again? :)
> > > 
> > > Of course!  Good thing we ran them again eh?
> > 
> > Okay, not my lucky day. There were still one or two more changes that needed
> to be guarded with SK_SUPPORT_LEGACY_BROKEN_LERP. But now every single one
> should be. One more try, with feeling?
> 
> here goes..

All green this time. Permission to press the big red button?

[mtklein_C, 2016-08-05 18:46:45]

Description was changed from

==========
SkBlendARGB32 and S32[A]_Blend_BlitRow32 are currently formulated as:
SkAlphaMulQ(src, src_scale) + SkAlphaMulQ(dst, dst_scale), which boils down to
((src*src_scale)>>8) + ((dst*dst_scale)>>8). In particular, note that the
intermediate precision is discarded before the two parts are added together,
causing the final result to possibly inaccurate. 

In Firefox, we use SkCanvas::saveLayer in combination with a backdrop that
initializes the layer to the background. When this is blended back onto
background using transparency, where the source and destination pixel colors are
the same, the resulting color after the blend is not preserved due to the lost
precision mentioned above. In cases where this operation is repeatedly
performed, this causes substantially noticeable differences in color as
evidenced in this downstream Firefox bug report: 
https://bugzilla.mozilla.org/show_bug.cgi?id=1200684

In the test-case in the downstream report, essentially it does
blend(src=0xFF2E3338, dst=0xFF2E3338, scale=217), which gives the result
0xFF2E3237, while we would expect to get back 0xFF2E3338.

This problem goes away if the blend is instead reformulated to effectively do
(src*src_scale + dst*dst_scale)>>8, which keeps the intermediate precision
during the addition before shifting it off.

This modifies the blending operations thusly. The performance should remain
mostly unchanged, or possibly improve slightly, so there should be no real
downside to doing this, with the benefit of making the results more accurate.
Without this, it is currently unsafe for Firefox to blend a layer back onto
itself that was initialized with a copy of its background.

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2097883002
CQ_INCLUDE_TRYBOTS=master.client.skia:Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-SKNX_NO_SIMD-Trybot
==========

to

==========
SkBlendARGB32 and S32[A]_Blend_BlitRow32 are currently formulated as:
SkAlphaMulQ(src, src_scale) + SkAlphaMulQ(dst, dst_scale), which boils down to
((src*src_scale)>>8) + ((dst*dst_scale)>>8). In particular, note that the
intermediate precision is discarded before the two parts are added together,
causing the final result to possibly inaccurate. 

In Firefox, we use SkCanvas::saveLayer in combination with a backdrop that
initializes the layer to the background. When this is blended back onto
background using transparency, where the source and destination pixel colors are
the same, the resulting color after the blend is not preserved due to the lost
precision mentioned above. In cases where this operation is repeatedly
performed, this causes substantially noticeable differences in color as
evidenced in this downstream Firefox bug report: 
https://bugzilla.mozilla.org/show_bug.cgi?id=1200684

In the test-case in the downstream report, essentially it does
blend(src=0xFF2E3338, dst=0xFF2E3338, scale=217), which gives the result
0xFF2E3237, while we would expect to get back 0xFF2E3338.

This problem goes away if the blend is instead reformulated to effectively do
(src*src_scale + dst*dst_scale)>>8, which keeps the intermediate precision
during the addition before shifting it off.

This modifies the blending operations thusly. The performance should remain
mostly unchanged, or possibly improve slightly, so there should be no real
downside to doing this, with the benefit of making the results more accurate.
Without this, it is currently unsafe for Firefox to blend a layer back onto
itself that was initialized with a copy of its background.

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2097883002
CQ_INCLUDE_TRYBOTS=master.client.skia:Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-SKNX_NO_SIMD-Trybot

[mtklein adds...]
No public API changes.
TBR=reed@google.com
==========

[mtklein_C, 2016-08-05 18:46:54]

fire away!

[lsalzman1, 2016-08-05 18:47:40]

The CQ bit was checked by lsalzman@mozilla.com

[lsalzman1, 2016-08-05 18:47:40]

The patchset sent to the CQ was uploaded after l-g-t-m from mtklein@google.com
Link to the patchset: https://codereview.chromium.org/2097883002/#ps80001
(title: "guard more changes with SK_SUPPORT_LEGACY_BROKEN_LERP")

[commit-bot: I haz the power, 2016-08-05 18:47:50]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[mtklein, 2016-08-05 18:48:25]

+Ethan, FYI

This change will cause many 8888 diffs.  I will triage them.

[commit-bot: I haz the power, 2016-08-05 18:48:49]

Description was changed from

==========
SkBlendARGB32 and S32[A]_Blend_BlitRow32 are currently formulated as:
SkAlphaMulQ(src, src_scale) + SkAlphaMulQ(dst, dst_scale), which boils down to
((src*src_scale)>>8) + ((dst*dst_scale)>>8). In particular, note that the
intermediate precision is discarded before the two parts are added together,
causing the final result to possibly inaccurate. 

In Firefox, we use SkCanvas::saveLayer in combination with a backdrop that
initializes the layer to the background. When this is blended back onto
background using transparency, where the source and destination pixel colors are
the same, the resulting color after the blend is not preserved due to the lost
precision mentioned above. In cases where this operation is repeatedly
performed, this causes substantially noticeable differences in color as
evidenced in this downstream Firefox bug report: 
https://bugzilla.mozilla.org/show_bug.cgi?id=1200684

In the test-case in the downstream report, essentially it does
blend(src=0xFF2E3338, dst=0xFF2E3338, scale=217), which gives the result
0xFF2E3237, while we would expect to get back 0xFF2E3338.

This problem goes away if the blend is instead reformulated to effectively do
(src*src_scale + dst*dst_scale)>>8, which keeps the intermediate precision
during the addition before shifting it off.

This modifies the blending operations thusly. The performance should remain
mostly unchanged, or possibly improve slightly, so there should be no real
downside to doing this, with the benefit of making the results more accurate.
Without this, it is currently unsafe for Firefox to blend a layer back onto
itself that was initialized with a copy of its background.

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2097883002
CQ_INCLUDE_TRYBOTS=master.client.skia:Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-SKNX_NO_SIMD-Trybot

[mtklein adds...]
No public API changes.
TBR=reed@google.com
==========

to

==========
SkBlendARGB32 and S32[A]_Blend_BlitRow32 are currently formulated as:
SkAlphaMulQ(src, src_scale) + SkAlphaMulQ(dst, dst_scale), which boils down to
((src*src_scale)>>8) + ((dst*dst_scale)>>8). In particular, note that the
intermediate precision is discarded before the two parts are added together,
causing the final result to possibly inaccurate. 

In Firefox, we use SkCanvas::saveLayer in combination with a backdrop that
initializes the layer to the background. When this is blended back onto
background using transparency, where the source and destination pixel colors are
the same, the resulting color after the blend is not preserved due to the lost
precision mentioned above. In cases where this operation is repeatedly
performed, this causes substantially noticeable differences in color as
evidenced in this downstream Firefox bug report: 
https://bugzilla.mozilla.org/show_bug.cgi?id=1200684

In the test-case in the downstream report, essentially it does
blend(src=0xFF2E3338, dst=0xFF2E3338, scale=217), which gives the result
0xFF2E3237, while we would expect to get back 0xFF2E3338.

This problem goes away if the blend is instead reformulated to effectively do
(src*src_scale + dst*dst_scale)>>8, which keeps the intermediate precision
during the addition before shifting it off.

This modifies the blending operations thusly. The performance should remain
mostly unchanged, or possibly improve slightly, so there should be no real
downside to doing this, with the benefit of making the results more accurate.
Without this, it is currently unsafe for Firefox to blend a layer back onto
itself that was initialized with a copy of its background.

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2097883002
CQ_INCLUDE_TRYBOTS=master.client.skia:Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-SKNX_NO_SIMD-Trybot

[mtklein adds...]
No public API changes.
TBR=reed@google.com

Committed:
https://skia.googlesource.com/skia/+/40254c2c2dc28a34f96294d5a1ad94a99b0be8a6
==========

[commit-bot: I haz the power, 2016-08-05 18:48:52]

Committed patchset #5 (id:80001) as
https://skia.googlesource.com/skia/+/40254c2c2dc28a34f96294d5a1ad94a99b0be8a6