check for wide-open inside quickContains

check for wide-open inside quickContains

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2263373002

Committed: https://skia.googlesource.com/skia/+/4d2cce442dc2d5a7b35c0ed65c725d0686071e23

[reed1, 2016-08-22 19:20:27]

Description was changed from

==========
check for wide-open inside quickContains

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

to

==========
check for wide-open inside quickContains

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2263373002
==========

[reed1, 2016-08-22 19:21:00]

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

[reed1, 2016-08-22 19:21:00]

had to relocate from https://codereview.chromium.org/2258403002/ since upload
failed on the old cl#

[reed1, 2016-08-22 19:21:06]

The CQ bit was checked by reed@google.com

[commit-bot: I haz the power, 2016-08-22 19:21:16]

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-22 19:21:18]

Note for Reviewers:
The CQ is waiting for an approval.
If you believe that the CL is not ready yet, or if you would like to L-G-T-M
with comments then please uncheck the CQ checkbox.

Waiting for LGTM from valid reviewer(s) till 2016-08-23 01:21 UTC

[bsalomon, 2016-08-22 19:47:56]

https://codereview.chromium.org/2263373002/diff/1/src/gpu/GrClipStackClip.cpp
File src/gpu/GrClipStackClip.cpp (right):

https://codereview.chromium.org/2263373002/diff/1/src/gpu/GrClipStackClip.cpp...
src/gpu/GrClipStackClip.cpp:29: if (!fStack || fStack->isWideOpen()) {
We don't need this call to isWideOpen() anymore, right?

https://codereview.chromium.org/2263373002/diff/1/src/gpu/GrClipStackClip.cpp...
src/gpu/GrClipStackClip.cpp:37: if (!fStack || fStack->isWideOpen()) {
or this one.

[reed1, 2016-08-22 19:49:49]

https://codereview.chromium.org/2263373002/diff/1/src/gpu/GrClipStackClip.cpp
File src/gpu/GrClipStackClip.cpp (right):

https://codereview.chromium.org/2263373002/diff/1/src/gpu/GrClipStackClip.cpp...
src/gpu/GrClipStackClip.cpp:29: if (!fStack || fStack->isWideOpen()) {
On 2016/08/22 19:47:56, bsalomon wrote:
> We don't need this call to isWideOpen() anymore, right?

Well, this is why my first version didn't put the check in clipstack: the cost
of making the rect itself (esp. since there are two int-to-float conversions)
was part of the slow-down. If I rely on the clipstack check, I still pay for the
rect.

[bsalomon, 2016-08-22 19:52:36]

On 2016/08/22 19:49:49, reed1 wrote:
> https://codereview.chromium.org/2263373002/diff/1/src/gpu/GrClipStackClip.cpp
> File src/gpu/GrClipStackClip.cpp (right):
> 
>
https://codereview.chromium.org/2263373002/diff/1/src/gpu/GrClipStackClip.cpp...
> src/gpu/GrClipStackClip.cpp:29: if (!fStack || fStack->isWideOpen()) {
> On 2016/08/22 19:47:56, bsalomon wrote:
> > We don't need this call to isWideOpen() anymore, right?
> 
> Well, this is why my first version didn't put the check in clipstack: the cost
> of making the rect itself (esp. since there are two int-to-float conversions)
> was part of the slow-down. If I rely on the clipstack check, I still pay for
the
> rect.

Oh! I misunderstood. lgtm

[reed1, 2016-08-22 19:52:45]

How about this: I create an alternate version that takes int w/h and it delays
the rect til after the check for wide-open ... ?

[reed1, 2016-08-22 19:55:47]

On 2016/08/22 19:52:45, reed1 wrote:
> How about this: I create an alternate version that takes int w/h and it delays
> the rect til after the check for wide-open ... ?

oh crap, I forgot, we're offsetting another rect by ints, not creating one. n/m

[commit-bot: I haz the power, 2016-08-22 20:03:49]

Description was changed from

==========
check for wide-open inside quickContains

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2263373002
==========

to

==========
check for wide-open inside quickContains

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2263373002

Committed:
https://skia.googlesource.com/skia/+/4d2cce442dc2d5a7b35c0ed65c725d0686071e23
==========

[commit-bot: I haz the power, 2016-08-22 20:03:50]

Committed patchset #1 (id:1) as
https://skia.googlesource.com/skia/+/4d2cce442dc2d5a7b35c0ed65c725d0686071e23