Revert of skrpb: evaluate color filters for constant shaders once.

Revert of skrpb: evaluate color filters for constant shaders once. (patchset #1 id:1 of https://codereview.chromium.org/2480823002/ )

Reason for revert:
Just testing if this CL really broke the ASAN bot.

Original issue's description:
> skrpb: evaluate color filters for constant shaders once.
> 
> The simplest thing to do here is just run shader+color filter pipeline at
> construction time to create a new constant color shader (replacing the paint
> color).
> 
> This reduces a pipeline like:
>   - constant_color  (paint color)
>   - matrix_4x5
>   - clamp_a
>   - load_d_foo, xfermode, lerp, store_foo
> to
>   - constant_color  (paint color -> matrix_4x5 -> clamp_a)
>   - load_d_foo, xfermode, lerp, store_foo
> 
> To implement this all, we add a new store_f32 stage that writes SkPM4f, and
> finally get around to implementing Sk8f::Store4() (store while reinterlacing).
> Sk4f::Store4() already exists for both SSE and NEON.
> 
> Next step: reduce simple constant_color -> store pipelines (src mode, full
> coverage) into non-pipeline memsets.
> GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2480823002
> 
> Committed: https://skia.googlesource.com/skia/+/a4a4488a4c3f16758f7e2b050168fe8d2f3b2a4d

TBR=msarett@google.com,reed@google.com,brianosman@google.com,mtklein@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2481023002
CQ_INCLUDE_TRYBOTS=master.client.skia:Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-SKNX_NO_SIMD-Trybot

[dogben, 2016-11-06 13:24:16]

Created Revert of skrpb: evaluate color filters for constant shaders once.

[mtklein_C, 2016-11-06 13:38:31]

Yeah, it's a big stretch to connect this CL to a memory leak.

I'll take a look at what's going on.  Please don't revert anything.

[dogben, 2016-11-06 13:44:20]

Description was changed from

==========
Revert of skrpb: evaluate color filters for constant shaders once. (patchset #1
id:1 of https://codereview.chromium.org/2480823002/ )

Reason for revert:
Just testing if this CL really broke the ASAN bot.

Original issue's description:
> skrpb: evaluate color filters for constant shaders once.
> 
> The simplest thing to do here is just run shader+color filter pipeline at
> construction time to create a new constant color shader (replacing the paint
> color).
> 
> This reduces a pipeline like:
>   - constant_color  (paint color)
>   - matrix_4x5
>   - clamp_a
>   - load_d_foo, xfermode, lerp, store_foo
> to
>   - constant_color  (paint color -> matrix_4x5 -> clamp_a)
>   - load_d_foo, xfermode, lerp, store_foo
> 
> To implement this all, we add a new store_f32 stage that writes SkPM4f, and
> finally get around to implementing Sk8f::Store4() (store while reinterlacing).
> Sk4f::Store4() already exists for both SSE and NEON.
> 
> Next step: reduce simple constant_color -> store pipelines (src mode, full
> coverage) into non-pipeline memsets.
> GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2480823002
> 
> Committed:
https://skia.googlesource.com/skia/+/a4a4488a4c3f16758f7e2b050168fe8d2f3b2a4d

TBR=msarett@google.com,reed@google.com,brianosman@google.com,mtklein@chromium...
# Not skipping CQ checks because original CL landed more than 1 days ago.
==========

to

==========
Revert of skrpb: evaluate color filters for constant shaders once. (patchset #1
id:1 of https://codereview.chromium.org/2480823002/ )

Reason for revert:
Just testing if this CL really broke the ASAN bot.

Original issue's description:
> skrpb: evaluate color filters for constant shaders once.
> 
> The simplest thing to do here is just run shader+color filter pipeline at
> construction time to create a new constant color shader (replacing the paint
> color).
> 
> This reduces a pipeline like:
>   - constant_color  (paint color)
>   - matrix_4x5
>   - clamp_a
>   - load_d_foo, xfermode, lerp, store_foo
> to
>   - constant_color  (paint color -> matrix_4x5 -> clamp_a)
>   - load_d_foo, xfermode, lerp, store_foo
> 
> To implement this all, we add a new store_f32 stage that writes SkPM4f, and
> finally get around to implementing Sk8f::Store4() (store while reinterlacing).
> Sk4f::Store4() already exists for both SSE and NEON.
> 
> Next step: reduce simple constant_color -> store pipelines (src mode, full
> coverage) into non-pipeline memsets.
> GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2480823002
> 
> Committed:
https://skia.googlesource.com/skia/+/a4a4488a4c3f16758f7e2b050168fe8d2f3b2a4d

TBR=msarett@google.com,reed@google.com,brianosman@google.com,mtklein@chromium...
# Not skipping CQ checks because original CL landed more than 1 days ago.
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2481023002
CQ_INCLUDE_TRYBOTS=master.client.skia:Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-SKNX_NO_SIMD-Trybot
==========

[dogben, 2016-11-06 14:38:04]

On 2016/11/06 at 13:38:31, mtklein wrote:
> Yeah, it's a big stretch to connect this CL to a memory leak.
> 
> I'll take a look at what's going on.  Please don't revert anything.

Yeah, I don't understand how this CL could cause the failure, but it does look
like reverting would fix the ASAN bot.

[mtklein_C, 2016-11-06 14:42:18]

On 2016/11/06 at 14:38:04, benjaminwagner wrote:
> On 2016/11/06 at 13:38:31, mtklein wrote:
> > Yeah, it's a big stretch to connect this CL to a memory leak.
> > 
> > I'll take a look at what's going on.  Please don't revert anything.
> 
> Yeah, I don't understand how this CL could cause the failure, but it does look
like reverting would fix the ASAN bot.

No, I don't think so.  I think it'd paper over the problem.  Please don't revert
this CL.

[mtklein_C, 2016-11-06 14:54:48]

Notes:

LSAN is telling us we're leaking the wideCS color space object allocated at the
start of draw_gamut_grid() in gm/gamut.cpp.

This particular blitter refuses to draw into 8888, and yet I see the leak when
drawing the GM into 8888, srgb, and f16.  This mostly rules out changes in this
blitter from being the root problem for now.  This CL did not change how memory
is managed in the blitter, nor did it change the conditions under which the
blitter is chosen.  If it's leaking, it should have been leaking longer than
this CL.

We'd probably see this leak when drawing into 565 too, but the GM explicitly
bails out in that case.  The GM itself looks fine, and I don't see any leaks
when drawing into the analogous GPU backends.  It appears that we're also
leaking refs on the global sRGB color space, but that won't show up as a memory
leak because it's a singleton.

The 4 simplest CellRenderers in the GM don't cause leaks.  The remaining 10 each
leak 1 ref.

[dogben, 2016-11-07 21:25:46]

Seems to be fixed; closing.