Add special case circle blur for Ganesh

Add special case circle blur for Ganesh

This makes the blurcircles bench go from ~33us to ~8us on Windows desktop.

It will require layout test suppressions

Committed: https://skia.googlesource.com/skia/+/30c4cae7d3a26252e7e45adf6e5722b34adf6848

[robertphillips, 2015-09-10 22:10:17]

robertphillips@google.com changed reviewers:
+ bsalomon@google.com

[bsalomon, 2015-09-11 13:05:46]

lgtm, small stuff inline

https://codereview.chromium.org/1311583005/diff/120001/gm/blurcircles.cpp
File gm/blurcircles.cpp (right):

https://codereview.chromium.org/1311583005/diff/120001/gm/blurcircles.cpp#new...
gm/blurcircles.cpp:43: kNormal_SkBlurStyle,
Given that there are only 4 blurs maybe we want to create the BMFs in the
onBeforePreOncePreDraw() function.

https://codereview.chromium.org/1311583005/diff/120001/include/core/SkColor.h
File include/core/SkColor.h (right):

https://codereview.chromium.org/1311583005/diff/120001/include/core/SkColor.h...
include/core/SkColor.h:149: return SkScalarToFixed(x) >> 8;
Maybe SkUnitScalarClampToByte(). Many of our SkXToY functions just assert that
the X fits in the Y.

https://codereview.chromium.org/1311583005/diff/120001/include/core/SkRRect.h
File include/core/SkRRect.h (right):

https://codereview.chromium.org/1311583005/diff/120001/include/core/SkRRect.h...
include/core/SkRRect.h:101: }
I wonder if these should take a tolerance (just a thought). Thinking about cases
where matrix has a large scale factor but the rrect is in local space.

https://codereview.chromium.org/1311583005/diff/120001/src/effects/GrCircleBl...
File src/effects/GrCircleBlurFragmentProcessor.h (right):

https://codereview.chromium.org/1311583005/diff/120001/src/effects/GrCircleBl...
src/effects/GrCircleBlurFragmentProcessor.h:28: static GrFragmentProcessor*
Create(GrTextureProvider*textureProvider,
const GrFP (most of our factories don't do this but probably should).

[robertphillips, 2015-09-11 15:55:35]

https://codereview.chromium.org/1311583005/diff/120001/gm/blurcircles.cpp
File gm/blurcircles.cpp (right):

https://codereview.chromium.org/1311583005/diff/120001/gm/blurcircles.cpp#new...
gm/blurcircles.cpp:43: kNormal_SkBlurStyle,
On 2015/09/11 13:05:45, bsalomon wrote:
> Given that there are only 4 blurs maybe we want to create the BMFs in the
> onBeforePreOncePreDraw() function.

Done.

https://codereview.chromium.org/1311583005/diff/120001/include/core/SkColor.h
File include/core/SkColor.h (right):

https://codereview.chromium.org/1311583005/diff/120001/include/core/SkColor.h...
include/core/SkColor.h:149: return SkScalarToFixed(x) >> 8;
On 2015/09/11 13:05:46, bsalomon wrote:
> Maybe SkUnitScalarClampToByte(). Many of our SkXToY functions just assert that
> the X fits in the Y.

Done.

https://codereview.chromium.org/1311583005/diff/120001/include/core/SkRRect.h
File include/core/SkRRect.h (right):

https://codereview.chromium.org/1311583005/diff/120001/include/core/SkRRect.h...
include/core/SkRRect.h:101: }
On 2015/09/11 13:05:46, bsalomon wrote:
> I wonder if these should take a tolerance (just a thought). Thinking about
cases
> where matrix has a large scale factor but the rrect is in local space.

Done. I've added a todo. Right now I only use isCircle in device space.

https://codereview.chromium.org/1311583005/diff/120001/src/effects/GrCircleBl...
File src/effects/GrCircleBlurFragmentProcessor.h (right):

https://codereview.chromium.org/1311583005/diff/120001/src/effects/GrCircleBl...
src/effects/GrCircleBlurFragmentProcessor.h:28: static GrFragmentProcessor*
Create(GrTextureProvider*textureProvider,
On 2015/09/11 13:05:46, bsalomon wrote:
> const GrFP (most of our factories don't do this but probably should).

Done.

[reed1, 2015-09-11 18:47:10]

reed@google.com changed reviewers:
+ reed@google.com

[reed1, 2015-09-11 18:47:11]

https://codereview.chromium.org/1311583005/diff/120001/include/core/SkColor.h
File include/core/SkColor.h (right):

https://codereview.chromium.org/1311583005/diff/120001/include/core/SkColor.h...
include/core/SkColor.h:149: return SkScalarToFixed(x) >> 8;
On 2015/09/11 13:05:46, bsalomon wrote:
> Maybe SkUnitScalarClampToByte(). Many of our SkXToY functions just assert that
> the X fits in the Y.

Independent of the clamping question...

This is a recent hot-topic for me. What *is* the right scaling function?

1. round(x * 255)
2. floor(x * 255.9999)

The current impl is "close" to #2, but not exactly either.

[reed1, 2015-09-11 18:49:50]

https://codereview.chromium.org/1311583005/diff/140001/include/core/SkColor.h
File include/core/SkColor.h (right):

https://codereview.chromium.org/1311583005/diff/140001/include/core/SkColor.h...
include/core/SkColor.h:142: static inline U8CPU SkUnitScalarClampToByte(SkScalar
x) {
Does this need to be public?

[robertphillips, 2015-09-15 16:35:06]

https://codereview.chromium.org/1311583005/diff/120001/include/core/SkColor.h
File include/core/SkColor.h (right):

https://codereview.chromium.org/1311583005/diff/120001/include/core/SkColor.h...
include/core/SkColor.h:149: return SkScalarToFixed(x) >> 8;
It seems like some pinned version of #2 would be best for this use case.

https://codereview.chromium.org/1311583005/diff/140001/include/core/SkColor.h
File include/core/SkColor.h (right):

https://codereview.chromium.org/1311583005/diff/140001/include/core/SkColor.h...
include/core/SkColor.h:142: static inline U8CPU SkUnitScalarClampToByte(SkScalar
x) {
On 2015/09/11 18:49:50, reed1 wrote:
> Does this need to be public?

Done. Moved to SkColorPriv.h.

[robertphillips, 2015-09-15 16:58:41]

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

[commit-bot: I haz the power, 2015-09-15 16:58:54]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1311583005/180001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1311583005/180001

[commit-bot: I haz the power, 2015-09-15 16:59:44]

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

[commit-bot: I haz the power, 2015-09-15 16:59:45]

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

[robertphillips, 2015-09-15 17:02:16]

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

[commit-bot: I haz the power, 2015-09-15 17:02:27]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1311583005/200001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1311583005/200001

[commit-bot: I haz the power, 2015-09-15 17:08:33]

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

[commit-bot: I haz the power, 2015-09-15 17:08:33]

Dry run: This issue passed the CQ dry run.

[robertphillips, 2015-09-15 17:20:02]

The CQ bit was checked by robertphillips@google.com

[robertphillips, 2015-09-15 17:20:02]

The patchset sent to the CQ was uploaded after l-g-t-m from bsalomon@google.com
Link to the patchset: https://codereview.chromium.org/1311583005/#ps200001
(title: "fix no-GPU build")

[commit-bot: I haz the power, 2015-09-15 17:20:15]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1311583005/200001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1311583005/200001

[commit-bot: I haz the power, 2015-09-15 17:20:58]

Committed patchset #11 (id:200001) as
https://skia.googlesource.com/skia/+/30c4cae7d3a26252e7e45adf6e5722b34adf6848